Refactor video capture to new design

ppapi/proxy/video_capture_resource.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.
+
+#ifndef PPAPI_PROXY_VIDEO_CAPTURE_RESOURCE_H_
+#define PPAPI_PROXY_VIDEO_CAPTURE_RESOURCE_H_
+
+#include "base/compiler_specific.h"
+#include "ppapi/c/dev/ppp_video_capture_dev.h"
+#include "ppapi/proxy/ppapi_proxy_export.h"
+#include "ppapi/proxy/plugin_resource.h"
+#include "ppapi/shared_impl/api_id.h"

[yzshen1, 2012/11/06 06:51:40]

why do you need this?

[victorhsieh, 2012/11/08 09:20:18]

Done.

+#include "ppapi/thunk/ppb_video_capture_api.h"
+
+namespace ppapi {
+namespace proxy {
+
+class PPAPI_PROXY_EXPORT VideoCaptureResource

[yzshen1, 2012/11/06 06:51:40]

I don't think it is necessary to be exported.
(And you could remove line 10 and the NON_EXPORTED_BASE below.)

[victorhsieh, 2012/11/08 09:20:18]

Done.  It runs well, but I saw many resource headers have it?

+    : public PluginResource,
+      public NON_EXPORTED_BASE(::ppapi::thunk::PPB_VideoCapture_API) {
+ public:
+  VideoCaptureResource(Connection connection, PP_Instance instance,

[yzshen1, 2012/11/06 06:51:40]

According to style guide, you should not put multiple parameters on the same
line, if they cannot all fit in the same line as the method name. (This is the
rule for declaration/definition, not for method call.)

[victorhsieh, 2012/11/08 09:20:18]

Done.

+                       PluginDispatcher* dispatcher);
+  ~VideoCaptureResource();

[yzshen1, 2012/11/06 06:51:40]

virtual

[victorhsieh, 2012/11/08 09:20:18]

Done.

+
+  void OnReplyReceived(const ResourceMessageReplyParams& params,

[yzshen1, 2012/11/06 06:51:40]

- virtual.
- add "<some_base_class> implementation." or "<some_base_class> overrides.",
depending on whether you implement an interface or override it. 

[victorhsieh, 2012/11/08 09:20:18]

Done and moved to private.

+                       const IPC::Message& msg) OVERRIDE;
+
+  thunk::PPB_VideoCapture_API* AsPPB_VideoCapture_API() OVERRIDE {

[yzshen1, 2012/11/06 06:51:40]

ditto.

[victorhsieh, 2012/11/08 09:20:18]

Done.

+    return this;
+  }
+
+  // PPB_VideoCapture_API
+  int32_t EnumerateDevices(PP_Resource* devices,
+                           scoped_refptr<TrackedCallback> callback) OVERRIDE;
+  int32_t Open(const std::string& device_id,
+               const PP_VideoCaptureDeviceInfo_Dev& requested_info,
+               uint32_t buffer_count,
+               scoped_refptr<TrackedCallback> callback) OVERRIDE;
+  int32_t StartCapture() OVERRIDE;
+  int32_t ReuseBuffer(uint32_t buffer) OVERRIDE;
+  int32_t StopCapture() OVERRIDE;
+  void Close() OVERRIDE;
+  const std::vector<DeviceRefData>& GetDeviceRefData() const OVERRIDE {

[yzshen1, 2012/11/06 06:51:40]

you don't need this method.

[victorhsieh, 2012/11/08 09:20:18]

It's still referenced by PepperFlashHost.

[yzshen1, 2012/11/10 01:14:40]

You have to also change PepperFlashHost. It will break with the current code
because we don't have a real resource at the host side anymore. Please talk to
Raymes.

On 2012/11/08 09:20:18, Victor Hsieh wrote:

[victorhsieh, 2012/11/13 03:10:53]

Done.

+    return devices_data_;
+  }
+
+ private:
+  enum OpenState {
+    BEFORE_OPEN,
+    OPENED,
+    CLOSED
+  };
+
+  void OnDeviceInfo(const ResourceMessageReplyParams& params,
+                    const struct PP_VideoCaptureDeviceInfo_Dev& info,
+                    const std::vector<HostResource>& buffers,
+                    uint32_t buffer_size);
+  void OnStatus(const ResourceMessageReplyParams& params,
+                uint32_t status);
+  void OnError(const ResourceMessageReplyParams& params,
+               uint32_t error);
+  void OnBufferReady(const ResourceMessageReplyParams& params,
+                     uint32_t buffer);
+
+  void OnOpenComplete(const ResourceMessageReplyParams& params);

[yzshen1, 2012/11/06 06:51:40]

nit: The convention is to use OnPluginMsg<the last part of the message name>.
So this one should be OnPluginMsgOpenReply.
(And same comment for other method names.)

[victorhsieh, 2012/11/08 09:20:18]

Done.

+  void OnEnumerateDevicesComplete(PP_Resource* devices_output,
+                                  scoped_refptr<TrackedCallback> callback,
+                                  const ResourceMessageReplyParams& params,
+                                  const std::vector<DeviceRefData>& devices);
+
+  void SetBufferInUse(uint32_t buffer_index);
+
+  // Points to the client implementation of pp::VideoCaptureClient_Dev.

[yzshen1, 2012/11/06 06:51:40]

nit: inaccurate comment, this doesn't point to the cpp wrapper, it is a pointer
to the c interface.

[victorhsieh, 2012/11/08 09:20:18]

Done.

+  const PPP_VideoCapture_Dev* ppp_video_capture_impl_;
+
+  // Indicates that the i-th buffer is currently in use.
+  std::vector<bool> buffer_in_use_;
+
+  // Holds a reference of the callback so that Close() can cancel it.
+  scoped_refptr<TrackedCallback> open_callback_;
+  OpenState open_state_;
+
+  // Saves the output of EnumerateDevices().
+  std::vector<DeviceRefData> devices_data_;

[yzshen1, 2012/11/06 06:51:40]

you don't need this one.

[victorhsieh, 2012/11/08 09:20:18]

ditto, still referenced.

[yzshen1, 2012/11/10 01:14:40]

ditto.
On 2012/11/08 09:20:18, Victor Hsieh wrote:

[victorhsieh, 2012/11/13 03:10:53]

Done.

+  bool has_pending_enum_devices_callback_;
+};

[yzshen1, 2012/11/06 06:51:40]

DISALLOW_COPY_AND_ASSIGN, please.

[victorhsieh, 2012/11/08 09:20:18]

Done.

+
+}  // namespace proxy
+}  // namespace ppapi
+
+#endif  // PPAPI_PROXY_VIDEO_CAPTURE_RESOURCE_H_