1047188 - MP4Reader still doesn't drain properly

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

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

I noticed when playing http://people.mozilla.org/~cpearce/video/h264_baseline_lvl3.mp4 that the MP4Reader gets stuck (on Windows at least) inside the loop in MP4Reader::Decode(), where it decides to call Drain(), sets data.mDrainComplete=false, calls Drain(), the decoder calls DrainComplete() (which resets mDrainComplete), but then we reenter the loop in Decode() again, and decide to Drain again and reset DrainComplete, and call Drain() again, repeat ad infinitum.

I need jya's patch from bug 1046549 applied to be able to play that file.

Comment 1

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

Attachment: Patch

* Don't call Drain() more than once.
* Add an EOS flag so that we can distinguish between "waiting for drain to complete", and "decoding normally and mDrainComplete is false".
* Add ResetDecode() which flushes decoders, and resets state.
* Don't call Flush and ResetDecode() in Seek(), since MediaDecoderStateMachine calls ResetDecode() for us.

Comment 2

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Comment on attachment 8465936 [details] [diff] [review]
Patch

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

::: content/media/fmp4/MP4Reader.cpp
@@ -669,5 @@
>      return NS_ERROR_FAILURE;
>    }
> -  Flush(kVideo);
> -  Flush(kAudio);
> -  ResetDecode();

Do we want to keep the call to ResetDecode() ?

Comment 3

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

(In reply to Anthony Jones (:kentuckyfriedtakahe, :k17e) from comment #2)
> Comment on attachment 8465936 [details] [diff] [review]
> Patch

> Do we want to keep the call to ResetDecode() ?


We don't need to; the caller of Seek() deliberately calls ResetDecode() before calling Seek(). This is requried by the caller, because async Readers must cancel async operations in their ResetDecode(). So the ResetDecode() call isn't going to go away.

Comment 4

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/18ad94c69dff

Comment 5

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/18ad94c69dff

Comment 6

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/18ad94c69dff