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

RESOLVED FIXED in Firefox 42

Status

()

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