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

Status: RESOLVED FIXED

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

Description

[Jean-Yves Avenard [:jya]]

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.

Comment 1

[Jean-Yves Avenard [:jya]]

Attachment: [MSE] Use latest demux end time to detect discontinuities.

The ContainerParser doesn't always return an accurate end time.

Comment 2

[Jean-Yves Avenard [:jya]]

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

Comment 3

[Jean-Yves Avenard [:jya]]

Attachment: [MSE] Use latest demux end time to detect discontinuities.

The ContainerParser doesn't always return an accurate end time.

Comment 4

[Jean-Yves Avenard [:jya]]

Attachment: [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+

Comment 5

[Jean-Yves Avenard [:jya]]

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/cec04b5766f9

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/cec04b5766f9

Comment 7

[Ralph Giles (:rillian)]

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

Comment 8

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8654583 [details] [diff] [review]
[MSE] Use latest demux end time to detect discontinuities.

Improve MSE, taking it.

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

I'm hitting conflicts trying to uplift this to aurora. Can we get a branch specific patch for this?

Comment 10

[Jean-Yves Avenard [:jya]]

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.

Comment 11

[Ralph Giles (:rillian)]

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.

Comment 12

[Carsten Book [:Tomcat]]

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 ?

Comment 13

[Jean-Yves Avenard [:jya]]

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