Use av_rescale_q() for converting FFmpeg timestamps to base::TimeDelta.

Previously we were using integer math to convert to microseconds, but depending on the frame rate and packet size we could introduce enough error that could accumulate and introduce audio/video synchronization drift.  av_rescale_q() is a simple function but is designed to minimize error as much as possible.

Review URL: http://codereview.chromium.org/113598

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@16453 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 57 insertions, 18 deletions

diff --git a/media/base/media_posix.cc b/media/base/media_posix.cc
index 4bddf6d..94c1a89 100644
--- a/media/base/media_posix.cc
+++ b/media/base/media_posix.cc
@@ -122,6 +122,11 @@ void av_free(void* ptr) {
   return av_free_ptr(ptr);
 }
 
+int64_t (*av_rescale_q_ptr)(int64_t a, AVRational bq, AVRational cq) = NULL;
+int64_t av_rescale_q(int64_t a, AVRational bq, AVRational cq) {
+  return av_rescale_q_ptr(a, bq, cq);
+}
+
 }  // extern "C"
 
 
@@ -240,6 +245,9 @@ bool InitializeMediaLibrary(const FilePath& module_dir) {
   av_free_ptr =
       reinterpret_cast<void (*)(void*)>(
           dlsym(libs[FILE_LIBAVUTIL], "av_free"));
+  av_rescale_q_ptr =
+      reinterpret_cast<int64_t (*)(int64_t, AVRational, AVRational)>(
+          dlsym(libs[FILE_LIBAVUTIL], "av_rescale_q"));
 
   // Check that all the symbols were loaded correctly before returning true.
   if (av_get_bits_per_sample_format_ptr &&
@@ -259,7 +267,8 @@ bool InitializeMediaLibrary(const FilePath& module_dir) {
       av_register_protocol_ptr &&
 
       av_malloc_ptr &&
-      av_free_ptr) {
+      av_free_ptr &&
+      av_rescale_q_ptr) {
     return true;
   }
 
diff --git a/media/filters/ffmpeg_audio_decoder.cc b/media/filters/ffmpeg_audio_decoder.cc
index 77b602d..e44880a 100644
--- a/media/filters/ffmpeg_audio_decoder.cc
+++ b/media/filters/ffmpeg_audio_decoder.cc
@@ -36,11 +36,16 @@ bool FFmpegAudioDecoder::OnInitialize(DemuxerStream* demuxer_stream) {
           QueryInterface<FFmpegDemuxerStream>(&ffmpeg_demuxer_stream))
     return false;
 
-  // Setting the media format.
+  // Grab the AVStream's codec context and make sure we have sensible values.
+  codec_context_ = ffmpeg_demuxer_stream->av_stream()->codec;
+  DCHECK_GT(codec_context_->channels, 0);
+  DCHECK_GT(av_get_bits_per_sample_format(codec_context_->sample_fmt), 0);
+  DCHECK_GT(codec_context_->sample_rate, 0);
+
+  // Set the media format.
   // TODO(hclam): Reuse the information provided by the demuxer for now, we may
   // need to wait until the first buffer is decoded to know the correct
   // information.
-  codec_context_ = ffmpeg_demuxer_stream->av_stream()->codec;
   media_format_.SetAsInteger(MediaFormat::kChannels, codec_context_->channels);
   media_format_.SetAsInteger(MediaFormat::kSampleBits,
       av_get_bits_per_sample_format(codec_context_->sample_fmt));
@@ -90,10 +95,29 @@ void FFmpegAudioDecoder::OnDecode(Buffer* input) {
     DataBuffer* result_buffer = new DataBuffer();
     memcpy(result_buffer->GetWritableData(output_buffer_size),
            output_buffer, output_buffer_size);
+
+    // Determine the duration if the demuxer couldn't figure it out, otherwise
+    // copy it over.
+    if (input->GetDuration().InMicroseconds() == 0) {
+      result_buffer->SetDuration(CalculateDuration(output_buffer_size));
+    } else {
+      result_buffer->SetDuration(input->GetDuration());
+    }
+
+    // Copy over the timestamp.
     result_buffer->SetTimestamp(input->GetTimestamp());
-    result_buffer->SetDuration(input->GetDuration());
+
     EnqueueResult(result_buffer);
   }
 }
 
+base::TimeDelta FFmpegAudioDecoder::CalculateDuration(size_t size) {
+  int64 denominator = codec_context_->channels *
+      av_get_bits_per_sample_format(codec_context_->sample_fmt) / 8 *
+      codec_context_->sample_rate;
+  double microseconds = size /
+      (denominator / static_cast<double>(base::Time::kMicrosecondsPerSecond));
+  return base::TimeDelta::FromMicroseconds(static_cast<int64>(microseconds));
+}
+
 }  // namespace
diff --git a/media/filters/ffmpeg_audio_decoder.h b/media/filters/ffmpeg_audio_decoder.h
index ffb99f8..f30f330 100644
--- a/media/filters/ffmpeg_audio_decoder.h
+++ b/media/filters/ffmpeg_audio_decoder.h
@@ -34,6 +34,10 @@ class FFmpegAudioDecoder : public DecoderBase<AudioDecoder, Buffer> {
   FFmpegAudioDecoder();
   virtual ~FFmpegAudioDecoder();
 
+  // Calculates the duration of an audio buffer based on the sample rate,
+  // channels and bits per sample given the size in bytes.
+  base::TimeDelta CalculateDuration(size_t size);
+
   // A FFmpeg defined structure that holds decoder information, this variable
   // is initialized in OnInitialize().
   AVCodecContext* codec_context_;
diff --git a/media/filters/ffmpeg_demuxer.cc b/media/filters/ffmpeg_demuxer.cc
index 077314e..ab3974b 100644
--- a/media/filters/ffmpeg_demuxer.cc
+++ b/media/filters/ffmpeg_demuxer.cc
@@ -51,7 +51,7 @@ class AVPacketBuffer : public Buffer {
 FFmpegDemuxerStream::FFmpegDemuxerStream(FFmpegDemuxer* demuxer,
                                          AVStream* stream)
     : demuxer_(demuxer),
-      av_stream_(stream),
+      stream_(stream),
       discontinuous_(false) {
   DCHECK(demuxer_);
 
@@ -70,12 +70,8 @@ FFmpegDemuxerStream::FFmpegDemuxerStream(FFmpegDemuxer* demuxer,
       break;
   }
 
-  // Calculate the time base and duration in microseconds.
-  int64 time_base_us = static_cast<int64>(av_q2d(stream->time_base) *
-      base::Time::kMicrosecondsPerSecond);
-  int64 duration_us = static_cast<int64>(time_base_us * stream->duration);
-  time_base_ = base::TimeDelta::FromMicroseconds(time_base_us);
-  duration_ = base::TimeDelta::FromMicroseconds(duration_us);
+  // Calculate the duration.
+  duration_ = ConvertTimestamp(stream->duration);
 }
 
 FFmpegDemuxerStream::~FFmpegDemuxerStream() {
@@ -106,8 +102,8 @@ bool FFmpegDemuxerStream::HasPendingReads() {
 }
 
 base::TimeDelta FFmpegDemuxerStream::EnqueuePacket(AVPacket* packet) {
-  base::TimeDelta timestamp = time_base_ * packet->pts;
-  base::TimeDelta duration = time_base_ * packet->duration;
+  base::TimeDelta timestamp = ConvertTimestamp(packet->pts);
+  base::TimeDelta duration = ConvertTimestamp(packet->duration);
   Buffer* buffer = new AVPacketBuffer(packet, timestamp, duration);
   DCHECK(buffer);
   {
@@ -166,6 +162,12 @@ bool FFmpegDemuxerStream::FulfillPendingReads() {
   return pending_reads;
 }
 
+base::TimeDelta FFmpegDemuxerStream::ConvertTimestamp(int64 timestamp) {
+  AVRational time_base = { 1, base::Time::kMicrosecondsPerSecond };
+  int64 microseconds = av_rescale_q(timestamp, stream_->time_base, time_base);
+  return base::TimeDelta::FromMicroseconds(microseconds);
+}
+
 
 //
 // FFmpegDemuxer
diff --git a/media/filters/ffmpeg_demuxer.h b/media/filters/ffmpeg_demuxer.h
index 9e92cd4..bd785b7 100644
--- a/media/filters/ffmpeg_demuxer.h
+++ b/media/filters/ffmpeg_demuxer.h
@@ -70,9 +70,7 @@ class FFmpegDemuxerStream : public DemuxerStream {
   virtual const MediaFormat& media_format();
   virtual void Read(Callback1<Buffer*>::Type* read_callback);
 
-  AVStream* av_stream() {
-    return av_stream_;
-  }
+  AVStream* av_stream() const { return stream_; }
 
   static const char* interface_id();
 
@@ -83,10 +81,12 @@ class FFmpegDemuxerStream : public DemuxerStream {
   // Returns true if there are still pending reads.
   bool FulfillPendingReads();
 
+  // Converts an FFmpeg stream timestamp into a base::TimeDelta.
+  base::TimeDelta ConvertTimestamp(int64 timestamp);
+
   FFmpegDemuxer* demuxer_;
-  AVStream* av_stream_;
+  AVStream* stream_;
   MediaFormat media_format_;
-  base::TimeDelta time_base_;
   base::TimeDelta duration_;
   bool discontinuous_;