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

webkit/media/audio_decoder.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 "webkit/media/audio_decoder.h"
 
 #include <vector>
 #include "base/basictypes.h"
 #include "base/string_util.h"
 #include "base/time.h"
@@ skipping 22 matching lines @@
   // Uses the FFmpeg library for audio file reading.
   InMemoryUrlProtocol url_protocol(reinterpret_cast<const uint8*>(data),
                                    data_size, false);
   AudioFileReader reader(&url_protocol);
 
   if (!reader.Open())
     return false;
 
   size_t number_of_channels = reader.channels();
   double file_sample_rate = reader.sample_rate();
-  double duration = reader.duration().InSecondsF();
   size_t number_of_frames = static_cast<size_t>(reader.number_of_frames());

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

Unnecessary static_cast.

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

Done.

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

Doesn't look changed in the latest patch set?

 
   // Apply sanity checks to make sure crazy values aren't coming out of
   // FFmpeg.
   if (!number_of_channels ||
       number_of_channels > static_cast<size_t>(media::limits::kMaxChannels) ||
       file_sample_rate < media::limits::kMinSampleRate ||
       file_sample_rate > media::limits::kMaxSampleRate)
     return false;
 
-  DVLOG(1) << "Decoding file data -"
-           << " data: " << data
-           << " data size: " << data_size
-           << " duration: " << duration
-           << " number of frames: " << number_of_frames
-           << " sample rate: " << file_sample_rate
-           << " number of channels: " << number_of_channels;
-
   // Allocate and configure the output audio channel data.
   destination_bus->initialize(number_of_channels,
                               number_of_frames,
                               file_sample_rate);
 
   // Wrap the channel pointers which will receive the decoded PCM audio.
   vector<float*> audio_data;
   audio_data.reserve(number_of_channels);
   for (size_t i = 0; i < number_of_channels; ++i) {
     audio_data.push_back(destination_bus->channelData(i));
   }
 
   scoped_ptr<AudioBus> audio_bus = AudioBus::WrapVector(
       number_of_frames, audio_data);
 
   // Decode the audio file data.
-  return reader.Read(audio_bus.get());
+  size_t actual_frames = static_cast<size_t>(reader.Read(audio_bus.get()));

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

This only works for down sizing and not up sizing since Read() will stop when
audio_bus->frames() are filled. E.g. if the frame estimate returned is too
small, you'll still lose frames.

FYI, if someone tries to load 2GB or 4GB files via this API on 32bit platforms
we may not OOM, but will hit a CHECK() failure in AudioBus::WrapVector(). Should
probably run a test to see what happens with a 2GB/4GB+ clip.

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

Added TODO that this case is still not handled.  The current CL is still
worthwhile, since it at least gets us the correct size when down-sizing, which
happens a lot.

+
+  // Adjust WebKit's bus to account for the actual file length
+  // and valid data read.
+  if (actual_frames != number_of_frames) {
+      DCHECK_LE(actual_frames, number_of_frames);

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

2 space indent.

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

Done.

+      destination_bus->resize(actual_frames);
+  }
+
+  double duration = actual_frames / file_sample_rate;
+
+  DVLOG(1) << "Decoded file data -"
+           << " data: " << data
+           << " data size: " << data_size
+           << " duration: " << duration
+           << " number of frames: " << actual_frames
+           << " sample rate: " << file_sample_rate
+           << " number of channels: " << number_of_channels;
+
+  return actual_frames > 0;
 }
 
 }  // namespace webkit_media