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

content/browser/renderer_host/media/video_capture_buffer_pool.cc

 // Copyright (c) 2013 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 "content/browser/renderer_host/media/video_capture_buffer_pool.h"
 
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
@@ skipping 24 matching lines @@
   if (!buffer) {
     NOTREACHED() << "Invalid buffer_id.";
     return base::SharedMemory::NULLHandle();
   }
   base::SharedMemoryHandle remote_handle;
   buffer->shared_memory.ShareToProcess(process_handle, &remote_handle);
   *memory_size = buffer->shared_memory.requested_size();
   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_);
   return ReserveForProducerInternal(size, buffer_id_to_drop);
 }
 
 void VideoCaptureBufferPool::RelinquishProducerReservation(int buffer_id) {
   base::AutoLock lock(lock_);
   Buffer* buffer = GetBuffer(buffer_id);
   if (!buffer) {
@@ skipping 29 matching lines @@
   if (!buffer) {
     NOTREACHED() << "Invalid buffer_id.";
     return;
   }
   DCHECK_GE(buffer->consumer_hold_count, num_clients);
 
   buffer->consumer_hold_count -= num_clients;
 }
 
 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;
     }
   }
   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) {
       if (buffer->shared_memory.requested_size() >= size) {
         // Existing buffer is big enough. Reuse it.
         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;
+  // 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_id_to_drop = realloc->first;
+    delete realloc->second;
+    buffers_.erase(realloc);
   }
 
-  // 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))
-      return kInvalidId;
-    buffer->held_by_producer = true;
-    buffers_[buffer_id] = buffer.release();
-    return buffer_id;
-  }
-
-  // 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(
     int buffer_id) {
   BufferMap::iterator it = buffers_.find(buffer_id);
   if (it == buffers_.end())
     return NULL;
   return it->second;
 }
 
 }  // namespace content