for readability review.

content/renderer/media/video_capture_impl.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.
 
 // VideoCaptureImpl represents a capture device in renderer process. It provides
 // interfaces for clients to Start/Stop capture. It also communicates to clients
-// when buffer is ready, state of capture device is changed.
+// when buffer is ready,  state of capture device is changed.
 
 #ifndef CONTENT_RENDERER_MEDIA_VIDEO_CAPTURE_IMPL_H_
 #define CONTENT_RENDERER_MEDIA_VIDEO_CAPTURE_IMPL_H_
 
 #include <list>
 #include <map>
 
 #include "content/common/content_export.h"
 #include "content/common/media/video_capture.h"
 #include "content/renderer/media/video_capture_message_filter.h"
 #include "media/video/capture/video_capture.h"
 #include "media/video/capture/video_capture_types.h"
 
 namespace base {
 class MessageLoopProxy;
 }
 

[zunger, 2012/07/09 20:08:26]

What's the thread-safety of this class?

[wjia(left Chromium), 2012/07/10 17:32:28]

All data processing happens on capture thread which is represented by
|capture_message_loop_proxy_|. Therefore, no lock/semaphore is used in this
class.

Whenever a client calls StartCapture or StopCapture, or
VideoCaptureMessageFilter calls its delegate functions, VideoCaptureImpl will
post a task to capture thread. This makes data access safe.

 class CONTENT_EXPORT VideoCaptureImpl
     : public media::VideoCapture, public VideoCaptureMessageFilter::Delegate {

[zunger, 2012/07/09 20:08:26]

I don't know if the Chromium codebase is somehow exempt from core bits of the
style guide, but the style guide forbids multiple inheritance except within
certain very narrowly defined guidelines. 

http://www.corp.google.com/eng/doc/cppguide.xml?showone=Multiple_Inheritance#...

[wjia(left Chromium), 2012/07/10 17:32:28]

Both base classes are pure interfaces. Please refer to
http://src.chromium.org/viewvc/chrome/trunk/src/content/renderer/media/video_...
and
http://src.chromium.org/viewvc/chrome/trunk/src/media/video/capture/video_cap...

  public:
   // media::VideoCapture interface.
   virtual void StartCapture(
       media::VideoCapture::EventHandler* handler,
       const media::VideoCaptureCapability& capability) OVERRIDE;
   virtual void StopCapture(media::VideoCapture::EventHandler* handler) OVERRIDE;
   virtual void FeedBuffer(scoped_refptr<VideoFrameBuffer> buffer) OVERRIDE;
   virtual bool CaptureStarted() OVERRIDE;
   virtual int CaptureWidth() OVERRIDE;
   virtual int CaptureHeight() OVERRIDE;
@@ skipping 44 matching lines @@
 
   // Helpers.
   bool ClientHasDIB();
 
   scoped_refptr<VideoCaptureMessageFilter> message_filter_;
   scoped_refptr<base::MessageLoopProxy> capture_message_loop_proxy_;
   scoped_refptr<base::MessageLoopProxy> io_message_loop_proxy_;
   int device_id_;
 
   // Pool of DIBs.
   typedef std::map<int /* buffer_id */, DIBBuffer*> CachedDIB;

[zunger, 2012/07/09 20:08:26]

/* */ comments forbidden.

[wjia(left Chromium), 2012/07/10 17:32:28]

Done.

   CachedDIB cached_dibs_;
 
   typedef std::map<media::VideoCapture::EventHandler*,
       media::VideoCaptureCapability> ClientInfo;
   ClientInfo clients_;
 
   ClientInfo clients_pending_on_filter_;
   ClientInfo clients_pending_on_restart_;
 
   media::VideoCaptureCapability::Format video_type_;
 
   // The parameter is being used in current capture session. A capture session
   // starts with StartCapture and ends with StopCapture.
   media::VideoCaptureParams current_params_;
 
   // The information about the device sent from browser process side.
   media::VideoCaptureParams device_info_;
   bool device_info_available_;
 
   video_capture::State state_;
 
   DISALLOW_COPY_AND_ASSIGN(VideoCaptureImpl);
 };
 
 #endif  // CONTENT_RENDERER_MEDIA_VIDEO_CAPTURE_IMPL_H_