Buffer events

[tchakabam]

@mangui This is introducing the new buffer events and separates MSE buffering and HLS scheduling concerns into separate functions.

Most important is that the streaming state machine does not run the appending anymore, it's all done via the buffer events and SourceBuffer ended callback. Same for the flushing.

There is still some shared state across the functions caring of MSE buffering (only in doAppending actually for the error state), but it should not be too hard to cleanly isolate that now.

Essentially all the logic has just been copy-pasted around, and in some place the state setup has been simply called before or after the event triggers, but I checked and there is no shared state in these cases between the state machine and the buffer event handlers.

The other important part is that we are doing the flushing, EOS and appending all async now, dispatched to the MSE callbacks if necessary when the SB is busy (see respective flags).

[mangui]

thanks @tchakabam this will be really easier to review and merge !

[mangui]

Hi @tchakabam I checked out your branch, and found an issue (reproducible in FF, not in Chrome)

load Big Buck Bunny, seek 60s from the end, wait for buffer EOS to be reached
then perform a smooth switching (set nextLevel to 240p for example)

buffer is not flushed appropriately, leading to smooth switching not working as expected.

see logs with your branch:

[log] > all media data available, signal endOfStream() to MediaSource and stop loading fragment hls.js:7508:7
[log] > media source ended hls.js:7508:7
[log] > set nextLevel:1 hls.js:7508:7
[log] > switching to level 1 hls.js:7508:7
[log] > (re)loading playlist for level 1 hls.js:7508:7
[log] > flush audio [530.018666,539.0032666666667], of [530.018666,634.182133], pos:542.428691 hls.js:7508:7
[log] > buffer flushed

and with master branch:

[log] > all media data available, signal endOfStream() to MediaSource and stop loading fragment hls.js:7450:7
[log] > media source ended hls.js:7450:7
[log] > set nextLevel:1 hls.js:7450:7
[log] > switching to level 1 hls.js:7450:7
[log] > (re)loading playlist for level 1 hls.js:7450:7
[log] > flush audio [0,260.014177], of [0,260.014177], pos:472.629411 hls.js:7450:7
[log] > level 1 loaded [0,63],duration:634.634 hls.js:7450:7
[log] > flush audio [460.033733,469.01831111111113], of [460.033733,634.182133], pos:472.629411 hls.js:7450:7
[log] > flush video [0,259.995555], of [0,259.995555], pos:472.664241 hls.js:7450:7
[log] > flush video [460,469.01831111111113], of [460,634.631288], pos:472.710681 hls.js:7450:7
[log] > buffer flushed hls.js:7450:7
[log] > flush audio [480.0029,Infinity], of [469.040488,634.182133], pos:472.710681 hls.js:7450:7
[log] > flush audio [480.0029,Infinity], of [469.040488,480.049333], pos:472.710681 hls.js:7450:7
[log] > flush audio [480.0029,Infinity], of [469.040488,480.049333], pos:472.791951 hls.js:7450:7
[log] > buffer flushed hls.js:7450:7

[mangui]

it is working fine on Chrome

[log] > all media data available, signal endOfStream() to MediaSource and stop loading fragment
logger.js:3 [log] > media source ended
logger.js:3 [log] > set nextLevel:1
logger.js:3 [log] > switching to level 1
logger.js:3 [log] > (re)loading playlist for level 1
logger.js:3 [log] > flush audio [0,89.997688], of [0,89.997688], pos:509.40338
logger.js:3 [log] > level 1 loaded [0,63],duration:634.634
logger.js:3 [log] > flush audio [490.033911,499.0185], of [490.033911,634.182133], pos:509.40338
logger.js:3 [log] > flush video [0,89.995555], of [0,89.995555], pos:509.40338
logger.js:3 [log] > flush video [490,499.0185], of [490,634.631288], pos:509.40338
logger.js:3 [log] > buffer flushed
logger.js:3 [log] > flush audio [520.0341,634.182133], of [499.040666,634.182133], pos:509.40338
logger.js:3 [log] > flush video [520.0341,634.631288], of [500,634.631288], pos:509.40338
logger.js:3 [log] > buffer flushed

[tchakabam]

OK interesting will check it out! Thanks for having done this test.

[mangui]

ok I understand what is going on. I am on it

[tchakabam]

@mangui hey that's great! because i haven't had time to really investigate it. and probably you have a better idea about these details.

maybe you can get things straight while keeping the separation between MSE objects and the "fragment fetch-and-demux machine" as laid out here.

i think things will get more clear as we separate these functions into different classes.

[mangui]

@tchakabam, IMHO there is a design issue with current pattern
previously when a buffer flushing was triggered, fragment loading was stopped.
now fragment loading/demuxing/appending continues in parallel of the flush.
this means that you flush on one hand, and append on the other hand...

also when you stress a little bit hls.nextLevel setter, you end up having FRAG_LOOP_LOADING_ERROR being raised, because fragmentLoader keeps reloading fragments that are flushed just after.

=> the flushing logic needs to be moved to another layer. and a controller (maybe directly in src/hls.js for now) should stop fragment/stream loader upon buffer flushing and restart it upon BUFFER_FLUSHED.

plz comment, suggestions welcome

[tchakabam]

@mangui ok that is a very obvious issue i agree, and also i think we can definitely solve it.

in fact it was probably my mistake to remove the BUFFER_FLUSHING state. i assumed that i was not necessary anymore since the flushing was happening dispatched on the SourceBuffer callbacks now.

adding this state back should solve it as as long as we are in flushing state, no further segment loading should happen, correct?

i will compare again with previous implementation and fix that aspect.

[mangui]

see dailymotion@523b177
this needs more testing, but it seems to work as expected

[tchakabam]

@mangui wow you were fast :) cool, hope this will work 👍

i think from a logical point of view we have covered the case that we obviously broke by removing that state from the fetching scheduler. so it should be good now.

but you are wise to be cautious ;)

[mangui]

closing as merged