Android: Propagate sample rate change to audio decoder job

media/base/android/media_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/media_decoder_job.h"
 
 #include "base/bind.h"
 #include "base/callback_helpers.h"
 #include "base/debug/trace_event.h"
 #include "base/message_loop/message_loop_proxy.h"
@@ skipping 427 matching lines @@
            status != MEDIA_CODEC_DEQUEUE_OUTPUT_AGAIN_LATER);
 
   if (status != MEDIA_CODEC_OK) {
     callback.Run(status, kNoTimestamp(), kNoTimestamp());
     return;
   }
 
   // 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);
   base::TimeDelta time_to_render;
   DCHECK(!start_time_ticks.is_null());
   if (render_output && ComputeTimeToRender()) {
     time_to_render = presentation_timestamp - (base::TimeTicks::Now() -
         start_time_ticks + start_presentation_timestamp);
   }
 
@@ skipping 76 matching lines @@
       DCHECK(false) << "Invalid output status";
       break;
   };
 
   if (status == MEDIA_CODEC_OUTPUT_END_OF_STREAM && drain_decoder_) {
     OnDecoderDrained();
     status = MEDIA_CODEC_OK;
   }
 
   if (status == MEDIA_CODEC_OUTPUT_FORMAT_CHANGED) {
-    if (UpdateOutputFormat())
-      config_changed_cb_.Run();
+    config_changed_cb_.Run();
     status = MEDIA_CODEC_OK;
   }
 
   if (release_resources_pending_) {
     ReleaseMediaCodecBridge();
     release_resources_pending_ = false;
     if (drain_decoder_)
       OnDecoderDrained();
   }
 
@@ skipping 84 matching lines @@
   return CreateMediaCodecBridgeInternal();
 }
 
 bool MediaDecoderJob::IsCodecReconfigureNeeded(
     const DemuxerConfigs& configs) const {
   if (!AreDemuxerConfigsChanged(configs))
     return false;
   return true;
 }
 
-bool MediaDecoderJob::UpdateOutputFormat() {
-  return false;
-}
-
 void MediaDecoderJob::ReleaseMediaCodecBridge() {
   if (!media_codec_bridge_)
     return;
 
   media_codec_bridge_.reset();
   input_buf_index_ = -1;
 }
 
 }  // namespace media