MSE: calling appendBuffer with no data doesn't emit updateend

RESOLVED FIXED in Firefox 36

Status

()

defect
P2
normal
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: jya, Assigned: jya)

Tracking

(Blocks 1 bug)

Trunk
mozilla38
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox36 fixed, firefox37 fixed, firefox38 fixed)

Details

Attachments

(2 attachments, 2 obsolete attachments)

Assignee

Description

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

We're supposed to run the segment parser loop algorithm.
Which states:
http://w3c.github.io/media-source/#sourcebuffer-segment-parser-loop

1.Loop Top: If the input buffer is empty, then jump to the need more data step below.

7.Need more data: Return control to the calling algorithm.

Which upon return:
3.Set the updating attribute to false.
4.Queue a task to fire a simple event named update at this SourceBuffer object.
5.Queue a task to fire a simple event named updateend at this SourceBuffer object.

Right now we abort, and as such no update and updateend events are fired.

This is what cause errors in testing/web-platform/tests/media-source/interfaces.html (and if we apply bug 1118589 now causes timeouts as no error is immediately returned)
Assignee

Comment 1

4 years ago
Posted patch Update web platform tests (obsolete) — Splinter Review
Update webref test results
Attachment #8546286 - Flags: review?(karlt)
Assignee

Updated

4 years ago
Assignee: nobody → jyavenard
Status: NEW → ASSIGNED
Attachment #8546286 - Flags: review?(karlt) → review+
Assignee

Comment 2

4 years ago
Do not error if data appended is empty, and properly issue update/updateend event
Attachment #8546301 - Flags: review?(cajbir.bugzilla)

Updated

4 years ago
Attachment #8546301 - Flags: review?(cajbir.bugzilla) → review+

Updated

4 years ago
Priority: -- → P2
Assignee

Comment 4

4 years ago
Rebase, so it doesn't rely on async appendBuffer
Assignee

Updated

4 years ago
Attachment #8546301 - Attachment is obsolete: true
Assignee

Comment 5

4 years ago
Use ./mach web-platform-tests-update to update patch instead. Yield exactly what I had done manually with extra CR
Assignee

Updated

4 years ago
Attachment #8546286 - Attachment is obsolete: true
Comment on attachment 8550027 [details] [diff] [review]
Do not abort when calling appendBuffer with no data

Approval Request Comment
[Feature/regressing bug #]: MSE
[User impact if declined]: Less consistent testing, possible problems with Youtube playback.
[Describe test coverage new/current, TBPL]: stable on inbound, presuming green on m-c.
[Risks and why]: MSE-specific change. Main risk is web platform test result variance from un-uplifted patches.
[String/UUID change made/needed]: None.

This request applies to all patches in this bug.
Attachment #8550027 - Flags: approval-mozilla-beta?
Attachment #8550027 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/670e66764052
https://hg.mozilla.org/mozilla-central/rev/20a4dd540338
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
Attachment #8550027 - Flags: approval-mozilla-beta?
Attachment #8550027 - Flags: approval-mozilla-beta+
Attachment #8550027 - Flags: approval-mozilla-aurora?
Attachment #8550027 - Flags: approval-mozilla-aurora+
This doesn't apply cleanly to beta. I'll need to do a backport before we can uplift this batch.
Or rather, figure out what the difference is, since this code should be the same on both branches. Looks like something with the bug 1118123 backout on 37.
Assignee

Comment 13

4 years ago
you likely would need bug 1118123 yes...
this bug is non-important, it's only required to make bug 1120084 land.
Ok, I traced the history. Bug 1118123 landed on firefox 37 when it was m-c, on January 9. I uplifted to 36 (then aurora) but backed it out per Jean-Yves' request. On January 12, m-c became firefox 38 and all the changed on 37 were merged into aurora. That explains why the patches from this bug apply to aurora 37, but not beta 36. I was confused because I remembered backing them out of Aurora, forgetting that was before branch uplift.
Flags: qe-verify-
Assignee

Comment 16

4 years ago
This has automatic coverage in the web-ref test: media-source/mediasource-append-buffer.html

this test that appendBuffer with no data properly emits update/updateend
You need to log in before you can comment on or make changes to this bug.