Break tight coupling of VideoCaptureDeviceClient to renderer_host

content/browser/renderer_host/media/video_capture_gpu_jpeg_decoder.h

Index: content/browser/renderer_host/media/video_capture_gpu_jpeg_decoder.h
diff --git a/content/browser/renderer_host/media/video_capture_gpu_jpeg_decoder.h b/content/browser/renderer_host/media/video_capture_gpu_jpeg_decoder.h
index c1541ec7d2b6cc8db91dff3b264962c710bf3e7f..9a6ae3d14410d9be0c446b47001b6c5297fd8aad 100644
--- a/content/browser/renderer_host/media/video_capture_gpu_jpeg_decoder.h
+++ b/content/browser/renderer_host/media/video_capture_gpu_jpeg_decoder.h
@@ -31,17 +31,7 @@ class VideoFrame;
 
 namespace content {
 
-// Adapter to GpuJpegDecodeAccelerator for VideoCaptureDevice::Client. It takes
-// care of GpuJpegDecodeAccelerator creation, shared memory, and threading
-// issues.
-//
-// All public methods except JpegDecodeAccelerator::Client ones should be called
-// on the same thread. JpegDecodeAccelerator::Client methods should be called on
-// the IO thread.
-class CONTENT_EXPORT VideoCaptureGpuJpegDecoder
-    : public media::JpegDecodeAccelerator::Client,
-      public base::NonThreadSafe,
-      public base::SupportsWeakPtr<VideoCaptureGpuJpegDecoder> {
+class CONTENT_EXPORT VideoCaptureJpegDecoder {
  public:
   // Enumeration of decoder status. The enumeration is published for clients to
   // decide the behavior according to STATUS.
@@ -52,31 +42,65 @@ class CONTENT_EXPORT VideoCaptureGpuJpegDecoder
                    // decode error.
   };
 
-  typedef base::Callback<void(
+  using DecodeDoneCB = base::Callback<void(
       std::unique_ptr<media::VideoCaptureDevice::Client::Buffer>,
-      const scoped_refptr<media::VideoFrame>&)>
-      DecodeDoneCB;
+      const scoped_refptr<media::VideoFrame>&)>;
 
-  // |decode_done_cb| is called on the IO thread when decode succeed. This can
-  // be on any thread. |decode_done_cb| is never called after
-  // VideoCaptureGpuJpegDecoder is destroyed.
-  explicit VideoCaptureGpuJpegDecoder(const DecodeDoneCB& decode_done_cb);
-  ~VideoCaptureGpuJpegDecoder() override;
+  virtual ~VideoCaptureJpegDecoder() {}
 
   // Creates and intializes decoder asynchronously.
-  void Initialize();
+  virtual void Initialize() = 0;
 
   // Returns initialization status.
-  STATUS GetStatus() const;
+  virtual STATUS GetStatus() = 0;
 
   // Decodes a JPEG picture.
+  virtual void DecodeCapturedData(
+      const uint8_t* data,
+      size_t in_buffer_size,
+      const media::VideoCaptureFormat& frame_format,
+      base::TimeTicks reference_time,
+      base::TimeDelta timestamp,
+      std::unique_ptr<media::VideoCaptureDevice::Client::Buffer>
+          out_buffer) = 0;
+};
+
+class CONTENT_EXPORT VideoCaptureJpegDecoderFactory {

[emircan, 2016/09/08 19:57:56]

Can you look if there is test coverage for this class? AFAIK, this only kicks in
for some CrOS devices when there is MJPEG camera input. They might not be
running on the waterfall.

[chfremer, 2016/09/09 00:55:38]

For this CL I am not too worried (yet) about breaking anything, since extracting
the interfaces seems relatively safe.

I will look into the test coverage question before moving on with the next CL.
If I find that there is no tests exercising this, I will consider adding some
before moving on.

Added this task as CL1.6.1.1 to the design doc [1].

[1]
https://docs.google.com/document/d/1Qw7rw1AJy0QHXjha36jZNiEuxsxWslJ_X-zpOhijv...

[emircan, 2016/09/10 00:08:46]

Thanks for adding the task. For now, we can test at least AppRTC loopback on
Cros with HW MJPEG. This CL actually touches the production code and most CrOS
testing is done manually AFAIK.

+ public:
+  virtual ~VideoCaptureJpegDecoderFactory() {}
+  virtual std::unique_ptr<VideoCaptureJpegDecoder> CreateJpegDecoder() = 0;
+};
+
+// Adapter to GpuJpegDecodeAccelerator for VideoCaptureDevice::Client. It takes
+// care of GpuJpegDecodeAccelerator creation, shared memory, and threading
+// issues.
+//
+// All public methods except JpegDecodeAccelerator::Client ones should be called
+// on the same thread. JpegDecodeAccelerator::Client methods should be called on
+// the IO thread.
+class CONTENT_EXPORT VideoCaptureGpuJpegDecoder
+    : public VideoCaptureJpegDecoder,
+      public media::JpegDecodeAccelerator::Client,
+      public base::NonThreadSafe,
+      public base::SupportsWeakPtr<VideoCaptureGpuJpegDecoder> {
+ public:
+  // |decode_done_cb| is called on the IO thread when decode succeed. This can
+  // be on any thread. |decode_done_cb| is never called after
+  // VideoCaptureGpuJpegDecoder is destroyed.
+  explicit VideoCaptureGpuJpegDecoder(const DecodeDoneCB& decode_done_cb);
+  ~VideoCaptureGpuJpegDecoder() override;
+
+  // Implementation of VideoCaptureGpuJpegDecoderInterface:
+  void Initialize() override;
+  STATUS GetStatus() override;
   void DecodeCapturedData(
       const uint8_t* data,
       size_t in_buffer_size,
       const media::VideoCaptureFormat& frame_format,
       base::TimeTicks reference_time,
       base::TimeDelta timestamp,
-      std::unique_ptr<media::VideoCaptureDevice::Client::Buffer> out_buffer);
+      std::unique_ptr<media::VideoCaptureDevice::Client::Buffer> out_buffer)
+      override;
 
   // JpegDecodeAccelerator::Client implementation.
   // These will be called on IO thread.