Dynamically allocate more memory to AudioBlockFifo

media/base/audio_block_fifo.cc

 // Copyright 2014 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_block_fifo.h"
 
 #include "base/logging.h"
 
 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() {}
 
 void AudioBlockFifo::Push(const void* source,
                           int frames,
                           int bytes_per_sample) {
   DCHECK(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;
   while (frames_to_push) {
     // Get the current write block.
     AudioBus* current_block = audio_blocks_[write_block_];
 
     // Figure out what segment sizes we need when adding the new content to
     // the FIFO.
     const int push_frames =
         std::min(block_frames_ - write_pos_, frames_to_push);
 
     // Deinterleave the content to the FIFO and update the |write_pos_|.
     current_block->FromInterleavedPartial(
         source_ptr, write_pos_, push_frames, bytes_per_sample);
     write_pos_ = (write_pos_ + push_frames) % block_frames_;
     if (!write_pos_) {
       // The current block is completely filled, increment |write_block_| and
       // |available_blocks_|.
       write_block_ = (write_block_ + 1) % audio_blocks_.size();
       ++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);
   }
 }
 
 const AudioBus* AudioBlockFifo::Consume() {
   DCHECK(available_blocks_);
   AudioBus* audio_bus = audio_blocks_[read_block_];
   read_block_ = (read_block_ + 1) % audio_blocks_.size();
   --available_blocks_;
@@ skipping 11 matching lines @@
   return available_blocks_ * block_frames_ + write_pos_;
 }
 
 int AudioBlockFifo::GetUnfilledFrames() const {
   const int unfilled_frames =
       (audio_blocks_.size() - available_blocks_) * block_frames_ - write_pos_;
   DCHECK_GE(unfilled_frames, 0);
   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