Closed Bug 1252762 Opened 6 years ago Closed 6 years ago

Decode at most one audio/video sample before finishing seeking

Categories

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

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla47
Tracking Status
firefox47 --- fixed

People

(Reporter: jwwang, Assigned: jwwang)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

This will slightly optimize our seeking process of MDSM. We should only decode next audio/video samples when Is{Audio,Video}SeekComplete returns false during seeking. This will allow us to finish seeking as soon as possible without decoding extra audio/video samples.
Assignee: nobody → jwwang
How is that different to what it does now?
With current code, we might end up having 2 or more audio/video samples in the queues before finishing seeking. The optimization is avoiding to spend time in decoding extra samples so we can finish seeking as soon as possible.
Blocks: 1252766
Decoding stopped for each of either audio or video is completed in CheckIfSeekComplete() 

EnsureVideoDecodeTaskQueued isn't called for either audio or video if the seek for that track as completed.

You only decode the frames you have to (as with video seeking completed on the keyframe before the seek target)

i don't see how more frames than necessary are decoded when seeking. where are the extra frames requested by the MDSM?
https://dxr.mozilla.org/mozilla-central/source/dom/media/MediaDecoderStateMachine.cpp#704
DispatchDecodeTasksIfNeeded() is called again when we push an audio sample to the queue. Therefore it is possible to decode 2 or more audio samples before the 1st video sample to come.

I added some assertions to CheckIfSeekComplete() and they did fire in some test cases for seeking.

 if (audioSeekComplete && videoSeekComplete) {
+  MOZ_RELEASE_ASSERT(AudioQueue().GetSize() <= 1);
+  MOZ_RELEASE_ASSERT(VideoQueue().GetSize() <= 1);
   SeekCompleted();
 }
The idea is to make NeedToDecode{Audio,Video} return false when mState is DECODER_STATE_SEEKING
so DispatchDecodeTasksIfNeeded() basically does nothing during seeking. And we call
Ensure{Audio,Video}DecodeTaskQueued instead during seeking to request audio/video samples
only when audio/video seeking is not completed.

Review commit: https://reviewboard.mozilla.org/r/37809/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/37809/
Attachment #8726099 - Flags: review?(cpearce)
Blocks: 1253184
Comment on attachment 8726099 [details]
MozReview Request: Bug 1252762 - Decode at most one audio/video sample before finishing seeking. r=cpearce.

https://reviewboard.mozilla.org/r/37809/#review34593

::: dom/media/MediaDecoderStateMachine.cpp:490
(Diff revision 1)
> -             IsVideoDecoding(), mDecodeToSeekTarget, mMinimizePreroll,
> +             IsVideoDecoding(), mMinimizePreroll, HaveEnoughDecodedVideo());

It looks like you've made mDecodeToSeekTarget write-only. Can you remove it?
Attachment #8726099 - Flags: review?(cpearce) → review+
Thanks! That would be bug 1252766.
https://hg.mozilla.org/mozilla-central/rev/291df6e8e8e0
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
You need to log in before you can comment on or make changes to this bug.