Allow seeking before finishing decoding first frames

RESOLVED FIXED in Firefox 53

Status

()

Core
Audio/Video: Playback
P3
normal
RESOLVED FIXED
11 months ago
10 months ago

People

(Reporter: jwwang, Assigned: jwwang)

Tracking

unspecified
mozilla53
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox53 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(4 attachments)

(Assignee)

Description

11 months ago
Thanks to bug 1309516, we have start time after decoding the metadata. So MDSM is able to seek even before decoding first frames.
(Assignee)

Updated

11 months ago
Assignee: nobody → jwwang
Blocks: 1295892
Depends on: 1309516
Priority: -- → P3
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 3

11 months ago
mozreview-review
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 4

11 months ago
mozreview-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+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
(Assignee)

Updated

11 months ago
Component: Audio/Video: Recording → Audio/Video: Playback

Comment 9

11 months ago
mozreview-review
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 10

11 months ago
mozreview-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+
(Assignee)

Comment 11

11 months ago
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).
(Assignee)

Updated

11 months ago
Depends on: 1320258
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 16

11 months ago
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

Comment 17

11 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/8aaa89d73c3f
https://hg.mozilla.org/mozilla-central/rev/03144421b4a6
https://hg.mozilla.org/mozilla-central/rev/3dc995228de4
https://hg.mozilla.org/mozilla-central/rev/e355836470f9
Status: NEW → RESOLVED
Last Resolved: 11 months ago
status-firefox53: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
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)
(Assignee)

Comment 19

11 months ago
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)
(Assignee)

Updated

11 months ago
Flags: needinfo?(jyavenard)

Updated

11 months ago
Flags: needinfo?(jyavenard)

Updated

10 months ago
Blocks: 1324357
You need to log in before you can comment on or make changes to this bug.