Dynamically allocate more memory to AudioBlockFifo

media/base/audio_block_fifo.cc

Index: media/base/audio_block_fifo.cc
diff --git a/media/base/audio_block_fifo.cc b/media/base/audio_block_fifo.cc
index 5e8c9b37949babea29b10a464b2a6e3fd9189610..7b18fb3d41c4b31097858c1c580f084fa39c1b65 100644
--- a/media/base/audio_block_fifo.cc
+++ b/media/base/audio_block_fifo.cc
@@ -9,17 +9,13 @@
 namespace media {
 
 AudioBlockFifo::AudioBlockFifo(int channels, int frames, int blocks)
-    : block_frames_(frames),
+    : channels_(channels),
+      block_frames_(frames),
       write_block_(0),
       read_block_(0),
       available_blocks_(0),
       write_pos_(0) {
-  // Create |blocks| of audio buses and push them to the containers.
-  audio_blocks_.reserve(blocks);
-  for (int i = 0; i < blocks; ++i) {
-    scoped_ptr<AudioBus> audio_bus = AudioBus::Create(channels, frames);
-    audio_blocks_.push_back(audio_bus.release());
-  }
+  IncreaseCapacity(blocks);
 }
 
 AudioBlockFifo::~AudioBlockFifo() {}
@@ -31,6 +27,7 @@ void AudioBlockFifo::Push(const void* source,
   DCHECK_GT(frames, 0);
   DCHECK_GT(bytes_per_sample, 0);
   DCHECK_LT(available_blocks_, static_cast<int>(audio_blocks_.size()));
+  CHECK_LE(frames, GetUnfilledFrames());
 
   const uint8* source_ptr = static_cast<const uint8*>(source);
   int frames_to_push = frames;
@@ -54,7 +51,7 @@ void AudioBlockFifo::Push(const void* source,
       ++available_blocks_;
     }
 
-    source_ptr += push_frames * bytes_per_sample * current_block->channels();
+    source_ptr += push_frames * bytes_per_sample * channels_;
     frames_to_push -= push_frames;
     DCHECK_GE(frames_to_push, 0);
   }
@@ -86,4 +83,23 @@ int AudioBlockFifo::GetUnfilledFrames() const {
   return unfilled_frames;
 }
 
+void AudioBlockFifo::IncreaseCapacity(int blocks) {

[DaleCurtis, 2014/09/16 19:36:53]

DCHECK_GT(blocks, 0); ?

[no longer working on chromium, 2014/09/18 12:42:49]

Done.

+  // Create |blocks| of audio buses and insert them to the containers.
+  audio_blocks_.reserve(audio_blocks_.size() + blocks);
+

[DaleCurtis, 2014/09/16 19:36:53]

Instead of repeatedly shifting the elements due to multiple inserts, here's a
more efficient add-everything-to-the-back-and-shift-once algorithm with
std::rotate():

const int original_size = audio_blocks_.size();
for (int i = 0; i < blocks; ++i) {
  audio_blocks_.push_back(
      AudioBus::Create(channels_, block_frames_).release());
}

if (!original_size)
  return;

std::rotate(audio_blocks_.begin() + read_block_,
            audio_blocks_.begin() + original_size,
            audio_blocks_.end());

[no longer working on chromium, 2014/09/18 12:42:49]

Done with using rotate.

Though I don't really think insert is that costly, and the new approach is using
|blocks| times of push_back and one rotate, it seems more operations to me :)

[DaleCurtis, 2014/09/18 17:12:03]

Vectors have contiguous memory layout and insert() inserts elements before the
specified iterator position, so under the hood for each inserted block it has to
move all elements after the insert position over 1 position.  std::rotate() will
ideally do this with a single move, though implementations may vary. With the
reserve() call above push_back is really just a constant time operation in also.

Since |blocks| is small this isn't a big deal, but I wanted to call it out and
recommend an alternative if you weren't aware of what was happening here and
found yourself in a similar situation in the future.

+  // Insert the new blocks of buffers right before the read block.

[no longer working on chromium, 2014/09/16 18:00:28]

we insert the new buffers before the read pointer so that those new buffers are
always last buffer to read in the FIFO.

+  ScopedVector<AudioBus>::iterator it = audio_blocks_.begin() + read_block_;
+  for (int i = 0; i < blocks; ++i) {
+    scoped_ptr<AudioBus> audio_bus = AudioBus::Create(channels_, block_frames_);
+    it = audio_blocks_.insert(it, audio_bus.release());
+  }
+
+  // Update the write pointer if it is on top of the new inserted blocks.
+  if (write_block_ >= read_block_)

[no longer working on chromium, 2014/09/16 18:00:28]

if write_block_ < read_block_, it means write pointer is below the new blocks of
buffer, so there is no need to update the write pointer.

[no longer working on chromium, 2014/09/18 12:42:49]

Done.

+    write_block_ += blocks % audio_blocks_.size();
+
+  // Update the read pointers correspondingly.
+  read_block_ += blocks % audio_blocks_.size();;

[DaleCurtis, 2014/09/16 19:36:53]

This should never wrap. You can remove the modulus if you gate it behind the
!original_size check I recommend above.  It might be nice to add a DCHECK_LT()
here too then.

[no longer working on chromium, 2014/09/18 12:42:49]

Done.

+}
+
 }  // namespace media