MediaCodecPlayer implementation (stage 3 - browser seek and surface change)

media/base/android/media_codec_decoder.cc

Index: media/base/android/media_codec_decoder.cc
diff --git a/media/base/android/media_codec_decoder.cc b/media/base/android/media_codec_decoder.cc
index dc51904ee953b070b33b21733a1091beaaea8d28..534a8414b8eaab6c205a383fbab14f53f231ae8b 100644
--- a/media/base/android/media_codec_decoder.cc
+++ b/media/base/android/media_codec_decoder.cc
@@ -40,6 +40,7 @@ MediaCodecDecoder::MediaCodecDecoder(
     const char* decoder_thread_name)
     : media_task_runner_(media_task_runner),
       decoder_thread_(decoder_thread_name),
+      needs_reconfigure_(false),
       external_request_data_cb_(external_request_data_cb),
       starvation_cb_(starvation_cb),
       stop_done_cb_(stop_done_cb),
@@ -82,8 +83,12 @@ void MediaCodecDecoder::ReleaseDecoderResources() {
 
   DVLOG(1) << class_name() << "::" << __FUNCTION__;
 
+  // Set [kInEmergencyStop| state to block already posted ProcessNextFrame().
+  SetState(kInEmergencyStop);
+
   decoder_thread_.Stop();  // synchronous
-  state_ = kStopped;
+
+  SetState(kStopped);
   media_codec_bridge_.reset();
 }
 
@@ -183,6 +188,19 @@ MediaCodecDecoder::ConfigStatus MediaCodecDecoder::Configure() {
     return CONFIG_FAILURE;
   }
 
+  if (needs_reconfigure_) {
+    DVLOG(1) << class_name() << "::" << __FUNCTION__
+             << ": needs reconfigure, deleting MediaCodec";
+    needs_reconfigure_ = false;
+    media_codec_bridge_.reset();
+
+    // No need to release these buffers since the MediaCodec is deleted, just
+    // remove their indexes from |delayed_buffers_|.
+
+    // Shall we move |delayed_buffers_| from VideoDecoder to Decoder class?

[wolenetz, 2015/07/29 22:37:02]

Seems there should be either a TODO or a CR comment instead of a question here.

[Tima Vaisburd, 2015/07/30 20:28:35]

Yes, but now I tend to think that having a method is actually cleaner in this
situation, so I will just delete the comment in the code, and the CR comment
already exists.

[wolenetz, 2015/07/30 21:10:39]

Acknowledged.

+    ClearDelayedBuffers(false);
+  }
+
   MediaCodecDecoder::ConfigStatus result;
   if (media_codec_bridge_) {
     DVLOG(1) << class_name() << "::" << __FUNCTION__
@@ -266,11 +284,15 @@ void MediaCodecDecoder::SyncStop() {
 
   // After this method returns, decoder thread will not be running.
 
+  // Set [kInEmergencyStop| state to block already posted ProcessNextFrame().
+  SetState(kInEmergencyStop);
+
   decoder_thread_.Stop();  // synchronous
-  state_ = kStopped;
+
+  SetState(kStopped);
 
   // Shall we move |delayed_buffers_| from VideoDecoder to Decoder class?

[wolenetz, 2015/07/29 22:37:02]

ditto

[Tima Vaisburd, 2015/07/30 20:28:35]

Same as above, removed the comment in the code as I came to like
ClearDelayerBuffers().

[wolenetz, 2015/07/30 21:10:39]

Acknowledged.

-  ReleaseDelayedBuffers();
+  ClearDelayedBuffers(true);  // release prior to clearing  |delayed_buffers_|.
 }
 
 void MediaCodecDecoder::RequestToStop() {
@@ -310,7 +332,8 @@ void MediaCodecDecoder::OnLastFrameRendered(bool completed) {
            << " completed:" << completed;
 
   decoder_thread_.Stop();  // synchronous
-  state_ = kStopped;
+
+  SetState(kStopped);
   completed_ = completed;
 
   media_task_runner_->PostTask(FROM_HERE, stop_done_cb_);
@@ -330,10 +353,10 @@ void MediaCodecDecoder::OnDemuxerDataAvailable(const DemuxerData& data) {
                                 : (aborted_data ? " skipped as aborted" : "");
 
   for (const auto& unit : data.access_units)
-    DVLOG(1) << class_name() << "::" << __FUNCTION__ << explain_if_skipped
+    DVLOG(2) << class_name() << "::" << __FUNCTION__ << explain_if_skipped
              << " au: " << unit;
   for (const auto& configs : data.demuxer_configs)
-    DVLOG(1) << class_name() << "::" << __FUNCTION__ << " configs: " << configs;
+    DVLOG(2) << class_name() << "::" << __FUNCTION__ << " configs: " << configs;
 #endif
 
   if (!is_incoming_data_invalid_ && !aborted_data)
@@ -344,7 +367,7 @@ void MediaCodecDecoder::OnDemuxerDataAvailable(const DemuxerData& data) {
 
   // Do not request data if we got kAborted. There is no point to request the
   // data after kAborted and before the OnDemuxerSeekDone.
-  if (state_ == kPrefetching && !aborted_data)
+  if (GetState() == kPrefetching && !aborted_data)
     PrefetchNextChunk();
 }
 
@@ -369,6 +392,14 @@ void MediaCodecDecoder::CheckLastFrame(bool eos_encountered,
 void MediaCodecDecoder::OnCodecError() {
   DCHECK(media_task_runner_->BelongsToCurrentThread());
 
+  // Ignore codec errors from the moment surface is changed till the
+  // |media_codec_bridge_| is deleted.
+  if (needs_reconfigure_) {
+    DVLOG(1) << class_name() << "::" << __FUNCTION__
+             << ": needs reconfigure, ignoring";
+    return;
+  }
+
   SetState(kError);
   error_cb_.Run();
 }
@@ -434,15 +465,8 @@ void MediaCodecDecoder::ProcessNextFrame() {
   if (!EnqueueInputBuffer())
     return;
 
-  bool eos_encountered = false;
-  if (!DepleteOutputBufferQueue(&eos_encountered))
-    return;
-
-  if (eos_encountered) {
-    DVLOG(1) << class_name() << "::" << __FUNCTION__
-             << " EOS dequeued, stopping frame processing";
+  if (!DepleteOutputBufferQueue())
     return;
-  }
 
   // We need a small delay if we want to stop this thread by
   // decoder_thread_.Stop() reliably.
@@ -566,7 +590,7 @@ bool MediaCodecDecoder::EnqueueInputBuffer() {
 }
 
 // Returns false if there was MediaCodec error.
-bool MediaCodecDecoder::DepleteOutputBufferQueue(bool* eos_encountered) {
+bool MediaCodecDecoder::DepleteOutputBufferQueue() {
   DCHECK(decoder_thread_.task_runner()->BelongsToCurrentThread());
 
   DVLOG(2) << class_name() << "::" << __FUNCTION__;
@@ -576,6 +600,7 @@ bool MediaCodecDecoder::DepleteOutputBufferQueue(bool* eos_encountered) {
   size_t size = 0;
   base::TimeDelta pts;
   MediaCodecStatus status;
+  bool eos_encountered = false;
 
   base::TimeDelta timeout =
       base::TimeDelta::FromMilliseconds(kOutputBufferTimeout);
@@ -585,7 +610,8 @@ bool MediaCodecDecoder::DepleteOutputBufferQueue(bool* eos_encountered) {
   // MEDIA_CODEC_OUTPUT_BUFFERS_CHANGED or MEDIA_CODEC_OUTPUT_FORMAT_CHANGED.
   do {
     status = media_codec_bridge_->DequeueOutputBuffer(
-        timeout, &buffer_index, &offset, &size, &pts, eos_encountered, nullptr);
+        timeout, &buffer_index, &offset, &size, &pts, &eos_encountered,
+        nullptr);
 
     // Reset the timeout to 0 for the subsequent DequeueOutputBuffer() calls
     // to quickly break the loop after we got all currently available buffers.
@@ -604,7 +630,7 @@ bool MediaCodecDecoder::DepleteOutputBufferQueue(bool* eos_encountered) {
 
       case MEDIA_CODEC_OK:
         // We got the decoded frame
-        Render(buffer_index, size, true, pts, *eos_encountered);
+        Render(buffer_index, size, true, pts, eos_encountered);
         break;
 
       case MEDIA_CODEC_DEQUEUE_OUTPUT_AGAIN_LATER:
@@ -623,9 +649,21 @@ bool MediaCodecDecoder::DepleteOutputBufferQueue(bool* eos_encountered) {
     }
 
   } while (status != MEDIA_CODEC_DEQUEUE_OUTPUT_AGAIN_LATER &&
-           status != MEDIA_CODEC_ERROR && !*eos_encountered);
+           status != MEDIA_CODEC_ERROR && !eos_encountered);
 
-  return status != MEDIA_CODEC_ERROR;
+  if (eos_encountered) {
+    DVLOG(1) << class_name() << "::" << __FUNCTION__
+             << " EOS dequeued, stopping frame processing";
+    return false;
+  }
+
+  if (status == MEDIA_CODEC_ERROR) {
+    DVLOG(1) << class_name() << "::" << __FUNCTION__
+             << " MediaCodec error, stopping frame processing";
+    return false;
+  }
+
+  return true;
 }
 
 MediaCodecDecoder::DecoderState MediaCodecDecoder::GetState() const {
@@ -634,7 +672,7 @@ MediaCodecDecoder::DecoderState MediaCodecDecoder::GetState() const {
 }
 
 void MediaCodecDecoder::SetState(DecoderState state) {
-  DVLOG(1) << class_name() << "::" << __FUNCTION__ << " " << state;
+  DVLOG(1) << class_name() << "::" << __FUNCTION__ << " " << AsString(state);
 
   base::AutoLock lock(state_lock_);
   state_ = state;
@@ -652,6 +690,7 @@ const char* MediaCodecDecoder::AsString(DecoderState state) {
     RETURN_STRING(kPrefetched);
     RETURN_STRING(kRunning);
     RETURN_STRING(kStopping);
+    RETURN_STRING(kInEmergencyStop);
     RETURN_STRING(kError);
     default:
       return "Unknown DecoderState";