WebMDemuxer/WebMContainerParser don't properly handle media segment (cluster) ending after the first block.

RESOLVED FIXED in Firefox 42

Status

()

RESOLVED FIXED
3 years ago
3 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

(URL)

Attachments

(2 attachments)

(Assignee)

Description

3 years ago
The issue was found investigating bug 1199583.

The test case was reproduced at the URL.

segment 19 is a continuation of segment 18; and contains a single block before starting a new cluster.

If the WebMTimeDataOffset returned by the WebMBufferedParser has a new cluster starting at index 1; WebMContainerParser::ParseStartAndEndTimestamps will return false ; even though we do have samples in it.

Expected Outcome:
It should return true and the start and end time of this block.
(Assignee)

Updated

3 years ago
Assignee: nobody → jyavenard
(Assignee)

Comment 1

3 years ago
Created attachment 8654464 [details] [diff] [review]
[MSE/webm] Properly calculate media segment duration.

We can know with certainty the duration of a block if we have a following one. We do not have to always rely on having a previous segment to estimate the duration.
Attachment #8654464 - Flags: review?(kinetik)
(Assignee)

Updated

3 years ago
Blocks: 1197083
(Assignee)

Comment 2

3 years ago
Created attachment 8654582 [details] [diff] [review]
[webm] P2. Hold on frames for which the duration can't be known or estimated.

Now that the ContainerParser can detect cluster's end without having to wait for the next one we have the situation were a cluster is made of a single block, with a single frame. Under such circumstances, the WebM demuxer would attempt to estimate the duration based on the last frame timestamp which by default is 0. With mediasource a media segment doesn't always start at 0.
For such cluster, and for mediasource to prevent an unforeseen regression, we hold off this frame until more is appended.
Attachment #8654582 - Flags: review?(kinetik)
(Assignee)

Comment 3

3 years ago
Matthew, I'm flying away early tomorrow morning.

Could you push it if you're happy with the changes? thanks
Flags: needinfo?(kinetik)
(Assignee)

Updated

3 years ago
Summary: WebMContainer doesn't properly handle media segment (cluster) ending after the first block. → WebMDemuxer/WebMContainerParser doesn't properly handle media segment (cluster) ending after the first block.
(Assignee)

Updated

3 years ago
Summary: WebMDemuxer/WebMContainerParser doesn't properly handle media segment (cluster) ending after the first block. → WebMDemuxer/WebMContainerParser don't properly handle media segment (cluster) ending after the first block.
Attachment #8654464 - Flags: review?(kinetik) → review+
Attachment #8654582 - Flags: review?(kinetik) → review+
Flags: needinfo?(kinetik)
https://hg.mozilla.org/mozilla-central/rev/8d9743e32b80
https://hg.mozilla.org/mozilla-central/rev/d92acf7fd4ce
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox43: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
Comment on attachment 8654464 [details] [diff] [review]
[MSE/webm] Properly calculate media segment duration.

Requesting 42 uplift of both changes on this bug.

Approval Request Comment
[Feature/regressing bug #]: MSE webm playbck
[User impact if declined]: playback can stall, harder to backport other changes.
[Describe test coverage new/current, TreeHerder]: Landed on m-c, manual testing.
[Risks and why]: Risk is minimal. This affects only WebM MSE playback, which is disabled on 42. I want it uplifted to reduce variance between the trees.
[String/UUID change made/needed]: None.
Attachment #8654464 - Flags: approval-mozilla-aurora?
status-firefox42: --- → affected
Comment on attachment 8654464 [details] [diff] [review]
[MSE/webm] Properly calculate media segment duration.

If this can simplify your work, taking it.
Attachment #8654464 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
I'm hitting conflicts trying to uplift this to aurora. Can we get branch specific patches for this?
Flags: needinfo?(jyavenard)
(Assignee)

Comment 9

3 years ago
This is the order in which the patches should be applied:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=6a328601136f

followed by bug 1199573, 1199878 and 1199879
Flags: needinfo?(jyavenard)
(Assignee)

Comment 10

3 years ago
bug 1195073 is missing in the uplift, and it's one of the most important one.
(In reply to Jean-Yves Avenard [:jya] from comment #10)
> bug 1195073 is missing in the uplift, and it's one of the most important one.

yeah seems it causing conflicts that need to be solved first.
(Assignee)

Comment 12

3 years ago
pushed it as part of 
https://hg.mozilla.org/releases/mozilla-aurora/pushloghtml?changeset=7437f28133fc
status-firefox42: affected → fixed
You need to log in before you can comment on or make changes to this bug.