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

RESOLVED FIXED in Firefox 42

Status

()

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: jya, Assigned: jya)

Tracking

(Blocks: 1 bug)

Trunk
mozilla43
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox42 fixed, firefox43 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

4 years ago
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

4 years ago
Assignee: nobody → jyavenard
(Assignee)

Comment 1

4 years ago
Created attachment 8654476 [details] [diff] [review]
[MSE] Use latest demux end time to detect discontinuities.

The ContainerParser doesn't always return an accurate end time.
Attachment #8654476 - Flags: review?(gsquelart)
(Assignee)

Comment 2

4 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

4 years ago
Created attachment 8654488 [details] [diff] [review]
[MSE] Use latest demux end time to detect discontinuities.

The ContainerParser doesn't always return an accurate end time.
Attachment #8654488 - Flags: review?(gsquelart)
(Assignee)

Updated

4 years ago
Blocks: 1197083
Attachment #8654488 - Flags: review?(gsquelart) → review+
(Assignee)

Comment 4

4 years ago
Created attachment 8654583 [details] [diff] [review]
[MSE] Use latest demux end time to detect discontinuities.

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

4 years ago
Keywords: checkin-needed
(Assignee)

Updated

4 years ago
Keywords: checkin-needed
Attachment #8654583 - Flags: review?(gsquelart) → review+
https://hg.mozilla.org/mozilla-central/rev/cec04b5766f9
Status: NEW → RESOLVED
Last Resolved: 4 years ago
status-firefox43: affected → fixed
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
status-firefox42: --- → affected
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

4 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)
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)
(Assignee)

Comment 13

4 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
status-firefox42: affected → fixed
Flags: needinfo?(jyavenard)
You need to log in before you can comment on or make changes to this bug.