Refactor MSE implementation on Android to simplify the logic and improve the performance

media/base/android/audio_decoder_job.cc

 // Copyright 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/audio_decoder_job.h"
 
 #include "base/bind.h"
 #include "base/lazy_instance.h"
 #include "base/threading/thread.h"
 #include "media/base/android/media_codec_bridge.h"
@@ skipping 14 matching lines @@
     Start();
   }
 };
 
 // TODO(qinmin): Check if it is tolerable to use worker pool to handle all the
 // decoding tasks so that we don't need a global thread here.
 // http://crbug.com/245750
 base::LazyInstance<AudioDecoderThread>::Leaky
     g_audio_decoder_thread = LAZY_INSTANCE_INITIALIZER;
 
-AudioDecoderJob* AudioDecoderJob::Create(
-    const AudioCodec audio_codec,
-    int sample_rate,
-    int channel_count,
-    const uint8* extra_data,
-    size_t extra_data_size,
-    jobject media_crypto,
-    const base::Closure& request_data_cb) {
-  scoped_ptr<AudioCodecBridge> codec(AudioCodecBridge::Create(audio_codec));
-  if (codec && codec->Start(audio_codec, sample_rate, channel_count, extra_data,
-                            extra_data_size, true, media_crypto)) {
-    scoped_ptr<AudioTimestampHelper> audio_timestamp_helper(
-        new AudioTimestampHelper(sample_rate));
-    return new AudioDecoderJob(
-        audio_timestamp_helper.Pass(), codec.Pass(),
-        kBytesPerAudioOutputSample * channel_count, request_data_cb);
-  }
-  LOG(ERROR) << "Failed to create AudioDecoderJob.";
-  return NULL;
+AudioDecoderJob::AudioDecoderJob(
+    const base::Closure& request_data_cb,
+    const base::Closure& on_demuxer_config_changed_cb)
+    : MediaDecoderJob(g_audio_decoder_thread.Pointer()->message_loop_proxy(),
+                      request_data_cb,
+                      on_demuxer_config_changed_cb),
+      audio_codec_(kUnknownAudioCodec),
+      num_channels_(0),
+      sampling_rate_(0),
+      volume_(-1.0),
+      bytes_per_frame_(0) {
 }
 
-AudioDecoderJob::AudioDecoderJob(
-    scoped_ptr<AudioTimestampHelper> audio_timestamp_helper,
-    scoped_ptr<AudioCodecBridge> audio_codec_bridge,
-    int bytes_per_frame,
-    const base::Closure& request_data_cb)
-    : MediaDecoderJob(g_audio_decoder_thread.Pointer()->message_loop_proxy(),
-                      audio_codec_bridge.get(), request_data_cb),
-      bytes_per_frame_(bytes_per_frame),
-      audio_codec_bridge_(audio_codec_bridge.Pass()),
-      audio_timestamp_helper_(audio_timestamp_helper.Pass()) {
-}
+AudioDecoderJob::~AudioDecoderJob() {}
 
-AudioDecoderJob::~AudioDecoderJob() {
+bool AudioDecoderJob::HasStream() const {
+  return audio_codec_ != kUnknownAudioCodec;
 }
 
 void AudioDecoderJob::SetVolume(double volume) {
-  audio_codec_bridge_->SetVolume(volume);
+  volume_ = volume;
+  SetVolumeInternal();
 }
 
 void AudioDecoderJob::SetBaseTimestamp(base::TimeDelta base_timestamp) {
-  audio_timestamp_helper_->SetBaseTimestamp(base_timestamp);
+  DCHECK(!is_decoding());
+  base_timestamp_ = base_timestamp;
+  if (audio_timestamp_helper_)
+    audio_timestamp_helper_->SetBaseTimestamp(base_timestamp_);
 }
 
 void AudioDecoderJob::ReleaseOutputBuffer(
     int output_buffer_index,
     size_t size,
     bool render_output,
     base::TimeDelta current_presentation_timestamp,
     const ReleaseOutputCompletionCallback& callback) {
   render_output = render_output && (size != 0u);
   if (render_output) {
-    int64 head_position = audio_codec_bridge_->PlayOutputBuffer(
-        output_buffer_index, size);
-    audio_timestamp_helper_->AddFrames(size / (bytes_per_frame_));
+    int64 head_position =(static_cast<AudioCodecBridge*>(
+        media_codec_bridge_.get()))->PlayOutputBuffer(
+            output_buffer_index, size);
+    audio_timestamp_helper_->AddFrames(size / bytes_per_frame_);
     int64 frames_to_play =
         audio_timestamp_helper_->frame_count() - head_position;
     DCHECK_GE(frames_to_play, 0);
     current_presentation_timestamp =
         audio_timestamp_helper_->GetTimestamp() -
         audio_timestamp_helper_->GetFrameDuration(frames_to_play);
   } else {
     current_presentation_timestamp = kNoTimestamp();
   }
-  audio_codec_bridge_->ReleaseOutputBuffer(output_buffer_index, false);
+  media_codec_bridge_->ReleaseOutputBuffer(output_buffer_index, false);
+
   callback.Run(current_presentation_timestamp,
                audio_timestamp_helper_->GetTimestamp());
 }
 
 bool AudioDecoderJob::ComputeTimeToRender() const {
   return false;
 }
 
+void AudioDecoderJob::UpdateDemuxerConfigs(const DemuxerConfigs& configs) {
+  audio_codec_ = configs.audio_codec;
+  num_channels_ = configs.audio_channels;
+  sampling_rate_ = configs.audio_sampling_rate;
+  set_is_content_encrypted(configs.is_audio_encrypted);

[xhwang, 2014/05/30 21:58:07]

Why |is_audio_encrypted| is special? 

Actually, can we just store a DemuxerConfigs in this class, instead all of it's
members? See
https://code.google.com/p/chromium/codesearch#chromium/src/media/base/video_d...

hmm, I see. DemuxerConfigs contains both audio and video configs... This is a
bit confusing. We probably should clean this up in a separate CL.

[qinmin, 2014/05/30 23:43:33]

is_content_encrypted_ is private in base class, so a set_is_content_encrypted_()
is needed. Other variables are not inherited.

ok, added a TODO here to split audio and video configs into 2 stuctures.

On 2014/05/30 21:58:07, xhwang wrote:

+  audio_extra_data_ = configs.audio_extra_data;
+  bytes_per_frame_ = kBytesPerAudioOutputSample * num_channels_;
+}
+
+bool AudioDecoderJob::IsDemuxerConfigChanged(
+    const DemuxerConfigs& configs) const {
+  return audio_codec_ != configs.audio_codec ||
+     num_channels_ != configs.audio_channels ||
+     sampling_rate_ != configs.audio_sampling_rate ||
+     is_content_encrypted() != configs.is_audio_encrypted ||
+     audio_extra_data_.size() != configs.audio_extra_data.size() ||
+     !std::equal(audio_extra_data_.begin(),
+                 audio_extra_data_.end(),
+                 configs.audio_extra_data.begin());

[xhwang, 2014/05/30 21:58:07]

If we separate DemuxerConfigs into AudioConfig and VideoConfig, we can just have
a Matches() helper function to do this, similar to:
https://code.google.com/p/chromium/codesearch#chromium/src/media/base/video_d...

+}
+
+bool AudioDecoderJob::CreateMediaCodecBridgeInternal() {
+  media_codec_bridge_.reset(AudioCodecBridge::Create(audio_codec_));
+  if (!media_codec_bridge_)
+    return false;
+
+  if (!(static_cast<AudioCodecBridge*>(media_codec_bridge_.get()))->Start(
+      audio_codec_, sampling_rate_, num_channels_, &audio_extra_data_[0],
+      audio_extra_data_.size(), true, GetMediaCrypto().obj())) {
+    media_codec_bridge_.reset();
+    return false;
+  }
+
+  SetVolumeInternal();
+
+  // Need to pass the base timestamp to the new decoder.
+  if (audio_timestamp_helper_)
+    base_timestamp_ = audio_timestamp_helper_->GetTimestamp();
+  audio_timestamp_helper_.reset(new AudioTimestampHelper(sampling_rate_));
+  audio_timestamp_helper_->SetBaseTimestamp(base_timestamp_);
+  return true;
+}
+
+void AudioDecoderJob::SetVolumeInternal() {
+  if (media_codec_bridge_) {
+    static_cast<AudioCodecBridge*>(media_codec_bridge_.get())->SetVolume(
+        volume_);
+  }
+}
+
 }  // namespace media