1323920 - Separate audio seek from video seek for MediaFormatReader

Status: RESOLVED WONTFIX

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

Description

[JW Wang [:jwwang] [:jw_wang]]

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.

Comment 1

[Jean-Yves Avenard [:jya]]

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

Comment 2

[Jean-Yves Avenard [:jya]]

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

Comment 3

[JW Wang [:jwwang] [:jw_wang]]

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.

Comment 4

[JW Wang [:jwwang] [:jw_wang]]

(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?

Comment 5

[Jean-Yves Avenard [:jya]]

(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.

Comment 6

[JW Wang [:jwwang] [:jw_wang]]

See bug 1319295 comment 17. This bug is not possible without exposing details of MFR. Mark as WONTFIX.