Remove the IPC to request DemuxerConfigs when config changes

When config changes, we currently use a round trip IPC to request the new config.
This increases the overall latency when processing config changes.
This CL appends the new config data to the DemuxerData which contains the config change access unit.

BUG=325528

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@269614 0039d316-1c4b-4281-b951-d872f2087c98

content/browser/media/android/browser_demuxer_android.cc

@@ -26,11 +26,6 @@ class BrowserDemuxerAndroid::Internal : public media::DemuxerAndroid {
     demuxer_->AddDemuxerClient(demuxer_client_id_, client);
   }
 
-  virtual void RequestDemuxerConfigs() OVERRIDE {
-    DCHECK(ClientIDExists()) << demuxer_client_id_;
-    demuxer_->Send(new MediaPlayerMsg_MediaConfigRequest(demuxer_client_id_));
-  }
-
   virtual void RequestDemuxerData(media::DemuxerStream::Type type) OVERRIDE {
     DCHECK(ClientIDExists()) << demuxer_client_id_;
     demuxer_->Send(new MediaPlayerMsg_ReadFromDemuxer(

content/common/media/media_player_messages_android.h

@@ -42,6 +42,7 @@ IPC_STRUCT_TRAITS_END()
 IPC_STRUCT_TRAITS_BEGIN(media::DemuxerData)
   IPC_STRUCT_TRAITS_MEMBER(type)
   IPC_STRUCT_TRAITS_MEMBER(access_units)
+  IPC_STRUCT_TRAITS_MEMBER(demuxer_configs)
 IPC_STRUCT_TRAITS_END()
 
 IPC_STRUCT_TRAITS_BEGIN(media::AccessUnit)
@@ -175,10 +176,6 @@ IPC_MESSAGE_CONTROL2(MediaPlayerMsg_ReadFromDemuxer,
                      int /* demuxer_client_id */,
                      media::DemuxerStream::Type /* type */)
 
-// The player needs new config data
-IPC_MESSAGE_CONTROL1(MediaPlayerMsg_MediaConfigRequest,
-                     int /* demuxer_client_id */)
-
 // Clank has connected to the remote device.
 IPC_MESSAGE_ROUTED2(MediaPlayerMsg_ConnectedToRemoteDevice,
                     int /* player_id */,

content/renderer/media/android/media_source_delegate.cc

@@ -398,14 +398,11 @@ void MediaSourceDelegate::OnBufferReady(
       break;
 
     case DemuxerStream::kConfigChanged:
-      // In case of kConfigChanged, need to read decoder_config once
-      // for the next reads.
-      // TODO(kjyoun): Investigate if we need to use this new config. See
-      // http://crbug.com/255783
-      if (is_audio) {
-        audio_stream_->audio_decoder_config();
-      } else {
-        gfx::Size size = video_stream_->video_decoder_config().coded_size();
+      CHECK((is_audio && audio_stream_) || (!is_audio && video_stream_));
+      data->demuxer_configs.resize(1);
+      CHECK(GetDemuxerConfigFromStream(&data->demuxer_configs[0], is_audio));
+      if (!is_audio) {
+        gfx::Size size = data->demuxer_configs[0].video_size;
         DVLOG(1) << "Video config is changed: " << size.width() << "x"
                  << size.height();
       }
@@ -649,13 +646,6 @@ void MediaSourceDelegate::DeleteSelf() {
   delete this;
 }
 
-void MediaSourceDelegate::OnMediaConfigRequest() {
-  DCHECK(media_loop_->BelongsToCurrentThread());
-  DVLOG(1) << __FUNCTION__ << " : " << demuxer_client_id_;
-  if (CanNotifyDemuxerReady())
-    NotifyDemuxerReady();
-}
-
 bool MediaSourceDelegate::CanNotifyDemuxerReady() {
   DCHECK(media_loop_->BelongsToCurrentThread());
   return is_demuxer_ready_;
@@ -667,24 +657,8 @@ void MediaSourceDelegate::NotifyDemuxerReady() {
   DCHECK(CanNotifyDemuxerReady());
 
   scoped_ptr<DemuxerConfigs> configs(new DemuxerConfigs());
-  if (audio_stream_) {
-    media::AudioDecoderConfig config = audio_stream_->audio_decoder_config();
-    configs->audio_codec = config.codec();
-    configs->audio_channels =
-        media::ChannelLayoutToChannelCount(config.channel_layout());
-    configs->audio_sampling_rate = config.samples_per_second();
-    configs->is_audio_encrypted = config.is_encrypted();
-    configs->audio_extra_data = std::vector<uint8>(
-        config.extra_data(), config.extra_data() + config.extra_data_size());
-  }
-  if (video_stream_) {
-    media::VideoDecoderConfig config = video_stream_->video_decoder_config();
-    configs->video_codec = config.codec();
-    configs->video_size = config.natural_size();
-    configs->is_video_encrypted = config.is_encrypted();
-    configs->video_extra_data = std::vector<uint8>(
-        config.extra_data(), config.extra_data() + config.extra_data_size());
-  }
+  GetDemuxerConfigFromStream(configs.get(), true);
+  GetDemuxerConfigFromStream(configs.get(), false);
   configs->duration = GetDuration();
 
   if (demuxer_client_)
@@ -770,4 +744,32 @@ base::TimeDelta MediaSourceDelegate::FindBufferedBrowserSeekTime_Locked(
   return seek_time;
 }
 
+bool MediaSourceDelegate::GetDemuxerConfigFromStream(
+    media::DemuxerConfigs* configs, bool is_audio) {
+  DCHECK(media_loop_->BelongsToCurrentThread());
+  if (!CanNotifyDemuxerReady())
+    return false;
+  if (is_audio && audio_stream_) {
+    media::AudioDecoderConfig config = audio_stream_->audio_decoder_config();
+    configs->audio_codec = config.codec();
+    configs->audio_channels =
+        media::ChannelLayoutToChannelCount(config.channel_layout());
+    configs->audio_sampling_rate = config.samples_per_second();
+    configs->is_audio_encrypted = config.is_encrypted();
+    configs->audio_extra_data = std::vector<uint8>(
+        config.extra_data(), config.extra_data() + config.extra_data_size());
+    return true;
+  }
+  if (!is_audio && video_stream_) {
+    media::VideoDecoderConfig config = video_stream_->video_decoder_config();
+    configs->video_codec = config.codec();
+    configs->video_size = config.natural_size();
+    configs->is_video_encrypted = config.is_encrypted();
+    configs->video_extra_data = std::vector<uint8>(
+        config.extra_data(), config.extra_data() + config.extra_data_size());
+    return true;
+  }
+  return false;
+}
+
 }  // namespace content

content/renderer/media/android/media_source_delegate.h

@@ -97,9 +97,6 @@ class MediaSourceDelegate : public media::DemuxerHost {
   // Called when DemuxerStreamPlayer needs to read data from ChunkDemuxer.
   void OnReadFromDemuxer(media::DemuxerStream::Type type);
 
-  // Called when the player needs the new config data from ChunkDemuxer.
-  void OnMediaConfigRequest();
-
   // Called by the Destroyer to destroy an instance of this object.
   void Destroy();
 
@@ -183,6 +180,11 @@ class MediaSourceDelegate : public media::DemuxerHost {
   base::TimeDelta FindBufferedBrowserSeekTime_Locked(
       const base::TimeDelta& seek_time) const;
 
+  // Get the demuxer configs for a particular stream identified by |is_audio|.
+  // Returns true on success, of false otherwise.
+  bool GetDemuxerConfigFromStream(media::DemuxerConfigs* configs,
+                                  bool is_audio);
+
   RendererDemuxerAndroid* demuxer_client_;
   int demuxer_client_id_;
 

content/renderer/media/android/renderer_demuxer_android.cc

@@ -43,7 +43,6 @@ bool RendererDemuxerAndroid::OnMessageReceived(const IPC::Message& message) {
   switch (message.type()) {
     case MediaPlayerMsg_DemuxerSeekRequest::ID:
     case MediaPlayerMsg_ReadFromDemuxer::ID:
-    case MediaPlayerMsg_MediaConfigRequest::ID:
       media_message_loop_->PostTask(FROM_HERE, base::Bind(
           &RendererDemuxerAndroid::DispatchMessage, this, message));
       return true;
@@ -82,7 +81,6 @@ void RendererDemuxerAndroid::DispatchMessage(const IPC::Message& message) {
   IPC_BEGIN_MESSAGE_MAP(RendererDemuxerAndroid, message)
     IPC_MESSAGE_HANDLER(MediaPlayerMsg_DemuxerSeekRequest, OnDemuxerSeekRequest)
     IPC_MESSAGE_HANDLER(MediaPlayerMsg_ReadFromDemuxer, OnReadFromDemuxer)
-    IPC_MESSAGE_HANDLER(MediaPlayerMsg_MediaConfigRequest, OnMediaConfigRequest)
   IPC_END_MESSAGE_MAP()
 }
 
@@ -103,10 +101,4 @@ void RendererDemuxerAndroid::OnDemuxerSeekRequest(
     delegate->Seek(time_to_seek, is_browser_seek);
 }
 
-void RendererDemuxerAndroid::OnMediaConfigRequest(int demuxer_client_id) {
-  MediaSourceDelegate* delegate = delegates_.Lookup(demuxer_client_id);
-  if (delegate)
-    delegate->OnMediaConfigRequest();
-}
-
 }  // namespace content

content/renderer/media/android/renderer_demuxer_android.h

@@ -70,7 +70,6 @@ class RendererDemuxerAndroid : public IPC::MessageFilter {
   void OnDemuxerSeekRequest(int demuxer_client_id,
                             const base::TimeDelta& time_to_seek,
                             bool is_browser_seek);
-  void OnMediaConfigRequest(int demuxer_client_id);
 
   base::AtomicSequenceNumber next_demuxer_client_id_;
 

media/base/android/demuxer_android.h

@@ -25,9 +25,6 @@ class MEDIA_EXPORT DemuxerAndroid {
   // Must be called prior to calling any other methods.
   virtual void Initialize(DemuxerAndroidClient* client) = 0;
 
-  // Called to request the current audio/video decoder configurations.
-  virtual void RequestDemuxerConfigs() = 0;
-
   // Called to request additional data from the demuxer.
   virtual void RequestDemuxerData(media::DemuxerStream::Type type) = 0;
 
@@ -44,12 +41,7 @@ class MEDIA_EXPORT DemuxerAndroid {
 // Defines the client callback interface.
 class MEDIA_EXPORT DemuxerAndroidClient {
  public:
-  // Called in response to RequestDemuxerConfigs() and also when the demuxer has
-  // initialized.
-  //
-  // TODO(scherkus): Perhaps clients should be required to call
-  // RequestDemuxerConfigs() to initialize themselves instead of the demuxer
-  // calling this method without being prompted.
+  // Called when the demuxer has initialized.
   virtual void OnDemuxerConfigsAvailable(const DemuxerConfigs& params) = 0;
 
   // Called in response to RequestDemuxerData().

media/base/android/demuxer_stream_player_params.h

@@ -54,6 +54,12 @@ struct MEDIA_EXPORT DemuxerData {
 
   DemuxerStream::Type type;
   std::vector<AccessUnit> access_units;
+  // If the last entry in |access_units| has a status equal to |kConfigChanged|,
+  // a corresponding DemuxerConfigs is added into this vector. The
+  // DemuxerConfigs should only contain information of the stream that is
+  // specified by |type|. This solves the issue that we need multiple IPCs when
+  // demuxer configs change.
+  std::vector<DemuxerConfigs> demuxer_configs;
 };
 
 };  // namespace media

media/base/android/media_decoder_job.cc

@@ -102,7 +102,7 @@ void MediaDecoderJob::Prefetch(const base::Closure& prefetch_cb) {
   RequestData(prefetch_cb);
 }
 
-bool MediaDecoderJob::Decode(
+scoped_ptr<DemuxerConfigs> MediaDecoderJob::Decode(
     base::TimeTicks start_time_ticks,
     base::TimeDelta start_presentation_timestamp,
     const DecoderCallback& callback) {
@@ -117,18 +117,19 @@ bool MediaDecoderJob::Decode(
                            base::Unretained(this),
                            start_time_ticks,
                            start_presentation_timestamp));
-    return true;
+    return scoped_ptr<DemuxerConfigs>();
   }
 
   if (DemuxerStream::kConfigChanged == CurrentAccessUnit().status) {
-    // Clear received data because we need to handle a config change.
     decode_cb_.Reset();
-    ClearData();
-    return false;
+    size_t index = CurrentReceivedDataChunkIndex();
+    CHECK_EQ(1u, received_data_[index].demuxer_configs.size());
+    return scoped_ptr<DemuxerConfigs>(new DemuxerConfigs(
+        received_data_[index].demuxer_configs[0]));
   }
 
   DecodeCurrentAccessUnit(start_time_ticks, start_presentation_timestamp);
-  return true;
+  return scoped_ptr<DemuxerConfigs>();
 }
 
 void MediaDecoderJob::StopDecode() {
@@ -462,11 +463,15 @@ void MediaDecoderJob::OnDecodeCompleted(
 const AccessUnit& MediaDecoderJob::CurrentAccessUnit() const {
   DCHECK(ui_task_runner_->BelongsToCurrentThread());
   DCHECK(HasData());
-  int index = NoAccessUnitsRemainingInChunk(true) ?
-      inactive_demuxer_data_index() : current_demuxer_data_index_;
+  size_t index = CurrentReceivedDataChunkIndex();
   return received_data_[index].access_units[access_unit_index_[index]];
 }
 
+size_t MediaDecoderJob::CurrentReceivedDataChunkIndex() const {
+  return NoAccessUnitsRemainingInChunk(true) ?
+      inactive_demuxer_data_index() : current_demuxer_data_index_;
+}
+
 bool MediaDecoderJob::NoAccessUnitsRemainingInChunk(
     bool is_active_chunk) const {
   DCHECK(ui_task_runner_->BelongsToCurrentThread());

media/base/android/media_decoder_job.h

@@ -54,11 +54,14 @@ 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 a config change is needed. |callback| is ignored
-  // and will not be called.
-  bool Decode(base::TimeTicks start_time_ticks,
+  // Returns a scoped pointer to a DemuxerConfig if a config change is detected,
+  // or an empty scoped pointer otherwise. In the case of requiring further data
+  // before commencing decode, an empty scoped pointer will also be returned
+  // although config change may be the next received access unit. |callback|
+  // will be called when the decode operation is complete. If a config change
+  // is detected, |callback| is ignored and will not be called.
+  scoped_ptr<DemuxerConfigs> Decode(
+      base::TimeTicks start_time_ticks,
       base::TimeDelta start_presentation_timestamp,
       const DecoderCallback& callback);
 
@@ -147,6 +150,9 @@ class MediaDecoderJob {
   // Helper function to get the current access unit that is being decoded.
   const AccessUnit& CurrentAccessUnit() const;
 
+  // Helper function to get the current data chunk index that is being decoded.
+  size_t CurrentReceivedDataChunkIndex() const;
+
   // Check whether a chunk has no remaining access units to decode. If
   // |is_active_chunk| is true, this function returns whether decoder has
   // consumed all data in |received_data_[current_demuxer_data_index_]|.

media/base/android/media_source_player.cc

@@ -19,7 +19,7 @@
 #include "media/base/android/media_drm_bridge.h"
 #include "media/base/android/media_player_manager.h"
 #include "media/base/android/video_decoder_job.h"
-#include "media/base/buffers.h"
+
 
 namespace media {
 
@@ -297,40 +297,21 @@ void MediaSourcePlayer::StartInternal() {
 void MediaSourcePlayer::OnDemuxerConfigsAvailable(
     const DemuxerConfigs& configs) {
   DVLOG(1) << __FUNCTION__;
+  DCHECK(!HasAudio() && !HasVideo());
   duration_ = configs.duration;
   clock_.SetDuration(duration_);
 
-  audio_codec_ = configs.audio_codec;
-  num_channels_ = configs.audio_channels;
-  sampling_rate_ = configs.audio_sampling_rate;
-  is_audio_encrypted_ = configs.is_audio_encrypted;
-  audio_extra_data_ = configs.audio_extra_data;
-  video_codec_ = configs.video_codec;
-  width_ = configs.video_size.width();
-  height_ = configs.video_size.height();
-  is_video_encrypted_ = configs.is_video_encrypted;
+  SetDemuxerConfigs(configs, true);
+  SetDemuxerConfigs(configs, false);
 
   manager()->OnMediaMetadataChanged(
       player_id(), duration_, width_, height_, true);
-
-  if (IsEventPending(CONFIG_CHANGE_EVENT_PENDING)) {
-    if (reconfig_audio_decoder_)
-      ConfigureAudioDecoderJob();
-
-    if (reconfig_video_decoder_)
-      ConfigureVideoDecoderJob();
-
-    ClearPendingEvent(CONFIG_CHANGE_EVENT_PENDING);
-
-    // Resume decoding after the config change if we are still playing.
-    if (playing_)
-      StartInternal();
-  }
 }
 
 void MediaSourcePlayer::OnDemuxerDataAvailable(const DemuxerData& data) {
   DVLOG(1) << __FUNCTION__ << "(" << data.type << ")";
   DCHECK_LT(0u, data.access_units.size());
+  CHECK_GE(1u, data.demuxer_configs.size());
 
   if (has_pending_audio_data_request_ && data.type == DemuxerStream::AUDIO) {
     has_pending_audio_data_request_ = false;
@@ -496,8 +477,16 @@ void MediaSourcePlayer::ProcessPendingEvents() {
   if (IsEventPending(CONFIG_CHANGE_EVENT_PENDING)) {
     DVLOG(1) << __FUNCTION__ << " : Handling CONFIG_CHANGE_EVENT.";
     DCHECK(reconfig_audio_decoder_ || reconfig_video_decoder_);
-    demuxer_->RequestDemuxerConfigs();
-    return;
+    manager()->OnMediaMetadataChanged(
+         player_id(), duration_, width_, height_, true);
+
+    if (reconfig_audio_decoder_)
+      ConfigureAudioDecoderJob();
+
+    if (reconfig_video_decoder_)
+      ConfigureVideoDecoderJob();
+
+    ClearPendingEvent(CONFIG_CHANGE_EVENT_PENDING);
   }
 
   if (IsEventPending(SURFACE_CHANGE_EVENT_PENDING)) {
@@ -524,7 +513,6 @@ void MediaSourcePlayer::ProcessPendingEvents() {
 
     DCHECK(audio_decoder_job_ || AudioFinished());
     DCHECK(video_decoder_job_ || VideoFinished());
-
     int count = (AudioFinished() ? 0 : 1) + (VideoFinished() ? 0 : 1);
 
     // It is possible that all streams have finished decode, yet starvation
@@ -649,12 +637,13 @@ void MediaSourcePlayer::MediaDecoderCallback(
       start_time_ticks_ = base::TimeTicks::Now();
   }
 
-  if (is_audio) {
+  if (is_audio)
     DecodeMoreAudio();
-    return;
-  }
-
+  else
     DecodeMoreVideo();
+
+  if (IsEventPending(CONFIG_CHANGE_EVENT_PENDING))
+    ProcessPendingEvents();
 }
 
 void MediaSourcePlayer::DecodeMoreAudio() {
@@ -662,21 +651,22 @@ void MediaSourcePlayer::DecodeMoreAudio() {
   DCHECK(!audio_decoder_job_->is_decoding());
   DCHECK(!AudioFinished());
 
-  if (audio_decoder_job_->Decode(
+  scoped_ptr<DemuxerConfigs> configs(audio_decoder_job_->Decode(
       start_time_ticks_,
       start_presentation_timestamp_,
       base::Bind(&MediaSourcePlayer::MediaDecoderCallback,
                  weak_factory_.GetWeakPtr(),
-                     true))) {
+                 true)));
+  if (!configs) {
     TRACE_EVENT_ASYNC_BEGIN0("media", "MediaSourcePlayer::DecodeMoreAudio",
                              audio_decoder_job_.get());
     return;
   }
 
   // Failed to start the next decode.
-  // Wait for demuxer ready message.
   DCHECK(!reconfig_audio_decoder_);
   reconfig_audio_decoder_ = true;
+  SetDemuxerConfigs(*configs, true);
 
   // Config change may have just been detected on the other stream. If so,
   // don't send a duplicate demuxer config request.
@@ -686,7 +676,6 @@ void MediaSourcePlayer::DecodeMoreAudio() {
   }
 
   SetPendingEvent(CONFIG_CHANGE_EVENT_PENDING);
-  ProcessPendingEvents();
 }
 
 void MediaSourcePlayer::DecodeMoreVideo() {
@@ -694,26 +683,26 @@ void MediaSourcePlayer::DecodeMoreVideo() {
   DCHECK(!video_decoder_job_->is_decoding());
   DCHECK(!VideoFinished());
 
-  if (video_decoder_job_->Decode(
+  scoped_ptr<DemuxerConfigs> configs(video_decoder_job_->Decode(
       start_time_ticks_,
       start_presentation_timestamp_,
       base::Bind(&MediaSourcePlayer::MediaDecoderCallback,
                  weak_factory_.GetWeakPtr(),
-                     false))) {
+                 false)));
+  if (!configs) {
     TRACE_EVENT_ASYNC_BEGIN0("media", "MediaSourcePlayer::DecodeMoreVideo",
                              video_decoder_job_.get());
     return;
   }
 
   // Failed to start the next decode.
-  // Wait for demuxer ready message.
-
   // After this detection of video config change, next video data received
   // will begin with I-frame.
   next_video_data_is_iframe_ = true;
 
   DCHECK(!reconfig_video_decoder_);
   reconfig_video_decoder_ = true;
+  SetDemuxerConfigs(*configs, false);
 
   // Config change may have just been detected on the other stream. If so,
   // don't send a duplicate demuxer config request.
@@ -723,7 +712,6 @@ void MediaSourcePlayer::DecodeMoreVideo() {
   }
 
   SetPendingEvent(CONFIG_CHANGE_EVENT_PENDING);
-  ProcessPendingEvents();
 }
 
 void MediaSourcePlayer::PlaybackCompleted(bool is_audio) {
@@ -974,6 +962,9 @@ void MediaSourcePlayer::OnPrefetchDone() {
 
   if (!VideoFinished())
     DecodeMoreVideo();
+
+  if (IsEventPending(CONFIG_CHANGE_EVENT_PENDING))
+    ProcessPendingEvents();
 }
 
 const char* MediaSourcePlayer::GetEventName(PendingEventFlags event) {
@@ -1014,4 +1005,20 @@ void MediaSourcePlayer::ClearPendingEvent(PendingEventFlags event) {
   pending_event_ &= ~event;
 }
 
+void MediaSourcePlayer::SetDemuxerConfigs(const DemuxerConfigs& configs,
+                                          bool is_audio) {
+  if (is_audio) {
+    audio_codec_ = configs.audio_codec;
+    num_channels_ = configs.audio_channels;
+    sampling_rate_ = configs.audio_sampling_rate;
+    is_audio_encrypted_ = configs.is_audio_encrypted;
+    audio_extra_data_ = configs.audio_extra_data;
+  } else {
+    video_codec_ = configs.video_codec;
+    width_ = configs.video_size.width();
+    height_ = configs.video_size.height();
+    is_video_encrypted_ = configs.is_video_encrypted;
+  }
+}
+
 }  // namespace media

media/base/android/media_source_player.h

@@ -168,6 +168,9 @@ class MEDIA_EXPORT MediaSourcePlayer : public MediaPlayerAndroid,
   // resync with audio and starts decoding.
   void OnPrefetchDone();
 
+  // Sets the demuxer configs for audio or video stream.
+  void SetDemuxerConfigs(const DemuxerConfigs& configs, bool is_audio);
+
   // Test-only method to setup hook for the completion of the next decode cycle.
   // This callback state is cleared when it is next run.
   // Prevent usage creep by only calling this from the