Make sure that DecodeAudioFileData() always returns an AudioBus of the exact length of the file

media/filters/audio_file_reader.cc

 // Copyright (c) 2012 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 "media/filters/audio_file_reader.h"
 
 #include <string>
 #include "base/basictypes.h"
 #include "base/string_util.h"
 #include "base/time.h"
@@ skipping 17 matching lines @@
 int AudioFileReader::channels() const {
   return codec_context_->channels;
 }
 
 int AudioFileReader::sample_rate() const {
   return codec_context_->sample_rate;
 }
 
 base::TimeDelta AudioFileReader::duration() const {
   const AVRational av_time_base = {1, AV_TIME_BASE};
-  return ConvertFromTimeBase(av_time_base, format_context_->duration);
+
+  // Add one microsecond to avoid rounding-down errors which can occur when
+  // |duration| has been calculated from an exact number of sample-frames.
+  // One microsecond is much less than the time of a single sample-frame
+  // at any real-world sample-rate.
+  return ConvertFromTimeBase(av_time_base, format_context_->duration + 1);
 }
 
 int64 AudioFileReader::number_of_frames() const {
   return static_cast<int64>(duration().InSecondsF() * sample_rate());
 }
 
 bool AudioFileReader::Open() {
   // Add our data reader to the protocol list and get our unique key.
   std::string key = FFmpegGlue::GetInstance()->AddProtocol(protocol_);
 
@@ skipping 54 matching lines @@
     avcodec_close(codec_context_);
     codec_context_ = NULL;
   }
 
   if (format_context_) {
     avformat_close_input(&format_context_);
     format_context_ = NULL;
   }
 }
 
-bool AudioFileReader::Read(AudioBus* audio_bus) {
+int AudioFileReader::Read(AudioBus* audio_bus) {
   DCHECK(format_context_ && codec_context_) <<
       "AudioFileReader::Read() : reader is not opened!";
 
   DCHECK_EQ(audio_bus->channels(), channels());

[DaleCurtis, 2012/10/12 22:42:02]

Style violation: Remove DCHECK() since this is handled by the if statement. Feel
free to replace with a DVLOG(1) or DLOG(ERROR) if you want.

[Chris Rogers, 2012/10/12 23:22:29]

This isn't part of this CL

On 2012/10/12 22:42:02, DaleCurtis wrote:

[DaleCurtis, 2012/10/13 01:10:35]

I know, but it's a good idea to fix these things if they're small when seen.

   if (audio_bus->channels() != channels())
-    return false;
+    return 0;
 
   size_t bytes_per_sample = av_get_bytes_per_sample(codec_context_->sample_fmt);
 
   // Holds decoded audio.
   scoped_ptr_malloc<AVFrame, ScopedPtrAVFree> av_frame(avcodec_alloc_frame());
 
   // Read until we hit EOF or we've read the requested number of frames.
   AVPacket packet;
   int current_frame = 0;
   bool continue_decoding = true;
@@ skipping 50 matching lines @@
 
       current_frame += frames_read;
     } while (packet_temp.size > 0);
     av_free_packet(&packet);
   }
 
   // Zero any remaining frames.
   audio_bus->ZeroFramesPartial(
       current_frame, audio_bus->frames() - current_frame);
 
-  // Fail if nothing has been decoded, otherwise return partial data.
-  return current_frame > 0;
+  // Returns the actual number of sample-frames decoded.
+  // Ideally this represents the "true" exact length of the file.
+  return current_frame;
 }
 
 }  // namespace media