media: add video capture device for ARC++ camera HAL v3

media/capture/video/chromeos/video_capture_device_factory_chromeos.h

Index: media/capture/video/chromeos/video_capture_device_factory_chromeos.h
diff --git a/media/capture/video/chromeos/video_capture_device_factory_chromeos.h b/media/capture/video/chromeos/video_capture_device_factory_chromeos.h
new file mode 100644
index 0000000000000000000000000000000000000000..0296173b81cbd9fdd42e5683da861240b845683c
--- /dev/null
+++ b/media/capture/video/chromeos/video_capture_device_factory_chromeos.h
@@ -0,0 +1,56 @@
+// Copyright 2017 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 MEDIA_CAPTURE_VIDEO_CHROMEOS_VIDEO_CAPTURE_DEVICE_FACTORY_CHROMEOS_H_
+#define MEDIA_CAPTURE_VIDEO_CHROMEOS_VIDEO_CAPTURE_DEVICE_FACTORY_CHROMEOS_H_
+
+#include <memory>
+
+#include "base/macros.h"
+#include "media/capture/video/chromeos/camera_hal_delegate.h"
+#include "media/capture/video/video_capture_device_factory.h"
+
+namespace media {
+
+class CAPTURE_EXPORT VideoCaptureDeviceFactoryChromeOS final
+    : public VideoCaptureDeviceFactory {
+ public:
+  explicit VideoCaptureDeviceFactoryChromeOS(
+      scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner);

[chfremer, 2017/05/24 23:36:43]

With the move to the video capture Mojo service, this may actually be running in
a utility process. In this context it is unclear what "ui_task_runner" would
mean. I recommend we rename this to indicate what the task runner is actually
being used for, e.g. task_runner_for_xyz.

[jcliang, 2017/05/25 14:23:48]

Done.

+
+  ~VideoCaptureDeviceFactoryChromeOS();
+
+  // VideoCaptureDeviceFactory interface implementations.
+  std::unique_ptr<VideoCaptureDevice> CreateDevice(
+      const VideoCaptureDeviceDescriptor& device_descriptor) final;
+  void GetSupportedFormats(
+      const VideoCaptureDeviceDescriptor& device_descriptor,
+      VideoCaptureFormats* supported_formats) final;
+  void GetDeviceDescriptors(
+      VideoCaptureDeviceDescriptors* device_descriptors) final;
+
+  // Initializes the factory. The factory is functional only after this call
+  // succeeds.  This method is called on the browser UI thread when the factory
+  // instance is created.

[chfremer, 2017/05/24 23:36:43]

Note, that with the move to the video capture Mojo service,
VideoCaptureDeviceFactories are no longer necessarily created and operated in
the browser process. This code should not make any assumptions about being run
on the browser UI thread.

[jcliang, 2017/05/25 14:23:48]

We do not depend on the thread being run on the browser UI thread. We can remove
or update the comments when we move the code to video capture Mojo service.

[chfremer, 2017/05/25 17:13:07]

The thing is that the code does not need to be moved to be used in the video
capture Mojo service. This code can (already today) be used in multiple
contexts, two of which are the in-process video capture stack and the video
capture Mojo service. A third one could be unit tests.

In general, I recommend not specifying anything about the context where a class
gets used in as part of the class itself, because, even if correct today, this
information very easily becomes inaccurate when the context changes and the
comment is not updated.

If there is any specific requirement on the thread that Init() must be called
on, please call out that requirement is specifically, e.g. "must be called from
a thread on which it is legal to do XYZ".

[jcliang, 2017/05/26 04:33:33]

I see. Thanks for explaining. I'll remove comments that specifically refer to
any browser process threads.

Are we using the video capture Mojo service in Chrome already? How can I force
the video capture device to run as a Mojo service? I'd like to verify running
the VCD in both the browser-process and the Mojo service contexts.

+  bool Init();
+
+ private:
+  const scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner_;
+
+  // The thread that all the Mojo operations of |camera_hal_delegate_| take
+  // place.  Started in Init and stopped when the class instance is destroyed.
+  base::Thread module_thread_;
+
+  // Communication interface to the camera HAL. |camera_hal_delegate_| is
+  // created on the thread where the VideoCaptureDeviceFactoryChromeOS was
+  // instantiated. All the Mojo communication that |camera_hal_delegate_|
+  // issues and receives must be sequenced through |module_thread_|.

[chfremer, 2017/05/24 23:36:43]

Nit: |camera_hal_delegate_| is actually create in the call to Init(), and there
is nothing in this class guaranteeing that this is the same thread as the on
calling the constructor. If we want to guarantee it, let us add a
base::SequenceChecker to enforce it. If we don't want to guarantee it, let us
relax the claims made in the comment.

[jcliang, 2017/05/25 14:23:48]

Updated the comment. We don't need to guarantee it.

+  scoped_refptr<CameraHalDelegate> camera_hal_delegate_;
+
+  DISALLOW_COPY_AND_ASSIGN(VideoCaptureDeviceFactoryChromeOS);
+};
+
+}  // namespace media
+
+#endif  // MEDIA_CAPTURE_VIDEO_CHROMEOS_VIDEO_CAPTURE_DEVICE_FACTORY_CHROMEOS_H_