Fix an issue that audio and video may run out of sync

media/base/android/media_source_player_unittest.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 <string>
 
 #include "base/basictypes.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/strings/stringprintf.h"
@@ skipping 713 matching lines @@
     EXPECT_EQ(1, demuxer_->num_seek_requests());
 
     player_.OnDemuxerSeekDone(kNoTimestamp());
     EXPECT_FALSE(manager_.playback_completed());
   }
 
   base::TimeTicks StartTimeTicks() {
     return player_.start_time_ticks_;
   }
 
+  AudioTimestampHelper* GetAudioTimestampHelper() {
+    return player_.audio_timestamp_helper_.get();
+  }
+
   base::MessageLoop message_loop_;
   MockMediaPlayerManager manager_;
   MockDemuxerAndroid* demuxer_;  // Owned by |player_|.
   MediaSourcePlayer player_;
 
   // Track whether a possibly async decoder callback test hook has run.
   bool decoder_callback_hook_executed_;
 
   // We need to keep the surface texture while the decoder is actively decoding.
   // Otherwise, it may trigger unexpected crashes on some devices. To switch
@@ skipping 1313 matching lines @@
   // player has not yet received media crypto.
   DemuxerConfigs configs = CreateVideoDemuxerConfigs();
   configs.is_video_encrypted = true;
 
   player_.OnDemuxerConfigsAvailable(configs);
   CreateNextTextureAndSetVideoSurface();
   EXPECT_FALSE(IsPendingSurfaceChange());
   EXPECT_FALSE(GetMediaDecoderJob(false));
 }
 
+TEST_F(MediaSourcePlayerTest, CurrentTimeUpdatedWhileDecoderStarved) {
+  SKIP_TEST_IF_MEDIA_CODEC_BRIDGE_IS_NOT_AVAILABLE();
+
+  // Test that current time is updated while decoder is starved.
+  StartAudioDecoderJob(true);
+  for (int i = 0; i < 3; ++i) {
+    player_.OnDemuxerDataAvailable(CreateReadFromDemuxerAckForAudio(i));
+    WaitForAudioDecodeDone();
+  }
+  base::TimeDelta current_time = player_.GetCurrentTime();
+
+  // Trigger starvation while the decoder is decoding.
+  player_.OnDemuxerDataAvailable(CreateReadFromDemuxerAckForAudio(3));
+  TriggerPlayerStarvation();
+  WaitForAudioDecodeDone();
+
+  // Current time should be updated.
+  EXPECT_LT(current_time.InMillisecondsF(),
+            player_.GetCurrentTime().InMillisecondsF());
+}
+
+TEST_F(MediaSourcePlayerTest,
+       AudioTimestampHelperNotRecreatedIfOnlyVideoConfigChanges) {
+  SKIP_TEST_IF_MEDIA_CODEC_BRIDGE_IS_NOT_AVAILABLE();
+
+  // Test that AudioTimestampHelper is not recreated if only video config
+  // changes.
+  CreateNextTextureAndSetVideoSurface();
+  Start(CreateAudioVideoDemuxerConfigs(), true);
+  AudioTimestampHelper* helper = GetAudioTimestampHelper();
+  for (int i = 0; i < 3; ++i) {
+    player_.OnDemuxerDataAvailable(CreateReadFromDemuxerAckForAudio(i));
+    WaitForAudioDecodeDone();
+  }
+  player_.OnDemuxerDataAvailable(CreateReadFromDemuxerAckForAudio(3));
+  player_.OnDemuxerDataAvailable(
+      CreateReadFromDemuxerAckWithConfigChanged(false, 0));
+  WaitForDecodeDone(true, true);
+  EXPECT_EQ(1, demuxer_->num_config_requests());
+  player_.OnDemuxerConfigsAvailable(CreateAudioVideoDemuxerConfigs());
+
+  EXPECT_EQ(helper, GetAudioTimestampHelper());

[wolenetz, 2014/04/04 17:27:43]

nit: Is there a better way than pointer comparison to ensure we haven't created
a new helper? I would prefer we don't further proliferate such a check that may
not always catch that the instance has changed, if the memory allocator re-used
the same location for the new instance. There's other locations in other tests
in this file that do this (e.g. to check if decoder job has/hasn't changed) that
I'm also concerned with. File bug + a TODO?

[qinmin, 2014/04/04 17:58:19]

Done.

+}
+
 }  // namespace media