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 637 matching lines @@
           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;
+
+  // Config change may have just been detected on the other stream. If so,
+  // don't send a duplicate demuxer config request.
+  if (IsEventPending(CONFIG_CHANGE_EVENT_PENDING)) {
+    DCHECK(reconfig_video_decoder_);

[xhwang, 2013/11/04 21:55:48]

hmm, isn't this always true given line 659? Or are we trying to remind ourselves
on that? Anyway, let's land this CL first!

[wolenetz, 2013/11/04 22:00:55]

This DCHECK is to ensure that the CONFIG_CHANGE_EVENT_PENDING was set already,
but for the *other* stream (the video stream). We already have line 658
protecting against attempted double-setting of this pending event for the same
stream. DecodeMoreVideo() has similar (with 'audio' and 'video' swapped).

[xhwang1, 2013/11/05 02:30:21]

Oh, I didn't realize it's "video", not "audio". Sorry for the confusion.

+    return;
+  }
+
   SetPendingEvent(CONFIG_CHANGE_EVENT_PENDING);
   ProcessPendingEvents();
 }
 
 void MediaSourcePlayer::DecodeMoreVideo() {
   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;
+
+  // Config change may have just been detected on the other stream. If so,
+  // don't send a duplicate demuxer config request.
+  if (IsEventPending(CONFIG_CHANGE_EVENT_PENDING)) {
+    DCHECK(reconfig_audio_decoder_);
+    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 233 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