Closed
Bug 1118123
Opened 9 years ago
Closed 9 years ago
MSE: Appending data to source buffer do not update MediaSource duration
Categories
(Core :: Audio/Video, defect, P2)
Core
Audio/Video
Tracking
()
RESOLVED
FIXED
mozilla37
People
(Reporter: jya, Assigned: jya)
References
(Blocks 1 open bug, )
Details
Attachments
(3 files, 2 obsolete files)
5.15 KB,
patch
|
cajbir
:
review+
Sylvestre
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
3.64 KB,
patch
|
cajbir
:
review+
|
Details | Diff | Splinter Review |
1.86 KB,
patch
|
Details | Diff | Splinter Review |
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•9 years ago
|
Assignee: nobody → jyavenard
Updated•9 years ago
|
Priority: -- → P2
Assignee | ||
Comment 1•9 years ago
|
||
Update mediasource duration as per spec following call to appendBuffer
Attachment #8545461 -
Flags: review?(cajbir.bugzilla)
Assignee | ||
Comment 2•9 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•9 years ago
|
||
Update web reftest to reflect changes. Sounds like more could be re-enabled
Attachment #8545475 -
Flags: review?(karlt)
Assignee | ||
Comment 4•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=84838a7133b8
Updated•9 years ago
|
Attachment #8545461 -
Flags: review?(cajbir.bugzilla) → review+
Updated•9 years ago
|
Attachment #8545474 -
Flags: review?(cajbir.bugzilla) → review+
Assignee | ||
Updated•9 years ago
|
Attachment #8545475 -
Flags: review?(karlt) → review?(cajbir.bugzilla)
Updated•9 years ago
|
Attachment #8545475 -
Flags: review?(cajbir.bugzilla) → review+
Assignee | ||
Comment 5•9 years ago
|
||
Carrying r+
Assignee | ||
Updated•9 years ago
|
Attachment #8545475 -
Attachment is obsolete: true
Assignee | ||
Comment 6•9 years ago
|
||
my hg queue got messed up for some reasons
Assignee | ||
Updated•9 years ago
|
Attachment #8546292 -
Attachment is obsolete: true
Comment 7•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/4cd36b909eef https://hg.mozilla.org/mozilla-central/rev/f56f0268eb5b https://hg.mozilla.org/mozilla-central/rev/d5dfcc81d26f
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
Comment 8•9 years ago
|
||
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?
Updated•9 years ago
|
status-firefox36:
--- → fixed
status-firefox37:
--- → fixed
Assignee | ||
Comment 9•9 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•9 years ago
|
Flags: needinfo?(giles)
Comment 10•9 years ago
|
||
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
Comment 11•9 years ago
|
||
These also caused mediasource-config-change-webm-v-bitrate.html test failures on the Aurora push. Possibly missing dependent patch.
Assignee | ||
Comment 12•9 years ago
|
||
ah yes, the need 1118123_web_platform_fix.patch applied.
Comment 13•9 years ago
|
||
Comment on attachment 8545461 [details] [diff] [review] Update mediasource duration following sourcebuffer::appendBuffer reset for now
Attachment #8545461 -
Flags: approval-mozilla-aurora?
Comment 14•9 years ago
|
||
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 15•9 years ago
|
||
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•9 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 17•9 years ago
|
||
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+
Comment 18•9 years ago
|
||
https://hg.mozilla.org/releases/mozilla-beta/rev/9a4a8602e6f4 https://hg.mozilla.org/releases/mozilla-beta/rev/61e917f920c9 https://hg.mozilla.org/releases/mozilla-beta/rev/7cd63f89473b
Comment 19•9 years ago
|
||
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.
Description
•