Handle config changes for MSE on android

media/base/android/media_source_player.cc

 // Copyright (c) 2013 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_source_player.h"
 
 #include "base/android/jni_android.h"
 #include "base/android/jni_string.h"
 #include "base/basictypes.h"
 #include "base/bind.h"
@@ skipping 37 matching lines @@
 
 }
 
 namespace media {
 
 MediaDecoderJob::MediaDecoderJob(base::Thread* thread, bool is_audio)
     : message_loop_(base::MessageLoopProxy::current()),
       thread_(thread),
       needs_flush_(false),
       is_audio_(is_audio),
-      weak_this_(this) {
+      weak_this_(this),
+      decoding_(false) {
 }
 
 MediaDecoderJob::~MediaDecoderJob() {}
 
 // Class for managing audio decoding jobs.
 class AudioDecoderJob : public MediaDecoderJob {
  public:
   AudioDecoderJob(
       const AudioCodec audio_codec, int sample_rate,
       int channel_count, const uint8* extra_data, size_t extra_data_size);
   virtual ~AudioDecoderJob() {}
 };
 
 // Class for managing video decoding jobs.
 class VideoDecoderJob : public MediaDecoderJob {
  public:
   VideoDecoderJob(
       const VideoCodec video_codec, const gfx::Size& size, jobject surface);
   virtual ~VideoDecoderJob() {}
 
   void Configure(
       const VideoCodec codec, const gfx::Size& size, jobject surface);
 };
 
 void MediaDecoderJob::Decode(
     const MediaPlayerHostMsg_ReadFromDemuxerAck_Params::AccessUnit& unit,
     const base::Time& start_wallclock_time,
     const base::TimeDelta& start_presentation_timestamp,
     const MediaDecoderJob::DecoderCallback& callback) {
+  decoding_ = true;

[acolwell GONE FROM CHROMIUM, 2013/06/04 15:19:14]

Should there be a DCHECK(!decoding_) here?

[qinmin, 2013/06/04 19:03:23]

Done.

   thread_->message_loop()->PostTask(FROM_HERE, base::Bind(
       &MediaDecoderJob::DecodeInternal, base::Unretained(this), unit,
       start_wallclock_time, start_presentation_timestamp, needs_flush_,
       callback));
   needs_flush_ = false;
 }
 
 void MediaDecoderJob::DecodeInternal(
     const MediaPlayerHostMsg_ReadFromDemuxerAck_Params::AccessUnit& unit,
     const base::Time& start_wallclock_time,
     const base::TimeDelta& start_presentation_timestamp,
     bool needs_flush,
     const MediaDecoderJob::DecoderCallback& callback) {
   if (needs_flush)
     media_codec_bridge_->Reset();
   base::TimeDelta timeout = base::TimeDelta::FromMicroseconds(
       kMediaCodecTimeoutInMicroseconds);
   int input_buf_index = media_codec_bridge_->DequeueInputBuffer(timeout);
+  if (input_buf_index == MediaCodecBridge::INFO_MEDIA_CODEC_ERROR) {
+    message_loop_->PostTask(FROM_HERE, base::Bind(
+        callback, false, start_presentation_timestamp, start_wallclock_time,
+        false));
+    return;
+  }
   // TODO(qinmin): skip frames if video is falling far behind.
   if (input_buf_index >= 0) {
     if (unit.end_of_stream) {
       media_codec_bridge_->QueueEOS(input_buf_index);
     } else {
       media_codec_bridge_->QueueInputBuffer(
           input_buf_index, &unit.data[0], unit.data.size(), unit.timestamp);
     }
   }
   size_t offset = 0;
@@ skipping 56 matching lines @@
   if (is_audio_) {
     static_cast<AudioCodecBridge*>(media_codec_bridge_.get())->PlayOutputBuffer(
         outputBufferIndex, size);
   }
   media_codec_bridge_->ReleaseOutputBuffer(outputBufferIndex, !is_audio_);
   message_loop_->PostTask(FROM_HERE, base::Bind(
       callback, true, presentation_timestamp, base::Time::Now(),
       end_of_stream));
 }
 
+void MediaDecoderJob::OnDecodeCompleted() {
+  decoding_ = false;
+}
+
 void MediaDecoderJob::Flush() {
   // Do nothing, flush when the next Decode() happens.
   needs_flush_ = true;
 }
 
 void MediaDecoderJob::Release() {
-  if (thread_->IsRunning() &&
-      thread_->message_loop() != base::MessageLoop::current()) {
+  if (decoding_ && thread_->message_loop() != base::MessageLoop::current())

[acolwell GONE FROM CHROMIUM, 2013/06/04 15:19:14]

This doesn't seem right. Shouldn't this code look something more like the
following?

if (decoding_ || thread_->message_loop() != base::MessageLoop::current()) {
  thread_->message_loop()->PostTask(FROM_HERE,
base::Bind(&MediaDecoderJob::Release, base::Unretained(this));
  return;
}

delete this;

This makes sure we are always done decoding and are on the proper thread before
deleting.

[qinmin, 2013/06/04 19:03:23]

Backgroud info: there is a problem in android that a java surface cannot be
assigned to 2 MediaCodecs at the same time, or otherwise that will cause an
exception.

If |decoding_| is false, there is nothing running on the decoder thread. So it
is safe to delete the MediaDecoderJob on the main thread. However, if we post a
task to the decoder thread to delete object, then we cannot immediately pass the
surface to a new MediaDecoderJob. So sth like "video_decoder_job_.reset(new
VideoDecoderJob())" will cause the app to crash. Basically we are blocked on the
main thread until the previous decoder got deleted. I can do a
PostTaskAndReply（）， but that will introduce longer delays when config change
happens. And it further complicates the logic in MediaSourcePlayer. Added a TODO
here to mention the bug and see if we can find a better way to handle this.

On 2013/06/04 15:19:14, acolwell wrote:

     thread_->message_loop()->DeleteSoon(FROM_HERE, this);
-  } else {
+  else
     delete this;
-  }
 }
 
 VideoDecoderJob::VideoDecoderJob(
     const VideoCodec video_codec, const gfx::Size& size, jobject surface)
     : MediaDecoderJob(g_video_decoder_thread.Pointer(), false) {
   scoped_ptr<VideoCodecBridge> codec(VideoCodecBridge::Create(video_codec));
   codec->Start(video_codec, size, surface);
   media_codec_bridge_.reset(codec.release());
 }
 
 AudioDecoderJob::AudioDecoderJob(
     const AudioCodec audio_codec,
     int sample_rate,
     int channel_count,
     const uint8* extra_data,
     size_t extra_data_size)
     : MediaDecoderJob(g_audio_decoder_thread.Pointer(), true) {
   scoped_ptr<AudioCodecBridge> codec(AudioCodecBridge::Create(audio_codec));
   codec->Start(audio_codec, sample_rate, channel_count, extra_data,
                extra_data_size, true);
   media_codec_bridge_.reset(codec.release());
 }
 
 MediaSourcePlayer::MediaSourcePlayer(
     int player_id,
     MediaPlayerManager* manager)
     : MediaPlayerAndroid(player_id, manager),
       pending_event_(NO_EVENT_PENDING),
       active_decoding_tasks_(0),
+      seek_request_id_(0),
       width_(0),
       height_(0),
       audio_codec_(kUnknownAudioCodec),
       video_codec_(kUnknownVideoCodec),
       num_channels_(0),
       sampling_rate_(0),
       seekable_(true),
       audio_finished_(true),
       video_finished_(true),
       playing_(false),
       audio_access_unit_index_(0),
       video_access_unit_index_(0),
       waiting_for_audio_data_(false),
       waiting_for_video_data_(false),
-      use_empty_surface_(true),
       weak_this_(this) {
 }
 
 MediaSourcePlayer::~MediaSourcePlayer() {
   Release();
 }
 
 void MediaSourcePlayer::SetVideoSurface(gfx::ScopedJavaSurface surface) {
-  use_empty_surface_ =  surface.IsSurfaceEmpty();
+  surface_ =  surface.Pass();
 
-  // If we haven't processed a surface change event, do so now.
-  if (active_decoding_tasks_ > 0) {
-    pending_event_ |= SURFACE_CHANGE_EVENT_PENDING;
-    // Request a seek so that the next decoder will decode an I-frame first.
-    // Or otherwise, MediaCodec might crash. See b/8950387.
-    pending_event_ |= SEEK_EVENT_PENDING;
-    ProcessPendingEvents();
-    return;
-  }
-
-  if (HasVideo()) {
-    video_decoder_job_.reset(new VideoDecoderJob(
-        video_codec_, gfx::Size(width_, height_), surface.j_surface().obj()));
-  }
-
-  // Inform the fullscreen view the player is ready.
-  // TODO(qinmin): refactor MediaPlayerBridge so that we have a better way
-  // to inform ContentVideoView.
-  OnMediaMetadataChanged(duration_, width_, height_, true);
-
-  if (pending_event_ & SURFACE_CHANGE_EVENT_PENDING) {
-    // We should already requested a seek in this case.
-    pending_event_ &= ~SURFACE_CHANGE_EVENT_PENDING;
-  } else {
-    // Perform a seek so the new decoder can get the I-frame first.
-    pending_event_ |= SEEK_EVENT_PENDING;
-    ProcessPendingEvents();
-    return;
-  }
-
-  if (playing_)
-    StartInternal();
+  pending_event_ |= SURFACE_CHANGE_EVENT_PENDING;
+  // Request a seek so that the next decoder will decode an I-frame first.
+  // Or otherwise, MediaCodec might crash. See b/8950387.

[acolwell GONE FROM CHROMIUM, 2013/06/04 15:19:14]

The seek is needed because you have to create a new decoder for the new surface
right? If so, you should probably updated the comment to indicate something
along these lines because it isn't obvious here that the new surface triggers a
new decoder.

[qinmin, 2013/06/04 19:03:23]

Done.

+  pending_event_ |= SEEK_EVENT_PENDING;
+  ProcessPendingEvents();
 }
 
 void MediaSourcePlayer::Start() {
   playing_ = true;
-  if (HasAudio() && !audio_decoder_job_) {
-    audio_decoder_job_.reset(new AudioDecoderJob(
-        audio_codec_, sampling_rate_, num_channels_,
-        &audio_extra_data_[0], audio_extra_data_.size()));
-  }
 
-  if (HasVideo() && !video_decoder_job_) {
-    // StartInternal() will be delayed until SetVideoSurface() gets called.
-    return;
-  }
+  CreateMediaDecoderJobs(false);
 
   StartInternal();
 }
 
 void MediaSourcePlayer::Pause() {
   playing_ = false;
   start_wallclock_time_ = base::Time();
 }
 
 bool MediaSourcePlayer::IsPlaying() {
@@ skipping 22 matching lines @@
   return duration_;
 }
 
 void MediaSourcePlayer::Release() {
   ClearDecodingData();
   audio_decoder_job_.reset();
   video_decoder_job_.reset();
   active_decoding_tasks_ = 0;
   playing_ = false;
   pending_event_ = NO_EVENT_PENDING;
+  surface_ = gfx::ScopedJavaSurface();
   ReleaseMediaResourcesFromManager();
 }
 
 void MediaSourcePlayer::SetVolume(float leftVolume, float rightVolume) {
 }
 
 bool MediaSourcePlayer::CanPause() {
   return seekable_;
 }
 
 bool MediaSourcePlayer::CanSeekForward() {
   return seekable_;
 }
 
 bool MediaSourcePlayer::CanSeekBackward() {
   return seekable_;
 }
 
 bool MediaSourcePlayer::IsPlayerReady() {
   return audio_decoder_job_ || video_decoder_job_;
 }
 
 void MediaSourcePlayer::StartInternal() {
   // Do nothing if the decoders are already running.
   if (active_decoding_tasks_ > 0 || pending_event_ != NO_EVENT_PENDING)
     return;
 
+  // If one of the decoder job is not ready, do nothing.
+  if ((HasAudio() && !audio_decoder_job_) ||
+      (HasVideo() && !video_decoder_job_)) {
+    return;
+  }
+
   if (HasAudio()) {
     audio_finished_ = false;
     DecodeMoreAudio();
   }
   if (HasVideo()) {
     video_finished_ = false;
     DecodeMoreVideo();
   }
 }
 
 void MediaSourcePlayer::DemuxerReady(
     const MediaPlayerHostMsg_DemuxerReady_Params& params) {
   if (params.duration_ms == std::numeric_limits<int>::max())
     seekable_ = false;
   duration_ = base::TimeDelta::FromMilliseconds(params.duration_ms);
   width_ = params.video_size.width();
   height_ = params.video_size.height();
   num_channels_ = params.audio_channels;
   sampling_rate_ = params.audio_sampling_rate;
   audio_codec_ = params.audio_codec;
   video_codec_ = params.video_codec;
   audio_extra_data_ = params.audio_extra_data;
   OnMediaMetadataChanged(duration_, width_, height_, true);
+  if (pending_event_ & CONFIG_CHANGE_EVENT_PENDING) {
+    CreateMediaDecoderJobs(true);
+    pending_event_ &= ~CONFIG_CHANGE_EVENT_PENDING;
+    // If there is a pending surface change, we can merge it with the config
+    // change.
+    pending_event_ &= ~SURFACE_CHANGE_EVENT_PENDING;
+    if (playing_ && pending_event_ == NO_EVENT_PENDING)
+      StartInternal();
+  }
 }
 
 void MediaSourcePlayer::ReadFromDemuxerAck(
     const MediaPlayerHostMsg_ReadFromDemuxerAck_Params& params) {
   if (params.type == DemuxerStream::AUDIO)
     waiting_for_audio_data_ = false;
   else
     waiting_for_video_data_ = false;
 
   // If there is a pending seek request, ignore the data from the chunk demuxer.
   // The data will be requested later when OnSeekRequestAck() is called.
   if (pending_event_ & SEEK_EVENT_PENDING)
     return;
 
   if (params.type == DemuxerStream::AUDIO) {
     DCHECK_EQ(0u, audio_access_unit_index_);
     received_audio_ = params;
     if (!pending_event_)
       DecodeMoreAudio();
   } else {
     DCHECK_EQ(0u, video_access_unit_index_);
     received_video_ = params;
     if (!pending_event_)
       DecodeMoreVideo();
   }
 }
 
-void MediaSourcePlayer::OnSeekRequestAck() {
+void MediaSourcePlayer::OnSeekRequestAck(unsigned request_id) {

[acolwell GONE FROM CHROMIUM, 2013/06/04 15:19:14]

nit:s/request/seek_request/

[qinmin, 2013/06/04 19:03:23]

Done.

+  // Do nothing until the most recent seek request is processed.
+  if (seek_request_id_ != request_id)
+    return;
   pending_event_ &= ~SEEK_EVENT_PENDING;
   OnSeekComplete();
-  if (playing_)
-    StartInternal();
+  ProcessPendingEvents();
 }
 
 void MediaSourcePlayer::UpdateTimestamps(
     const base::TimeDelta& presentation_timestamp,
     const base::Time& wallclock_time) {
   last_presentation_timestamp_ = presentation_timestamp;
   OnTimeUpdated();
   if (start_wallclock_time_.is_null() && playing_) {
     start_wallclock_time_ = wallclock_time;
     start_presentation_timestamp_ = last_presentation_timestamp_;
   }
 }
 
 void MediaSourcePlayer::ProcessPendingEvents() {
-  // Wait for all the decoding jobs to finish before sending a seek request.
+  // Wait for all the decoding jobs to finish before processing pending tasks.
   if (active_decoding_tasks_ > 0)
     return;
 
-  DCHECK(pending_event_ != NO_EVENT_PENDING);
-  if (use_empty_surface_ && (pending_event_ & SURFACE_CHANGE_EVENT_PENDING)) {
-    video_decoder_job_.reset();
+  if (pending_event_ & SEEK_EVENT_PENDING) {
+    ClearDecodingData();
+    manager()->OnMediaSeekRequest(
+        player_id(), last_presentation_timestamp_, ++seek_request_id_);
+    return;
+  }
+
+  start_wallclock_time_ = base::Time();
+  if (pending_event_ & CONFIG_CHANGE_EVENT_PENDING) {
+    manager()->OnMediaConfigRequest(player_id());
+    return;
+  }
+
+  if (pending_event_ & SURFACE_CHANGE_EVENT_PENDING) {
+    CreateMediaDecoderJobs(false);
     pending_event_ &= ~SURFACE_CHANGE_EVENT_PENDING;
   }
 
-  ClearDecodingData();
-  manager()->OnMediaSeekRequest(player_id(),
-                                last_presentation_timestamp_,
-                                pending_event_ & SURFACE_CHANGE_EVENT_PENDING);
+  if (playing_ && pending_event_ == NO_EVENT_PENDING)
+    StartInternal();
 }
 
 void MediaSourcePlayer::MediaDecoderCallback(
     bool is_audio, bool decode_succeeded,
     const base::TimeDelta& presentation_timestamp,
     const base::Time& wallclock_time, bool end_of_stream) {
   if (active_decoding_tasks_ > 0)
     active_decoding_tasks_--;
 
+  if (is_audio && audio_decoder_job_)
+    audio_decoder_job_->OnDecodeCompleted();
+  if (!is_audio && video_decoder_job_)
+    video_decoder_job_->OnDecodeCompleted();
+
   if (!decode_succeeded) {
     Release();
     OnMediaError(MEDIA_ERROR_DECODE);
     return;
   }
 
   if (pending_event_ != NO_EVENT_PENDING) {
     ProcessPendingEvents();
     return;
   }
@@ skipping 19 matching lines @@
   if (audio_access_unit_index_ >= received_audio_.access_units.size()) {
     if (!waiting_for_audio_data_) {
       manager()->OnReadFromDemuxer(player_id(), DemuxerStream::AUDIO, true);
       received_audio_ = MediaPlayerHostMsg_ReadFromDemuxerAck_Params();
       audio_access_unit_index_ = 0;
       waiting_for_audio_data_ = true;
     }
     return;
   }
 
+  if (DemuxerStream::kConfigChanged ==
+      received_audio_.access_units[audio_access_unit_index_].status) {
+    // Wait for demuxer ready message.
+    pending_event_ |= CONFIG_CHANGE_EVENT_PENDING;
+    received_audio_ = MediaPlayerHostMsg_ReadFromDemuxerAck_Params();
+    audio_access_unit_index_ = 0;
+    ProcessPendingEvents();
+    return;
+  }
+
   audio_decoder_job_->Decode(
       received_audio_.access_units[audio_access_unit_index_],
       start_wallclock_time_, start_presentation_timestamp_,
       base::Bind(&MediaSourcePlayer::MediaDecoderCallback,
                  weak_this_.GetWeakPtr(), true));
   active_decoding_tasks_++;
   audio_access_unit_index_++;
 }
 
 void MediaSourcePlayer::DecodeMoreVideo() {
   if (video_access_unit_index_ >= received_video_.access_units.size()) {
     if (!waiting_for_video_data_) {
       manager()->OnReadFromDemuxer(player_id(), DemuxerStream::VIDEO, true);
       received_video_ = MediaPlayerHostMsg_ReadFromDemuxerAck_Params();
       video_access_unit_index_ = 0;
       waiting_for_video_data_ = true;
     }
     return;
   }
 
+  if (DemuxerStream::kConfigChanged ==
+      received_video_.access_units[video_access_unit_index_].status) {
+    // Wait for demuxer ready message.
+    pending_event_ |= CONFIG_CHANGE_EVENT_PENDING;
+    received_video_ = MediaPlayerHostMsg_ReadFromDemuxerAck_Params();
+    video_access_unit_index_ = 0;
+    ProcessPendingEvents();
+    return;
+  }
+
   video_decoder_job_->Decode(
       received_video_.access_units[video_access_unit_index_],
       start_wallclock_time_, start_presentation_timestamp_,
       base::Bind(&MediaSourcePlayer::MediaDecoderCallback,
                  weak_this_.GetWeakPtr(), false));
   active_decoding_tasks_++;
   video_access_unit_index_++;
 }
 
 
@@ skipping 23 matching lines @@
 }
 
 bool MediaSourcePlayer::HasVideo() {
   return kUnknownVideoCodec != video_codec_;
 }
 
 bool MediaSourcePlayer::HasAudio() {
   return kUnknownAudioCodec != audio_codec_;
 }
 
+void MediaSourcePlayer::CreateMediaDecoderJobs(bool config_change) {
+  DCHECK_EQ(0, active_decoding_tasks_);
+
+  // Create audio decoder job only if config changes.
+  if (HasAudio() && (config_change || !audio_decoder_job_)) {

[acolwell GONE FROM CHROMIUM, 2013/06/04 15:19:14]

I don't think this is quite right. Audio & Video config changes are not
necessarily always aligned. They can happen at different times. It looks like
this code resets both decoders here if it sees a config change from either
DemuxerStream. It should only reset the decoder for the stream the signalled the
change.

[qinmin, 2013/06/04 19:03:23]

added 2 new variable, |reconfig_audio_decoder_| and |reconfig_video_decoder_| to
indicate whether we need to recreate the audio/video decoder.

On 2013/06/04 15:19:14, acolwell wrote:

+    audio_decoder_job_.reset(new AudioDecoderJob(
+        audio_codec_, sampling_rate_, num_channels_,
+        &audio_extra_data_[0], audio_extra_data_.size()));
+  }
+
+  video_decoder_job_.reset();
+  if (!HasVideo() || surface_.IsSurfaceEmpty())
+    return;
+
+  // For video, the java surface might have changed even if config does not
+  // change. So always recreate the video decoder job.
+  video_decoder_job_.reset(new VideoDecoderJob(
+      video_codec_, gfx::Size(width_, height_), surface_.j_surface().obj()));
+
+  // Inform the fullscreen view the player is ready.
+  // TODO(qinmin): refactor MediaPlayerBridge so that we have a better way
+  // to inform ContentVideoView.
+  OnMediaMetadataChanged(duration_, width_, height_, true);
+}
+
 }  // namespace media