Fix off-by-one error, which lead to only BufferMax-1 segments getting transcoded per batch

[hheimbuerger]

Potential fix for #60

The simplest possible fix is to just remove the -1 in the for loop, changing the current

for i := index; i < segmentsTotal-1; i++ {

to the new

for i := index; i < segmentsTotal; i++ {

I guess another possible fix would be to keep the -1, but change the operator to <=:

for i := index; i <= segmentsTotal-1; i++ {

This PR suggests a slightly more involved approach, by also renaming all the local variables, so the algorithm is easier to read and reason about.

[m1k1o]

I think when we already address this issue, it would be good to create unit tests that highlight the issue that is being fixed.

[hheimbuerger]

I think when we already address this issue, it would be good to create unit tests that highlight the issue that is being fixed.

Of course, I don't know why I didn't think of this. I'm gonna look into it!

[hheimbuerger]

Added a unit test.

It did require little bit of dependency injection, to be able to stub out the FFmpeg calls in the test.
The alternative would have been to bring in a monkey patching library, but I didn't really want to do that just yet.

On master:

PS D:\Development\go-transcode> go test ./hlsvod -run TestTranscodeFromSegmentBufferSize
--- FAIL: TestTranscodeFromSegmentBufferSize (0.00s)
    manager_test.go:45: expected 5 queued segments, got 4
FAIL
FAIL    github.com/m1k1o/go-transcode/hlsvod    0.405s
FAIL

On the fix branch:

PS D:\Development\go-transcode> go test ./hlsvod -run TestTranscodeFromSegmentBufferSize
ok      github.com/m1k1o/go-transcode/hlsvod    0.407s

[hheimbuerger]

Hmm, I'm not so sure anymore.

I've just had o3 call this a sentinel entry, and argue that by design, only segmentsBufferMax-1 segments are being created per batch. It says due to breakpoints == segments-1, the valid segment indices go from 0 to segmentsTotal-2.

Is that correct? Is the whole "encode only segmentsBufferMax-1 segments per batch" intentional, maybe? Even still, it feels like the configuration setting is confusingly named then.

[m1k1o]

Huh. I am also confused. Simply, all the segments that are advertised to be available should be available. It now doesn't really matter if we do open or closed set.

[hheimbuerger]

segments that are advertised

What do you mean by "segments that are advertised"?

This is about the 'prefetching' (or rather: speculative transcoding) of not-yet-requested segments via the segmentBufferMax configuration. If by "advertised", you're referring to the m3u8, I think they're all advertised in advance in the VOD use case.