VideoCaptureBufferPool: Refactor to allow support of non-ShMem backed buffers

content/browser/renderer_host/media/video_capture_buffer_pool.h

Index: content/browser/renderer_host/media/video_capture_buffer_pool.h
diff --git a/content/browser/renderer_host/media/video_capture_buffer_pool.h b/content/browser/renderer_host/media/video_capture_buffer_pool.h
index 54cb0c34bc5abd1bd649e306f1baafeb1d74085e..080d852ace1f9412455b12537fa83614eeb5134c 100644
--- a/content/browser/renderer_host/media/video_capture_buffer_pool.h
+++ b/content/browser/renderer_host/media/video_capture_buffer_pool.h
@@ -14,21 +14,17 @@
 #include "base/process/process.h"
 #include "base/synchronization/lock.h"
 #include "content/common/content_export.h"
+#include "media/base/video_capture_types.h"
 #include "ui/gfx/geometry/size.h"
 
-namespace media {
-
-class VideoFrame;
-
-}  // namespace media
-
 namespace content {
 
 // A thread-safe class that does the bookkeeping and lifetime management for a
-// pool of shared-memory pixel buffers cycled between an in-process producer
-// (e.g. a VideoCaptureDevice) and a set of out-of-process consumers. The pool
-// is intended to be orchestrated by a VideoCaptureController, but is designed
-// to outlive the controller if necessary.
+// pool of pixel buffers cycled between an in-process producer (e.g. a
+// VideoCaptureDevice) and a set of out-of-process consumers. The pool is
+// intended to be orchestrated by a VideoCaptureController, but is designed
+// to outlive the controller if necessary. The pixel buffers may be backed by a
+// SharedMemory, but this is not compulsory.
 //
 // Producers get a buffer by calling ReserveForProducer(), and may pass on their
 // ownership to the consumer by calling HoldForConsumers(), or drop the buffer
@@ -58,7 +54,7 @@ class CONTENT_EXPORT VideoCaptureBufferPool
 
   // Query the memory parameters of |buffer_id|. Fills in parameters in the
   // pointer arguments, and returns true iff the buffer exists.
-  bool GetBufferInfo(int buffer_id, void** memory, size_t* size);
+  bool GetBufferInfo(int buffer_id, void** storage, size_t* size);
 
   // Reserve or allocate a buffer of at least |size| bytes and return its id.
   // This will fail (returning kInvalidId) if the pool already is at its |count|
@@ -73,7 +69,8 @@ class CONTENT_EXPORT VideoCaptureBufferPool
   // On occasion, this call will decide to free an old buffer to make room for a
   // new allocation at a larger size. If so, the ID of the destroyed buffer is
   // returned via |buffer_id_to_drop|.
-  int ReserveForProducer(size_t size, int* buffer_id_to_drop);
+  int ReserveForProducer(const media::VideoCaptureFormat& format,
+                         int* buffer_id_to_drop);
 
   // Indicate that a buffer held for the producer should be returned back to the
   // pool without passing on to the consumer. This effectively is the opposite
@@ -93,29 +90,47 @@ class CONTENT_EXPORT VideoCaptureBufferPool
   int count() const { return count_; }
 
  private:
-  friend class base::RefCountedThreadSafe<VideoCaptureBufferPool>;
-
-  // Per-buffer state.
-  struct Buffer {
-    Buffer();
-
-    // The memory created to be shared with renderer processes.
-    base::SharedMemory shared_memory;
-
-    // Tracks whether this buffer is currently referenced by the producer.
-    bool held_by_producer;
-
-    // Number of consumer processes which hold this shared memory.
-    int consumer_hold_count;
+  class SharedMemTracker;
+  // Generic class to keep track of the state of a given mappable resource.
+  class Tracker {
+   public:
+    static scoped_ptr<Tracker> CreateTracker();
+
+    Tracker() : held_by_producer_(false), consumer_hold_count_(0) {}
+    virtual bool Init(const gfx::Size& dimensions) = 0;
+    virtual ~Tracker();
+
+    bool held_by_producer() const { return held_by_producer_; }
+    void set_held_by_producer(bool value) { held_by_producer_ = value; }
+    int consumer_hold_count() const { return consumer_hold_count_; }
+    void set_consumer_hold_count(int value) { consumer_hold_count_ = value; }
+
+    // Returns a void* to the underlying storage, be that a memory block for
+    // Shared Memory, or a GpuMemoryBuffer.
+    virtual void* storage() = 0;
+    // Amount of bytes requested when first created. Can be zero if it does not
+    // need RAM, e.g. is allocated in GPU memory.
+    virtual size_t requested_size() = 0;
+    // The actual size of the underlying backing resource.
+    virtual size_t mapped_size() = 0;
+
+    virtual bool ShareToProcess(base::ProcessHandle process_handle,
+                      base::SharedMemoryHandle* new_handle) = 0;
+
+   private:
+    // Indicates whether this Tracker is currently referenced by the producer.
+    bool held_by_producer_;
+    // Number of consumer processes which hold this Tracker.
+    int consumer_hold_count_;
   };
 
-  typedef std::map<int, Buffer*> BufferMap;
-
+  friend class base::RefCountedThreadSafe<VideoCaptureBufferPool>;
   virtual ~VideoCaptureBufferPool();
 
-  int ReserveForProducerInternal(size_t size, int* buffer_id_to_drop);
+  int ReserveForProducerInternal(const media::VideoCaptureFormat& format,
+                                 int* buffer_id_to_drop);
 
-  Buffer* GetBuffer(int buffer_id);
+  Tracker* GetBuffer(int buffer_id);

[miu, 2015/04/08 01:20:00]

naming: Should be GetTracker() now.  Also, there seems to be a lot of variables
in the .cc file named "buffer" that should be renamed.

[mcasas, 2015/04/08 22:07:05]

Done.

 
   // The max number of buffers that the pool is allowed to have at any moment.
   const int count_;
@@ -127,6 +142,7 @@ class CONTENT_EXPORT VideoCaptureBufferPool
   int next_buffer_id_;
 
   // The buffers, indexed by |buffer_id|.
+  typedef std::map<int, Tracker*> BufferMap;

[miu, 2015/04/08 01:20:00]

Consider the newer, more readable C++11 style:

using BufferMap = std::map<int, Tracker*>;

[mcasas, 2015/04/08 22:07:05]

Done.

   BufferMap buffers_;
 
   DISALLOW_IMPLICIT_CONSTRUCTORS(VideoCaptureBufferPool);