Separate audio seek from video seek for MediaFormatReader

RESOLVED WONTFIX

Status

()

P3
normal
RESOLVED WONTFIX
2 years ago
2 years ago

People

(Reporter: jwwang, Assigned: jwwang)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Assignee)

Description

2 years ago
This is required by bug 1286129 and also makes the code of MFR simpler:

1. MDSM takes care of A/V sync without MFR's effort. MFR just needs to seek to what it is told to.
2. MFR doesn't need to know video-only seek anymore which is the concern of MDSM.
3. http://searchfox.org/mozilla-central/rev/09ae31fddfbc229ef1b8f1a3ccd87ad05680e695/dom/media/MediaFormatReader.cpp#2042
   We want MFR to reject a seek promise when waiting for data. So MDSM can call WaitForData() and seek again when the wait promise is resolved.

Note 3. is needed to fix bug 1286129.
(Assignee)

Updated

2 years ago
Assignee: nobody → jwwang
Blocks: 1319295, 1286129
Priority: -- → P3
Attention need to be paid that under some circumstances you need to know where video seeked compare to audio seek as it caused a bug with non aligned segment.

This caused stalls with YouTube.

Example.
we have an audio buffered range = [5, 10]
video buffered range is [4.8, 10]

seeking to 4.8 will cause a stall, as a work around those incorrectly aligned segment, the MFR checks if it has audio near where video seeked and attempt to seek to that point instead
see: https://dxr.mozilla.org/mozilla-central/rev/6dbc6e9f62a705d5f523cc750811bd01c8275ec6/dom/media/MediaFormatReader.cpp#2042
so yes, it will make the MFR simpler, but only to make the MDSM more complicated as you'll need to duplicate that logic. Except that the MDSM currently doesn't have access to the data to make that decision.

not sure it's a simplification of the code overall
(Assignee)

Comment 3

2 years ago
Yes, most of the code is moved from MFR to MDSM in that sense. I think MDSM is the right place to hold the code in order to implement features such as video-suspending or dormant mechanism.
(Assignee)

Comment 4

2 years ago
(In reply to Jean-Yves Avenard [:jya] from comment #1)
> This caused stalls with YouTube.
> 
> Example.
> we have an audio buffered range = [5, 10]
> video buffered range is [4.8, 10]
> 
> seeking to 4.8 will cause a stall, as a work around those incorrectly
> aligned segment, the MFR checks if it has audio near where video seeked and
> attempt to seek to that point instead
> see:
> https://dxr.mozilla.org/mozilla-central/rev/
> 6dbc6e9f62a705d5f523cc750811bd01c8275ec6/dom/media/MediaFormatReader.cpp#2042

What is the stall like? I expect it to play from the position of 4.8s with 0.2s silence to begin with. Is that a problem?
(In reply to JW Wang [:jwwang] [:jw_wang] from comment #4)

> What is the stall like? I expect it to play from the position of 4.8s with
> 0.2s silence to begin with. Is that a problem?

the seek never completes because we have too big of a gap, and the demuxer returns WAITING_FOR_DATA

YouTube waits on the "playing" event to be fired prior loading more data. As the seek never completes, playing isn't fired, and so we never get more data for the seek to complete.
(Assignee)

Comment 6

2 years ago
See bug 1319295 comment 17. This bug is not possible without exposing details of MFR. Mark as WONTFIX.
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.