Update how MINIMUM_SEGMENT_SIZE is used in the SegmentInventory

[peaBerberian]

Fix a rebuffering issue seen since a fix proposed by #1416

The MINIMUM_SEGMENT_SIZE

Basically the MINIMUM_SEGMENT_SIZE value, which defined a minimum time duration a media segment can have to be considered by the RxPlayer (mainly to prevent some rounding errors in the RxPlayer, though this is far from the only mechanism for that) was also sometimes relied on for "chunks", which is the term the RxPlayer use for the units of media data that will be transmitted to media buffers.

The latter (chunks) can be smaller than the former (segments) in (and only in for now) low-latency scenarios.

Problem with chunks

"Chunks" can be much much smaller than "segments" (the unit of media data we actually load from the network through a single HTTP(S) request) and it is very possible for a chunk to be smaller than 5 milliseconds, the original value (before this PR) for MINIMUM_SEGMENT_SIZE.

Before the fix proposed by #1416, multiple chunks were generally grouped together before being pushed at once to a buffer (though still as a smaller group than the whole segment) and it seemed those groups always were bigger than MINIMUM_SEGMENT_SIZE.

However, the #1416 fix now separates every one of those chunks (this is actually performed at its lowest level, by always considering carefully a moof and an mdat box for ISOBMFF-compatible containers) which leads to frequently obtaining chunks smaller than the MINIMUM_SEGMENT_SIZE.

In turn, due to some processing of the SegmentInventory (which lists the chunks believed to be currently buffered by the browser), the RxPlayer would then believe that some of those chunks have been garbage-collected.

For some reasons not yet explored (this gets messy very quickly from there sadly), the RxPlayer first seems to think that it cannot buffer those segments, then any of the following segment, and then enters a rebuffering status for around 20 seconds, before restarting playback as if nothing happened.

This would merit more investigation, but for now I mostly spent time fixing the original MINIMUM_SEGMENT_SIZE issue.

The fixes

There's actually three fixes. It gets pretty technical so I guess only people knowing how the RxPlayer's SegmentInventory works could decipher this (even I, have issues explaining what I did :p):

1. Before, we would have considered in the SegmentInventory that if a chunk only had less than MINIMUM_SEGMENT_SIZE (5 milliseconds originally) from its starting time to the end of the currently-considered contiguous buffered time range (so basically range.end - chunk.start < MINIMUM_SEGMENT_SIZE), then it was considered as part of the next range, unless there's no next range in which case it was considered garbage-collected (see fix 3).

   The fix here is to also check if there's more than MINIMUM_SEGMENT_SIZE after that same range's end in that scenario.

2. MINIMUM_SEGMENT_SIZE has been updated from 5 milliseconds to 1 millisecond. This has risks but there's risks in both sides and I here noticed an issue due to its value being here "too high", so I chose to lower it for resiliency.

3. We previously considered in the SegmentInventory that chunks that were not associated to any buffered time range (due to the fact that we already found the last chunk associated to the last buffered time range) must have since been garbage-collected.

   It theoretically makes sense, yet we didn't perform a check on their insertionTs, a property storing a timestamp of when the chunk has been pushed, and which protects against race condition where the browser wouldn't have yet updated its buffered time range, despite the chunk having been pushed.

   Now we do check that at least SEGMENT_SYNCHRONIZATION_DELAY milliseconds have elapsed since a chunk's insertionTs before defining if it has been garbage-collected or not.

Note that in the diff, thisSegment actually refers to a chunk and not a segment (it would have been too easy if we had the right variable name)

[peaBerberian]

I added a log so it becomes easier to determine if a SegmentInventory's synchronization result (logged at the end) do seem to correspond to its SourceBuffer's buffered property at that time (logged here)

[peaBerberian]

I broke the while condition into separate if -> break for readability purposes (yes your heard right, I relied on the much-hated break statements for readability purposes :p)

[Florent-Bouisset]

Shouldn't it be the opposite of the previous condition ?

[peaBerberian]

I think you're right, I just replaced >= by <