Closed Bug 1199879 Opened 5 years ago Closed 5 years ago

TrackBuffersManager unnecessarily recreate a new demuxer after parsing a media segment.

Categories

(Core :: Audio/Video: Playback, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla43
Tracking Status
firefox42 --- fixed
firefox43 --- fixed

People

(Reporter: jya, Assigned: jya)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 2 obsolete files)

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: nobody → jyavenard
The ContainerParser doesn't always return an accurate end time.
Attachment #8654476 - Flags: review?(gsquelart)
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)
The ContainerParser doesn't always return an accurate end time.
Attachment #8654488 - Flags: review?(gsquelart)
Blocks: 1197083
Attachment #8654488 - Flags: review?(gsquelart) → review+
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)
Keywords: checkin-needed
Keywords: checkin-needed
Attachment #8654583 - Flags: review?(gsquelart) → review+
https://hg.mozilla.org/mozilla-central/rev/cec04b5766f9
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
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?
Attachment #8654488 - Attachment is obsolete: true
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)
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)
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)
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)
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.