Separate MediaCodecLoop from MediaCodecAudioDecoder

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 d8f91dfda4cd091de88a9c0879f0ea012d83568f..7318ff5cd45c905018f6ad5be41a80cce47f0c97 100644
--- a/media/filters/android/media_codec_audio_decoder.cc
+++ b/media/filters/android/media_codec_audio_decoder.cc
@@ -22,48 +22,10 @@ namespace {
 // Android MediaCodec can only output 16bit PCM audio.
 const int kBytesPerOutputSample = 2;
 
-// Valid MediaCodec buffer indexes are zero or positive.
-const int kInvalidBufferIndex = -1;
-
-inline const base::TimeDelta DecodePollDelay() {
-  return base::TimeDelta::FromMilliseconds(10);
-}
-
-inline const base::TimeDelta NoWaitTimeout() {
-  return base::TimeDelta::FromMicroseconds(0);
-}
-
-inline const base::TimeDelta IdleTimerTimeout() {
-  return base::TimeDelta::FromSeconds(1);
-}
-
 inline int GetChannelCount(const AudioDecoderConfig& config) {
   return ChannelLayoutToChannelCount(config.channel_layout());
 }
 
-std::unique_ptr<MediaCodecBridge> CreateMediaCodec(
-    const AudioDecoderConfig& config,
-    jobject media_crypto) {
-  DVLOG(1) << __FUNCTION__ << ": config:" << config.AsHumanReadableString();
-
-  std::unique_ptr<AudioCodecBridge> audio_codec_bridge(
-      AudioCodecBridge::Create(config.codec()));
-  if (!audio_codec_bridge) {
-    DVLOG(0) << __FUNCTION__ << " failed: cannot create AudioCodecBridge";
-    return nullptr;
-  }
-
-  const bool do_play = false;  // Do not create AudioTrack object.
-
-  if (!audio_codec_bridge->ConfigureAndStart(config, do_play, media_crypto)) {
-    DVLOG(0) << __FUNCTION__ << " failed: cannot configure audio codec for "
-             << config.AsHumanReadableString();
-    return nullptr;
-  }
-
-  return std::move(audio_codec_bridge);
-}
-
 // Converts interleaved data into planar data and writes it to |planes|.
 // The planes are populated in the order of channels in the interleaved frame.
 // If |channel_count| is less than the number of available planes the extra
@@ -94,7 +56,6 @@ MediaCodecAudioDecoder::MediaCodecAudioDecoder(
     : task_runner_(task_runner),
       state_(STATE_UNINITIALIZED),
       channel_count_(0),
-      pending_input_buf_index_(kInvalidBufferIndex),
       media_drm_bridge_cdm_context_(nullptr),
       cdm_registration_id_(0),
       weak_factory_(this) {
@@ -104,7 +65,7 @@ MediaCodecAudioDecoder::MediaCodecAudioDecoder(
 MediaCodecAudioDecoder::~MediaCodecAudioDecoder() {
   DVLOG(1) << __FUNCTION__;
 
-  media_codec_.reset();
+  codec_loop_.reset();
 
   if (media_drm_bridge_cdm_context_) {
     DCHECK(cdm_registration_id_);
@@ -156,8 +117,8 @@ void MediaCodecAudioDecoder::Initialize(const AudioDecoderConfig& config,
     return;
   }
 
-  media_codec_ = CreateMediaCodec(config_, nullptr);
-  if (!media_codec_) {
+  CreateCodecLoop();
+  if (!codec_loop_) {
     bound_init_cb.Run(false);
     return;
   }
@@ -166,6 +127,32 @@ void MediaCodecAudioDecoder::Initialize(const AudioDecoderConfig& config,
   bound_init_cb.Run(true);
 }
 
+void MediaCodecAudioDecoder::CreateCodecLoop() {
+  DVLOG(1) << __FUNCTION__ << ": config:" << config_.AsHumanReadableString();
+
+  codec_loop_.reset();
+
+  std::unique_ptr<AudioCodecBridge> audio_codec_bridge(
+      AudioCodecBridge::Create(config_.codec()));
+  if (!audio_codec_bridge) {
+    DVLOG(0) << __FUNCTION__ << " failed: cannot create AudioCodecBridge";
+    return;
+  }
+
+  const bool do_play = false;  // Do not create AudioTrack object.

[Tima Vaisburd, 2016/06/01 00:52:40]

nit: Min in some other review suggested to get rid of this var and just pass
false.

[liberato (no reviews please), 2016/06/01 21:47:54]

Done.

+
+  jobject media_crypto_obj = media_crypto_ ? media_crypto_->obj() : nullptr;
+
+  if (!audio_codec_bridge->ConfigureAndStart(config_, do_play,
+                                             media_crypto_obj)) {
+    DVLOG(0) << __FUNCTION__ << " failed: cannot configure audio codec for "
+             << config_.AsHumanReadableString();
+    return;
+  }
+
+  codec_loop_.reset(new MediaCodecLoop(this, std::move(audio_codec_bridge)));
+}
+
 void MediaCodecAudioDecoder::Decode(const scoped_refptr<DecoderBuffer>& buffer,
                                     const DecodeCB& decode_cb) {
   DecodeCB bound_decode_cb = BindToCurrentLoop(decode_cb);
@@ -177,6 +164,7 @@ void MediaCodecAudioDecoder::Decode(const scoped_refptr<DecoderBuffer>& buffer,
     return;
   }
 
+  // Note that we transition to STATE_ERROR if |codec_loop_| does.
   if (state_ == STATE_ERROR) {
     // We get here if an error happens in DequeueOutput() or Reset().
     DVLOG(2) << __FUNCTION__ << " " << buffer->AsHumanReadableString()
@@ -186,9 +174,13 @@ void MediaCodecAudioDecoder::Decode(const scoped_refptr<DecoderBuffer>& buffer,
     return;
   }
 
+  DCHECK(codec_loop_);
+
   DVLOG(2) << __FUNCTION__ << " " << buffer->AsHumanReadableString();
 
   DCHECK_EQ(state_, STATE_READY) << " unexpected state " << AsString(state_);
+  DCHECK_EQ(codec_loop_->GetState(), MediaCodecLoop::STATE_READY)
+      << " unexpected codec state " << AsString(state_);
 
   // AudioDecoder requires that "Only one decode may be in flight at any given
   // time".
@@ -196,31 +188,23 @@ void MediaCodecAudioDecoder::Decode(const scoped_refptr<DecoderBuffer>& buffer,
 
   input_queue_.push_back(std::make_pair(buffer, bound_decode_cb));
 
-  DoIOTask();
+  codec_loop_->DoIOTask();
 }
 
 void MediaCodecAudioDecoder::Reset(const base::Closure& closure) {
   DVLOG(1) << __FUNCTION__;
 
-  io_timer_.Stop();
-
   ClearInputQueue(DecodeStatus::ABORTED);
 
   // Flush if we can, otherwise completely recreate and reconfigure the codec.
   // Prior to JellyBean-MR2, flush() had several bugs (b/8125974, b/8347958) so
   // we have to completely destroy and recreate the codec there.
-  bool success = false;
-  if (state_ != STATE_ERROR && state_ != STATE_DRAINED &&
-      base::android::BuildInfo::GetInstance()->sdk_int() >= 18) {
-    // media_codec_->Reset() calls MediaCodec.flush().
-    success = (media_codec_->Reset() == MEDIA_CODEC_OK);
-  }
+  bool success = codec_loop_->TryFlush();
 
+  // If the flush failed, then we have to re-create the codec.
   if (!success) {
-    media_codec_.reset();
-    jobject media_crypto_obj = media_crypto_ ? media_crypto_->obj() : nullptr;
-    media_codec_ = CreateMediaCodec(config_, media_crypto_obj);
-    success = !!media_codec_;
+    CreateCodecLoop();
+    success = !!codec_loop_;
   }
 
   // Reset AudioTimestampHelper.
@@ -263,6 +247,15 @@ void MediaCodecAudioDecoder::SetCdm(CdmContext* cdm_context,
                  weak_factory_.GetWeakPtr(), init_cb)));
 }
 
+void MediaCodecAudioDecoder::OnKeyAdded() {
+  DVLOG(1) << __FUNCTION__;
+
+  // We don't register |codec_loop_| directly with the DRM bridge, since it's
+  // subject to replacement.
+  if (codec_loop_)
+    codec_loop_->OnKeyAdded();
+}
+
 void MediaCodecAudioDecoder::OnMediaCryptoReady(
     const InitCB& init_cb,
     MediaDrmBridgeCdmContext::JavaObjectPtr media_crypto,
@@ -282,12 +275,11 @@ void MediaCodecAudioDecoder::OnMediaCryptoReady(
 
   // We assume this is a part of the initialization process, thus MediaCodec
   // is not created yet.
-  DCHECK(!media_codec_);
+  DCHECK(!codec_loop_);
 
   // After receiving |media_crypto_| we can configure MediaCodec.
-
-  media_codec_ = CreateMediaCodec(config_, media_crypto->obj());
-  if (!media_codec_) {
+  CreateCodecLoop();
+  if (!codec_loop_) {
     SetState(STATE_UNINITIALIZED);
     init_cb.Run(false);
     return;
@@ -299,198 +291,40 @@ void MediaCodecAudioDecoder::OnMediaCryptoReady(
   init_cb.Run(true);
 }
 
-void MediaCodecAudioDecoder::OnKeyAdded() {
-  DVLOG(1) << __FUNCTION__;
-
-  if (state_ == STATE_WAITING_FOR_KEY)
-    SetState(STATE_READY);
-
-  DoIOTask();
-}
-
-void MediaCodecAudioDecoder::DoIOTask() {
-  if (state_ == STATE_ERROR)
-    return;
-
-  const bool did_input = QueueInput();
-  const bool did_output = DequeueOutput();
-
-  ManageTimer(did_input || did_output);
-}
-
-bool MediaCodecAudioDecoder::QueueInput() {
-  DVLOG(2) << __FUNCTION__;
-
-  bool did_work = false;
-  while (QueueOneInputBuffer())
-    did_work = true;
-
-  return did_work;
-}
-
-bool MediaCodecAudioDecoder::QueueOneInputBuffer() {
-  DVLOG(2) << __FUNCTION__;
-
-  if (input_queue_.empty())
+bool MediaCodecAudioDecoder::IsAnyInputPending() {
+  if (state_ != STATE_READY)
     return false;
 
-  if (state_ == STATE_WAITING_FOR_KEY || state_ == STATE_DRAINING ||
-      state_ == STATE_DRAINED)
-    return false;
-
-  // DequeueInputBuffer() may set STATE_ERROR.
-  InputBufferInfo input_info = DequeueInputBuffer();
-
-  if (input_info.buf_index == kInvalidBufferIndex) {
-    if (state_ == STATE_ERROR)
-      ClearInputQueue(DecodeStatus::DECODE_ERROR);
-
-    return false;
-  }
-
-  // EnqueueInputBuffer() may set STATE_DRAINING, STATE_WAITING_FOR_KEY or
-  // STATE_ERROR.
-  bool did_work = EnqueueInputBuffer(input_info);
-
-  switch (state_) {
-    case STATE_READY: {
-      const DecodeCB& decode_cb = input_queue_.front().second;
-      decode_cb.Run(DecodeStatus::OK);
-      input_queue_.pop_front();
-    } break;
-
-    case STATE_DRAINING:
-      // After queueing EOS we need to flush decoder, i.e. receive this EOS at
-      // the output, and then call corresponding decoder_cb.
-      // Keep the EOS buffer in the input queue.
-      break;
-
-    case STATE_WAITING_FOR_KEY:
-      // Keep trying to enqueue the same input buffer.
-      // The buffer is owned by us (not the MediaCodec) and is filled with data.
-      break;
-
-    case STATE_ERROR:
-      ClearInputQueue(DecodeStatus::DECODE_ERROR);
-      break;
-
-    default:
-      NOTREACHED() << ": internal error, unexpected state " << AsString(state_);
-      SetState(STATE_ERROR);
-      ClearInputQueue(DecodeStatus::DECODE_ERROR);
-      did_work = false;
-      break;
-  }
-
-  return did_work;
+  return !input_queue_.empty();
 }
 
-MediaCodecAudioDecoder::InputBufferInfo
-MediaCodecAudioDecoder::DequeueInputBuffer() {
+MediaCodecLoop::InputBufferData MediaCodecAudioDecoder::ProvideInputData() {
   DVLOG(2) << __FUNCTION__;
 
-  // Do not dequeue a new input buffer if we failed with MEDIA_CODEC_NO_KEY.
-  // That status does not return the input buffer back to the pool of
-  // available input buffers. We have to reuse it in QueueSecureInputBuffer().
-  if (pending_input_buf_index_ != kInvalidBufferIndex) {
-    InputBufferInfo result(pending_input_buf_index_, true);
-    pending_input_buf_index_ = kInvalidBufferIndex;
-    return result;
-  }
-
-  int input_buf_index = kInvalidBufferIndex;
-
-  media::MediaCodecStatus status =
-      media_codec_->DequeueInputBuffer(NoWaitTimeout(), &input_buf_index);
-  switch (status) {
-    case media::MEDIA_CODEC_DEQUEUE_INPUT_AGAIN_LATER:
-      DVLOG(2) << __FUNCTION__ << ": MEDIA_CODEC_DEQUEUE_INPUT_AGAIN_LATER";
-      break;
-
-    case media::MEDIA_CODEC_ERROR:
-      DVLOG(1) << __FUNCTION__ << ": MEDIA_CODEC_ERROR from DequeInputBuffer";
-      SetState(STATE_ERROR);
-      break;
-
-    case media::MEDIA_CODEC_OK:
-      break;
-
-    default:
-      NOTREACHED() << "Unknown DequeueInputBuffer status " << status;
-      SetState(STATE_ERROR);
-      break;
-  }
-
-  return InputBufferInfo(input_buf_index, false);
-}
-
-bool MediaCodecAudioDecoder::EnqueueInputBuffer(
-    const InputBufferInfo& input_info) {
-  DVLOG(2) << __FUNCTION__ << ": index:" << input_info.buf_index;
-
-  DCHECK_NE(input_info.buf_index, kInvalidBufferIndex);
-
   scoped_refptr<DecoderBuffer> decoder_buffer = input_queue_.front().first;
 
+  MediaCodecLoop::InputBufferData input_data;
   if (decoder_buffer->end_of_stream()) {
-    media_codec_->QueueEOS(input_info.buf_index);
-
-    SetState(STATE_DRAINING);
-    return true;
-  }
-
-  media::MediaCodecStatus status = MEDIA_CODEC_OK;
-
-  const DecryptConfig* decrypt_config = decoder_buffer->decrypt_config();
-  if (decrypt_config && decrypt_config->is_encrypted()) {
-    // A pending buffer is already filled with data, no need to copy it again.
-    const uint8_t* memory =
-        input_info.is_pending ? nullptr : decoder_buffer->data();
-
-    DVLOG(2) << __FUNCTION__ << ": QueueSecureInputBuffer:"
-             << " index:" << input_info.buf_index
-             << " pts:" << decoder_buffer->timestamp()
-             << " size:" << decoder_buffer->data_size();
-
-    status = media_codec_->QueueSecureInputBuffer(
-        input_info.buf_index, memory, decoder_buffer->data_size(),
-        decrypt_config->key_id(), decrypt_config->iv(),
-        decrypt_config->subsamples(), decoder_buffer->timestamp());
-
+    input_data.is_eos = true;
+    // We do not set the completion callback here, nor do we remove the EOS
+    // from the queue.  Unlike normal decodes, the EOS will be signalled once
+    // it shows up in the decoder's output.
   } else {
-    DVLOG(2) << __FUNCTION__ << ": QueueInputBuffer:"
-             << " index:" << input_info.buf_index
-             << " pts:" << decoder_buffer->timestamp()
-             << " size:" << decoder_buffer->data_size();
-
-    status = media_codec_->QueueInputBuffer(
-        input_info.buf_index, decoder_buffer->data(),
-        decoder_buffer->data_size(), decoder_buffer->timestamp());
+    input_data.memory = static_cast<const uint8_t*>(decoder_buffer->data());
+    input_data.length = decoder_buffer->data_size();
+    const DecryptConfig* decrypt_config = decoder_buffer->decrypt_config();
+    if (decrypt_config && decrypt_config->is_encrypted()) {
+      input_data.is_encrypted = true;
+      input_data.key_id = decrypt_config->key_id();
+      input_data.iv = decrypt_config->iv();
+      input_data.subsamples = decrypt_config->subsamples();
+    }
+    input_data.presentation_time = decoder_buffer->timestamp();
+    input_data.completion_cb = input_queue_.front().second;
+    input_queue_.pop_front();
   }
 
-  bool did_work = false;
-  switch (status) {
-    case MEDIA_CODEC_ERROR:
-      DVLOG(0) << __FUNCTION__ << ": MEDIA_CODEC_ERROR from QueueInputBuffer";
-      SetState(STATE_ERROR);
-      break;
-
-    case MEDIA_CODEC_NO_KEY:
-      DVLOG(1) << "QueueSecureInputBuffer failed: MEDIA_CODEC_NO_KEY";
-      pending_input_buf_index_ = input_info.buf_index;
-      SetState(STATE_WAITING_FOR_KEY);
-      break;
-
-    case MEDIA_CODEC_OK:
-      did_work = true;
-      break;
-
-    default:
-      NOTREACHED() << "Unknown Queue(Secure)InputBuffer status " << status;
-      SetState(STATE_ERROR);
-      break;
-  }
-  return did_work;
+  return input_data;
 }
 
 void MediaCodecAudioDecoder::ClearInputQueue(DecodeStatus decode_status) {
@@ -502,123 +336,41 @@ void MediaCodecAudioDecoder::ClearInputQueue(DecodeStatus decode_status) {
   input_queue_.clear();
 }
 
-bool MediaCodecAudioDecoder::DequeueOutput() {
-  DVLOG(2) << __FUNCTION__;
-
-  MediaCodecStatus status;
-  OutputBufferInfo out;
-  bool did_work = false;
-  do {
-    status = media_codec_->DequeueOutputBuffer(NoWaitTimeout(), &out.buf_index,
-                                               &out.offset, &out.size, &out.pts,
-                                               &out.is_eos, &out.is_key_frame);
-
-    switch (status) {
-      case MEDIA_CODEC_OUTPUT_BUFFERS_CHANGED:
-        // Output buffers are replaced in MediaCodecBridge, nothing to do.
-        DVLOG(2) << __FUNCTION__ << " MEDIA_CODEC_OUTPUT_BUFFERS_CHANGED";
-        did_work = true;
-        break;
-
-      case MEDIA_CODEC_OUTPUT_FORMAT_CHANGED:
-        DVLOG(2) << __FUNCTION__ << " MEDIA_CODEC_OUTPUT_FORMAT_CHANGED";
-        OnOutputFormatChanged();
-        did_work = true;
-        break;
-
-      case MEDIA_CODEC_OK:
-        // We got the decoded frame.
-        if (out.is_eos)
-          OnDecodedEos(out);
-        else
-          OnDecodedFrame(out);
-
-        did_work = true;
-        break;
-
-      case MEDIA_CODEC_DEQUEUE_OUTPUT_AGAIN_LATER:
-        // Nothing to do.
-        DVLOG(2) << __FUNCTION__ << " MEDIA_CODEC_DEQUEUE_OUTPUT_AGAIN_LATER";
-        break;
-
-      case MEDIA_CODEC_ERROR:
-        DVLOG(0) << __FUNCTION__
-                 << ": MEDIA_CODEC_ERROR from DequeueOutputBuffer";
-
-        // Next Decode() will report the error to the pipeline.
-        SetState(STATE_ERROR);
-        break;
-
-      default:
-        NOTREACHED() << "Unknown DequeueOutputBuffer status " << status;
-        // Next Decode() will report the error to the pipeline.
-        SetState(STATE_ERROR);
-        break;
-    }
-  } while (status != MEDIA_CODEC_DEQUEUE_OUTPUT_AGAIN_LATER &&
-           status != MEDIA_CODEC_ERROR && !out.is_eos);
-
-  return did_work;
-}
-
-void MediaCodecAudioDecoder::ManageTimer(bool did_work) {
-  bool should_be_running = true;
-
-  base::TimeTicks now = base::TimeTicks::Now();
-  if (did_work || idle_time_begin_ == base::TimeTicks()) {
-    idle_time_begin_ = now;
-  } else {
-    // Make sure that we have done work recently enough, else stop the timer.
-    if (now - idle_time_begin_ > IdleTimerTimeout())
-      should_be_running = false;
-  }
-
-  if (should_be_running && !io_timer_.IsRunning()) {
-    io_timer_.Start(FROM_HERE, DecodePollDelay(), this,
-                    &MediaCodecAudioDecoder::DoIOTask);
-  } else if (!should_be_running && io_timer_.IsRunning()) {
-    io_timer_.Stop();
-  }
-}
-
 void MediaCodecAudioDecoder::SetState(State new_state) {
   DVLOG(1) << __FUNCTION__ << ": " << AsString(state_) << "->"
            << AsString(new_state);
   state_ = new_state;
 }
 
-void MediaCodecAudioDecoder::OnDecodedEos(const OutputBufferInfo& out) {
-  DVLOG(2) << __FUNCTION__ << " pts:" << out.pts;
-
-  DCHECK_NE(out.buf_index, kInvalidBufferIndex);
-  DCHECK(media_codec_);
-
-  DCHECK_EQ(state_, STATE_DRAINING);
+void MediaCodecAudioDecoder::OnStateChanged(MediaCodecLoop::State new_state) {
+  // If the codec transitions into the error state, then so should we.
+  if (new_state == MediaCodecLoop::STATE_ERROR)
+    SetState(STATE_ERROR);
+}
 
-  media_codec_->ReleaseOutputBuffer(out.buf_index, false);
+void MediaCodecAudioDecoder::OnDecodedEos(
+    const MediaCodecLoop::OutputBufferInfo& out) {
+  DVLOG(2) << __FUNCTION__ << " pts:" << out.pts;
 
   // Report the end of decoding for EOS DecoderBuffer.
   DCHECK(!input_queue_.empty());
   DCHECK(input_queue_.front().first->end_of_stream());
 
+  // Now that the EOS has reached the output, signal that it has been decoded.
   const DecodeCB& decode_cb = input_queue_.front().second;
   decode_cb.Run(DecodeStatus::OK);
-
-  // Remove the EOS buffer from the input queue.
   input_queue_.pop_front();
-
-  // Set state STATE_DRAINED after we have received EOS frame at the output.
-  // media_decoder_job.cc says: once output EOS has occurred, we should
-  // not be asked to decode again.
-  SetState(STATE_DRAINED);
 }
 
-void MediaCodecAudioDecoder::OnDecodedFrame(const OutputBufferInfo& out) {
+void MediaCodecAudioDecoder::OnDecodedFrame(
+    const MediaCodecLoop::OutputBufferInfo& out) {
   DVLOG(2) << __FUNCTION__ << " pts:" << out.pts;
 
   DCHECK_NE(out.size, 0U);
-  DCHECK_NE(out.buf_index, kInvalidBufferIndex);
-  DCHECK(media_codec_);
+  DCHECK_NE(out.buf_index, MediaCodecLoop::kInvalidBufferIndex);
+  DCHECK(codec_loop_);
+  MediaCodecBridge* media_codec = codec_loop_->GetCodec();

[Tima Vaisburd, 2016/06/01 00:52:40]

It seems we use MediaCodecBridge to only get access to the data.
In that case I would add a method in MediaCodecLoop that does just that,
e.g. GetOutputBufferAddress(const OutputBufferInfo&).

media_codec->CopyFromOutputBuffer() is just a combination of getting this
address and memcpy().

[liberato (no reviews please), 2016/06/01 21:47:54]

OnFormatChanged uses it as well.  please see my reply in the .h

+  DCHECK(media_codec);
 
   // For proper |frame_count| calculation we need to use the actual number
   // of channels which can be different from |config_| value.
@@ -639,7 +391,7 @@ void MediaCodecAudioDecoder::OnDecodedFrame(const OutputBufferInfo& out) {
     // Copy data into AudioBuffer.
     CHECK_LE(out.size, audio_buffer->data_size());
 
-    MediaCodecStatus status = media_codec_->CopyFromOutputBuffer(
+    MediaCodecStatus status = media_codec->CopyFromOutputBuffer(
         out.buf_index, out.offset, audio_buffer->channel_data()[0], out.size);
 
     // TODO(timav,watk): This CHECK maintains the behavior of this call before
@@ -652,7 +404,7 @@ void MediaCodecAudioDecoder::OnDecodedFrame(const OutputBufferInfo& out) {
 
     const uint8_t* interleaved_data = nullptr;
     size_t interleaved_capacity = 0;
-    MediaCodecStatus status = media_codec_->GetOutputBufferAddress(
+    MediaCodecStatus status = media_codec->GetOutputBufferAddress(
         out.buf_index, out.offset, &interleaved_data, &interleaved_capacity);
 
     // TODO(timav): Handle wrong status properly, http://crbug.com/585978.

[Tima Vaisburd, 2016/06/01 00:52:40]

nit: Now we have a chance to implement it, maybe return bool instead of void ?

[liberato (no reviews please), 2016/06/01 21:47:54]

done.  MCL transitions to STATE_ERROR if OnDecodedFrame fails.

@@ -666,7 +418,7 @@ void MediaCodecAudioDecoder::OnDecodedFrame(const OutputBufferInfo& out) {
   }
 
   // Release MediaCodec output buffer.
-  media_codec_->ReleaseOutputBuffer(out.buf_index, false);
+  media_codec->ReleaseOutputBuffer(out.buf_index, false);
 
   // Calculate and set buffer timestamp.
 
@@ -684,16 +436,19 @@ void MediaCodecAudioDecoder::OnDecodedFrame(const OutputBufferInfo& out) {
   output_cb_.Run(audio_buffer);
 }
 
-void MediaCodecAudioDecoder::OnOutputFormatChanged() {
+bool MediaCodecAudioDecoder::OnOutputFormatChanged() {
   DVLOG(2) << __FUNCTION__;
+  MediaCodecBridge* media_codec = codec_loop_->GetCodec();
+
+  // Note that if we return false to transition |codec_loop_| to the error
+  // state, then we'll also transition to the error state when it notifies us.
 
   int new_sampling_rate = 0;
   MediaCodecStatus status =
-      media_codec_->GetOutputSamplingRate(&new_sampling_rate);
+      media_codec->GetOutputSamplingRate(&new_sampling_rate);
   if (status != MEDIA_CODEC_OK) {
     DLOG(ERROR) << "GetOutputSamplingRate failed.";
-    SetState(STATE_ERROR);
-    return;
+    return false;
   }
   if (new_sampling_rate != config_.samples_per_second()) {
     // We do not support the change of sampling rate on the fly
@@ -701,15 +456,13 @@ void MediaCodecAudioDecoder::OnOutputFormatChanged() {
                 << GetDisplayName() << " (detected change "
                 << config_.samples_per_second() << "->" << new_sampling_rate
                 << ")";
-    SetState(STATE_ERROR);
-    return;
+    return false;
   }
 
-  status = media_codec_->GetOutputChannelCount(&channel_count_);
+  status = media_codec->GetOutputChannelCount(&channel_count_);
   if (status != MEDIA_CODEC_OK) {
     DLOG(ERROR) << "GetOutputChannelCount failed.";
-    SetState(STATE_ERROR);
-    return;
+    return false;
   }
 
   const int config_channel_count = GetChannelCount(config_);
@@ -719,9 +472,10 @@ void MediaCodecAudioDecoder::OnOutputFormatChanged() {
   if (channel_count_ > config_channel_count) {
     DLOG(ERROR) << "Actual channel count " << channel_count_
                 << " is greater than configured " << config_channel_count;
-    SetState(STATE_ERROR);
-    return;
+    return false;
   }
+
+  return true;
 }
 
 #undef RETURN_STRING
@@ -735,9 +489,6 @@ const char* MediaCodecAudioDecoder::AsString(State state) {
     RETURN_STRING(STATE_UNINITIALIZED);
     RETURN_STRING(STATE_WAITING_FOR_MEDIA_CRYPTO);
     RETURN_STRING(STATE_READY);
-    RETURN_STRING(STATE_WAITING_FOR_KEY);
-    RETURN_STRING(STATE_DRAINING);
-    RETURN_STRING(STATE_DRAINED);
     RETURN_STRING(STATE_ERROR);
   }
   NOTREACHED() << "Unknown state " << state;