BufferedDataSource: don't reallocate buffer unnecessarily

media/blink/buffered_data_source.cc

Index: media/blink/buffered_data_source.cc
diff --git a/media/blink/buffered_data_source.cc b/media/blink/buffered_data_source.cc
index 91ed1a254c8c43cba43aa8011d0e2501cb6a3c51..eef5f5fd89a891a3c79664639a14dde1f6ea5c4f 100644
--- a/media/blink/buffered_data_source.cc
+++ b/media/blink/buffered_data_source.cc
@@ -90,8 +90,6 @@ BufferedDataSource::BufferedDataSource(
       total_bytes_(kPositionNotSpecified),
       streaming_(false),
       frame_(frame),
-      intermediate_read_buffer_(new uint8[kInitialReadBufferSize]),
-      intermediate_read_buffer_size_(kInitialReadBufferSize),
       render_task_runner_(task_runner),
       stop_signal_received_(false),
       media_has_played_(false),
@@ -104,6 +102,7 @@ BufferedDataSource::BufferedDataSource(
       weak_factory_(this) {
   DCHECK(host_);
   DCHECK(!downloading_cb_.is_null());
+  intermediate_read_buffer_.resize(kInitialReadBufferSize);

[xhwang, 2014/09/24 16:27:16]

You can do this in the initialization list by using vector's ctor:

explicit vector (size_type n, const value_type& val = value_type(),
                 const allocator_type& alloc = allocator_type());

 }
 
 BufferedDataSource::~BufferedDataSource() {}
@@ -314,14 +313,13 @@ void BufferedDataSource::ReadInternal() {
 
   // First we prepare the intermediate read buffer for BufferedResourceLoader
   // to write to.
-  if (size > intermediate_read_buffer_size_) {
-    intermediate_read_buffer_.reset(new uint8[size]);
-  }
+  if (static_cast<int>(intermediate_read_buffer_.size()) < size)
+    intermediate_read_buffer_.resize(size);

[xhwang, 2014/09/24 16:27:16]

So the buffer size can only grow and never shrinks... I am not super familiar
with how this is used. Not sure what's the typical size of this buffer, and
whether we could be holding too much memory... 

[xhwang, 2014/09/24 16:27:16]

For the record: This has another benefit. If the new size is less than the
capacity of the vector, then we don't actually allocate a new buffer.

[xhwang, 2014/09/24 16:39:27]

I did a quick test on Youtube and a test clip and it seems the read size is
relatively stable. So this shouldn't be a concern.

 
   // Perform the actual read with BufferedResourceLoader.
   loader_->Read(position,
                 size,
-                intermediate_read_buffer_.get(),
+                &intermediate_read_buffer_[0],

[xhwang, 2014/09/24 16:27:16]

This is safe since the intermediate_read_buffer_ is never empty. But add a
DCHECK(!intermediate_read_buffer_.empty()) at l.319 to make this explicit?

                 base::Bind(&BufferedDataSource::ReadCallback,
                            weak_factory_.GetWeakPtr()));
 }
@@ -448,7 +446,7 @@ void BufferedDataSource::ReadCallback(
   }
 
   if (bytes_read > 0) {
-    memcpy(read_op_->data(), intermediate_read_buffer_.get(), bytes_read);
+    memcpy(read_op_->data(), &intermediate_read_buffer_[0], bytes_read);
   } else if (bytes_read == 0 && total_bytes_ == kPositionNotSpecified) {
     // We've reached the end of the file and we didn't know the total size
     // before. Update the total size so Read()s past the end of the file will