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

media/base/android/media_codec_player.cc

Index: media/base/android/media_codec_player.cc
diff --git a/media/base/android/media_codec_player.cc b/media/base/android/media_codec_player.cc
index dfa5abbdca55f75a4fc04c11e4fdf5cb6d752d88..fd76dd40df2e08c6b3cefa1c54af126d335126ac 100644
--- a/media/base/android/media_codec_player.cc
+++ b/media/base/android/media_codec_player.cc
@@ -73,6 +73,7 @@ MediaCodecPlayer::MediaCodecPlayer(
       base::Bind(&MediaPlayerManager::OnPlaybackComplete, manager, player_id);
   seek_done_cb_ =
       base::Bind(&MediaPlayerManager::OnSeekComplete, manager, player_id);
+  error_cb_ = base::Bind(&MediaPlayerManager::OnError, manager, player_id);
   attach_listener_cb_ = base::Bind(&MediaPlayerAndroid::AttachListener,
                                    WeakPtrForUIThread(), nullptr);
   detach_listener_cb_ =
@@ -130,16 +131,56 @@ void MediaCodecPlayer::SetVideoSurface(gfx::ScopedJavaSurface surface) {
 
   DVLOG(1) << __FUNCTION__ << (surface.IsEmpty() ? " empty" : " non-empty");
 
-  // I assume that if video decoder already has the surface,
-  // there will be two calls:
-  // (1) SetVideoSurface(0)
-  // (2) SetVideoSurface(new_surface)
-  video_decoder_->SetPendingSurface(surface.Pass());
+  // Save the empty-ness before we pass the surface to the decoder.
+  bool surface_is_empty = surface.IsEmpty();
 
-  if (video_decoder_->HasPendingSurface() &&
-      state_ == STATE_WAITING_FOR_SURFACE) {
-    SetState(STATE_PLAYING);
-    StartPlaybackDecoders();
+  // Apparently RemoveVideoSurface() can be called several times in a row,
+  // ignore the second and subsequent calls.
+  if (surface_is_empty && !video_decoder_->HasVideoSurface()) {
+    DVLOG(1) << __FUNCTION__ << ": surface already removed, ignoring";
+    return;
+  }
+
+  video_decoder_->SetVideoSurface(surface.Pass());
+
+  if (surface_is_empty) {
+    // Remove video surface.
+    switch (state_) {
+      case STATE_PLAYING:
+        DVLOG(1) << __FUNCTION__ << ": stopping and restarting";
+        // Stop decoders as quickly as possible.
+        StopDecoders();  // synchronous stop

[qinmin, 2015/07/22 17:30:33]

we only need to do this if video is available.

[Tima Vaisburd, 2015/07/23 00:37:49]

I added checks in both remove and replace sections.

+
+        // Prefetch or wait for initial configuration.
+        if (HasAudio() || HasVideo()) {
+          SetState(STATE_PREFETCHING);
+          StartPrefetchDecoders();
+        } else {
+          SetState(STATE_WAITING_FOR_CONFIG);
+        }
+        break;
+
+      default:
+        break;  // ignore
+    }
+  } else {
+    // Replace video surface.
+    switch (state_) {
+      case STATE_WAITING_FOR_SURFACE:
+        SetState(STATE_PLAYING);
+        StartPlaybackOrBrowserSeek();
+        break;
+
+      case STATE_PLAYING:
+        DVLOG(1) << __FUNCTION__ << ": requesting to stop and restart";
+        SetState(STATE_STOPPING);
+        RequestToStopDecoders();
+        SetPendingStart(true);
+        break;
+
+      default:
+        break;  // ignore
+    }
   }
 }
 
@@ -471,17 +512,17 @@ void MediaCodecPlayer::OnPrefetchDone() {
     // No configuration at all after prefetching.
     // This is an error, initial configuration is expected
     // before the first data chunk.
-    GetMediaTaskRunner()->PostTask(FROM_HERE, error_cb_);
+    GetMediaTaskRunner()->PostTask(FROM_HERE, internal_error_cb_);
     return;
   }
 
-  if (HasVideo() && !HasPendingSurface()) {
+  if (HasVideo() && !video_decoder_->HasVideoSurface()) {
     SetState(STATE_WAITING_FOR_SURFACE);
     return;
   }
 
   SetState(STATE_PLAYING);
-  StartPlaybackDecoders();
+  StartPlaybackOrBrowserSeek();
 }
 
 void MediaCodecPlayer::OnStopDone() {
@@ -540,6 +581,9 @@ void MediaCodecPlayer::OnError() {
   SetState(STATE_ERROR);
 
   ReleaseDecoderResources();
+
+  ui_task_runner_->PostTask(FROM_HERE,
+                            base::Bind(error_cb_, MEDIA_ERROR_DECODE));
 }
 
 void MediaCodecPlayer::OnStarvation(DemuxerStream::Type type) {
@@ -606,17 +650,6 @@ void MediaCodecPlayer::SetState(PlayerState new_state) {
   state_ = new_state;
 }
 
-void MediaCodecPlayer::SetPendingSurface(gfx::ScopedJavaSurface surface) {
-  DCHECK(GetMediaTaskRunner()->BelongsToCurrentThread());
-  DVLOG(1) << __FUNCTION__;
-
-  video_decoder_->SetPendingSurface(surface.Pass());
-}
-
-bool MediaCodecPlayer::HasPendingSurface() {
-  return video_decoder_->HasPendingSurface();
-}
-
 void MediaCodecPlayer::SetPendingStart(bool need_to_start) {
   DCHECK(GetMediaTaskRunner()->BelongsToCurrentThread());
   DVLOG(1) << __FUNCTION__ << ": " << need_to_start;
@@ -700,33 +733,63 @@ void MediaCodecPlayer::StartPrefetchDecoders() {
     video_decoder_->Prefetch(prefetch_cb);
 }
 
-void MediaCodecPlayer::StartPlaybackDecoders() {
+void MediaCodecPlayer::StartPlaybackOrBrowserSeek() {
   DCHECK(GetMediaTaskRunner()->BelongsToCurrentThread());
   DVLOG(1) << __FUNCTION__;
 
-  // Configure all streams before the start since
-  // we may discover that browser seek is required.
+  // TODO(timav): consider replacing this method with posting a
+  // browser seek task (i.e. generate an event) from StartPlaybackDecoders().
+
+  StartStatus status = StartPlaybackDecoders();
+
+  switch (status) {
+    case START_BROWSER_SEEK_REQUIRED:
+      // Browser seek
+      SetState(STATE_WAITING_FOR_SEEK);
+      SetPendingStart(true);
+      StopDecoders();
+      RequestDemuxerSeek(GetInterpolatedTime(), true);
+      break;
+    case START_FAILED:
+      GetMediaTaskRunner()->PostTask(FROM_HERE, internal_error_cb_);
+      break;
+    case START_OK:
+      break;
+  }
+}
+
+MediaCodecPlayer::StartStatus MediaCodecPlayer::StartPlaybackDecoders() {
+  DCHECK(GetMediaTaskRunner()->BelongsToCurrentThread());
+  DVLOG(1) << __FUNCTION__;
 
   bool do_audio = !AudioFinished();
   bool do_video = !VideoFinished();
 
-  // If there is nothing to play, the state machine should determine
-  // this at the prefetch state and never call this method.
+  // If there is nothing to play, the state machine should determine this at the
+  // prefetch state and never call this method.
   DCHECK(do_audio || do_video);
 
-  if (do_audio) {
-    MediaCodecDecoder::ConfigStatus status = audio_decoder_->Configure();
-    if (status != MediaCodecDecoder::CONFIG_OK) {
-      GetMediaTaskRunner()->PostTask(FROM_HERE, error_cb_);
-      return;
-    }
-  }
+  // Configure all streams before the start since we may discover that browser
+  // seek is required. Start with video: if browser seek is required it would
+  // not make sense to configure audio.
 
   if (do_video) {
     MediaCodecDecoder::ConfigStatus status = video_decoder_->Configure();
+    switch (status) {
+      case MediaCodecDecoder::CONFIG_OK:
+        break;
+      case MediaCodecDecoder::CONFIG_KEY_FRAME_REQUIRED:
+        // TODO(timav): post a task or return the status?
+        return START_BROWSER_SEEK_REQUIRED;
+      case MediaCodecDecoder::CONFIG_FAILURE:
+        return START_FAILED;
+    }
+  }
+
+  if (do_audio) {
+    MediaCodecDecoder::ConfigStatus status = audio_decoder_->Configure();
     if (status != MediaCodecDecoder::CONFIG_OK) {
-      GetMediaTaskRunner()->PostTask(FROM_HERE, error_cb_);
-      return;
+      return START_FAILED;
     }
   }
 
@@ -738,8 +801,7 @@ void MediaCodecPlayer::StartPlaybackDecoders() {
 
   if (do_audio) {
     if (!audio_decoder_->Start(current_time)) {
-      GetMediaTaskRunner()->PostTask(FROM_HERE, error_cb_);
-      return;
+      return START_FAILED;
     }
 
     // Attach listener on UI thread
@@ -748,18 +810,19 @@ void MediaCodecPlayer::StartPlaybackDecoders() {
 
   if (do_video) {
     if (!video_decoder_->Start(current_time)) {
-      GetMediaTaskRunner()->PostTask(FROM_HERE, error_cb_);
-      return;
+      return START_FAILED;
     }
   }
+
+  return START_OK;
 }
 
 void MediaCodecPlayer::StopDecoders() {
   DCHECK(GetMediaTaskRunner()->BelongsToCurrentThread());
   DVLOG(1) << __FUNCTION__;
 
-  audio_decoder_->SyncStop();
   video_decoder_->SyncStop();
+  audio_decoder_->SyncStop();
 }
 
 void MediaCodecPlayer::RequestToStopDecoders() {
@@ -790,7 +853,7 @@ void MediaCodecPlayer::RequestDemuxerSeek(base::TimeDelta seek_time,
                                           bool is_browser_seek) {
   DCHECK(GetMediaTaskRunner()->BelongsToCurrentThread());
   DVLOG(1) << __FUNCTION__ << " " << seek_time
-           << (is_browser_seek ? " browser_seek" : "");
+           << (is_browser_seek ? " BROWSER_SEEK" : "");
 
   // Flush decoders before requesting demuxer.
   audio_decoder_->Flush();
@@ -821,14 +884,15 @@ void MediaCodecPlayer::CreateDecoders() {
   DCHECK(GetMediaTaskRunner()->BelongsToCurrentThread());
   DVLOG(1) << __FUNCTION__;
 
-  error_cb_ = base::Bind(&MediaCodecPlayer::OnError, media_weak_this_);
+  internal_error_cb_ = base::Bind(&MediaCodecPlayer::OnError, media_weak_this_);
 
   audio_decoder_.reset(new MediaCodecAudioDecoder(
       GetMediaTaskRunner(), base::Bind(&MediaCodecPlayer::RequestDemuxerData,
                                        media_weak_this_, DemuxerStream::AUDIO),
       base::Bind(&MediaCodecPlayer::OnStarvation, media_weak_this_,
                  DemuxerStream::AUDIO),
-      base::Bind(&MediaCodecPlayer::OnStopDone, media_weak_this_), error_cb_,
+      base::Bind(&MediaCodecPlayer::OnStopDone, media_weak_this_),
+      internal_error_cb_,
       base::Bind(&MediaCodecPlayer::OnTimeIntervalUpdate, media_weak_this_,
                  DemuxerStream::AUDIO)));
 
@@ -837,7 +901,8 @@ void MediaCodecPlayer::CreateDecoders() {
                                        media_weak_this_, DemuxerStream::VIDEO),
       base::Bind(&MediaCodecPlayer::OnStarvation, media_weak_this_,
                  DemuxerStream::VIDEO),
-      base::Bind(&MediaCodecPlayer::OnStopDone, media_weak_this_), error_cb_,
+      base::Bind(&MediaCodecPlayer::OnStopDone, media_weak_this_),
+      internal_error_cb_,
       base::Bind(&MediaCodecPlayer::OnTimeIntervalUpdate, media_weak_this_,
                  DemuxerStream::VIDEO),
       base::Bind(&MediaCodecPlayer::OnVideoResolutionChanged, media_weak_this_),