Android: Propagate sample rate change to audio decoder job

media/base/android/media_decoder_job.cc

Index: media/base/android/media_decoder_job.cc
diff --git a/media/base/android/media_decoder_job.cc b/media/base/android/media_decoder_job.cc
index d96785e2121de96e6162c280da8ee3f6ef6dbdbf..115d0096c9d53e0064b7134372acb5466b2b5ffc 100644
--- a/media/base/android/media_decoder_job.cc
+++ b/media/base/android/media_decoder_job.cc
@@ -445,8 +445,13 @@ void MediaDecoderJob::DecodeInternal(
   // TODO(xhwang/qinmin): This logic is correct but strange. Clean it up.
   if (output_eos_encountered_)
     status = MEDIA_CODEC_OUTPUT_END_OF_STREAM;
-  else if (has_format_change)
-    status = MEDIA_CODEC_OUTPUT_FORMAT_CHANGED;
+  else if (has_format_change) {

[qinmin, 2015/01/12 23:57:15]

This is on the decoder thread, you should never call updateOutputFormat() here
as it updates the variables belongs to the main thread.

Instead, you should handle the format change when OnDecodeCompleted() is called.

[kjoswiak, 2015/01/13 00:14:30]

Ahh, hmm. See my reply to Jesse's comment in patch 1 as to why I did this.
Instead, what if I added status as a parameter to
MediaDecoderJob::ReleaseOutputBuffer, and did this logic there?

[kjoswiak, 2015/01/13 00:39:42]

Looking closer, this fix wouldn't change anything.

However, I'm also not sure I understand where the issue is. What main / ui
thread variables are we editing? From what I see, the only variables we end up
changing belong to the MediaDecoderJob (or more accurately its subclasses), so I
would think it should run on MediaDecoderJob's thread, and isn't that
decoder_task_runner_ / what we are on?

[gunsch, 2015/01/13 00:45:31]

Kyle: check the first line of each function. Some are executed on
decoder_task_runner_ (decoder thread), some are executed on ui_task_runner_
(browser main thread).

[qinmin, 2015/01/13 00:46:51]

you are touching variables like time_stamp_helper, output_width_, output_height_
on the decoder thread.
MediaDecoder runs on 2 thread, ui_task_runner and decoder_task_runner_.
Some of the variables (format, timestamp, etc) should be maintained on the ui
thread, while others are in the decoder thread.

[kjoswiak, 2015/01/13 01:05:58]

Hmm I see. We use audio_timestamp_helper_ in ReleaseOutputBuffer, which runs on
decoder thread. Is the fact this object can be changed by ui thread while we are
feeding it not a concern? Shouldn't we update it in the thread that uses it?

+    status = UpdateOutputFormat();
+    if (status == MEDIA_CODEC_ERROR) {
+      callback.Run(status, kNoTimestamp(), kNoTimestamp());
+      return;
+    }
+  }
 
   bool render_output  = presentation_timestamp >= preroll_timestamp_ &&
       (status != MEDIA_CODEC_OUTPUT_END_OF_STREAM || size != 0u);
@@ -543,8 +548,7 @@ void MediaDecoderJob::OnDecodeCompleted(
   }
 
   if (status == MEDIA_CODEC_OUTPUT_FORMAT_CHANGED) {
-    if (UpdateOutputFormat())
-      config_changed_cb_.Run();
+    config_changed_cb_.Run();
     status = MEDIA_CODEC_OK;
   }
 
@@ -649,10 +653,6 @@ bool MediaDecoderJob::IsCodecReconfigureNeeded(
   return true;
 }
 
-bool MediaDecoderJob::UpdateOutputFormat() {
-  return false;
-}
-
 void MediaDecoderJob::ReleaseMediaCodecBridge() {
   if (!media_codec_bridge_)
     return;