1188233 - If decoder can't be recreated, MediaFormatReader will not notify parent decoder

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect)

Description

[Jean-Yves Avenard [:jya]]

When a decoder can't be recreated for whatever reason an error is dispatched.  MediaFormatReader when encountering an error attempts to drain the current decoder ; which doesn't exist.

MediaFormatReader then waits on DrainComplete to be called. Call that will never occur

Comment 1

[Jean-Yves Avenard [:jya]]

Attachment: Ensure drainComplete flag is set upon failure to create decoder.

Comment 2

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

Comment on attachment 8639675 [details] [diff] [review]
Ensure drainComplete flag is set upon failure to create decoder.

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

::: dom/media/MediaFormatReader.cpp
@@ +987,1 @@
>    decoder.mOutputRequested = true;

Maybe the "decoder.mOutputRequested = true;" line should be down right before the "decoder.mDecoder->Drain();" call, so we don't set it if we bail out here in the NotifyDrainComplete() path?

Comment 3

[Jean-Yves Avenard [:jya]]

yes, it's probably more elegant that way.
Having said that, both case reports a problem with the current decoder, and no output will be coming our way anyway.

Comment 4

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9c48bac3e0d3

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/277e0210d08f

Comment 6

[Jean-Yves Avenard [:jya]]

I backed it out, appears to cause timeout on all eme test.

will check that out

Comment 7

[Jean-Yves Avenard [:jya]]

Well, turned out that you do want to set mOutputRequested to true, otherwise NotifyDrainComplete() still thinks we're within the flush()

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/0582b7cd2052

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/0582b7cd2052