1123202 - Call to removeSourceBuffer should abort pending buffer append

Status: RESOLVED FIXED

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

Description

[Jean-Yves Avenard [:jya]]

Per spec:
http://w3c.github.io/media-source/#widl-MediaSource-removeSourceBuffer-void-SourceBuffer-sourceBuffer

"When this method is invoked, the user agent must run the following steps:

    1.If sourceBuffer specifies an object that is not in sourceBuffers then throw a NotFoundError exception and abort these steps.
    2.If the sourceBuffer.updating attribute equals true, then run the following steps:
        1.Abort the buffer append and stream append loop algorithms if they are running.
        2.Set the sourceBuffer.updating attribute to false.
        3.Queue a task to fire a simple event named abort at sourceBuffer.
        4.Queue a task to fire a simple event named updateend at sourceBuffer.
"

Currently it does nothing.

Comment 1

[Jean-Yves Avenard [:jya]]

Attachment: Execute abort() when detaching source buffer

properly issue abort/updateend when a source buffer is removed.

Comment 2

[cajbir (:cajbir)]

Comment on attachment 8553001 [details] [diff] [review]
Execute abort() when detaching source buffer

Review of attachment 8553001 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/media/mediasource/SourceBuffer.h
@@ +78,5 @@
>    void AppendBuffer(const ArrayBuffer& aData, ErrorResult& aRv);
>    void AppendBuffer(const ArrayBufferView& aData, ErrorResult& aRv);
>  
>    void Abort(ErrorResult& aRv);
> +  void Abort();

Add comment differentiating this from the Abort function above it.

Comment 3

[Jean-Yves Avenard [:jya]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1efd2c0dfbb4

Comment 4

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/1efd2c0dfbb4

Comment 5

[Ralph Giles (:rillian)]

Comment on attachment 8553001 [details] [diff] [review]
Execute abort() when detaching source buffer

Approval Request Comment
[Feature/regressing bug #]: MSE
[User impact if declined]: Less consistent testing. Sites more likely to serve flash video.
[Describe test coverage new/current, TreeHerder]: Landed on m-c.
[Risks and why]: This is an MSE-specific change for spec compliance. Risk is low.
[String/UUID change made/needed]: None.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/f60d058bf105
https://hg.mozilla.org/releases/mozilla-beta/rev/d52554d9a8f0

Comment 7

[Jean-Yves Avenard [:jya]]

There's another place that I had missed

Comment 8

[Jean-Yves Avenard [:jya]]

Spawned into bug 1128115. This patch actually fixed SourceBufferList.Remove rather than MediaSource.removeSourceBuffer.