1252762 - Decode at most one audio/video sample before finishing seeking

Status: RESOLVED FIXED

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

Description

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

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.

Comment 1

[Jean-Yves Avenard [:jya]]

How is that different to what it does now?

Comment 2

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

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.

Comment 3

[Jean-Yves Avenard [:jya]]

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?

Comment 4

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

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();
 }

Comment 5

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

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=4566366ea618

Comment 6

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

Attachment: MozReview Request: Bug 1252762 - Decode at most one audio/video sample before finishing seeking. r=cpearce.

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/

Comment 7

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

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?

Comment 8

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

Thanks! That would be bug 1252766.

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/291df6e8e8e0

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/291df6e8e8e0