wait for both main and audio loaders for endOfStream if main starting media unknown #44
Conversation
| @@ -623,10 +623,11 @@ export class MasterPlaylistController extends videojs.EventTarget { | |||
|
|
|||
| if (this.mediaTypes_.AUDIO.activePlaylistLoader) { | |||
| // if the audio playlist loader exists, then alternate audio is active | |||
| if (this.mainSegmentLoader_.startingMedia_ && | |||
| if (!this.mainSegmentLoader_.startingMedia_ || | |||
There was a problem hiding this comment.
What happens in the event that we only have alternate audio, it hits the end of its playlist, and mainSegmentLoader_.startingMedia_ is not yet set?
There was a problem hiding this comment.
I do not believe this is a scenario we can get into. STREAM-INFs are REQUIRED to have a URI line by the spec. So there will always be a main stream that the main segment loader is loading from. In addition to that, until the way audio buffer ownership is overhauled, there isn't actually any mechanism to stop the main segment loader in an audio only stream with alternate audio activated. So even if audio only and alternate audio activated, the main segment loader will continue to request its audio segments, and then the VirtualSourceBuffer just drops them on the floor because it doesn't own the audio buffer. (Yes this is wasted bandwidth, even more reason to change the way we handle audio)
| // audio source buffer | ||
| MPC.mediaSource.sourceBuffers[1].trigger('updateend'); | ||
|
|
||
| assert.equal(videoEnded, 0, 'main segment loader triggered endded'); |
| MPC.mediaSource.sourceBuffers[1].trigger('updateend'); | ||
|
|
||
| assert.equal(videoEnded, 0, 'main segment loader triggered endded'); | ||
| assert.equal(audioEnded, 1, 'audio segment loader did not trigger ended'); |
There was a problem hiding this comment.
I think this comment and the one above are backwards (this one did trigger, other one didn't)
There was a problem hiding this comment.
oops, botched copy paste
| // main source buffer | ||
| MPC.mediaSource.sourceBuffers[0].trigger('updateend'); | ||
|
|
||
| assert.equal(videoEnded, 1, 'main segment loader did not trigger ended again'); |
There was a problem hiding this comment.
These descriptions may also be backwards
|
Tested 👍 |
This fixes an issue introduced by #34 that can happen with very short playlists with demuxed audio on very slow connections. If the connection is slow enough such that the audio segment loader can complete requesting the entire audio playlist and trigger
endedbefore the main segment loader has completed its request for the first video segment, the changes in #34 will assume the stream is audio only and just rely on the audio segment loaders ended state, ignoring the main loader, becausestartingMedia_is set after the first request has been completed. This change will force waiting for both the main and audio loaders to trigger ended if the starting media of the main loader is still unknown.