Closed Bug 1130826 Opened 8 years ago Closed 8 years ago

sourcebuffer.abort() shouldn't abort range removal algorithm


(Core :: Audio/Video, defect, P2)




Tracking Status
firefox37 --- fixed
firefox38 --- fixed


(Reporter: jya, Assigned: jya)


(Blocks 1 open bug)



(1 file)

As per W3C spec:

"If the updating attribute equals true, then run the following steps:

    Abort the buffer append and stream append loop algorithms if they are running.
    Set the updating attribute to false.
    Queue a task to fire a simple event named abort at this SourceBuffer object.
    Queue a task to fire a simple event named updateend at this SourceBuffer object.


Right now, abort will abort the current buffer append as per spec; and set the updating attribute to false.

When the queued range removal gets to run, it checks if the updating attribute is true ; if not it aborts.

This is the cause for failing YouTube MSE conformance test 42 (bug 1116651).

Now, I'm rather confused at what events should be fired once abort() is run.

Right now, we always have a matching number of updatestart and updateend , even if aborted.

If we do as per spec as I'm reading them, we would have:
updatestart (caused by range removal algorithm starting), then abort / updateend (due to abort(), and then update/updatend (once range removal gets to complete)
Something rather inconsistent:

"The append or remove was aborted by an abort() call. updating transitions from true to false."

which indicates that remove will be aborted by abort.
Now range removal algorithm can be started by modifying the mediasource.duration attribute.

Would this mean that the remove() function can be aborted, but changing the duration won't ?

Actually, upon re-reading the spec, I think we got it wrong: changing the duration currently immediately change the duration of the mediasource object; however this should only queue the range removal algorithm with the new duration ; and this is what will set the duration later ; and as such can be aborted.
Flags: needinfo?(karlt)
Further to comment 2, actually we are supposed to immediately change the duration and that the range removal algorithm is to be run (and that's asynchronous)..

So back to the original problem on what events to fired...
Summary: sourcebuffer.abort() shouldn't abort range removal algorithm → Changing mediasource.duration attribute should

This adds 10 x 1s segment ; and then set the mediasource object's duration to 5s ; and immediately call sourceBuffer.abort()

Chrome issues:

and the sourceBuffer.buffered appears as { 0, 5 }, and mediaSource.duration = 5

Nightly shows that buffered range is still { 0, 10.03 }, and mediaSource.duration = 5

IE: Doesn't fire the abort() event but only updatestart/update/updatend, and the buffered range is {0,5ish} and mediaSource.duration = 5

IE behaviour seems more correct, though none of those behaviour are right according to the spec.
Summary: Changing mediasource.duration attribute should → sourcebuffer.abort() shouldn't abort range removal algorithm
Blocks: 1116651
Only run range removal asynchronously when called following sourcebuffer.remove() ; when run following a change to mediasource.duration, we always run it, so data is properly removed. Events fired followed the rules of the last command run (e.g. if abort(): abort/updateend, otherwise update/updateend)
Attachment #8560988 - Flags: review?(cajbir.bugzilla)
Assignee: nobody → jyavenard
Attachment #8560988 - Flags: review?(cajbir.bugzilla) → review+
(In reply to Jean-Yves Avenard [:jya] from comment #4)
> Lodged a bug at W3C:

Thanks for filing this.  abort() is useful for resetting the segment parser if the client decides to start appending a different stream.  In this situation, aborting an append currently in progress seems more reasonable than aborting a remove.  However, perhaps the only *need* for aborting an algorithm might be to allow subsequent appends without waiting for updateend, and that situation is similar with removes.

I also filed about asynchronicity in the duration change algorithm when reducing duration.
Flags: needinfo?(karlt)
Priority: -- → P2
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
Comment on attachment 8560988 [details] [diff] [review]
Run range removal algorithm when setting mediasource duration

Approval Request Comment
[Feature/regressing bug #]: MSE
[User impact if declined]: Spec compliance, less consistent testing. Harder to port later fixes.
[Describe test coverage new/current, TreeHerder]: Landed on m-c.
[Risks and why]: MSE-specific so low.
[String/UUID change made/needed]: None.
Attachment #8560988 - Flags: approval-mozilla-aurora?
Comment on attachment 8560988 [details] [diff] [review]
Run range removal algorithm when setting mediasource duration

As stated, this bug was pre-approved to land with a set of changes for MSE. Marking the approval after the fact.
Attachment #8560988 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.