1030527 - Seeking using Media Source Extensions fails with corrupted video message since async decoder landing

Status: VERIFIED FIXED

(Core :: Audio/Video, defect)

Description

[cajbir (:cajbir)]

The landing of bug 979104 seems to have broken seeking in MSE. Steps to reproduce:

1) Go to http://cd.pn/mse/ytdemo/dash-player.html?url=http://cd.pn/mse/ytdemo/feelings2.mpd.
2) When playback starts, seek somewhere forwards.

Expected results:

3) Video seeks

Acutal results:

4) Video displays "Video cannot be played because the file is corrupt".

Backing out the bug 979104 landing fixes the issue.

Comment 1

[cajbir (:cajbir)]

When a seek is requested the existing SubBufferDecoder is discarded and a new one created for decoding the new part of the stream. The new decoder starts reading data. At some point the old decoder comes back to life and the data from that is read which corrupts the stream.

When discarded the old decoder is still active in the list of of decoders held by the MediaSourceReader. It appears to re-activate when the new decoder has no time range for the seeked-to time. This happens at the beginning of the seek because no data has been read yet.

A quick change to disable the old discarded decoder results in the seek completing successfully. I need to:

1) Remove or deactivate the old decoder in the list of decoders when discarded.
2) Write a test to make sure seeking doesn't break in the future.

Comment 2

[cajbir (:cajbir)]

Attachment: Fix

When a decoder is discarded it is marked as discarded. Discarded decoders are ignored when switching decoders. When testing if we can switch decoders we ignore the requirement to fit in the time range if we are seeking as we haven't got the relevant time data for that yet. This fixes the seeking issue identified by this bug. Try push: https://tbpl.mozilla.org/?tree=Try&rev=c43cb15aa94e.

Comment 3

[Matthew Gregan [:kinetik]]

Comment on attachment 8453511 [details] [diff] [review]
Fix

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

r+ with comment

::: content/media/mediasource/MediaSourceDecoder.cpp
@@ +197,4 @@
>          continue;
>        }
> +
> +      if (!decoder->IsDiscarded() && (aState == SWITCHSTATE_SEEKING || mTimeThreshold >= mDecoders[i]->GetMediaStartTime())) {

Move the IsDiscarded test up to the HasVideo test, since they're both used for skipping a decoder that doesn't meet simple rejection criteria.

Comment 4

[cajbir (:cajbir)]

Pushed with review comment addressed: 

https://hg.mozilla.org/integration/mozilla-inbound/rev/3988e2a46e9f

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/3988e2a46e9f

Comment 6

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

Setting for contributors to verify. Before trying steps from comment 0, make sure "media.mediasource.enabled" preference is set to true in about:config.

Comment 7

[vasilicamihasca]

I was able to reproduce this bug on Firefox 33.0a1 (2014-06-25) using Windows 7 x64 and Ubuntu 12.04 x64.

Verified fixed on Windows 7 x64 and Ubuntu 12.04 x64 using Firefox 36.0a1 (2014-10-14).

This fix can be marked as verified.

[bugday-20141015]