MSE: Appending data to source buffer do not update MediaSource duration

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
mozilla37
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +
qe-verify -

Firefox Tracking Flags

(firefox36 fixed, firefox37 fixed)

Details

(URL)

Attachments

(3 attachments, 2 obsolete attachments)

(Assignee)

Description

4 years ago
When appending data to a buffer, the mediasource duration attribute should be updated with the new duration.

The attached test case shows that after appending 10s of video, mediaSource.duration attribute is still 0.

From the spec:
"appendBuffer(), appendStream() and endOfStream() can update the duration under certain circumstances."

This will call the Coded Frame Processing Algorithm
http://w3c.github.io/media-source/#sourcebuffer-coded-frame-processing

"If the media segment contains data beyond the current duration, then run the duration change algorithm with new duration set to the maximum of the current duration and the group end timestamp."
(Assignee)

Updated

4 years ago
Assignee: nobody → jyavenard
Priority: -- → P2
(Assignee)

Comment 1

4 years ago
Update mediasource duration as per spec following call to appendBuffer
Attachment #8545461 - Flags: review?(cajbir.bugzilla)
(Assignee)

Comment 2

4 years ago
mochitest to verify behaviour. Some hoops to do so as loading the init segment would set an original duration of 4s. So we have to wait for that duration to change, then change it back to 0. And only then call appendBuffer
Attachment #8545474 - Flags: review?(cajbir.bugzilla)
(Assignee)

Comment 3

4 years ago
Update web reftest to reflect changes. Sounds like more could be re-enabled
Attachment #8545475 - Flags: review?(karlt)

Updated

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

Updated

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

Updated

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

Updated

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

Comment 5

4 years ago
Carrying r+
(Assignee)

Updated

4 years ago
Attachment #8545475 - Attachment is obsolete: true
(Assignee)

Comment 6

4 years ago
my hg queue got messed up for some reasons
(Assignee)

Updated

4 years ago
Attachment #8546292 - Attachment is obsolete: true
Comment on attachment 8545461 [details] [diff] [review]
Update mediasource duration following sourcebuffer::appendBuffer

Landed the first two patches on Aurora with a=mse pre-approval.

https://hg.mozilla.org/releases/mozilla-aurora/rev/dda3f8613738
https://hg.mozilla.org/releases/mozilla-aurora/rev/cfe3df775521

Skipped the third patch, as it adjusts web platform test changes which we haven't been uplifting.

Main risk is that this will change test results and we haven't uplifted the new expectations. Code changes are MSE-specific.
Attachment #8545461 - Flags: approval-mozilla-aurora?
status-firefox36: --- → fixed
status-firefox37: --- → fixed
(Assignee)

Comment 9

4 years ago
Ralph, I would actually back them out..

There is a possibility (albeit rare) that CheckEndTime() is called from RangeRemoval: in which case we would either assert as mUpdating is true; or would end up issuing two update/updateend in a row (with no matching updatestart)...

Currently the issue is mostly hidden because it's hidden by another bug (we always treat the initial duration as 0, and it only ever grows under most circumstances).
But bug 1119757 and bug 1118126 expose them more.

I have a fix. Ran out of time to commit them
(Assignee)

Updated

4 years ago
Flags: needinfo?(giles)
Ok, backed out. We'll reland with the fixes from bug 1118126 when they're ready.

https://hg.mozilla.org/releases/mozilla-aurora/rev/893ed355ee54
status-firefox36: fixed → affected
Depends on: 1118126
Flags: needinfo?(giles)
These also caused mediasource-config-change-webm-v-bitrate.html test failures on the Aurora push. Possibly missing dependent patch.
(Assignee)

Comment 12

4 years ago
ah yes, the need 1118123_web_platform_fix.patch applied.
Comment on attachment 8545461 [details] [diff] [review]
Update mediasource duration following sourcebuffer::appendBuffer

reset for now
Attachment #8545461 - Flags: approval-mozilla-aurora?
Jean-Yves, are we still waiting on bug 1118126 before uplifting this? It was on your 'nice to have' list for beta 2, but in comment #9 you said to wait.
Flags: needinfo?(jyavenard)
Comment on attachment 8545461 [details] [diff] [review]
Update mediasource duration following sourcebuffer::appendBuffer

Approval Request Comment
[Feature/regressing bug #]: MSE
[User impact if declined]: Less consistent testing, subsequent fixes don't backport cleanly.
[Describe test coverage new/current, TBPL]: Landed on m-c, m-a.
[Risks and why]: Risk is low; we've had no problem with these patches on 37.
[String/UUID change made/needed]: None.

This request is for all patches on this bug. Jean-Yves doesn't seem to be around, but re-nominating for beta anyway. I think it's better to be consistent between the three branches at this point, so this should go in.
Attachment #8545461 - Flags: approval-mozilla-beta?
(Assignee)

Comment 16

4 years ago
(In reply to Ralph Giles (:rillian) from comment #14)
> Jean-Yves, are we still waiting on bug 1118126 before uplifting this? It was
> on your 'nice to have' list for beta 2, but in comment #9 you said to wait.

bug 1118126 is the range removal implementation we won't be ready for another day or say from my side.

bug 1120079 is what prevents the problem from occurring. Problem with that one is that it relies on a whole heaps.

My list of bug to uplift were in regards to Beta 2. Now that we've passed that deadline, and we're for Beta 3 I think all of them should be uplifted.
Flags: needinfo?(jyavenard)
Comment on attachment 8545461 [details] [diff] [review]
Update mediasource duration following sourcebuffer::appendBuffer

It is not too late for beta 2. A bunch of patches didn't land.
Attachment #8545461 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Appears to have test automation coverage.
Flags: qe-verify-
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.