Use cached Key frames to avoid browser seek

This change allows MediaSourcePlayer to skip browser seek if a key frame is found in cache.
Key frames can be identified by an empty data buffer, a config change or is_key_frame field.

BUG=304234

Review URL: https://codereview.chromium.org/898843002

Cr-Commit-Position: refs/heads/master@{#314699}

media/base/android/audio_decoder_job.cc

@@ -135,16 +135,17 @@ bool AudioDecoderJob::AreDemuxerConfigsChanged(
                  configs.audio_extra_data.begin());
 }
 
-bool AudioDecoderJob::CreateMediaCodecBridgeInternal() {
+MediaDecoderJob::MediaDecoderJobStatus
+    AudioDecoderJob::CreateMediaCodecBridgeInternal() {
   media_codec_bridge_.reset(AudioCodecBridge::Create(audio_codec_));
   if (!media_codec_bridge_)
-    return false;
+    return STATUS_FAILURE;
 
   if (!(static_cast<AudioCodecBridge*>(media_codec_bridge_.get()))->Start(
       audio_codec_, config_sampling_rate_, num_channels_, &audio_extra_data_[0],
       audio_extra_data_.size(), true, GetMediaCrypto().obj())) {
     media_codec_bridge_.reset();
-    return false;
+    return STATUS_FAILURE;
   }
 
   SetVolumeInternal();
@@ -153,7 +154,7 @@ bool AudioDecoderJob::CreateMediaCodecBridgeInternal() {
   frame_count_ = 0;
   ResetTimestampHelper();
 
-  return true;
+  return STATUS_SUCCESS;
 }
 
 void AudioDecoderJob::SetVolumeInternal() {

media/base/android/audio_decoder_job.h

@@ -49,7 +49,7 @@ class AudioDecoderJob : public MediaDecoderJob {
   virtual bool ComputeTimeToRender() const override;
   virtual bool AreDemuxerConfigsChanged(
       const DemuxerConfigs& configs) const override;
-  virtual bool CreateMediaCodecBridgeInternal() override;
+  virtual MediaDecoderJobStatus CreateMediaCodecBridgeInternal() override;
   virtual void OnOutputFormatChanged() override;
 
   // Helper method to set the audio output volume.

media/base/android/media_decoder_job.cc

@@ -110,7 +110,7 @@ void MediaDecoderJob::Prefetch(const base::Closure& prefetch_cb) {
   RequestData(prefetch_cb);
 }
 
-bool MediaDecoderJob::Decode(
+MediaDecoderJob::MediaDecoderJobStatus MediaDecoderJob::Decode(
     base::TimeTicks start_time_ticks,
     base::TimeDelta start_presentation_timestamp,
     const DecoderCallback& callback) {
@@ -120,10 +120,11 @@ bool MediaDecoderJob::Decode(
   if (!media_codec_bridge_ || need_to_reconfig_decoder_job_) {
     if (drain_decoder_)
       OnDecoderDrained();
-    need_to_reconfig_decoder_job_ = !CreateMediaCodecBridge();
+    MediaDecoderJobStatus status = CreateMediaCodecBridge();
+    need_to_reconfig_decoder_job_ = (status != STATUS_SUCCESS);
     skip_eos_enqueue_ = true;
     if (need_to_reconfig_decoder_job_)
-      return false;
+      return status;
   }
 
   decode_cb_ = callback;
@@ -133,11 +134,11 @@ bool MediaDecoderJob::Decode(
                            base::Unretained(this),
                            start_time_ticks,
                            start_presentation_timestamp));
-    return true;
+    return STATUS_SUCCESS;
   }
 
   DecodeCurrentAccessUnit(start_time_ticks, start_presentation_timestamp);
-  return true;
+  return STATUS_SUCCESS;
 }
 
 void MediaDecoderJob::StopDecode() {
@@ -205,6 +206,32 @@ base::android::ScopedJavaLocalRef<jobject> MediaDecoderJob::GetMediaCrypto() {
   return media_crypto;
 }
 
+bool MediaDecoderJob::SetCurrentFrameToPreviouslyCachedKeyFrame() {
+  const std::vector<AccessUnit>& access_units =
+      received_data_[current_demuxer_data_index_].access_units;
+  // If the current data chunk is empty, the player must be in an initial or
+  // seek state. The next access unit will always be a key frame.
+  if (access_units.size() == 0)
+    return true;
+
+  // Find key frame in all the access units the decoder have decoded,
+  // or is about to decode.
+  int i = std::min(access_unit_index_[current_demuxer_data_index_],
+                   access_units.size() - 1);
+  for (; i >= 0; --i) {
+    // Config change is always the last access unit, and it always come with
+    // a key frame afterwards.
+    if (access_units[i].status == DemuxerStream::kConfigChanged)
+      return true;
+    if (access_units[i].is_key_frame) {
+      access_unit_index_[current_demuxer_data_index_] = i;
+      return true;
+    }
+  }
+  return false;
+}
+
+
 void MediaDecoderJob::Release() {
   DCHECK(ui_task_runner_->BelongsToCurrentThread());
   DVLOG(1) << __FUNCTION__;
@@ -518,11 +545,8 @@ void MediaDecoderJob::OnDecodeCompleted(
     case MEDIA_CODEC_DEQUEUE_OUTPUT_AGAIN_LATER:
     case MEDIA_CODEC_OUTPUT_FORMAT_CHANGED:
     case MEDIA_CODEC_OUTPUT_END_OF_STREAM:
-      if (!input_eos_encountered_) {
-        CurrentDataConsumed(
-            CurrentAccessUnit().status == DemuxerStream::kConfigChanged);
+      if (!input_eos_encountered_)
         access_unit_index_[current_demuxer_data_index_]++;
-      }
       break;
 
     case MEDIA_CODEC_DEQUEUE_INPUT_AGAIN_LATER:
@@ -589,7 +613,7 @@ void MediaDecoderJob::RequestCurrentChunkIfEmpty() {
 
   // Requests new data if the the last access unit of the next chunk is not EOS.
   current_demuxer_data_index_ = inactive_demuxer_data_index();
-  const AccessUnit last_access_unit =
+  const AccessUnit& last_access_unit =
       received_data_[current_demuxer_data_index_].access_units.back();
   if (!last_access_unit.is_end_of_stream &&
       last_access_unit.status != DemuxerStream::kAborted) {
@@ -616,26 +640,26 @@ void MediaDecoderJob::OnDecoderDrained() {
   // Increase the access unit index so that the new decoder will not handle
   // the config change again.
   access_unit_index_[current_demuxer_data_index_]++;
-  CurrentDataConsumed(true);
 }
 
-bool MediaDecoderJob::CreateMediaCodecBridge() {
+MediaDecoderJob::MediaDecoderJobStatus
+    MediaDecoderJob::CreateMediaCodecBridge() {
   DVLOG(1) << __FUNCTION__;
   DCHECK(ui_task_runner_->BelongsToCurrentThread());
   DCHECK(decode_cb_.is_null());
 
   if (!HasStream()) {
     ReleaseMediaCodecBridge();
-    return false;
+    return STATUS_FAILURE;
   }
 
   // Create |media_codec_bridge_| only if config changes.
   if (media_codec_bridge_ && !need_to_reconfig_decoder_job_)
-    return true;
+    return STATUS_SUCCESS;
 
   base::android::ScopedJavaLocalRef<jobject> media_crypto = GetMediaCrypto();
   if (is_content_encrypted_ && media_crypto.is_null())
-    return false;
+    return STATUS_FAILURE;
 
   ReleaseMediaCodecBridge();
   DVLOG(1) << __FUNCTION__ << " : creating new media codec bridge";

media/base/android/media_decoder_job.h

@@ -27,6 +27,13 @@ class MediaDrmBridge;
 // data request will be sent to the renderer.
 class MediaDecoderJob {
  public:
+  // Return value when Decode() is called.
+  enum MediaDecoderJobStatus {
+    STATUS_SUCCESS,
+    STATUS_KEY_FRAME_REQUIRED,
+    STATUS_FAILURE,
+  };
+
   struct Deleter {
     inline void operator()(MediaDecoderJob* ptr) const { ptr->Release(); }
   };
@@ -57,11 +64,10 @@ class MediaDecoderJob {
   // Called by MediaSourcePlayer to decode some data.
   // |callback| - Run when decode operation has completed.
   //
-  // Returns true if the next decode was started and |callback| will be
-  // called when the decode operation is complete.
-  // Returns false if |media_codec_bridge_| cannot be created; |callback| is
-  // ignored and will not be called.
-  bool Decode(base::TimeTicks start_time_ticks,
+  // Returns STATUS_SUCCESS on success, or STATUS_FAILURE on failure, or
+  // STATUS_KEY_FRAME_REQUIRED if a browser seek is required. |callback| is
+  // ignored and will not be called for the latter 2 cases.
+  MediaDecoderJobStatus Decode(base::TimeTicks start_time_ticks,
                                base::TimeDelta start_presentation_timestamp,
                                const DecoderCallback& callback);
 
@@ -132,6 +138,11 @@ class MediaDecoderJob {
   // Releases the |media_codec_bridge_|.
   void ReleaseMediaCodecBridge();
 
+  // Sets the current frame to a previously cached key frame. Returns true if
+  // a key frame is found, or false otherwise.
+  // TODO(qinmin): add UMA to study the cache hit ratio for key frames.
+  bool SetCurrentFrameToPreviouslyCachedKeyFrame();
+
   MediaDrmBridge* drm_bridge() { return drm_bridge_; }
 
   void set_is_content_encrypted(bool is_content_encrypted) {
@@ -209,18 +220,16 @@ class MediaDecoderJob {
   // Called when the decoder is completely drained and is ready to be released.
   void OnDecoderDrained();
 
-  // Creates |media_codec_bridge_| for decoding purpose. Returns true if it is
-  // created, or false otherwise.
-  bool CreateMediaCodecBridge();
-
-  // Called when an access unit is consumed by the decoder. |is_config_change|
-  // indicates whether the current access unit is a config change. If it is
-  // true, the next access unit is guarateed to be an I-frame.
-  virtual void CurrentDataConsumed(bool is_config_change) {}
+  // Creates |media_codec_bridge_| for decoding purpose.
+  // Returns STATUS_SUCCESS on success, or STATUS_FAILURE on failure, or
+  // STATUS_KEY_FRAME_REQUIRED if a browser seek is required.
+  MediaDecoderJobStatus CreateMediaCodecBridge();
 
   // Implemented by the child class to create |media_codec_bridge_| for a
-  // particular stream. Returns true if it is created, or false otherwise.
-  virtual bool CreateMediaCodecBridgeInternal() = 0;
+  // particular stream.
+  // Returns STATUS_SUCCESS on success, or STATUS_FAILURE on failure, or
+  // STATUS_KEY_FRAME_REQUIRED if a browser seek is required.
+  virtual MediaDecoderJobStatus CreateMediaCodecBridgeInternal() = 0;
 
   // Returns true if the |configs| doesn't match the current demuxer configs
   // the decoder job has.

media/base/android/media_source_player.cc

@@ -564,18 +564,25 @@ void MediaSourcePlayer::DecodeMoreAudio() {
   DCHECK(!audio_decoder_job_->is_decoding());
   DCHECK(!AudioFinished());
 
-  if (audio_decoder_job_->Decode(
+  MediaDecoderJob::MediaDecoderJobStatus status = audio_decoder_job_->Decode(
       start_time_ticks_,
       start_presentation_timestamp_,
-      base::Bind(&MediaSourcePlayer::MediaDecoderCallback, weak_this_, true))) {
+      base::Bind(&MediaSourcePlayer::MediaDecoderCallback, weak_this_, true));
+
+  switch (status) {
+    case MediaDecoderJob::STATUS_SUCCESS:
       TRACE_EVENT_ASYNC_BEGIN0("media", "MediaSourcePlayer::DecodeMoreAudio",
                                audio_decoder_job_.get());
-    return;
-  }
-
+      break;
+    case MediaDecoderJob::STATUS_KEY_FRAME_REQUIRED:
+      NOTREACHED();
+      break;
+    case MediaDecoderJob::STATUS_FAILURE:
       is_waiting_for_audio_decoder_ = true;
       if (!IsEventPending(DECODER_CREATION_EVENT_PENDING))
         SetPendingEvent(DECODER_CREATION_EVENT_PENDING);
+      break;
+  }
 }
 
 void MediaSourcePlayer::DecodeMoreVideo() {
@@ -583,25 +590,26 @@ void MediaSourcePlayer::DecodeMoreVideo() {
   DCHECK(!video_decoder_job_->is_decoding());
   DCHECK(!VideoFinished());
 
-  if (video_decoder_job_->Decode(
+  MediaDecoderJob::MediaDecoderJobStatus status = video_decoder_job_->Decode(
       start_time_ticks_,
       start_presentation_timestamp_,
       base::Bind(&MediaSourcePlayer::MediaDecoderCallback, weak_this_,
-                 false))) {
+                 false));
+
+  switch (status) {
+    case MediaDecoderJob::STATUS_SUCCESS:
       TRACE_EVENT_ASYNC_BEGIN0("media", "MediaSourcePlayer::DecodeMoreVideo",
                                video_decoder_job_.get());
-    return;
-  }
-
-  // If the decoder is waiting for iframe, trigger a browser seek.
-  if (!video_decoder_job_->next_video_data_is_iframe()) {
+      break;
+    case MediaDecoderJob::STATUS_KEY_FRAME_REQUIRED:
       BrowserSeekToCurrentTime();
-    return;
-  }
-
+      break;
+    case MediaDecoderJob::STATUS_FAILURE:
       is_waiting_for_video_decoder_ = true;
       if (!IsEventPending(DECODER_CREATION_EVENT_PENDING))
         SetPendingEvent(DECODER_CREATION_EVENT_PENDING);
+      break;
+  }
 }
 
 void MediaSourcePlayer::PlaybackCompleted(bool is_audio) {

media/base/android/media_source_player_unittest.cc

@@ -435,6 +435,10 @@ class MediaSourcePlayerTest : public testing::Test {
     return data;
   }
 
+  bool HasData(bool is_audio) {
+    return GetMediaDecoderJob(is_audio)->HasData();
+  }
+
   // Helper method for use at test start. It starts an audio decoder job and
   // immediately feeds it some data to decode. Then, without letting the decoder
   // job complete a decode cycle, it also starts player SeekTo(). Upon return,
@@ -1043,7 +1047,9 @@ TEST_F(MediaSourcePlayerTest, SetEmptySurfaceAndStarveWhileDecoding) {
 
   // Playback resumes once a non-empty surface is passed.
   CreateNextTextureAndSetVideoSurface();
-  EXPECT_EQ(1, demuxer_->num_browser_seek_requests());
+  EXPECT_EQ(0, demuxer_->num_browser_seek_requests());
+  while(demuxer_->num_browser_seek_requests() != 1)
+    message_loop_.RunUntilIdle();
   WaitForVideoDecodeDone();
 }
 
@@ -1534,6 +1540,34 @@ TEST_F(MediaSourcePlayerTest, BrowserSeek_MidStreamReleaseAndStart) {
   EXPECT_EQ(1, demuxer_->num_seek_requests());
 }
 
+TEST_F(MediaSourcePlayerTest, NoBrowserSeekWithKeyFrameInCache) {
+  SKIP_TEST_IF_MEDIA_CODEC_BRIDGE_IS_NOT_AVAILABLE();
+
+  // Test that browser seek is not needed if a key frame is found in data
+  // cache.
+  CreateNextTextureAndSetVideoSurface();
+  StartVideoDecoderJob();
+  DemuxerData data = CreateReadFromDemuxerAckForVideo(false);
+  data.access_units[0].is_key_frame = true;
+
+  // Simulate demuxer's response to the video data request.
+  player_.OnDemuxerDataAvailable(data);
+
+  // Trigger decoder recreation later by changing surfaces.
+  CreateNextTextureAndSetVideoSurface();
+
+  // Wait for the media codec bridge to finish decoding and be reset.
+  WaitForVideoDecodeDone();
+  EXPECT_FALSE(HasData(false));
+
+  // Send a non key frame to decoder so that decoder can continue. This will
+  // not trigger any browser seeks as the previous key frame is still in the
+  // buffer.
+  player_.OnDemuxerDataAvailable(CreateReadFromDemuxerAckForVideo(false));
+  WaitForVideoDecodeDone();
+  EXPECT_EQ(0, demuxer_->num_browser_seek_requests());
+}
+
 TEST_F(MediaSourcePlayerTest, PrerollAudioAfterSeek) {
   SKIP_TEST_IF_MEDIA_CODEC_BRIDGE_IS_NOT_AVAILABLE();
 

media/base/android/video_decoder_job.cc

@@ -37,8 +37,7 @@ VideoDecoderJob::VideoDecoderJob(
       config_height_(0),
       output_width_(0),
       output_height_(0),
-      request_resources_cb_(request_resources_cb),
-      next_video_data_is_iframe_(true) {
+      request_resources_cb_(request_resources_cb) {
 }
 
 VideoDecoderJob::~VideoDecoderJob() {}
@@ -61,11 +60,6 @@ bool VideoDecoderJob::HasStream() const {
   return video_codec_ != kUnknownVideoCodec;
 }
 
-void VideoDecoderJob::Flush() {
-  MediaDecoderJob::Flush();
-  next_video_data_is_iframe_ = true;
-}
-
 void VideoDecoderJob::ReleaseDecoderResources() {
   MediaDecoderJob::ReleaseDecoderResources();
   surface_ = gfx::ScopedJavaSurface();
@@ -124,16 +118,17 @@ bool VideoDecoderJob::AreDemuxerConfigsChanged(
       config_height_ != configs.video_size.height();
 }
 
-bool VideoDecoderJob::CreateMediaCodecBridgeInternal() {
+MediaDecoderJob::MediaDecoderJobStatus
+    VideoDecoderJob::CreateMediaCodecBridgeInternal() {
   if (surface_.IsEmpty()) {
     ReleaseMediaCodecBridge();
-    return false;
+    return STATUS_FAILURE;
   }
 
-  // If the next data is not iframe, return false so that the player need to
-  // perform a browser seek.
-  if (!next_video_data_is_iframe_)
-    return false;
+  // If we cannot find a key frame in cache, browser seek is needed.
+  bool next_video_data_is_iframe = SetCurrentFrameToPreviouslyCachedKeyFrame();
+  if (!next_video_data_is_iframe)
+    return STATUS_KEY_FRAME_REQUIRED;
 
   bool is_secure = is_content_encrypted() && drm_bridge() &&
       drm_bridge()->IsProtectedSurfaceRequired();
@@ -143,14 +138,10 @@ bool VideoDecoderJob::CreateMediaCodecBridgeInternal() {
       surface_.j_surface().obj(), GetMediaCrypto().obj()));
 
   if (!media_codec_bridge_)
-    return false;
+    return STATUS_FAILURE;
 
   request_resources_cb_.Run();
-  return true;
-}
-
-void VideoDecoderJob::CurrentDataConsumed(bool is_config_change) {
-  next_video_data_is_iframe_ = is_config_change;
+  return STATUS_SUCCESS;
 }
 
 bool VideoDecoderJob::UpdateOutputFormat() {

media/base/android/video_decoder_job.h

@@ -33,14 +33,9 @@ class VideoDecoderJob : public MediaDecoderJob {
 
   // MediaDecoderJob implementation.
   virtual bool HasStream() const override;
-  virtual void Flush() override;
   virtual void ReleaseDecoderResources() override;
   virtual void SetDemuxerConfigs(const DemuxerConfigs& configs) override;
 
-  bool next_video_data_is_iframe() {
-    return next_video_data_is_iframe_;
-  }
-
   int output_width() const { return output_width_; }
   int output_height() const { return output_height_; }
 
@@ -57,8 +52,7 @@ class VideoDecoderJob : public MediaDecoderJob {
       const DemuxerConfigs& configs) const override;
   virtual bool AreDemuxerConfigsChanged(
       const DemuxerConfigs& configs) const override;
-  virtual bool CreateMediaCodecBridgeInternal() override;
-  virtual void CurrentDataConsumed(bool is_config_change) override;
+  virtual MediaDecoderJobStatus CreateMediaCodecBridgeInternal() override;
   virtual bool UpdateOutputFormat() override;
 
   // Returns true if a protected surface is required for video playback.
@@ -80,11 +74,6 @@ class VideoDecoderJob : public MediaDecoderJob {
   base::Closure request_resources_cb_;
   base::Closure release_resources_cb_;
 
-  // Track whether the next access unit is an I-frame. The first access
-  // unit after Flush() and CurrentDataConsumed(true) is guaranteed to be an
-  // I-frame.
-  bool next_video_data_is_iframe_;
-
   DISALLOW_COPY_AND_ASSIGN(VideoDecoderJob);
 };