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

Status: RESOLVED FIXED

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

Description

[Jean-Yves Avenard [:jya]]

As per W3C spec:
http://w3c.github.io/media-source/#widl-SourceBuffer-abort-void

"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)

Comment 1

[Jean-Yves Avenard [:jya]]

Something rather inconsistent:
http://w3c.github.io/media-source/#sourcebuffer-events

states:
"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.

Comment 2

[Jean-Yves Avenard [:jya]]

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...

Comment 3

[Jean-Yves Avenard [:jya]]

http://people.mozilla.org/~jyavenard/tests/mse_mp4/1130826.html

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

Chrome issues:
updatestart
abort
updateend

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.

Comment 4

[Jean-Yves Avenard [:jya]]

Lodged a bug at W3C:
https://www.w3.org/Bugs/Public/show_bug.cgi?id=27980

Comment 5

[Jean-Yves Avenard [:jya]]

Attachment: Run range removal algorithm when setting mediasource duration

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)

Comment 6

[Karl Tomlinson (:karlt)]

(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 https://www.w3.org/Bugs/Public/show_bug.cgi?id=27982 about asynchronicity in the duration change algorithm when reducing duration.

Comment 7

[Jean-Yves Avenard [:jya]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5adf82c926e3

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/5adf82c926e3

Comment 9

[Ralph Giles (:rillian)]

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.

Comment 10

[Ralph Giles (:rillian)]

Pushed to aurora with pre-approval from lmandel.

https://hg.mozilla.org/releases/mozilla-aurora/rev/d8e630336213

Comment 11

[Lawrence Mandel [:lmandel] (use needinfo)]

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.