media: Fix MediaCodecAudioDecoder reinitialization

media/filters/android/media_codec_audio_decoder.cc

Index: media/filters/android/media_codec_audio_decoder.cc
diff --git a/media/filters/android/media_codec_audio_decoder.cc b/media/filters/android/media_codec_audio_decoder.cc
index a38e57a3770d17ad947a4fed7eada3b0b9349f9b..9c8fb77fc128da1f18cb0597f5ec5502fdcb44dc 100644
--- a/media/filters/android/media_codec_audio_decoder.cc
+++ b/media/filters/android/media_codec_audio_decoder.cc
@@ -57,11 +57,20 @@ void MediaCodecAudioDecoder::Initialize(const AudioDecoderConfig& config,
                                         const InitCB& init_cb,
                                         const OutputCB& output_cb) {
   DVLOG(1) << __func__ << ": " << config.AsHumanReadableString();
-  DCHECK_EQ(state_, STATE_UNINITIALIZED);
+  DCHECK_NE(state_, STATE_WAITING_FOR_MEDIA_CRYPTO);

[Tima Vaisburd, 2017/01/19 21:43:57]

Shall we do something about STATE_READY?

[xhwang, 2017/01/19 22:36:06]

When it's STATE_READY, we'll just go ahead and recreate the MediaCodecLoop. ISTM
we don't need to do anything special in that case...

[Tima Vaisburd, 2017/01/19 23:22:38]

The explanation in CL was really helpful :-)
If I get it right, right now nothing prevents the second Initialize() to happen
without Reset(), although description says Reset() should come first. In that
case it seems we'd miss ClearInputQueue(). Is this something to worry about?

[Tima Vaisburd, 2017/01/19 23:22:38]

Sorry, is the man intention of this CL to allow initialization in STATE_READY?
Then never mind. The details in the description would be helpful though.

[Tima Vaisburd, 2017/01/19 23:25:41]

Silly, silly rietveld and silly, silly me! Please ignore this particular
comment.

[xhwang, 2017/01/20 18:49:03]

Sorry I got confused about the API as well. I read it again and it turns out we
don't necessarily call Reset() before reinitialization. The only cases we cannot
call it are during pending decode or pending reset.

When reinit happens, we should drop all internal buffers and reconfig the
decoder using the new config.
Good point! Since we should not reinitialize during pending decode, the input
queue should be empty. I added a DCHECK() to cover this.

Also, I didn't call codec_loop_->TryFlush() since CreateMediaCodecLoop() should
cover it. Is that correct?

[Tima Vaisburd, 2017/01/20 20:02:27]

Yes, as far as I understand CreateMediaCodecLoop() fully destroys and recreates
MediaCodec.
As well you can just call ClearInputQueue() instead of DCHECK(). Would it hurt
anything?

lgtm

[xhwang, 2017/01/20 20:07:49]

DCHECK() helps us enforce the API contract and helps reader understand the
assumptions. So I would like to keep it.

Added ClearInputQueue() just in case we have bugs in release build. Though we
don't usually do it :)

 
   InitCB bound_init_cb = BindToCurrentLoop(init_cb);
 
+  if (state_ == STATE_ERROR) {
+    DVLOG(1) << "Decoder is in error state.";
+    bound_init_cb.Run(false);
+    return;
+  }
+
   // We can support only the codecs that AudioCodecBridge can decode.
+  // TODO(xhwang): Get this list from AudioCodecBridge or just rely on
+  // AudioCodecBridge::ConfigureAndStart() to determine whether the codec is
+  // supported.
   const bool is_codec_supported = config.codec() == kCodecVorbis ||
                                   config.codec() == kCodecAAC ||
                                   config.codec() == kCodecOpus;
@@ -82,11 +91,11 @@ void MediaCodecAudioDecoder::Initialize(const AudioDecoderConfig& config,
 
   SetInitialConfiguration();
 
-  if (config_.is_encrypted()) {
+  if (config_.is_encrypted() && !media_crypto_) {
     // Postpone initialization after MediaCrypto is available.
     // SetCdm uses init_cb in a method that's already bound to the current loop.
-    SetCdm(cdm_context, init_cb);
     SetState(STATE_WAITING_FOR_MEDIA_CRYPTO);
+    SetCdm(cdm_context, init_cb);
     return;
   }