1096823 - MediaDecoderStateMachine::GetBuffered could go wrong when mStartTime is not ready

Status: RESOLVED INACTIVE

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

Description

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

1. MediaDecoderStateMachine::GetBuffered could be called before we decode the 1st frame to get mStartTime.
2. WebMReader::GetBuffered needs the start time to get correct results.

Proposal: fall back to MediaDecoderReader::GetBuffered which doesn't require the start time.

For the proposal to work, we have to remove the hack at [1] which sets mStartTime to 0 when mStartTime is -1. The hack prevents us from checking |mStartTime == -1| to determine if we get the start time from the 1st frame or not. Also it is also confusion that |mStartTime == 0| doesn't really mean the true start time is zero.

[1] http://hg.mozilla.org/mozilla-central/file/c0d559389a5c/dom/media/MediaDecoderStateMachine.cpp#l1302

Comment 1

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

Hi Chris,
Can you share your thoughts about the proposal? Thanks.

Comment 2

[Chris Pearce [:cpearce (Not reading bugmail)]]

I would be happy for GetBuffered() to return empty ranges in this case, rather than guesstimating.

Could we add a flag to denote when loadeddata has fired in MediaDecoder, and bail out in MediaDecoder::GetBuffered() if the flag is not set?

Note bug 1091008 added a dependency in the MSEReader on the start time being set to 0. MSE requires this it seems.

I would be very happy if this whole StartTime/EndTime/Duration mess could be cleaned up.

Comment 3

[Bobby Holley (:bholley)]

Note that I touch this stuff a bit in my patches in bug 1091008, which should land tonight.

See https://github.com/bholley/gecko-dev/commits/mse_buffertime

Comment 4

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

The problem is the hack at [1] sets mStartTime to 0 when mStartTime is -1 and shadows the truth that we don't actually the start time yet. WebMReader::GetBuffered and OggReader::GetBuffered just happen to return correct results when the start time turns out to be 0 in the future as is not case for split.webm which has a non-zero start time.

test_buffered.html will fail on split.webm when MediaDecoderStateMachine::GetBuffered is called before we decode the 1st frame to get mStartTime under the condition where the file is not fully downloaded [2].

[1] http://hg.mozilla.org/mozilla-central/file/c0d559389a5c/dom/media/MediaDecoderStateMachine.cpp#l1302
[2] http://hg.mozilla.org/mozilla-central/file/c60fc2c11c0e/dom/media/webm/WebMReader.cpp#l1103

Comment 5

[Chris Pearce [:cpearce (Not reading bugmail)]]

(In reply to JW Wang [:jwwang] from comment #4)
> The problem is the hack at [1] sets mStartTime to 0 when mStartTime is -1
> and shadows the truth that we don't actually the start time yet.

The assumption there was that if we set the duration from some other source, the duration should take into account a non-zero start time, and if we later discover the start time is non-zero we can adjust the end time to preserve the duration.

Comment 6

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

(In reply to Chris Pearce (:cpearce) from comment #2)
> Could we add a flag to denote when loadeddata has fired in MediaDecoder, and
> bail out in MediaDecoder::GetBuffered() if the flag is not set?

We might check mLoadedDataFired in HTMLMediaElement::Buffered.

Comment 7

[Nils Ohlmeier [:drno]]

Mass closing do to inactivity.
Feel free to re-open if still needed.