AudioBus: Add a ToInterleavedFloat() method to AudioBus

media/base/audio_bus.h

Index: media/base/audio_bus.h
diff --git a/media/base/audio_bus.h b/media/base/audio_bus.h
index f827f9db87d396667e39d3105f10a33f85e39d11..c592a2166ce151fbeabce76a6c3fb95c303d3e3a 100644
--- a/media/base/audio_bus.h
+++ b/media/base/audio_bus.h
@@ -18,11 +18,15 @@
 namespace media {
 class AudioParameters;
 
-// Scoped container for "busing" audio channel data around.  Each channel is
-// stored in planar format and guaranteed to be aligned by kChannelAlignment.
-// AudioBus objects can be created normally or via wrapping.  Normally, AudioBus
-// will dice up a contiguous memory block for channel data.  When wrapped,
-// AudioBus instead routes requests for channel data to the wrapped object.
+// Represents a sequence of audio frames containing frames() audio samples for
+// each of channels() channels. The data is stored as a set of contiguous
+// float arrays with one array per channel. The memory for the arrays is either
+// allocated and owned by the AudioBus or it is provided to one of the factory
+// methods. AudioBus guarantees that it allocates memory such that float array
+// for each channel is aligned by AudioBus::kChannelAlignment bytes and it
+// requires the same for memory passed to its Wrap...() factory methods.
+// TODO(chfremer): There are currently no unit tests involving CreateWrapper and
+// SetChannelData, so we need to add them.
 class MEDIA_EXPORT AudioBus {
  public:
   // Guaranteed alignment of each channel's data; use 16-byte alignment for easy
@@ -35,8 +39,8 @@ class MEDIA_EXPORT AudioBus {
   static std::unique_ptr<AudioBus> Create(const AudioParameters& params);
 
   // Creates a new AudioBus with the given number of channels, but zero length.
-  // It's expected to be used with SetChannelData() and set_frames() to
-  // wrap externally allocated memory.
+  // Clients are expected to subsequently call SetChannelData() and set_frames()
+  // to wrap externally allocated memory.
   static std::unique_ptr<AudioBus> CreateWrapper(int channels);
 
   // Creates a new AudioBus from an existing channel vector.  Does not transfer
@@ -54,31 +58,82 @@ class MEDIA_EXPORT AudioBus {
                                               void* data);
   static std::unique_ptr<AudioBus> WrapMemory(const AudioParameters& params,
                                               void* data);
-  static int CalculateMemorySize(const AudioParameters& params);
 
-  // Calculates the required size for an AudioBus given the number of channels
-  // and frames.
+  // Based on the given number of channels and frames, calculates the minimum
+  // required size in bytes of a contiguous block of memory to be passed to
+  // AudioBus for storage of the audio data.
+  // Uses channels() and frames_per_buffer() from AudioParameters if given.
   static int CalculateMemorySize(int channels, int frames);
+  static int CalculateMemorySize(const AudioParameters& params);
+
+  // Methods that are expected to be called after AudioBus::CreateWrapper() in
+  // order to wrap externally allocated memory. Note: It is illegal to call
+  // these methods when using a factory method other than CreateWrapper().
+  void SetChannelData(int channel, float* data);
+  void set_frames(int frames);
 
-  // Helper methods for converting an AudioBus from and to interleaved integer
-  // data.  Expects interleaving to be [ch0, ch1, ..., chN, ch0, ch1, ...] with
-  // |bytes_per_sample| per value.  Values are scaled and bias corrected during
-  // conversion.  ToInterleaved() will also clip values to format range.
-  // Handles uint8_t, int16_t, and int32_t currently.  FromInterleaved() will
-  // zero out
-  // any unfilled frames when |frames| is less than frames().
+  // Overwrites the sample values stored in this AudioBus instance with values
+  // from a given interleaved |source_buffer| with expected layout
+  // [ch0, ch1, ..., chN, ch0, ch1, ...] and sample values in the format
+  // corresponding to the given SourceSampleTypeTraits.
+  // The sample values are converted to float values by means of the method
+  // convert_to_float32() provided by the SourceSampleTypeTraits. For a list of
+  // ready-to-use SampleTypeTraits, see file audio_sample_types.h.
+  // If |num_frames_to_write| is less than frames(), the remaining frames are
+  // zeroed out. If |num_frames_to_write| is more than frames(), this results in
+  // undefined behavior.
+  template <class SourceSampleTypeTraits>
+  void FromInterleaved(
+      const typename SourceSampleTypeTraits::ValueType* source_buffer,
+      int num_frames_to_write);
+
+  // DEPRECATED
+  // Please use the version templated with SourceSampleTypeTraits instead.
   void FromInterleaved(const void* source, int frames, int bytes_per_sample);
-  void ToInterleaved(int frames, int bytes_per_sample, void* dest) const;
-  void ToInterleavedPartial(int start_frame, int frames, int bytes_per_sample,
-                            void* dest) const;
 
-  // Similar to FromInterleaved() above, but meant for streaming sources.  Does
-  // not zero out remaining frames, the caller is responsible for doing so using
-  // ZeroFramesPartial().  Frames are deinterleaved from the start of |source|
-  // to channel(x)[start_frame].
+  // Similar to FromInterleaved...(), but overwrites the frames starting at a
+  // given offset |write_offset_in_frames| and does not zero out frames that are
+  // not overwritten.
+  template <class SourceSampleTypeTraits>
+  void FromInterleavedPartial(
+      const typename SourceSampleTypeTraits::ValueType* source_buffer,
+      int write_offset_in_frames,
+      int num_frames_to_write);
+
+  // DEPRECATED
+  // Please use the version templated with SourceSampleTypeTraits instead.
   void FromInterleavedPartial(const void* source, int start_frame, int frames,
                               int bytes_per_sample);
 
+  // Reads the sample values stored in this AudioBus instance and places them
+  // into the given |dest_buffer| in interleaved format using the sample format
+  // specified by TargetSampleTypeTraits. For a list of ready-to-use
+  // SampleTypeTraits, see file audio_sample_types.h. If |num_frames_to_read| is
+  // larger than frames(), this results in undefined behavior.
+  template <class TargetSampleTypeTraits>
+  void ToInterleaved(
+      int num_frames_to_read,
+      typename TargetSampleTypeTraits::ValueType* dest_buffer) const;
+
+  // DEPRECATED

[mcasas, 2016/06/12 09:41:48]

nit: mention the bug
// DEPRECATED (https://crbug.com/580391)

And maybe you can even:
// TODO(crfremer): deprecated, remove (https://crbug.com/abcdef) 

:)

[chfremer, 2016/06/13 17:44:11]

Done.

+  // Please use the version templated with TargetSampleTypeTraits instead.
+  void ToInterleaved(int frames, int bytes_per_sample, void* dest) const;
+
+  // Similar to ToInterleaved(), but reads the frames starting at a given
+  // offset |read_offset_in_frames|.
+  template <class TargetSampleTypeTraits>
+  void ToInterleavedPartial(
+      int read_offset_in_frames,
+      int num_frames_to_read,
+      typename TargetSampleTypeTraits::ValueType* dest_buffer) const;
+
+  // DEPRECATED

[mcasas, 2016/06/12 09:41:48]

Idem.

[chfremer, 2016/06/13 17:44:11]

Done.

+  // Please use the version templated with TargetSampleTypeTraits instead.
+  void ToInterleavedPartial(int start_frame,
+                            int frames,
+                            int bytes_per_sample,
+                            void* dest) const;
+
   // Helper method for copying channel data from one AudioBus to another.  Both
   // AudioBus object must have the same frames() and channels().
   void CopyTo(AudioBus* dest) const;
@@ -97,11 +152,11 @@ class MEDIA_EXPORT AudioBus {
   // inf, nan, or between [-1.0, 1.0]) values in the channel data.
   float* channel(int channel) { return channel_data_[channel]; }
   const float* channel(int channel) const { return channel_data_[channel]; }
-  void SetChannelData(int channel, float* data);
 
+  // Returns the number of channels.
   int channels() const { return static_cast<int>(channel_data_.size()); }
+  // Returns the number of frames.
   int frames() const { return frames_; }
-  void set_frames(int frames);
 
   // Helper method for zeroing out all channels of audio data.
   void Zero();
@@ -129,12 +184,32 @@ class MEDIA_EXPORT AudioBus {
 
  private:
   // Helper method for building |channel_data_| from a block of memory.  |data|
-  // must be at least BlockSize() bytes in size.
+  // must be at least CalculateMemorySize(...) bytes in size.
   void BuildChannelData(int channels, int aligned_frame, float* data);
 
+  static void CheckOverflow(int start_frame, int frames, int total_frames);
+
+  template <class SourceSampleTypeTraits>
+  static void CopyConvertFromInterleavedSourceToAudioBus(
+      const typename SourceSampleTypeTraits::ValueType* source_buffer,
+      int write_offset_in_frames,
+      int num_frames_to_write,
+      AudioBus* dest);
+
+  template <class TargetSampleTypeTraits>
+  static void CopyConvertFromAudioBusToInterleavedTarget(
+      const AudioBus* source,
+      int read_offset_in_frames,
+      int num_frames_to_read,
+      typename TargetSampleTypeTraits::ValueType* dest_buffer);
+
   // Contiguous block of channel memory.
   std::unique_ptr<float, base::AlignedFreeDeleter> data_;
 
+  // One float pointer per channel pointing to a contiguous block of memory for
+  // that channel. If the memory is owned by this instance, this will
+  // point to the memory in |data_|. Otherwise, it may point to memory provided
+  // by the client.
   std::vector<float*> channel_data_;
   int frames_;
 
@@ -144,6 +219,87 @@ class MEDIA_EXPORT AudioBus {
   DISALLOW_COPY_AND_ASSIGN(AudioBus);
 };
 
+// Delegates to FromInterleavedPartial()
+template <class SourceSampleTypeTraits>
+void AudioBus::FromInterleaved(
+    const typename SourceSampleTypeTraits::ValueType* source_buffer,
+    int num_frames_to_write) {
+  FromInterleavedPartial<SourceSampleTypeTraits>(source_buffer, 0,
+                                                 num_frames_to_write);
+  // Zero any remaining frames.
+  ZeroFramesPartial(num_frames_to_write, frames_ - num_frames_to_write);
+}
+
+template <class SourceSampleTypeTraits>
+void AudioBus::FromInterleavedPartial(
+    const typename SourceSampleTypeTraits::ValueType* source_buffer,
+    int write_offset_in_frames,
+    int num_frames_to_write) {
+  CheckOverflow(write_offset_in_frames, num_frames_to_write, frames_);
+  CopyConvertFromInterleavedSourceToAudioBus<SourceSampleTypeTraits>(
+      source_buffer, write_offset_in_frames, num_frames_to_write, this);
+}
+
+// Delegates to ToInterleavedPartial()
+template <class TargetSampleTypeTraits>
+void AudioBus::ToInterleaved(
+    int num_frames_to_read,
+    typename TargetSampleTypeTraits::ValueType* dest_buffer) const {
+  ToInterleavedPartial<TargetSampleTypeTraits>(0, num_frames_to_read,
+                                               dest_buffer);
+}
+
+// TODO(dalecurtis): See if intrinsic optimizations help any here (for each
+// sample type)
+template <class TargetSampleTypeTraits>
+void AudioBus::ToInterleavedPartial(
+    int read_offset_in_frames,
+    int num_frames_to_read,
+    typename TargetSampleTypeTraits::ValueType* dest) const {
+  CheckOverflow(read_offset_in_frames, num_frames_to_read, frames_);
+  CopyConvertFromAudioBusToInterleavedTarget<TargetSampleTypeTraits>(
+      this, read_offset_in_frames, num_frames_to_read, dest);
+}
+
+template <class SourceSampleTypeTraits>
+void AudioBus::CopyConvertFromInterleavedSourceToAudioBus(
+    const typename SourceSampleTypeTraits::ValueType* source_buffer,
+    int write_offset_in_frames,
+    int num_frames_to_write,
+    AudioBus* dest) {
+  const int channels = dest->channels();
+  for (int ch = 0; ch < channels; ++ch) {
+    float* channel_data = dest->channel(ch);
+    for (int target_frame_index = write_offset_in_frames,
+             read_pos_in_source = ch;
+         target_frame_index < write_offset_in_frames + num_frames_to_write;
+         ++target_frame_index, read_pos_in_source += channels) {
+      auto source_value = source_buffer[read_pos_in_source];
+      channel_data[target_frame_index] =
+          SourceSampleTypeTraits::ConvertToFloat(source_value);
+    }
+  }
+}
+
+template <class TargetSampleTypeTraits>
+void AudioBus::CopyConvertFromAudioBusToInterleavedTarget(

[mcasas, 2016/06/12 09:41:48]

I understand that miu@ commented on the
appropriateness of using NEON instructions here;
in that case, make a http://crbug.com/new and
note it here:
//TODO(chfremer): Consider using super duper vector
// instructions here, https://crbug.com/abcdef.

(and/or in the previous deinterlacing method).

[chfremer, 2016/06/13 17:44:11]

Done.

+    const AudioBus* source,
+    int read_offset_in_frames,
+    int num_frames_to_read,
+    typename TargetSampleTypeTraits::ValueType* dest_buffer) {
+  const int channels = source->channels();
+  for (int ch = 0; ch < channels; ++ch) {
+    const float* channel_data = source->channel(ch);
+    for (int source_frame_index = read_offset_in_frames, write_pos_in_dest = ch;
+         source_frame_index < read_offset_in_frames + num_frames_to_read;
+         ++source_frame_index, write_pos_in_dest += channels) {
+      float sourceSampleValue = channel_data[source_frame_index];
+      dest_buffer[write_pos_in_dest] =
+          TargetSampleTypeTraits::ConvertFromFloat(sourceSampleValue);
+    }
+  }
+}
+
 // RefCounted version of AudioBus. This is not meant for general use. Only use
 // this when your lifetime requirements make it impossible to use an
 // AudioBus scoped_ptr.