Closed Bug 1062020 Opened 5 years ago Closed 5 years ago

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

Categories

(Core :: Audio/Video, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla35

People

(Reporter: cajbir, Assigned: cajbir)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 2 obsolete files)

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.
Blocks: MSE, 1056440
Summary: Forward video/audio sample discontinuity flag in mSE when skipping samples → Forward video/audio sample discontinuity flag in MSE when skipping samples
Assignee: nobody → cajbir.bugzilla
Status: NEW → ASSIGNED
Attachment #8483160 - Flags: review?(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.
Attachment #8483160 - Flags: review?(kinetik) → review+
Fix review comments. The adding of the assertion required enough changes that I'm re-requesting review.
Attachment #8483160 - Attachment is obsolete: true
Attachment #8483279 - Flags: review?(kinetik)
Attachment #8483279 - Flags: review?(kinetik) → review+
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.
Attachment #8483279 - Attachment is obsolete: true
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.
Attachment #8483305 - Flags: review?(kinetik)
Attachment #8483305 - Flags: review?(kinetik) → review+
https://hg.mozilla.org/mozilla-central/rev/30a34467eec1
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Blocks: 1041374
Blocks: 1040552
You need to log in before you can comment on or make changes to this bug.