Closed Bug 1319706 Opened 3 years ago Closed 3 years ago

Allow seeking before finishing decoding first frames

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox53 --- fixed

People

(Reporter: jwwang, Assigned: jwwang)

References

Details

Attachments

(4 files)

Thanks to bug 1309516, we have start time after decoding the metadata. So MDSM is able to seek even before decoding first frames.
Assignee: nobody → jwwang
Blocks: 1295892
Depends on: 1309516
Priority: -- → P3
Comment on attachment 8813591 [details]
Bug 1319706. Part 1 - remove DecodingFirstFrameState::mPendingSeek since it can handle seek requests immediately.

https://reviewboard.mozilla.org/r/95028/#review95198
Attachment #8813591 - Flags: review?(kaku) → review+
Comment on attachment 8813592 [details]
Bug 1319706. Part 2 - remove the SeekJob parameter from DecodingFirstFrameState::Enter().

https://reviewboard.mozilla.org/r/95030/#review95200
Attachment #8813592 - Flags: review?(kaku) → review+
Component: Audio/Video: Recording → Audio/Video: Playback
Comment on attachment 8813974 [details]
Bug 1319706. Part 3 - remove the assertion that is no longer valid.

https://reviewboard.mozilla.org/r/95274/#review95454
Attachment #8813974 - Flags: review?(kaku) → review+
Comment on attachment 8813975 [details]
Bug 1319706. Part 4 - add some logs to debug 'ended' not fired.

https://reviewboard.mozilla.org/r/95276/#review95456
Attachment #8813975 - Flags: review?(kaku) → review+
dom/media/test/test_bug1242338.html times out because:

http://searchfox.org/mozilla-central/rev/feef954874af9a18168e61a75629a9406b847c53/dom/media/MediaDecoderStateMachine.cpp#2720
mFirstFrameLoadedEvent won't be notified until mBufferedUpdateRequest is resolved which could happen after

http://searchfox.org/mozilla-central/rev/feef954874af9a18168e61a75629a9406b847c53/dom/media/MediaDecoderStateMachine.cpp#1066
notifying mOnPlaybackEvent when seeking to the end of the media (as test_bug1242338.html does).

http://searchfox.org/mozilla-central/rev/feef954874af9a18168e61a75629a9406b847c53/dom/media/MediaDecoder.cpp#983
MediaDecoder::PlaybackEnded() returns early for mPlayState == PLAY_STATE_LOADING (which is changed in FirstFrameLoaded).
Depends on: 1320258
Pushed by jwwang@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8aaa89d73c3f
Part 1 - remove DecodingFirstFrameState::mPendingSeek since it can handle seek requests immediately. r=kaku
https://hg.mozilla.org/integration/autoland/rev/03144421b4a6
Part 2 - remove the SeekJob parameter from DecodingFirstFrameState::Enter(). r=kaku
https://hg.mozilla.org/integration/autoland/rev/3dc995228de4
Part 3 - remove the assertion that is no longer valid. r=kaku
https://hg.mozilla.org/integration/autoland/rev/e355836470f9
Part 4 - add some logs to debug 'ended' not fired. r=kaku
was just thinking that this could cause potential regression if we seek prior decoding the first frame for decoders such as Opus or Vorbis (and AAC once bug 1321249 is done)

The Opus decoder has a PreSkip value; that is the number of frames that needs to be dropped from the first decoded sample.

This should only be done on the first sample in the stream, not the first sample ever decoded. Which if you seek before decoding is what will happen.

if not, we need to modify all demuxers to add a flag to indicate that a packet is the first in the stream.
Flags: needinfo?(jwwang)
So what's the path forward? Should I back out this bug  or you will modify the demuxer to add the flag? Will this issue happen to MSE as well (which allows seeking even before decoding the 1st sample) or it doesn't support Opus and Vorbis at all?
Flags: needinfo?(jwwang)
Flags: needinfo?(jyavenard)
Flags: needinfo?(jyavenard)
Blocks: 1324357
You need to log in before you can comment on or make changes to this bug.