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

RESOLVED FIXED in Firefox 36

Status

()

P2
normal
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: jya, Assigned: jya)

Tracking

(Blocks: 1 bug)

Trunk
mozilla38
Points:
---

Firefox Tracking Flags

(firefox36 fixed, firefox37 fixed, firefox38 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

4 years ago
As per spec:
http://w3c.github.io/media-source/#sourcebuffer-append-error

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.
(Assignee)

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)
(Assignee)

Updated

4 years ago
Assignee: nobody → jyavenard
Status: NEW → ASSIGNED

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+
(Assignee)

Comment 3

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

Updated

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?
https://hg.mozilla.org/mozilla-central/rev/0ba9865709b5
Status: ASSIGNED → RESOLVED
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.