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

media/base/android/media_source_player.cc

Index: media/base/android/media_source_player.cc
diff --git a/media/base/android/media_source_player.cc b/media/base/android/media_source_player.cc
index e52d0e49fc2e651e0d6352b391a96432fc8cb6bc..663c85549c5bc21debfcb7f7d1daafcaaeabf390 100644
--- a/media/base/android/media_source_player.cc
+++ b/media/base/android/media_source_player.cc
@@ -316,9 +316,11 @@ void MediaSourcePlayer::OnDemuxerConfigsAvailable(
   is_audio_encrypted_ = configs.is_audio_encrypted;
   audio_extra_data_ = configs.audio_extra_data;
   if (HasAudio()) {
-    DCHECK_GT(num_channels_, 0);
-    audio_timestamp_helper_.reset(new AudioTimestampHelper(sampling_rate_));
-    audio_timestamp_helper_->SetBaseTimestamp(GetCurrentTime());
+    if (reconfig_audio_decoder_ || !audio_decoder_job_) {
+      DCHECK_GT(num_channels_, 0);
+      audio_timestamp_helper_.reset(new AudioTimestampHelper(sampling_rate_));
+      audio_timestamp_helper_->SetBaseTimestamp(GetCurrentTime());
+    }
   } else {
     audio_timestamp_helper_.reset();
   }
@@ -471,16 +473,29 @@ void MediaSourcePlayer::OnDemuxerSeekDone(
 }
 
 void MediaSourcePlayer::UpdateTimestamps(
-    base::TimeDelta presentation_timestamp, size_t audio_output_bytes) {
-  base::TimeDelta new_max_time = presentation_timestamp;
-
+    base::TimeDelta presentation_timestamp, size_t audio_output_bytes,
+    int64 audio_head_position, base::TimeTicks audio_render_time) {
+  // If audio is present and there is no audio data passed to the AudioTrack,
+  // just let the clock progress by itself.
   if (audio_output_bytes > 0) {
     audio_timestamp_helper_->AddFrames(
         audio_output_bytes / (kBytesPerAudioOutputSample * num_channels_));
-    new_max_time = audio_timestamp_helper_->GetTimestamp();
+    int64 frames_to_play =
+        audio_timestamp_helper_->frame_count() - audio_head_position;
+    DCHECK_GE(frames_to_play, 0);
+    base::TimeDelta new_max_time = audio_timestamp_helper_->GetTimestamp();
+    // Do a rough estimation of the current presentation time, assuming
+    // playback is smooth since the last time send the data to the AudioTrack.
+    base::TimeDelta current_presentation_timestamp =
+        base::TimeTicks::Now() - audio_render_time + new_max_time -
+        audio_timestamp_helper_->GetFrameDuration(frames_to_play);
+    if (current_presentation_timestamp >= new_max_time)
+      current_presentation_timestamp = new_max_time;

[acolwell GONE FROM CHROMIUM, 2014/04/04 19:25:58]

It feels like the bulk of this logic should be in the AudioDecoderJob. It seems
like there would be a lot fewer changes if you moved the audio_timestamp_helper_
into AudioDecoderJob and simply posted a task to this thread with a new
current_presentation_timestamp and new_max_time. That would be more in line with
what the pipeline code does and as long as there aren't significant delays on
this thread, it should be pretty accurate.

Am I missing something key about why this couldn't work?

[qinmin, 2014/04/04 19:42:51]

ok， I will move the logic there. 
However, the received presentation timestamp is a past thingy due to the delays
introduced by the posted task. I think we can ignore the delay for now, unless
we found the delays to be too significant to ignore.

On 2014/04/04 19:25:58, acolwell wrote:

+    clock_.SetTime(current_presentation_timestamp, new_max_time);
+  } else if (!HasAudio()) {
+    clock_.SetMaxTime(presentation_timestamp);
   }
 
-  clock_.SetMaxTime(new_max_time);
   manager()->OnTimeUpdate(player_id(), GetCurrentTime());
 }
 
@@ -581,8 +596,11 @@ void MediaSourcePlayer::ProcessPendingEvents() {
 
 void MediaSourcePlayer::MediaDecoderCallback(
     bool is_audio, MediaCodecStatus status,
-    base::TimeDelta presentation_timestamp, size_t audio_output_bytes) {
+    base::TimeDelta presentation_timestamp, size_t audio_output_bytes,
+    int64 audio_head_position, base::TimeTicks audio_render_time) {
   DVLOG(1) << __FUNCTION__ << ": " << is_audio << ", " << status;
+  DCHECK(is_audio || (audio_output_bytes == 0 && audio_head_position == 0 &&
+      audio_render_time.is_null()));
 
   // TODO(xhwang): Drop IntToString() when http://crbug.com/303899 is fixed.
   if (is_audio) {
@@ -625,6 +643,13 @@ void MediaSourcePlayer::MediaDecoderCallback(
     return;
   }
 
+  if (status == MEDIA_CODEC_OK && is_clock_manager &&
+      presentation_timestamp != kNoTimestamp()) {
+    UpdateTimestamps(
+        presentation_timestamp, audio_output_bytes, audio_head_position,
+        audio_render_time);
+  }
+
   if (status == MEDIA_CODEC_OUTPUT_END_OF_STREAM)
     PlaybackCompleted(is_audio);
 
@@ -636,11 +661,6 @@ void MediaSourcePlayer::MediaDecoderCallback(
   if (status == MEDIA_CODEC_OUTPUT_END_OF_STREAM)
     return;
 
-  if (status == MEDIA_CODEC_OK && is_clock_manager &&
-      presentation_timestamp != kNoTimestamp()) {
-    UpdateTimestamps(presentation_timestamp, audio_output_bytes);
-  }
-
   if (!playing_) {
     if (is_clock_manager)
       clock_.Pause();