This is an archived snapshot of W3C's public bugzilla bug tracker, decommissioned in April 2019. Please see the home page for more details.

Bug 26316 - [MSE] End of stream algorithm closes, re-opens MediaSource
Summary: [MSE] End of stream algorithm closes, re-opens MediaSource
Status: RESOLVED FIXED
Alias: None
Product: HTML WG
Classification: Unclassified
Component: Media Source Extensions (show other bugs)
Version: unspecified
Hardware: PC All
: P2 normal
Target Milestone: ---
Assignee: Aaron Colwell
QA Contact: HTML WG Bugzilla archive list
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-07-12 04:40 UTC by Jer Noble
Modified: 2014-07-25 18:14 UTC (History)
4 users (show)

See Also:


Attachments

Description Jer Noble 2014-07-12 04:40:34 UTC
2.4.7 End of stream algorithm
> 1. Change the readyState attribute value to "ended".
> 2. Queue a task to fire a simple event named sourceended at the MediaSource.
> 3. ↪︎ If error is not set
>  3.1 Run the duration change algorithm with new duration set to the highest end time reported by the buffered attribute across all SourceBuffer objects in sourceBuffers.

2.4.6 Duration change
> 4. If the new duration is less than old duration, then call remove(new duration, old duration) on all objects in sourceBuffers.

3.2 Methods, remove
> 6. If the readyState attribute of the parent media source is in the "ended" state then run the following steps:
>  6.1 Set the readyState attribute of the parent media source to "open""
>  6.2 Queue a task to fire a simple event named sourceopen at the parent media source .

Calling endOfStream() on a MediaSource with a partially-appended SourceBuffer will result in:

MediaSource.readyState: open -> ended
MediaSource.readyState: ended -> open
MediaSource 'sourceended' fires
MediaSource 'sourceopen' fires
Comment 1 Jer Noble 2014-07-12 05:38:44 UTC
Moving §2.4.7 steps 1 & 2 after step 3 seems to solve the problem.
Comment 2 Matt Wolenetz 2014-07-14 20:47:55 UTC
I'm not sure that I understand the problem.
As part of endOfStream(), "If the updating attribute equals true on any SourceBuffer in sourceBuffers, then throw an INVALID_STATE_ERR exception and abort these steps."
This occurs prior to running any of the end of stream algorithm steps.
If the agent was in the middle of a SourceBuffer.append() operation, then updating should be true and INVALID_STATE_ERR should result on an attempt by the app to call endOfStream().
Do I misunderstand the scenario? I'm quoting from the latest editor's draft (08 July 2014)
Comment 3 Jer Noble 2014-07-14 21:30:45 UTC
(In reply to Matt Wolenetz from comment #2)
> I'm not sure that I understand the problem.
> As part of endOfStream(), "If the updating attribute equals true on any
> SourceBuffer in sourceBuffers, then throw an INVALID_STATE_ERR exception and
> abort these steps."
> This occurs prior to running any of the end of stream algorithm steps.
> If the agent was in the middle of a SourceBuffer.append() operation, then
> updating should be true and INVALID_STATE_ERR should result on an attempt by
> the app to call endOfStream().
> Do I misunderstand the scenario? I'm quoting from the latest editor's draft
> (08 July 2014)

I believe the source of the misunderstanding is that a single append() operation may not append samples for [0, duration).  So after one or more append() operations:

- "updateend" is fired
- /updating/ is unset
- duration is N
- buffered is (0, M), where M < N.

Calling endOfStream() in these circumstances will lead to the events listed in the initial bug report.
Comment 4 Jer Noble 2014-07-14 21:34:09 UTC
(In reply to Jer Noble from comment #3)
> (In reply to Matt Wolenetz from comment #2)
> > I'm not sure that I understand the problem.
> > As part of endOfStream(), "If the updating attribute equals true on any
> > SourceBuffer in sourceBuffers, then throw an INVALID_STATE_ERR exception and
> > abort these steps."
> > This occurs prior to running any of the end of stream algorithm steps.
> > If the agent was in the middle of a SourceBuffer.append() operation, then
> > updating should be true and INVALID_STATE_ERR should result on an attempt by
> > the app to call endOfStream().
> > Do I misunderstand the scenario? I'm quoting from the latest editor's draft
> > (08 July 2014)
> 
> I believe the source of the misunderstanding is that a single append()
> operation may not append samples for [0, duration).  So after one or more
> append() operations:
> 
> - "updateend" is fired
> - /updating/ is unset
> - duration is N
> - buffered is (0, M), where M < N.
> 
> Calling endOfStream() in these circumstances will lead to the events listed
> in the initial bug report.

Note, however, that duration must either have been set explicitly from script, or a duration must have been present in the initialization segment, for duration to be greater than the buffered range.
Comment 5 Matt Wolenetz 2014-07-14 23:32:14 UTC
Thank you for clarifying; I was confused by the "partially-appended SourceBuffer" text in the original post.

Note that duration defaults to +Infinity in the initialization segment received algorithm, if duration was previously NaN and is not present in the init segment. This increases the potential for this bug to be hit due to endOfStream()'s execution of the duration change algorithm resulting in remove(new duration, +Infinity or some value > new duration).
Comment 6 Aaron Colwell 2014-07-24 18:03:25 UTC
Jer: I don't think your proposed fix is quite right since deferring the readyState transition causes SourceBuffer.buffered to return a different value than it currently does.

I can think of 2 possible alternate solutions:
- Replace step 4 in the "duration change algorithm" with steps 7-13 from remove().
- Change step 4 of "duration change algorithm" to only call remove() if any SourceBuffer.buffered contains one or more ranges that intersect with [new duration, old duration). I believe in the case of endOfStream() this should always be false since we are using the end ranges to determine the new duration.

Any preference?
Comment 7 Jer Noble 2014-07-24 18:31:52 UTC
(In reply to Aaron Colwell from comment #6)
> Jer: I don't think your proposed fix is quite right since deferring the
> readyState transition causes SourceBuffer.buffered to return a different
> value than it currently does.
> 
> I can think of 2 possible alternate solutions:
> - Replace step 4 in the "duration change algorithm" with steps 7-13 from
> remove().

I think this is the right way to go, because:

> - Change step 4 of "duration change algorithm" to only call remove() if any
> SourceBuffer.buffered contains one or more ranges that intersect with [new
> duration, old duration). I believe in the case of endOfStream() this should
> always be false since we are using the end ranges to determine the new
> duration.

Well, that's a different issue, which I haven't written up yet.  But if you use the highest buffered end time across all MediaSource.sourceBuffers, you'll end up stalling (dropping to HAVE_METADATA) if you try to play to the (new) duration, as the currentTime enters into the unbuffered range of the shorter-buffered SourceBuffer.  And given that the general use case is to have a separate SourceBuffer for each audio and video stream, and given that each will have very slightly different durations, due to the irreconcilable differences in sample durations between the two media types.

Also, the HTMLMediaElement.buffered attribute is also defined to inflate shorter SourceBuffer.buffered values to the highest end time across all MediaSource.sourceBuffers.

So either the "SourceBuffer Monitoring" state algorithm has to be amended to account for these "virtual buffered ranges", or the endOfStream() behavior has to be changed to truncate the SourceBuffers to the least highest buffered end time.
Comment 8 Aaron Colwell 2014-07-24 20:24:42 UTC
(In reply to Jer Noble from comment #7)
> (In reply to Aaron Colwell from comment #6)
> > Jer: I don't think your proposed fix is quite right since deferring the
> > readyState transition causes SourceBuffer.buffered to return a different
> > value than it currently does.
> > 
> > I can think of 2 possible alternate solutions:
> > - Replace step 4 in the "duration change algorithm" with steps 7-13 from
> > remove().
> 
> I think this is the right way to go, because:

Ok. I'll do this then.

> 
> > - Change step 4 of "duration change algorithm" to only call remove() if any
> > SourceBuffer.buffered contains one or more ranges that intersect with [new
> > duration, old duration). I believe in the case of endOfStream() this should
> > always be false since we are using the end ranges to determine the new
> > duration.
> 
> Well, that's a different issue, which I haven't written up yet.  But if you
> use the highest buffered end time across all MediaSource.sourceBuffers,
> you'll end up stalling (dropping to HAVE_METADATA) if you try to play to the
> (new) duration, as the currentTime enters into the unbuffered range of the
> shorter-buffered SourceBuffer.  And given that the general use case is to
> have a separate SourceBuffer for each audio and video stream, and given that
> each will have very slightly different durations, due to the irreconcilable
> differences in sample durations between the two media types.
> 
> Also, the HTMLMediaElement.buffered attribute is also defined to inflate
> shorter SourceBuffer.buffered values to the highest end time across all
> MediaSource.sourceBuffers.
> 
> So either the "SourceBuffer Monitoring" state algorithm has to be amended to
> account for these "virtual buffered ranges", or the endOfStream() behavior
> has to be changed to truncate the SourceBuffers to the least highest
> buffered end time.

I'm pretty sure the "SourceBuffer Monitoring" algorithm needs to be updated. It hasn't gotten much love in a long time and I wouldn't be surprised if there are bugs lurking in there. readyState transitions should definitely be driven of HTMLMediaElement.buffered ranges and not individual SourceBuffer.buffered since during non-endOfStream playback the intersection used should make sure that we don't try to play unbuffered regions. I'll take a look at the algorithm and file a bugs for any problems with the algorithm I see.
Comment 9 Aaron Colwell 2014-07-24 21:05:06 UTC
Change committed.
https://dvcs.w3.org/hg/html-media/rev/b3f086e62b53

Factored the steps out into a "range removal" algorithm and updated remove() and the "duration change" algorithms to use that just to avoid these 2 mechanisms from accidentally diverging.
Comment 10 Jer Noble 2014-07-25 18:14:05 UTC
> I'm pretty sure the "SourceBuffer Monitoring" algorithm needs to be updated.
> It hasn't gotten much love in a long time and I wouldn't be surprised if
> there are bugs lurking in there. readyState transitions should definitely be
> driven of HTMLMediaElement.buffered ranges and not individual
> SourceBuffer.buffered since during non-endOfStream playback the intersection
> used should make sure that we don't try to play unbuffered regions. I'll
> take a look at the algorithm and file a bugs for any problems with the
> algorithm I see.

Okay great.  I've filed Bug #26436 to track this separate issue.