Tab Capture: Backing store readbacks to YV12 VideoFrames.

content/browser/renderer_host/media/video_capture_controller.h

 // Copyright (c) 2012 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.
 
 // VideoCaptureController is the glue between VideoCaptureHost,
 // VideoCaptureManager and VideoCaptureDevice.
 // It provides functions for VideoCaptureHost to start a VideoCaptureDevice and
 // is responsible for keeping track of shared DIBs and filling them with I420
 // video frames for IPC communication between VideoCaptureHost and
 // VideoCaptureMessageFilter.
@@ skipping 47 matching lines @@
   // prematurely closed.
   void StopSession(int session_id);
 
   // Return a buffer previously given in
   // VideoCaptureControllerEventHandler::OnBufferReady.
   void ReturnBuffer(const VideoCaptureControllerID& id,
                     VideoCaptureControllerEventHandler* event_handler,
                     int buffer_id);
 
   // Implement media::VideoCaptureDevice::EventHandler.
-  virtual void OnIncomingCapturedFrame(const uint8* data,
-                                       int length,
+  virtual void OnIncomingCapturedFrame(const uint8* data, int length,
                                        base::Time timestamp) OVERRIDE;
+  virtual void OnIncomingCapturedVideoFrame(media::VideoFrame* frame,
+                                            base::Time timestamp) OVERRIDE;
   virtual void OnError() OVERRIDE;
   virtual void OnFrameInfo(
       const media::VideoCaptureCapability& info) OVERRIDE;
 
  protected:
   virtual ~VideoCaptureController();
 
  private:
   friend class base::RefCountedThreadSafe<VideoCaptureController>;
 
   struct ControllerClient;
   typedef std::list<ControllerClient*> ControllerClients;
 
   // Callback when manager has stopped device.
   void OnDeviceStopped();
 
   // Worker functions on IO thread.
   void DoIncomingCapturedFrameOnIOThread(int buffer_id, base::Time timestamp);
   void DoFrameInfoOnIOThread();
   void DoErrorOnIOThread();
   void DoDeviceStoppedOnIOThread();
 
   // Send frame info and init buffers to |client|.
   void SendFrameInfoAndBuffers(ControllerClient* client, int buffer_size);
 
   // Helpers.

[scherkus (not reviewing), 2013/02/05 22:40:44]

nit: this comment is useless -- mind removing?

[ncarter (slow), 2013/02/06 23:54:44]

Done, but I welcome wjia to contradict.

FWIW, I interpreted this comment as "from here on down these are implementation
helper functions, as opposed to trampoline targets." Touching this code for the
first time, I benefited from that hint. I also interpreted the lack of spacing
between the definitions was done so that it was clear that the 'Helpers' comment
applied to everything in the wall of text. The wall of text effect disappears if
you use an editor which syntax-colors the comments, there is a nice visual flow
in that case, so personally I am not bothered by such a lack of vertical
whitespace.

[scherkus (not reviewing), 2013/02/07 18:25:03]

My $0.02 is making it readable for the lowest common denominator means
readability will always be improved regardless of someone's editor preference.

Example: this code review

   // Find a client of |id| and |handler| in |clients|.
   ControllerClient* FindClient(
       const VideoCaptureControllerID& id,
       VideoCaptureControllerEventHandler* handler,
       const ControllerClients& clients);
   // Find a client of |session_id| in |clients|.
   ControllerClient* FindClient(
       int session_id,
       const ControllerClients& clients);
   // Decide what to do after kStopping state. Dependent on events, controller
   // can stay in kStopping state, or go to kStopped, or restart capture.
   void PostStopping();
   // Check if any DIB is used by client.
   bool ClientHasDIB();
+  // DIB allocator. Locate a free DIB object, reserve it, and return its id

[scherkus (not reviewing), 2013/02/05 22:40:44]

nit: can you add blank lines before each of these // comments to avoid
walloftext code?

[ncarter (slow), 2013/02/06 23:54:44]

Done.

+  // as well as pointers to its color planes. Returns true if successful.
+  bool ReserveSharedMemory(int* buffer_id_out,

[scherkus (not reviewing), 2013/02/05 22:40:44]

nit: the comment is a bit misleading as there is no allocation being performed
but rather finding an available DIB in owned_dibs_

[ncarter (slow), 2013/02/06 23:54:44]

I see what you mean. Fixed the comment language.

+                           uint8** yplane,
+                           uint8** uplane,
+                           uint8** vplane);
 
   // Lock to protect free_dibs_ and owned_dibs_.
   base::Lock lock_;
 
   struct SharedDIB;
   typedef std::map<int /*buffer_id*/, SharedDIB*> DIBMap;
   // All DIBs created by this object.
   // It's modified only on IO thread.
   DIBMap owned_dibs_;
 
@@ skipping 21 matching lines @@
 
   bool device_in_use_;
   VideoCaptureState state_;
 
   DISALLOW_IMPLICIT_CONSTRUCTORS(VideoCaptureController);
 };
 
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_RENDERER_HOST_MEDIA_VIDEO_CAPTURE_CONTROLLER_H_