Video Capture Mojo (1.4b): Implement ability to use fake device instance

services/video_capture/device_client_mojo_to_media_adapter.h

+// Copyright 2016 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 SERVICES_VIDEO_CAPTURE_DEVICE_CLIENT_MOJO_TO_MEDIA_ADAPTER_H_
+#define SERVICES_VIDEO_CAPTURE_DEVICE_CLIENT_MOJO_TO_MEDIA_ADAPTER_H_
+
+#include "media/capture/video/video_capture_device.h"
+#include "services/video_capture/public/interfaces/video_capture_device_client.mojom.h"
+
+namespace media {
+class MojoSharedBufferVideoFrame;
+}
+
+namespace video_capture {
+
+// Adapter that allows a mojom::VideoCaptureDeviceClient to be used in place of
+// a media::VideoCaptureDevice::Client.
+class DeviceClientMojoToMediaAdapter

[mcasas, 2016/08/30 01:20:26]

Hmm this name doesn't sound 100% right somehow.
Name wise, I see there are few _shim, and about
the same _adapter and _bridge, albeit >50% of the
adapters are in media related code. But I guess
adapter is fine. 
However, what this class does is to adapt the data
flow coming from a media::VCD towards a 
mojom::VideoCaptureDeviceClient, so I'd call it
MediaVideoCaptureDeviceToMojomAdapter :-) :-)
or some shortened version of it, WDYT?

[chfremer, 2016/08/30 18:22:54]

It seems that the terms "shim", "adapter", "bridge", and "wrapper" could all be
used here. I prefer "adapter", because it refers to the Adapter Pattern [1] as
described in "standard literature" of software engineering [2][3]. In addition
to just expressing that calls are redirected, it expresses the purpose of the
redirection, which is "The Adapter design pattern allows otherwise incompatible
classes to work together by converting the interface of one class into an
interface expected by the clients". I would avoid the term "bridge" as it could
cause confusion with the Bridge Pattern [4].

As for the naming of the adapter, there are two opposite but equivalent ways of
thinking about it. Your suggestion takes the perspective of client calls
arriving at the outer interface (media) and being adapted to the inner interface
(mojo). My naming choice takes the perspective of factory code providing an
instance exposing the inner interface (mojo) and using an adapter to convert it
to the outer interface (media). I prefer the second perspective, because the
factory code is the place where the name of the adapter class appears, and
clients of the outer interface should know nothing about the adapter.

[1] https://en.wikipedia.org/wiki/Adapter_pattern
[2] https://en.wikipedia.org/wiki/Design_Patterns
[3] http://www.headfirstlabs.com/books/hfdp/
[4] https://en.wikipedia.org/wiki/Bridge_pattern

+    : public media::VideoCaptureDevice::Client {
+ public:
+  using VideoCaptureFormat = media::VideoCaptureFormat;
+  using VideoPixelFormat = media::VideoPixelFormat;
+  using VideoPixelStorage = media::VideoPixelStorage;
+  using VideoFrame = media::VideoFrame;
+
+  DeviceClientMojoToMediaAdapter(mojom::VideoCaptureDeviceClientPtr client);
+  ~DeviceClientMojoToMediaAdapter() override;
+
+  // media::VideoCaptureDevice::Client:
+  void OnIncomingCapturedData(const uint8_t* data,
+                              int length,
+                              const VideoCaptureFormat& frame_format,
+                              int clockwise_rotation,
+                              base::TimeTicks reference_time,
+                              base::TimeDelta timestamp) override;
+  std::unique_ptr<Buffer> ReserveOutputBuffer(
+      const gfx::Size& dimensions,
+      VideoPixelFormat format,
+      VideoPixelStorage storage) override;
+  void OnIncomingCapturedBuffer(std::unique_ptr<Buffer> buffer,
+                                const VideoCaptureFormat& frame_format,
+                                base::TimeTicks reference_time,
+                                base::TimeDelta timestamp) override;
+  void OnIncomingCapturedVideoFrame(
+      std::unique_ptr<Buffer> buffer,
+      const scoped_refptr<VideoFrame>& frame) override;
+  std::unique_ptr<Buffer> ResurrectLastOutputBuffer(
+      const gfx::Size& dimensions,
+      VideoPixelFormat format,
+      VideoPixelStorage storage) override;
+  void OnError(const tracked_objects::Location& from_here,
+               const std::string& reason) override;
+  double GetBufferPoolUtilization() const override;
+
+ private:
+  mojom::VideoCaptureDeviceClientPtr client_;
+  scoped_refptr<media::MojoSharedBufferVideoFrame> dummy_frame_;

[mcasas, 2016/08/30 01:20:26]

nit: I'd add here the comment about this variable being 
temporary instead of l.31-32 of the .cc file and, just in 
case, quote the bug number 584797.

[chfremer, 2016/08/30 18:22:54]

Done.

+};
+
+}  // namespace video_capture
+
+#endif  // SERVICES_VIDEO_CAPTURE_DEVICE_CLIENT_MOJO_TO_MEDIA_ADAPTER_H_