MediaCodecPlayer - preroll and reconfiguration fixes

media/base/android/media_codec_player.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "media/base/android/media_codec_player.h"
 
 #include "base/barrier_closure.h"
 #include "base/bind.h"
 #include "base/lazy_instance.h"
 #include "base/logging.h"
@@ skipping 423 matching lines @@
          seek_info_->seek_time <= actual_browser_seek_time);
 
   // Restrict the current time to be equal to seek_time
   // for the next StartPlaybackDecoders() call.
 
   base::TimeDelta seek_time = seek_info_->is_browser_seek
                                   ? actual_browser_seek_time
                                   : seek_info_->seek_time;
 
   interpolator_.SetBounds(seek_time, seek_time);
+
   audio_decoder_->SetBaseTimestamp(seek_time);
 
+  audio_decoder_->SetPrerollTimestamp(seek_time);
+  video_decoder_->SetPrerollTimestamp(seek_time);
+
   // The Flush() might set the state to kStateError.
   if (state_ == kStateError) {
     // Notify the Renderer.
     if (!seek_info_->is_browser_seek)
       ui_task_runner_->PostTask(FROM_HERE,
                                 base::Bind(seek_done_cb_, seek_time));
 
     seek_info_.reset();
     return;
   }
@@ skipping 145 matching lines @@
   }
 
   SetState(kStatePlaying);
   StartPlaybackOrBrowserSeek();
 }
 
 void MediaCodecPlayer::OnPrerollDone() {
   DCHECK(GetMediaTaskRunner()->BelongsToCurrentThread());
   DVLOG(1) << __FUNCTION__;
 
+  if (state_ != kStatePlaying) {
+    DVLOG(1) << __FUNCTION__ << ": in state " << AsString(state_)
+             << ", ignoring";
+    return;
+  }
+
   StartStatus status = StartDecoders();
   if (status != kStartOk)
     GetMediaTaskRunner()->PostTask(FROM_HERE, internal_error_cb_);
 }
 
 void MediaCodecPlayer::OnDecoderDrained(DemuxerStream::Type type) {
   DCHECK(GetMediaTaskRunner()->BelongsToCurrentThread());
   DVLOG(1) << __FUNCTION__ << " " << type;
 
   // We expect that OnStopDone() comes next.
 
   DCHECK(type == DemuxerStream::AUDIO || type == DemuxerStream::VIDEO);
   DCHECK(state_ == kStatePlaying || state_ == kStateStopping)
       << __FUNCTION__ << " illegal state: " << AsString(state_);
 
   switch (state_) {
     case kStatePlaying:
       SetState(kStateStopping);
       SetPendingStart(true);
 
       if (type == DemuxerStream::AUDIO && !VideoFinished()) {
         DVLOG(1) << __FUNCTION__ << " requesting to stop video";
-        video_decoder_->SetDecodingUntilOutputIsPresent();
         video_decoder_->RequestToStop();
       } else if (type == DemuxerStream::VIDEO && !AudioFinished()) {
         DVLOG(1) << __FUNCTION__ << " requesting to stop audio";
-        audio_decoder_->SetDecodingUntilOutputIsPresent();
         audio_decoder_->RequestToStop();
       }
       break;
 
-    case kStateStopping:
-      if (type == DemuxerStream::AUDIO && !VideoFinished())
-        video_decoder_->SetDecodingUntilOutputIsPresent();
-      else if (type == DemuxerStream::VIDEO && !AudioFinished())
-        audio_decoder_->SetDecodingUntilOutputIsPresent();
-      break;
-
     default:
-      NOTREACHED();
       break;
   }
 }
 
 void MediaCodecPlayer::OnStopDone(DemuxerStream::Type type) {
   DCHECK(GetMediaTaskRunner()->BelongsToCurrentThread());
   DVLOG(1) << __FUNCTION__ << " " << type
            << " interpolated time:" << GetInterpolatedTime();
 
   if (!(audio_decoder_->IsStopped() && video_decoder_->IsStopped())) {
@@ skipping 26 matching lines @@
       // Unexpected stop means completion
       SetState(kStatePaused);
       break;
     default:
       // DVLOG(0) << __FUNCTION__ << " illegal state: " << AsString(state_);
       // NOTREACHED();
       // Ignore! There can be a race condition: audio posts OnStopDone,
       // then video posts, then first OnStopDone arrives at which point
       // both streams are already stopped, then second OnStopDone arrives. When
       // the second one arrives, the state us not kStateStopping any more.
-      break;
+      return;

[Tima Vaisburd, 2015/09/03 19:55:27]

Changed |break| to |return| so we do not post notifications to the upper layer
if we are in the unexpected state.

   }
 
   // DetachListener to UI thread
   ui_task_runner_->PostTask(FROM_HERE, detach_listener_cb_);
 
   if (AudioFinished() && VideoFinished())
     ui_task_runner_->PostTask(FROM_HERE, completion_cb_);
 }
 
 void MediaCodecPlayer::OnError() {
@@ skipping 244 matching lines @@
   if (do_audio) {
     MediaCodecDecoder::ConfigStatus status = audio_decoder_->Configure();
     if (status != MediaCodecDecoder::kConfigOk) {
       return kStartFailed;
     }
   }
 
   return kStartOk;
 }
 
 MediaCodecPlayer::StartStatus MediaCodecPlayer::MaybePrerollDecoders(

[liberato (no reviews please), 2015/09/04 21:59:14]

now that the preroll timestamp is being maintained differently in the decoders,
i'm not sure that we need all of this.  could we always go through the preroll
state, and get rid of NotCompletedAndNeedsPreroll()?

i don't know if it would work exactly as it is now, but i think it can get
there.

[Tima Vaisburd, 2015/09/04 23:17:35]

We do not always perform preroll. For instance, in case of plain pause and
restart we did not do it before this change and do not do it now.

After this change the number of cases where we skip preroll just increased.

Consider e.g. reconfiguration (with drain) on one stream. Now the stream that
drained does the preroll, whereas the other one does nothing, it waits for the
preroll on the other stream to finish. (Then both resume).

Because of that I do not think we could get rid of
NotCompletedAndNeedsPreroll(), this CL heavily relies on this.

I think I could have made the preroll always happen but sometimes do nothing,
but I decided against for the sake of the simplicity of the change.

     bool* preroll_required) {
   DCHECK(GetMediaTaskRunner()->BelongsToCurrentThread());
 
-  base::TimeDelta current_time = GetInterpolatedTime();
-
-  DVLOG(1) << __FUNCTION__ << " current_time:" << current_time;
+  DVLOG(1) << __FUNCTION__ << " current_time:" << GetInterpolatedTime();
 
   // If requested, preroll is always done in the beginning of the playback,
   // after prefetch. The request might not happen at all though, in which case
   // we won't have prerolling phase. We need the prerolling when we (re)create
   // the decoder, because its configuration and initialization (getting input,
   // but not making output) can take time, and after the seek because there
   // could be some data to be skipped and there is again initialization after
   // the flush.
 
   *preroll_required = false;
 
   int count = 0;
-  const bool do_audio_preroll = audio_decoder_->NotCompletedAndNeedsPreroll();
-  if (do_audio_preroll)
+  const bool do_audio = audio_decoder_->NotCompletedAndNeedsPreroll();
+  if (do_audio)
     ++count;
 
-  const bool do_video_preroll = video_decoder_->NotCompletedAndNeedsPreroll();
-  if (do_video_preroll)
+  const bool do_video = video_decoder_->NotCompletedAndNeedsPreroll();
+  if (do_video)
     ++count;
 
   if (count == 0) {
     DVLOG(1) << __FUNCTION__ << ": preroll is not required, skipping";
     return kStartOk;
   }
 
   *preroll_required = true;
 
   DCHECK(count > 0);
-  DCHECK(do_audio_preroll || do_video_preroll);
+  DCHECK(do_audio || do_video);
 
   DVLOG(1) << __FUNCTION__ << ": preroll for " << count << " stream(s)";
 
   base::Closure preroll_cb = base::BarrierClosure(
       count, base::Bind(&MediaCodecPlayer::OnPrerollDone, media_weak_this_));
 
-  if (do_audio_preroll) {
-    if (!audio_decoder_->Preroll(current_time, preroll_cb))
-      return kStartFailed;
-  }
+  if (do_audio && !audio_decoder_->Preroll(preroll_cb))
+    return kStartFailed;
 
-  if (do_video_preroll) {
-    if (!video_decoder_->Preroll(current_time, preroll_cb))
-      return kStartFailed;
-  }
+  if (do_video && !video_decoder_->Preroll(preroll_cb))
+    return kStartFailed;
 
   return kStartOk;
 }
 
 MediaCodecPlayer::StartStatus MediaCodecPlayer::StartDecoders() {
   DCHECK(GetMediaTaskRunner()->BelongsToCurrentThread());
 
   if (!interpolator_.interpolating())
     interpolator_.StartInterpolating();
 
@@ skipping 147 matching lines @@
     RETURN_STRING(kStateWaitingForSurface);
     RETURN_STRING(kStateWaitingForSeek);
     RETURN_STRING(kStateError);
   }
   return nullptr;  // crash early
 }
 
 #undef RETURN_STRING
 
 }  // namespace media