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

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.

Second attempt since I forgot to manually resolve av_rescale_q() for unittests.

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

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

7 files changed, 64 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_;
 
diff --git a/media/filters/ffmpeg_demuxer_unittest.cc b/media/filters/ffmpeg_demuxer_unittest.cc
index a882c58..f4c989a 100644
--- a/media/filters/ffmpeg_demuxer_unittest.cc
+++ b/media/filters/ffmpeg_demuxer_unittest.cc
@@ -134,6 +134,12 @@ int av_find_stream_info(AVFormatContext* format) {
   return g_av_find_stream_info;
 }
 
+int64 av_rescale_q(int64 a, AVRational bq, AVRational cq) {
+  int64 num = bq.num * cq.den;
+  int64 den = cq.num * bq.den;
+  return a * num / den;
+}
+
 void av_free(void* ptr) {
   if (ptr) {
     EXPECT_EQ(&g_format, ptr);
diff --git a/third_party/ffmpeg/avutil-50.def b/third_party/ffmpeg/avutil-50.def
index 23bb5db..79e5ddc 100644
--- a/third_party/ffmpeg/avutil-50.def
+++ b/third_party/ffmpeg/avutil-50.def
@@ -2,3 +2,4 @@ LIBRARY avutil-50
 EXPORTS
   av_free
   av_malloc
+  av_rescale_q