Allow simultaneous audio and video config change

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 <limits>
 
 #include "base/android/jni_android.h"
 #include "base/android/jni_string.h"
@@ skipping 585 matching lines @@
     // If we have a valid timestamp, start the starvation callback. Otherwise,
     // reset the |start_time_ticks_| so that the next frame will not suffer
     // from the decoding delay caused by the current frame.
     if (presentation_timestamp != kNoTimestamp())
       StartStarvationCallback(presentation_timestamp);
     else
       start_time_ticks_ = base::TimeTicks::Now();
   }
 
   if (is_audio) {
-    DecodeMoreAudio();
+    DecodeMoreAudio(false);
     return;
   }
 
-  DecodeMoreVideo();
+  DecodeMoreVideo(false);
 }
 
-void MediaSourcePlayer::DecodeMoreAudio() {
+void MediaSourcePlayer::DecodeMoreAudio(bool video_change_already_pending) {
   DVLOG(1) << __FUNCTION__;
   DCHECK(!audio_decoder_job_->is_decoding());
 
   if (audio_decoder_job_->Decode(
           start_time_ticks_, start_presentation_timestamp_, base::Bind(
               &MediaSourcePlayer::MediaDecoderCallback,
               weak_this_.GetWeakPtr(), true))) {
     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;
+  if (video_change_already_pending) {

[acolwell GONE FROM CHROMIUM, 2013/11/04 18:42:07]

I don't think the xxx_change_already_pending parameters are providing much value
here and below. It also makes the call sites confusing. I think you should just
use IsEventPending(CONFIG_CHANGE_EVENT_PENDING) as the condition here and just
keep a DCHECK(reconfig_xxx_decoder_) in the block before returning.

[wolenetz, 2013/11/04 19:30:25]

Done. Thank you for the rapid CR :)

+    DCHECK(reconfig_video_decoder_ &&
+           IsEventPending(CONFIG_CHANGE_EVENT_PENDING));
+    return;
+  }
+
   SetPendingEvent(CONFIG_CHANGE_EVENT_PENDING);
   ProcessPendingEvents();
 }
 
-void MediaSourcePlayer::DecodeMoreVideo() {
+void MediaSourcePlayer::DecodeMoreVideo(bool audio_change_already_pending) {
   DVLOG(1) << __FUNCTION__;
   DCHECK(!video_decoder_job_->is_decoding());
 
   if (video_decoder_job_->Decode(
           start_time_ticks_, start_presentation_timestamp_, base::Bind(
               &MediaSourcePlayer::MediaDecoderCallback,
               weak_this_.GetWeakPtr(), false))) {
     TRACE_EVENT_ASYNC_BEGIN0("media", "MediaSourcePlayer::DecodeMoreVideo",
                              video_decoder_job_.get());
     return;
   }
 
   // Failed to start the next decode.
   // Wait for demuxer ready message.
-  reconfig_video_decoder_ = true;
 
   // 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;
+  if (audio_change_already_pending) {
+    DCHECK(reconfig_audio_decoder_ &&
+           IsEventPending(CONFIG_CHANGE_EVENT_PENDING));
+    return;
+  }
+
   SetPendingEvent(CONFIG_CHANGE_EVENT_PENDING);
   ProcessPendingEvents();
 }
 
 void MediaSourcePlayer::PlaybackCompleted(bool is_audio) {
   DVLOG(1) << __FUNCTION__ << "(" << is_audio << ")";
   if (is_audio)
     audio_finished_ = true;
   else
     video_finished_ = true;
@@ skipping 179 matching lines @@
     ProcessPendingEvents();
     return;
   }
 
   start_time_ticks_ = base::TimeTicks::Now();
   start_presentation_timestamp_ = GetCurrentTime();
   if (!clock_.IsPlaying())
     clock_.Play();
 
   if (audio_decoder_job_)
-    DecodeMoreAudio();
+    DecodeMoreAudio(false);
   if (video_decoder_job_)
-    DecodeMoreVideo();
+    DecodeMoreVideo(IsEventPending(CONFIG_CHANGE_EVENT_PENDING));
 }
 
 const char* MediaSourcePlayer::GetEventName(PendingEventFlags event) {
   static const char* kPendingEventNames[] = {
     "SEEK",
     "SURFACE_CHANGE",
     "CONFIG_CHANGE",
     "PREFETCH_REQUEST",
     "PREFETCH_DONE",
   };
@@ skipping 21 matching lines @@
 
 void MediaSourcePlayer::ClearPendingEvent(PendingEventFlags event) {
   DVLOG(1) << __FUNCTION__ << "(" << GetEventName(event) << ")";
   DCHECK_NE(event, NO_EVENT_PENDING);
   DCHECK(IsEventPending(event)) << GetEventName(event);
 
   pending_event_ &= ~event;
 }
 
 }  // namespace media