cajbir (:cajbir) Assignee
	Description • 10 years ago

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.

	Matthew Gregan [:kinetik]
	Comment 2 • 10 years ago

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.

	cajbir (:cajbir) Assignee
	Comment 3 • 10 years ago

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

	cajbir (:cajbir) Assignee
	Comment 4 • 10 years ago

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.

	cajbir (:cajbir) Assignee
	Comment 5 • 10 years ago

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.

	cajbir (:cajbir) Assignee
	Comment 6 • 10 years ago

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

	Ryan VanderMeulen [:RyanVM]
	Comment 7 • 10 years ago

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