Adds AudioFifo class to Chrome media.

media/base/audio_fifo.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/base/audio_fifo.h"
+
+#include "media/audio/audio_parameters.h"
+#include "base/logging.h"
+
+namespace media {
+
+static void GetSizes(int pos, int max_size, int src_size,
+    int* size, int* wrap_size) {

[DaleCurtis, 2012/09/05 10:34:18]

Indent to start with (. (Or move the line above down here and keep 4 space
indent)

[henrika (OOO until Aug 14), 2012/09/05 12:49:04]

Done.

+  if (pos + src_size > max_size) {

[DaleCurtis, 2012/09/05 10:34:18]

CHECK() size, wrap_size?

[henrika (OOO until Aug 14), 2012/09/05 12:49:04]

See comment above about using CHECK() in this class.

+    // Wrapping is required => derive size of each segment.
+    *size = max_size - pos;
+    *wrap_size = src_size - *size;
+  } else {
+    // Wrapping is not required.
+    *size = src_size;
+    *wrap_size = 0;
+  }
+}
+
+static int UpdatePos(int pos, int step, int max_size) {
+  return ((pos + step) % max_size);
+}
+
+AudioFifo::AudioFifo(int channels, int frames)
+    : audio_bus_(AudioBus::Create(channels, frames)),
+      size_(0),
+      read_pos_(0),
+      write_pos_(0) {
+  audio_bus_->Zero();

[Chris Rogers, 2012/09/04 20:07:20]

I don't think Zero() is necessary since Create() should return an already zeroed
bus

[henrika (OOO until Aug 14), 2012/09/05 10:14:36]

removed.

[DaleCurtis, 2012/09/05 10:34:18]

Create() doesn't Zero, so if it needs to be Zero'd that must be done here. I
don't it's necessary though since we'll just Underflow if no data has been
added. otherwise the data will be written to anyways through Append().

+}
+
+AudioFifo::~AudioFifo() {}
+
+bool AudioFifo::Append(const AudioBus* source) {
+  DCHECK(source);
+  DCHECK_EQ(source->channels(), audio_bus_->channels());
+
+  // Ensure that there is space for the new data in the FIFO.
+  const int source_size = source->frames();
+  if (size_ + source_size > max_size()) {
+    DLOG(ERROR) << "FIFO overflow.";
+    return false;
+  }
+
+  // Figure out if wrapping is needed and if so what segment sizes we need
+  // when adding the new audio bus content to the FIFO.
+  int append_size = 0;

[DaleCurtis, 2012/09/05 10:34:18]

Can these names include "frames" or something more specific than size? At a
glance I interpreted this as the size in bytes.

+  int wrap_size = 0;
+  GetSizes(write_pos_, max_size(), source_size, &append_size, &wrap_size);
+
+  // Copy all channels from the source to the FIFO. Wrap around if needed.
+  for (int ch = 0; ch < source->channels(); ++ch) {
+    float* dest = audio_bus_->channel(ch);
+    const float* src = source->channel(ch);
+
+    // Append part of (or the complete) source to the FIFO.
+    memcpy(&dest[write_pos_], &src[0], append_size * sizeof(src[0]));

[Chris Rogers, 2012/09/04 20:07:20]

why "&dest[write_pos_]" instead of "dest + write_pos"
and "&src[0]" instead of "src"

[henrika (OOO until Aug 14), 2012/09/05 10:14:36]

I prefer the "C++ way" and not the "C way" since &foo[0] can deal with cases
like:

char char_array[];
std::vector<char> std_array;

memcpy(&char_array[0], &src[0], size);  // OK
memcpy(&std_array[0], &src[0], size);  // OK

memcpy(char_array + 0, &src[0], size);  // OK, the compiler will cast char_array
to a pointer.
memcpy(std_array + 0, &src[0], size);  // Error since std_array is not a
pointer.

+    if (wrap_size > 0) {
+      // Wrapping is needed: copy remaining part from the source to the FIFO.
+      memcpy(&dest[0], &src[append_size], wrap_size * sizeof(src[0]));

[Chris Rogers, 2012/09/04 20:07:20]

why not "dest" instead of "&dest[0]"

[henrika (OOO until Aug 14), 2012/09/05 10:14:36]

ditto

+    }
+  }
+
+  size_ += source_size;
+  DCHECK_LE(size_, max_size());
+  write_pos_ = UpdatePos(write_pos_, source_size, max_size());
+  return true;
+}
+
+bool AudioFifo::Remove(AudioBus* destination) {
+  DCHECK(destination);
+  DCHECK_EQ(destination->channels(), audio_bus_->channels());
+
+  // It is not possible to ask for more data than what is available in the FIFO.
+  const int dest_size = destination->frames();
+  if (dest_size > size_) {
+    DLOG(ERROR) << "FIFO underrun.";
+    return false;
+  }
+
+  // Figure out if wrapping is needed and if so what segment sizes we need
+  // when removing audio bus content from the FIFO.
+  int remove_size = 0;
+  int wrap_size = 0;
+  GetSizes(read_pos_, max_size(), dest_size, &remove_size, &wrap_size);
+
+  // Remove all channels from the FIFO and copy the content to the

[Chris Rogers, 2012/09/04 20:07:20]

"Remove all channels from the FIFO" sounds a little misleading.  How about: "For
all channels, remove the requested amount of data..."

[henrika (OOO until Aug 14), 2012/09/05 10:14:36]

Done.

+  // destination. Wrap around if needed.
+  for (int ch = 0; ch < destination->channels(); ++ch) {
+    float* dest = destination->channel(ch);
+    const float* src = audio_bus_->channel(ch);
+
+    // Append part of (or the complete) source to the destination.

[DaleCurtis, 2012/09/05 10:34:18]

Update comment.

+    memcpy(&dest[0], &src[read_pos_], remove_size * sizeof(src[0]));

[Chris Rogers, 2012/09/04 20:07:20]

"dest" instead of "&dest[0]", etc. here and in memcpy() below

[henrika (OOO until Aug 14), 2012/09/05 10:14:36]

ditto

+    if (wrap_size > 0) {
+      // Wrapping is needed: copy remaining part to the destination.
+      memcpy(&dest[wrap_size], &src[0], wrap_size * sizeof(src[0]));
+    }
+  }
+
+  size_ -= dest_size;
+  read_pos_ = UpdatePos(read_pos_, dest_size, max_size());
+  return true;
+}
+
+void AudioFifo::Clear() {
+  size_ = 0;
+  read_pos_ = 0;
+  write_pos_ = 0;
+}
+
+}  // namespace media