Closed
Bug 1199879
Opened 9 years ago
Closed 9 years ago
TrackBuffersManager unnecessarily recreate a new demuxer after parsing a media segment.
Categories
(Core :: Audio/Video: Playback, defect)
Core
Audio/Video: Playback
Tracking
()
RESOLVED
FIXED
mozilla43
People
(Reporter: jya, Assigned: jya)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 2 obsolete files)
5.66 KB,
patch
|
mozbugz
:
review+
Sylvestre
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
Since bug 1195073 ; the TrackBuffersManager monitor the incoming stream to detect out of order append and when finding one will recreate a demuxer so a demuxer is always ever fed data with timestamps monotonically increasing.
However, to do so, it relies on the start and end time returned by the ContainerParser.
Unfortunately, the ContainerParser returns the start/end time of the entire binary range provided, not just the of the first media segment (this is required for the old MSE).
As such, the end time returned, being of the entire data will always be greater than the start time of the new media segment parsed.
We should tune the discontinuity detector so it handles this case and remove the need to recreate a demuxer ; which is a rather expensive operation.
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → jyavenard
Assignee | ||
Comment 1•9 years ago
|
||
The ContainerParser doesn't always return an accurate end time.
Attachment #8654476 -
Flags: review?(gsquelart)
Assignee | ||
Comment 2•9 years ago
|
||
Comment on attachment 8654476 [details] [diff] [review]
[MSE] Use latest demux end time to detect discontinuities.
GroupEndTimeStamp isn't reset upon a call to Coded Frame Removal... so thats not reliable enough
Attachment #8654476 -
Attachment is obsolete: true
Attachment #8654476 -
Flags: review?(gsquelart)
Assignee | ||
Comment 3•9 years ago
|
||
The ContainerParser doesn't always return an accurate end time.
Attachment #8654488 -
Flags: review?(gsquelart)
Attachment #8654488 -
Flags: review?(gsquelart) → review+
Assignee | ||
Comment 4•9 years ago
|
||
The ContainerParser doesn't always return an accurate end time.
Remove stray reset that got introduced in previous commit.
Carrying r+
Attachment #8654583 -
Flags: review?(gsquelart)
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 5•9 years ago
|
||
Attachment #8654583 -
Flags: review?(gsquelart) → review+
Comment 6•9 years ago
|
||
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
Comment 7•9 years ago
|
||
Comment on attachment 8654583 [details] [diff] [review]
[MSE] Use latest demux end time to detect discontinuities.
Approval Request Comment
[Feature/regressing bug #]: MSE
[User impact if declined]: Higher cpu and memory usage in video playback.
[Describe test coverage new/current, TreeHerder]: Landed on m-c some days ago.
[Risks and why]: Risk looks low to me. MSE-specific code.
[String/UUID change made/needed]: none
Attachment #8654583 -
Flags: approval-mozilla-aurora?
Updated•9 years ago
|
Attachment #8654488 -
Attachment is obsolete: true
Updated•9 years ago
|
status-firefox42:
--- → affected
Comment 8•9 years ago
|
||
Comment on attachment 8654583 [details] [diff] [review]
[MSE] Use latest demux end time to detect discontinuities.
Improve MSE, taking it.
Attachment #8654583 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
I'm hitting conflicts trying to uplift this to aurora. Can we get a branch specific patch for this?
Flags: needinfo?(jyavenard)
Assignee | ||
Comment 10•9 years ago
|
||
There shouldn't be any conflicts. If there are it only means something that should have been uplifted didn't..
Anything in Dom/media/mediasource should be identical between aurora and master branch.
Flags: needinfo?(jyavenard) → needinfo?(giles)
Comment 11•9 years ago
|
||
Did you uplift on top of the intervening changes? see bug 1197083. Feel free to do those, I had trouble with the rebase and wasn't able to get something I was confident with while I still had time to watch the tree.
Flags: needinfo?(giles)
Comment 12•9 years ago
|
||
hey jya, when trying to uplift this i get :
Tomcats-MacBook-Pro:mozilla-central Tomcat$ hg graft --edit -r cec04b5766f9
grafting 295968:cec04b5766f9 "Bug 1199879: [MSE] Use latest demux end time to detect discontinuities. r=gerald"
merging dom/media/mediasource/TrackBuffersManager.cpp
note: graft of 295968:cec04b5766f9 created no changes to commit
is this expected ?
Flags: needinfo?(jyavenard)
Assignee | ||
Comment 13•9 years ago
|
||
Yes ; it's expected , I did all the uplifts yesterday. sorry , I thought I had updated all the bugs, appears I had missed this one
pushed it as part of
https://hg.mozilla.org/releases/mozilla-aurora/pushloghtml?changeset=7437f28133fc
Flags: needinfo?(jyavenard)
You need to log in
before you can comment on or make changes to this bug.
Description
•