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..0ac2e7a1da23c2bdb369b4411830bafc4978a19c 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,25 @@ 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_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();
+    base::TimeDelta current_presentation_timestamp =

[wolenetz, 2014/04/03 18:14:40]

nit: Please document this is guessing the current_presentation_timestamp due to
lack of current info from the renderer. In later CL that uses kitkat support, do
I understand correctly that this code would change (for kitkat) to no longer
guess?

[qinmin, 2014/04/03 21:59:45]

Comments added.
For kitkat, we still need to do the guess. The kitkat call returns the timestamp
A a particular frame B is rendered in the past. So we still need to calculate
the current presentation time based on the timestamp A and position of frame B.

On 2014/04/03 18:14:40, wolenetz wrote:

+        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;
+    clock_.SetTime(current_presentation_timestamp, new_max_time);
+  } else {
+    clock_.SetMaxTime(presentation_timestamp);

[wolenetz, 2014/04/03 18:14:40]

Is it possible for audio decode+render to report 0 audio output bytes? If so,
would we want so instead use the audio_timestamp_helper_->GetTimestamp() here
(of course, for audio, not video)?

[qinmin, 2014/04/03 21:59:45]

Yes, this is possible. For example, during the preroll phase. Changed the else
statement to else if (!HasAudio()).
If HasAudio() is true and we are not queueing any output to the AudioTrack, just
do nothing and the clock will progress by itself. 

On 2014/04/03 18:14:40, wolenetz wrote:

   }
 
-  clock_.SetMaxTime(new_max_time);
   manager()->OnTimeUpdate(player_id(), GetCurrentTime());
 }
 
@@ -581,7 +592,8 @@ 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) {

[wolenetz, 2014/04/03 18:14:40]

nit: DCHECK that, if |is_audio| is true, that all of these |audio_xxx|
parameters are 0 as required in the ReleaseOutputCallback's comments.

[qinmin, 2014/04/03 21:59:45]

you mean |!is_audio|? Done.

   DVLOG(1) << __FUNCTION__ << ": " << is_audio << ", " << status;
 
   // TODO(xhwang): Drop IntToString() when http://crbug.com/303899 is fixed.
@@ -625,6 +637,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 +655,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();