Introduce AudioBus to replace std::vector<float*>.

media/base/audio_bus.h

+// 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.
+
+#ifndef MEDIA_BASE_AUDIO_BUS_H_
+#define MEDIA_BASE_AUDIO_BUS_H_
+
+#include <vector>
+
+#include "base/logging.h"
+#include "base/memory/aligned_memory.h"
+#include "base/memory/scoped_ptr.h"
+#include "media/audio/audio_parameters.h"
+#include "media/base/channel_layout.h"
+
+namespace media {
+
+// Scoped container for "busing" audio channel data around.
+class MEDIA_EXPORT AudioBus {
+ public:
+  ~AudioBus();

[scherkus (not reviewing), 2012/08/06 18:55:34]

if you only return scoped_ptr<> this can be private w/ scoped_ptr<AudioBus> a
friend class

I notice in 10823175 that you're passing raw pointers, do doing this will forbid
those areas of code from being able to delete the pointer

[DaleCurtis, 2012/08/07 01:36:32]

Nice!

+
+  // Creates a new AudioBus and allocates |channels| of length |frames|.
+  static scoped_ptr<AudioBus> Create(int channels, int frames);
+
+  // Same thing as above, except extracts |channels| and |frames| from the given
+  // AudioParameters structure.  Additionally sets |channel_layout_|.

[scherkus (not reviewing), 2012/08/06 18:55:34]

nit: I'd just move this up by the other Create() and have one comment for both

[DaleCurtis, 2012/08/07 01:36:32]

Done.

+  static scoped_ptr<AudioBus> Create(const AudioParameters& params);
+
+  // Creates a new AudioBus and from an existing channel vector.

[scherkus (not reviewing), 2012/08/06 18:55:34]

s/and//

I'd doc that this isn't ownership transfer nor a copy and that the lifetime of
the float* inside |audio_data_vector| must outlive this object

[DaleCurtis, 2012/08/07 01:36:32]

Done.

+  static scoped_ptr<AudioBus> WrapVector(
+      int frames,
+      const std::vector<float*>& audio_data_vector);

[scherkus (not reviewing), 2012/08/06 18:55:34]

s/audio_data_vector/channel_data?

[DaleCurtis, 2012/08/07 01:36:32]

Done.

+
+  // Returns a raw pointer to internal data for reconstruction of an AudioBus

[scherkus (not reviewing), 2012/08/06 18:55:34]

I'd drop reference to shared memory -- that's something content/ specific

[DaleCurtis, 2012/08/07 01:36:32]

Done.

+  // object across shared memory.  Can not be used with an AudioBus constructed
+  // via wrapping.
+  void* audio_data() {

[scherkus (not reviewing), 2012/08/06 18:55:34]

s/audio_// here + below (no need to prefix methods w/ audio for a class that has
audio in the name)

[DaleCurtis, 2012/08/07 01:36:32]

Done.

+    DCHECK(audio_data_.get());

[scherkus (not reviewing), 2012/08/06 18:55:34]

I see what you mean about the DCHECK() -- I would de-inline and move to .cc

[DaleCurtis, 2012/08/07 01:36:32]

Done.

+    return audio_data_.get();
+  }
+
+  // Size in bytes of internal data.  Should only be used for transferring an
+  // AudioBus object across shared memory.
+  int audio_data_size() const {
+    DCHECK(audio_data_.get());
+    return audio_data_size_;
+  }
+
+  // Returns a raw pointer to the requested channel.  Pointer is guaranteed to
+  // have a 16-byte alignment.
+  float* channel(int channel) { return audio_data_vector_[channel]; }
+  const float* channel(int channel) const {

[scherkus (not reviewing), 2012/08/06 18:55:34]

what's up with the overloaded const version? where are we using const AudioBus
objects?

[DaleCurtis, 2012/08/07 01:36:32]

In AudioUtil::InterleaveFloatToInt, see:
http://codereview.chromium.org/10823175/diff/1/media/audio/audio_util.cc
SincResampler/MultiChannelResampler should actually take a const ref as input
too. Possibly other use cases I'll find during conversion.

In any case, I ended up not needing the overload, since technically channel()
can be marked const.

+    return audio_data_vector_[channel];
+  }
+
+  int channels() const { return audio_data_vector_.size(); }
+  int frames() const { return frames_; }
+
+  // Track channel layout information in case of channel remixing.  Defaults to
+  // CHANNEL_LAYOUT_NONE if no layout is provided.
+  ChannelLayout channel_layout() const { return channel_layout_; }

[scherkus (not reviewing), 2012/08/06 18:55:34]

I don't see channel layout used in 10823175 -- can we remove until needed?

[DaleCurtis, 2012/08/07 01:36:32]

Done.

+  void set_channel_layout(ChannelLayout layout) {
+    DCHECK_EQ(static_cast<size_t>(ChannelLayoutToChannelCount(layout)),
+              audio_data_vector_.size());
+    channel_layout_ = layout;
+  }
+
+  // Helper method for zeroing out all channels of audio data.
+  void Zero(int frames);
+  void Zero() { Zero(frames_); }

[scherkus (not reviewing), 2012/08/06 18:55:34]

instead of overload (generally avoided), how about ZeroAll OR change
Zero(frames) to ZeroFrames()

[DaleCurtis, 2012/08/07 01:36:32]

Done.

+
+ private:
+  AudioBus(int channels, int frames, ChannelLayout channel_layout);
+  AudioBus(int frames, const std::vector<float*>& audio_data_vector);

[scherkus (not reviewing), 2012/08/06 18:55:34]

ditto

[DaleCurtis, 2012/08/07 01:36:32]

Done.

+
+  // Contiguous block of channel memory.
+  scoped_ptr_malloc<float, base::ScopedPtrAlignedFree> audio_data_;

[scherkus (not reviewing), 2012/08/06 18:55:34]

similar to above this can be data_ if you want

[DaleCurtis, 2012/08/07 01:36:32]

Done.

+  int audio_data_size_;
+
+  // Vector of pointers to each channel's audio data.
+  std::vector<float*> audio_data_vector_;

[scherkus (not reviewing), 2012/08/06 18:55:34]

similar to above this can be channel_data_

[DaleCurtis, 2012/08/07 01:36:32]

Done.

+  int frames_;
+
+  ChannelLayout channel_layout_;
+
+  DISALLOW_COPY_AND_ASSIGN(AudioBus);
+};
+
+}  // namespace media
+
+#endif  // MEDIA_BASE_AUDIO_BUS_H_