MSE: In case of error, appendBuffer, sourcebuffer never fire the error and updateend events

RESOLVED FIXED in Firefox 36



4 years ago
4 years ago


(Reporter: jya, Assigned: jya)


(Blocks: 1 bug)


Firefox Tracking Flags

(firefox36 fixed, firefox37 fixed, firefox38 fixed)



(1 attachment, 1 obsolete attachment)



4 years ago
As per spec:

In case of error during appendBuffer, then the sourcebuffer append error algorithm should be called.
"This algorithm is called when an error occurs during an append. This algorithm takes a decode error parameter that indicates whether endOfStream() should be called.

    1.Run the reset parser state algorithm.
    2.Set the updating attribute to false.
    3.Queue a task to fire a simple event named error at this SourceBuffer object.
    3.Queue a task to fire a simple event named updateend at this SourceBuffer object.
    4.If decode error is true, then run the end of stream algorithm with the error parameter set to "decode".

We don't do any of those steps..
We currently call endOfStream under all circumstances and that's it.

So we fire the updatestart event; and from that point on nothing more happens.

Comment 1

4 years ago
Partial implementation of Append Error Algorithm. We still should perform a 'Reset Parser State' algorithm, but our TrackBuffer implementation has little to do with what MSE spec describes, and some of the concepts do not apply here
Attachment #8547245 - Flags: review?(cajbir.bugzilla)


4 years ago
Assignee: nobody → jyavenard

Comment 2

4 years ago
Comment on attachment 8547245 [details] [diff] [review]
Implement MSE's AppendErrorAlgorithm

Review of attachment 8547245 [details] [diff] [review]:

::: dom/media/mediasource/SourceBuffer.h
@@ +139,5 @@
>                      double aTimestampOffset);
> +  // Implement the "Append Error Algorithm".
> +  // Will call endOfStream() with "decode" error if aDecodeError is true.
> +  void AppendError(bool aDecoderError);

Reference spec number in comment if spec numbers are stable.

::: dom/media/mediasource/TrackBuffer.cpp
@@ +567,5 @@
> +void
> +TrackBuffer::ResetParserState()
> +{
> +  // TODO

Raise a bug for this. Reference the bug in the TODO.

::: dom/media/mediasource/TrackBuffer.h
@@ +70,5 @@
>    // Call ResetDecode() on each decoder in mDecoders.
>    void ResetDecode();
> +  // MSE's Reset Parser State
> +  void ResetParserState();

Change comment to "Run MSE Reset Parser State Algorithm". If the spec numbers are stable, refer to the number.
Attachment #8547245 - Flags: review?(cajbir.bugzilla) → review+

Comment 3

4 years ago
Carrying r+, and rebase so it doesn't rely on asynchronous appendBuffer (bug 1118589)


4 years ago
Attachment #8547245 - Attachment is obsolete: true
status-firefox36: --- → affected
status-firefox37: --- → affected
status-firefox38: --- → affected
Comment on attachment 8550036 [details] [diff] [review]
Implement MSE's AppendErrorAlgorithm

Approval Request Comment
[Feature/regressing bug #]: MSE
[User impact if declined]: Less consistent testing.
[Describe test coverage new/current, TBPL]: stable on inbound; presuming green on m-c.
[Risks and why]: MSE-specific, so low.
[String/UUID change made/needed]: None.
Attachment #8550036 - Flags: approval-mozilla-beta?
Attachment #8550036 - Flags: approval-mozilla-aurora?
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
status-firefox38: affected → fixed
Attachment #8550036 - Flags: approval-mozilla-beta?
Attachment #8550036 - Flags: approval-mozilla-beta+
Attachment #8550036 - Flags: approval-mozilla-aurora?
Attachment #8550036 - Flags: approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.