MediaCodecPlayer implementation (stage 5 - reconfiguration)

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 e6300b2f6a6927b396c118d542d52f69e4543c0a..695f0f0df3a7384be1cdce8e74d70c2282001f42 100644
--- a/media/base/android/media_codec_decoder.cc
+++ b/media/base/android/media_codec_decoder.cc
@@ -35,18 +35,22 @@ MediaCodecDecoder::MediaCodecDecoder(
     const scoped_refptr<base::SingleThreadTaskRunner>& media_task_runner,
     const base::Closure& external_request_data_cb,
     const base::Closure& starvation_cb,
+    const base::Closure& decoder_drained_cb,
     const base::Closure& stop_done_cb,
     const base::Closure& error_cb,
     const char* decoder_thread_name)
     : media_task_runner_(media_task_runner),
       decoder_thread_(decoder_thread_name),
       needs_reconfigure_(false),
+      drain_decoder_(false),
+      always_reconfigure_for_tests_(false),
       external_request_data_cb_(external_request_data_cb),
       starvation_cb_(starvation_cb),
+      decoder_drained_cb_(decoder_drained_cb),
       stop_done_cb_(stop_done_cb),
       error_cb_(error_cb),
       state_(kStopped),
-      needs_preroll_(true),
+      preroll_mode_(kPrerollTillPTS),
       eos_enqueued_(false),
       completed_(false),
       last_frame_posted_(false),
@@ -88,6 +92,7 @@ void MediaCodecDecoder::Flush() {
 
   eos_enqueued_ = false;
   completed_ = false;
+  drain_decoder_ = false;
   au_queue_.Flush();
 
 #ifndef NDEBUG
@@ -99,7 +104,7 @@ void MediaCodecDecoder::Flush() {
   verify_next_frame_is_key_ = true;
 #endif
 
-  needs_preroll_ = true;
+  preroll_mode_ = kPrerollTillPTS;
 
   if (media_codec_bridge_) {
     // MediaCodecBridge::Reset() performs MediaCodecBridge.flush()
@@ -118,7 +123,7 @@ void MediaCodecDecoder::ReleaseMediaCodec() {
   DVLOG(1) << class_name() << "::" << __FUNCTION__;
 
   media_codec_bridge_.reset();
-  needs_preroll_ = true;
+  preroll_mode_ = kPrerollTillPTS;
 }
 
 bool MediaCodecDecoder::IsPrefetchingOrPlaying() const {
@@ -158,7 +163,14 @@ bool MediaCodecDecoder::IsCompleted() const {
 bool MediaCodecDecoder::NotCompletedAndNeedsPreroll() const {
   DCHECK(media_task_runner_->BelongsToCurrentThread());
 
-  return HasStream() && needs_preroll_ && !completed_;
+  return HasStream() && preroll_mode_ != kNoPreroll && !completed_;
+}
+
+void MediaCodecDecoder::SetPrerollTillNextFrame() {
+  DCHECK(media_task_runner_->BelongsToCurrentThread());
+  DVLOG(1) << class_name() << "::" << __FUNCTION__;
+
+  preroll_mode_ = kPrerollTillNextFrame;
 }
 
 base::android::ScopedJavaLocalRef<jobject> MediaCodecDecoder::GetMediaCrypto() {
@@ -201,24 +213,31 @@ MediaCodecDecoder::ConfigStatus MediaCodecDecoder::Configure() {
     ReleaseMediaCodec();
   }
 
-  MediaCodecDecoder::ConfigStatus result;
   if (media_codec_bridge_) {
     DVLOG(1) << class_name() << "::" << __FUNCTION__
              << ": reconfiguration is not required, ignoring";
-    result = kConfigOk;
-  } else {
-    result = ConfigureInternal();
+    return kConfigOk;
+  }
+
+  // Read all |kConfigChanged| units preceding the data one.
+  AccessUnitQueue::Info au_info = au_queue_.GetInfo();
+  while (au_info.configs) {
+    SetDemuxerConfigs(*au_info.configs);
+    au_queue_.Advance();
+    au_info = au_queue_.GetInfo();
+  }
+
+  MediaCodecDecoder::ConfigStatus result = ConfigureInternal();
 
 #ifndef NDEBUG
-    // We check and reset |verify_next_frame_is_key_| on Decoder thread.
-    // This DCHECK ensures we won't need to lock this variable.
-    DCHECK(!decoder_thread_.IsRunning());
+  // We check and reset |verify_next_frame_is_key_| on Decoder thread.
+  // This DCHECK ensures we won't need to lock this variable.
+  DCHECK(!decoder_thread_.IsRunning());
 
-    // For video the first frame after reconfiguration must be key frame.
-    if (result == kConfigOk)
-      verify_next_frame_is_key_ = true;
+  // For video the first frame after reconfiguration must be key frame.
+  if (result == kConfigOk)
+    verify_next_frame_is_key_ = true;
 #endif
-  }
 
   return result;
 }
@@ -244,13 +263,15 @@ bool MediaCodecDecoder::Preroll(base::TimeDelta preroll_timestamp,
   }
 
   DCHECK(!decoder_thread_.IsRunning());
-  DCHECK(needs_preroll_);
+  DCHECK(preroll_mode_ != kNoPreroll);
 
   preroll_done_cb_ = preroll_done_cb;
 
   // We only synchronize video stream.
   DissociatePTSFromTime();  // associaton will happen after preroll is done.
-  preroll_timestamp_ = preroll_timestamp;
+
+  preroll_timestamp_ = (preroll_mode_ == kPrerollTillPTS) ? preroll_timestamp

[liberato (no reviews please), 2015/08/27 20:42:26]

i don't understand this part.

in TillNextFrame, it looks like preroll_timestamp will be in the past (from the
first preroll).  won't that end up rendering frames during the preroll?

i'm pretty sure that i've misunderstood something.

[Tima Vaisburd, 2015/08/27 22:04:18]

I understood your words as "in the case TillNextFrame is set, the preroll
timestamp will be in the past anyway". Please correct me if I'm wrong :-)

But if I understood you correctly, then no, there is no guarantee that it will
be in the past.

Consider the case with video reconfiguration while audio goes ahead.

--------+-------------+----------------------------> video
     drained      next video pts

                          last audio pts
---------------+-----------------+----------+------> audio
             stop request              preroll ts


(Not to scale!)

Last audio means before we resume, of course. In the absence of the second
preroll mode we would preroll to preroll ts, and the start.
 
The time it takes the request to propagate from video to audio thread depended
on the device and Android version. On one of my two devices (Nexus 5 with
Android 5) it was long enough to have preroll ts past the next video pts. During
the regular preroll we would skip all frames before preroll pts, thus skipping
this next video frame.

Please ask for more details if it does not make sense.

[Tima Vaisburd, 2015/08/27 22:13:11]

Actually, now I'm not sure I got your point. In the TillNextFrame mode the
"preroll" is supposed to report "I'm done" at the moment it gets the output
buffer. We never render frames during the preroll, strictly speaking (if you do
not count the intentional cheating in video with kRenderAfterPreroll).

[liberato (no reviews please), 2015/08/28 15:21:42]

thanks for the explanation.  the root cause of my comment was that i misread
DepleteOutputBuffers().  in the TilNextFrame case in Preroll(), preroll_pts is
TimeDelta() (time 0, "in the past"). this will cause the next call to
DepleteOuputBuffers() to choose render_mode==kRenderAfterPreroll.  i mis-read
that last part as choosing kRender, which made no sense.

that diagram was also quite helpful, thanks for providing it.

+                                                          : base::TimeDelta();
 
   last_frame_posted_ = false;
 
@@ -356,7 +377,7 @@ void MediaCodecDecoder::RequestToStop() {
     case kStopped:
     case kPrefetching:
     case kPrefetched:
-      // There is nothing to wait for, we can sent nofigication right away.
+      // There is nothing to wait for, we can sent notification right away.
       DCHECK(!decoder_thread_.IsRunning());
       SetState(kStopped);
       media_task_runner_->PostTask(FROM_HERE, stop_done_cb_);
@@ -367,20 +388,29 @@ void MediaCodecDecoder::RequestToStop() {
   }
 }
 
-void MediaCodecDecoder::OnLastFrameRendered(bool completed) {
+void MediaCodecDecoder::OnLastFrameRendered(bool eos_encountered) {
   DCHECK(media_task_runner_->BelongsToCurrentThread());
 
-  DVLOG(0) << class_name() << "::" << __FUNCTION__
-           << " completed:" << completed;
+  DVLOG(1) << class_name() << "::" << __FUNCTION__
+           << " eos_encountered:" << eos_encountered;
 
   decoder_thread_.Stop();  // synchronous
 
   SetState(kStopped);
-  completed_ = completed;
+  completed_ = (eos_encountered && !drain_decoder_);
 
-  if (completed_ && !preroll_done_cb_.is_null())
+  if (completed_ && !preroll_done_cb_.is_null()) {
     media_task_runner_->PostTask(FROM_HERE,
                                  base::ResetAndReturn(&preroll_done_cb_));
+  }
+
+  if (eos_encountered && drain_decoder_) {
+    ReleaseMediaCodec();
+    drain_decoder_ = false;
+    eos_enqueued_ = false;
+    preroll_mode_ = kPrerollTillNextFrame;
+    media_task_runner_->PostTask(FROM_HERE, decoder_drained_cb_);
+  }
 
   media_task_runner_->PostTask(FROM_HERE, stop_done_cb_);
 }
@@ -390,7 +420,7 @@ void MediaCodecDecoder::OnPrerollDone() {
 
   DVLOG(1) << class_name() << "::" << __FUNCTION__;
 
-  needs_preroll_ = false;
+  preroll_mode_ = kNoPreroll;
 
   // The state might be kStopping.
   if (GetState() == kPrerolling) {
@@ -437,6 +467,16 @@ bool MediaCodecDecoder::IsPrerollingForTests() const {
   return GetState() == kPrerolling;
 }
 
+void MediaCodecDecoder::SetAlwaysReconfigureForTests() {
+  // UI task runner.
+  always_reconfigure_for_tests_ = true;
+}
+
+void MediaCodecDecoder::SetCodecCreatedCallbackForTests(base::Closure cb) {
+  // UI task runner.
+  codec_created_for_tests_cb_ = cb;
+}
+
 int MediaCodecDecoder::NumDelayedRenderTasks() const {
   return 0;
 }
@@ -501,7 +541,8 @@ void MediaCodecDecoder::PrefetchNextChunk() {
 
   AccessUnitQueue::Info au_info = au_queue_.GetInfo();
 
-  if (eos_enqueued_ || au_info.length >= kPrefetchLimit || au_info.has_eos) {
+  if (eos_enqueued_ || au_info.data_length >= kPrefetchLimit ||
+      au_info.has_eos) {
     // We are done prefetching
     SetState(kPrefetched);
     DVLOG(1) << class_name() << "::" << __FUNCTION__ << " posting PrefetchDone";
@@ -582,33 +623,58 @@ bool MediaCodecDecoder::EnqueueInputBuffer() {
 
   // Get the next frame from the queue and the queue info
 
-  AccessUnitQueue::Info au_info = au_queue_.GetInfo();
+  // Retrieve access units from the |au_queue_| in a loop until we either get
+  // a non-config front unit or until the new config is incompatible and we need
+  // to drain decoder.
 
-  // Request the data from Demuxer
-  if (au_info.length <= kPlaybackLowLimit && !au_info.has_eos)
-    media_task_runner_->PostTask(FROM_HERE, request_data_cb_);
+  AccessUnitQueue::Info au_info;

[liberato (no reviews please), 2015/08/28 15:21:42]

this block can probably be split out into a separate function, since this one's
getting somewhat big.

if (AdvanceUntilNonConfigAccessUnit())
  return true;  // starvation
AccessUnitQueue::Info au_info = au_queue_.GetInfo();
// We are ready to enqueue the front unit.

[Tima Vaisburd, 2015/08/28 21:26:23]

Made a method AdvanceAccessUnitQueue().

After thinking on your words about stravation I though it would be better to
change the logic and report starvation if we cannot advance to the data unit
event if we need to drain decoder. Although for the drain case we could have
waited I thought that it's better to make the logic uniform.

You might want to take another look.

+  do {
+    // |drain_decoder_| can be set here upon the first time we enter the loop if
+    // we could not dequeue the input buffer for it right away.
+    if (drain_decoder_)
+      break;
 
-  // Get the next frame from the queue
+    au_info = au_queue_.GetInfo();
 
-  if (!au_info.length) {
-    // Report starvation and return, Start() will be called again later.
-    DVLOG(1) << class_name() << "::" << __FUNCTION__ << ": starvation detected";
-    media_task_runner_->PostTask(FROM_HERE, starvation_cb_);
-    return true;
-  }
+    // Request the data from Demuxer
+    if (au_info.data_length <= kPlaybackLowLimit && !au_info.has_eos)
+      media_task_runner_->PostTask(FROM_HERE, request_data_cb_);
 
-  if (au_info.configs) {
-    DVLOG(1) << class_name() << "::" << __FUNCTION__
-             << ": received new configs, not implemented";
-    // post an error for now?
-    media_task_runner_->PostTask(FROM_HERE, internal_error_cb_);
-    return false;
-  }
+    // Get the next frame from the queue
+
+    if (!au_info.length) {
+      // Report starvation and return, Start() will be called again later.
+      DVLOG(1) << class_name() << "::" << __FUNCTION__
+               << ": starvation detected";
+      media_task_runner_->PostTask(FROM_HERE, starvation_cb_);
+      return true;
+    }
+
+    if (au_info.configs) {
+      DVLOG(1) << class_name() << "::" << __FUNCTION__ << ": received configs "
+               << (*au_info.configs);
+
+      // Compare the new and current configs.
+      if (IsCodecReconfigureNeeded(*au_info.configs)) {
+        DVLOG(1) << class_name() << "::" << __FUNCTION__
+                 << ": reconfiguration required";
+        drain_decoder_ = true;
+      }
+
+      // Replace the current configs.
+      SetDemuxerConfigs(*au_info.configs);
+
+      // Remove the configs from the queue.
+      au_queue_.Advance();
+    }
+  } while (au_info.configs);
 
   // We are ready to enqueue the front unit.
 
+  DCHECK(drain_decoder_ || au_info.front_unit);
+
 #ifndef NDEBUG
-  if (verify_next_frame_is_key_) {
+  if (verify_next_frame_is_key_ && !drain_decoder_) {
     verify_next_frame_is_key_ = false;
     VerifyUnitIsKeyFrame(au_info.front_unit);
   }
@@ -646,16 +712,18 @@ bool MediaCodecDecoder::EnqueueInputBuffer() {
   DCHECK_GE(index, 0);
 
   const AccessUnit* unit = au_info.front_unit;
-  DCHECK(unit);
 
-  if (unit->is_end_of_stream) {
+  if (drain_decoder_ || unit->is_end_of_stream) {
     DVLOG(1) << class_name() << "::" << __FUNCTION__ << ": QueueEOS";
     media_codec_bridge_->QueueEOS(index);
     eos_enqueued_ = true;
     return true;
   }
 
-  DVLOG(2) << class_name() << ":: QueueInputBuffer pts:" << unit->timestamp;
+  DCHECK(unit);
+
+  DVLOG(2) << class_name() << "::" << __FUNCTION__
+           << ": QueueInputBuffer pts:" << unit->timestamp;
 
   status = media_codec_bridge_->QueueInputBuffer(
       index, &unit->data[0], unit->data.size(), unit->timestamp);
@@ -726,7 +794,8 @@ bool MediaCodecDecoder::DepleteOutputBufferQueue() {
         Render(buffer_index, offset, size, render_mode, pts, eos_encountered);
 
         if (render_mode == kRenderAfterPreroll) {
-          DVLOG(1) << class_name() << "::" << __FUNCTION__ << " pts:" << pts
+          DVLOG(1) << class_name() << "::" << __FUNCTION__ << " pts " << pts
+                   << " >= preroll timestamp " << preroll_timestamp_
                    << " preroll done, stopping frame processing";
           media_task_runner_->PostTask(FROM_HERE, internal_preroll_done_cb_);
           return false;