Remove media::VideoFrame from media::VideoCaptureDevice::Client interface

content/browser/renderer_host/media/video_capture_buffer_pool.cc

Index: content/browser/renderer_host/media/video_capture_buffer_pool.cc
diff --git a/content/browser/renderer_host/media/video_capture_buffer_pool.cc b/content/browser/renderer_host/media/video_capture_buffer_pool.cc
index f6486a5405128c6006329a2462a794fbf7c4a38c..557defa7c527738f2b80f15a3939edbdfac6f5ea 100644
--- a/content/browser/renderer_host/media/video_capture_buffer_pool.cc
+++ b/content/browser/renderer_host/media/video_capture_buffer_pool.cc
@@ -42,6 +42,22 @@ base::SharedMemoryHandle VideoCaptureBufferPool::ShareToProcess(
   return remote_handle;
 }
 
+bool VideoCaptureBufferPool::GetBufferInfo(int buffer_id,
+                                           void** memory,
+                                           size_t* size) {
+  base::AutoLock lock(lock_);
+
+  Buffer* buffer = GetBuffer(buffer_id);
+  if (!buffer) {
+    NOTREACHED() << "Invalid buffer_id.";
+    return false;
+  }
+
+  *memory = buffer->shared_memory.memory();
+  *size = buffer->shared_memory.mapped_size();
+  return true;
+}
+
 int VideoCaptureBufferPool::ReserveForProducer(size_t size,
                                                int* buffer_id_to_drop) {
   base::AutoLock lock(lock_);
@@ -91,10 +107,10 @@ void VideoCaptureBufferPool::RelinquishConsumerHold(int buffer_id,
 }
 
 int VideoCaptureBufferPool::RecognizeReservedBuffer(
-    base::SharedMemoryHandle maybe_belongs_to_pool) {
+    const void* maybe_belongs_to_pool) {
   base::AutoLock lock(lock_);
   for (BufferMap::iterator it = buffers_.begin(); it != buffers_.end(); it++) {
-    if (it->second->shared_memory.handle() == maybe_belongs_to_pool) {
+    if (it->second->shared_memory.memory() == maybe_belongs_to_pool) {
       DCHECK(it->second->held_by_producer);
       return it->first;
     }
@@ -102,58 +118,23 @@ int VideoCaptureBufferPool::RecognizeReservedBuffer(
   return kInvalidId;  // Buffer is not from our pool.
 }
 
-scoped_refptr<media::VideoFrame> VideoCaptureBufferPool::ReserveI420VideoFrame(
-    const gfx::Size& size,
-    int rotation,
-    int* buffer_id_to_drop) {
-  base::AutoLock lock(lock_);
-
-  size_t frame_bytes =
-      media::VideoFrame::AllocationSize(media::VideoFrame::I420, size);
-
-  int buffer_id = ReserveForProducerInternal(frame_bytes, buffer_id_to_drop);
-  if (buffer_id == kInvalidId)
-    return NULL;
-
-  base::Closure disposal_handler = base::Bind(
-      &VideoCaptureBufferPool::RelinquishProducerReservation,
-      this,
-      buffer_id);
-
-  Buffer* buffer = GetBuffer(buffer_id);
-  // Wrap the buffer in a VideoFrame container.
-  scoped_refptr<media::VideoFrame> frame =
-      media::VideoFrame::WrapExternalSharedMemory(
-          media::VideoFrame::I420,
-          size,
-          gfx::Rect(size),
-          size,
-          static_cast<uint8*>(buffer->shared_memory.memory()),
-          frame_bytes,
-          buffer->shared_memory.handle(),
-          base::TimeDelta(),
-          disposal_handler);
-
-  if (buffer->rotation != rotation) {
-    // TODO(jiayl): Generalize the |rotation| mechanism.
-    media::FillYUV(frame.get(), 0, 128, 128);
-    buffer->rotation = rotation;
-  }
-
-  return frame;
-}
-
 VideoCaptureBufferPool::Buffer::Buffer()
-    : rotation(0),
-      held_by_producer(false),
-      consumer_hold_count(0) {}
+    : held_by_producer(false), consumer_hold_count(0) {}
 
 int VideoCaptureBufferPool::ReserveForProducerInternal(size_t size,
                                                        int* buffer_id_to_drop) {
   lock_.AssertAcquired();
 
-  // Look for a buffer that's allocated, big enough, and not in use.
+  // Even zero-sized placeholder buffers need some memory allocation, so that
+  // RecognizeReservedBuffer() can have some pointer to recognize.

[ncarter (slow), 2013/11/09 01:26:38]

Is there an alternative to creating a dummy shared memory allocation? You're not
allocating a byte here, you're almost certainly allocating full 4KB page (as
this is shareable memory). And having the pointer around seems like it has the
potential to create some confusion.

Now that we're handing out int buffer_ids, it seems like we don't need
RecognizeReservedBuffer at all?

[sheu, 2013/11/11 22:37:44]

I was hesitant about exporting buffer_id, but I've decided to go ahead and do
that.

RecongizeReservedBuffer removed.

+  if (!size)
+    size = 1;
+
+  // Look for a buffer that's allocated, big enough, and not in use. Track the
+  // largest one that's not big enough, in case we have to reallocate a buffer.

[jiayl, 2013/11/08 22:16:46]

Why is the largest one preferred for reallocation? It seems to me reallocating
the smallest one could potentially minimize reallocations in the future.

[sheu, 2013/11/09 00:59:30]

I'd like reallocations of large buffer first so we can reduce memory footprint,
as soon as possible after switching to a smaller buffer size.

   *buffer_id_to_drop = kInvalidId;
+  size_t realloc_size = 0;
+  BufferMap::iterator realloc = buffers_.end();
   for (BufferMap::iterator it = buffers_.begin(); it != buffers_.end(); it++) {
     Buffer* buffer = it->second;
     if (!buffer->consumer_hold_count && !buffer->held_by_producer) {
@@ -162,35 +143,33 @@ int VideoCaptureBufferPool::ReserveForProducerInternal(size_t size,
         buffer->held_by_producer = true;
         return it->first;
       }
+      if (buffer->shared_memory.requested_size() > realloc_size) {
+        realloc_size = buffer->shared_memory.requested_size();
+        realloc = it;
+      }
     }
   }
 
-  // Look for a buffer that's not in use, that we can reallocate.
-  for (BufferMap::iterator it = buffers_.begin(); it != buffers_.end(); it++) {
-    Buffer* buffer = it->second;
-    if (!buffer->consumer_hold_count && !buffer->held_by_producer) {
-      // Existing buffer is too small. Free it so we can allocate a new one
-      // after the loop.
-      *buffer_id_to_drop = it->first;
-      buffers_.erase(it);
-      delete buffer;
-      break;
-    }
-  }
-
-  // If possible, grow the pool by creating a new buffer.
-  if (static_cast<int>(buffers_.size()) < count_) {
-    int buffer_id = next_buffer_id_++;
-    scoped_ptr<Buffer> buffer(new Buffer());
-    if (!buffer->shared_memory.CreateAndMapAnonymous(size))
+  // Preferentially grow the pool by creating a new buffer. If we're at maximum
+  // size, then reallocate by deleting an existing one instead.
+  if (buffers_.size() == static_cast<size_t>(count_)) {
+    if (realloc == buffers_.end()) {
+      // We're out of space, and can't find an unused buffer to reallocate.
       return kInvalidId;
-    buffer->held_by_producer = true;
-    buffers_[buffer_id] = buffer.release();
-    return buffer_id;
+    }
+    *buffer_id_to_drop = realloc->first;
+    delete realloc->second;
+    buffers_.erase(realloc);
   }
 
-  // The pool is at its size limit, and all buffers are in use.
-  return kInvalidId;
+  // Create the new buffer.
+  int buffer_id = next_buffer_id_++;
+  scoped_ptr<Buffer> buffer(new Buffer());
+  if (!buffer->shared_memory.CreateAndMapAnonymous(size))
+    return kInvalidId;
+  buffer->held_by_producer = true;
+  buffers_[buffer_id] = buffer.release();
+  return buffer_id;
 }
 
 VideoCaptureBufferPool::Buffer* VideoCaptureBufferPool::GetBuffer(