1062020 - Forward video/audio sample discontinuity flag in MSE when skipping samples

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[cajbir (:cajbir)]

In MediaSourceReader::On{Video,Audio}Decoded we skip samples that don't match a threshold. The first of these samples contains a flag, mDiscontinuity, that the media decoder state machine uses to determine if seeking has completed. This is dropped and the state machine does not correctly cleanup the seek.

Comment 1

[cajbir (:cajbir)]

Attachment: Forward discontinuity flag on so media decoder state machine can react to it

Comment 2

[Matthew Gregan [:kinetik]]

Comment on attachment 8483160 [details] [diff] [review]
Forward discontinuity flag on so media decoder state machine can react to it

Review of attachment 8483160 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/media/mediasource/MediaSourceReader.cpp
@@ +75,5 @@
>        return;
>      }
>      mDropAudioBeforeThreshold = false;
> +
> +    // While dropping the video samples we also dropped the first sample

"audio samples"

@@ +77,5 @@
>      mDropAudioBeforeThreshold = false;
> +
> +    // While dropping the video samples we also dropped the first sample
> +    // that has the discontinuous flag set. This flag is used by the media
> +    // stage machine to determine end of seeking. Set this flag before

"state machine"

@@ +79,5 @@
> +    // While dropping the video samples we also dropped the first sample
> +    // that has the discontinuous flag set. This flag is used by the media
> +    // stage machine to determine end of seeking. Set this flag before
> +    // passing back to the state machine callback.
> +    aSample->mDiscontinuity = true;

Can you please add a MOZ_ASSERT that verifies we saw and discarded a sample with mDiscontinuity == true?

@@ +131,5 @@
> +    // While dropping the video samples we also dropped the first sample
> +    // that has the discontinuous flag set. This flag is used by the media
> +    // stage machine to determine end of seeking. Set this flag before
> +    // passing back to the state machine callback.
> +    aSample->mDiscontinuity = true;

Last two comments for the audio chunk apply here too.

Comment 3

[cajbir (:cajbir)]

Attachment: Forward discontinuity flag on so media decoder state machine can react to it

Fix review comments. The adding of the assertion required enough changes that I'm re-requesting review.

Comment 4

[cajbir (:cajbir)]

Comment on attachment 8483279 [details] [diff] [review]
Forward discontinuity flag on so media decoder state machine can react to it

Patch hits assertion constantly with the new code in bug 1059058 and sporadically in the old code. Obsoleting for a different fix.

Comment 5

[cajbir (:cajbir)]

Attachment: Set video/audio sample discontinuity flag in MSE after seek

As discussed offline. Set flag that we are seeking and then set the discontinuity field of the sample on the first decoded frame after the seek that we want to pass on to the media decoder state machine.

Comment 6

[cajbir (:cajbir)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/30a34467eec1

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/30a34467eec1