MJPEG acceleration for video capture, browser part

content/browser/renderer_host/gpu_jpeg_decoder.h

Index: content/browser/renderer_host/gpu_jpeg_decoder.h
diff --git a/content/browser/renderer_host/gpu_jpeg_decoder.h b/content/browser/renderer_host/gpu_jpeg_decoder.h
new file mode 100644
index 0000000000000000000000000000000000000000..13a34a514cba5f4aabb9900b4eea8f5fb5daf9e9
--- /dev/null
+++ b/content/browser/renderer_host/gpu_jpeg_decoder.h
@@ -0,0 +1,121 @@
+// Copyright 2015 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 CONTENT_BROWSER_RENDERER_HOST_GPU_JPEG_DECODER_H_
+#define CONTENT_BROWSER_RENDERER_HOST_GPU_JPEG_DECODER_H_
+
+#include <string>
+
+#include "base/callback.h"
+#include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/memory/weak_ptr.h"
+#include "base/single_thread_task_runner.h"
+#include "base/threading/thread.h"
+#include "content/common/content_export.h"
+#include "media/video/capture/video_capture_device.h"

[Pawel Osciak, 2015/06/22 08:51:23]

Would it be enough to declare media::VideoCaptureDevice::Client::Buffer?

[kcwu, 2015/06/23 13:58:25]

AFAIK, there is no way to forward declare an inner class.
http://stackoverflow.com/questions/1021793/

+#include "media/video/jpeg_decode_accelerator.h"
+
+namespace media {
+class VideoFrame;
+}
+
+namespace content {
+class GpuChannelHost;
+
+// 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.

[Pawel Osciak, 2015/06/22 08:51:23]

Should we mention explicitly that Client methods should be called on Browser IO
thread?

[kcwu, 2015/06/23 13:58:25]

Done.

+class CONTENT_EXPORT GpuJpegDecoder

[Pawel Osciak, 2015/06/22 08:51:23]

Since this is a Capture-specific class, I'd prefer renaming it to indicate that.
Otherwise this looks like a general GpuJpegDecoder that could be reused for
other uses, which it is not. Perhaps GpuJpegDecoderForVideoCapture? Or maybe
even have a subclass adapter that handles the Capture-specific parts?

[kcwu, 2015/06/22 09:18:48]

I was thinking it's an adapter in the other direction and named it 
GpuJpegDecodeAcceleratorAdapter and wucheng suggested current one.
@wuchengli, what do you think?

[kcwu, 2015/06/23 13:58:25]

wucheng has flaky network and we chatted off-line.
He said he prefer video_capture_gpu_jpeg_decoder among
 video_capture_jpeg_decoder, video_capture_gpu_jpeg_decoder, and
gpu_jpeg_decoder.
He also mentioned this class should be under renderer_host/media.

If there is no objection, I will rename this class to VideoCaptureGpuJpegDecoder
and move the file to renderer_host/media tomorrow.

+    : public media::JpegDecodeAccelerator::Client,
+      public base::NonThreadSafe,
+      public base::SupportsWeakPtr<GpuJpegDecoder> {
+ public:
+  typedef base::Callback<void(
+      scoped_ptr<media::VideoCaptureDevice::Client::Buffer>,
+      const scoped_refptr<media::VideoFrame>&,
+      const base::TimeTicks&)> DecodeDoneCB;
+  typedef base::Callback<void(const std::string&)> ErrorCB;
+
+  // Returns true if JPEG hardware decoding is supported on this device.
+  static bool Supported();
+
+  // |decode_done_cb| is called when decode succeed.
+  // |error_cb| is called when error.
+  // Both |decode_done_cb| and |error_cb| are called on the IO thread and never
+  // after GpuJpegDecoder is destroyed.
+  GpuJpegDecoder(const DecodeDoneCB& decode_done_cb, const ErrorCB& error_cb);
+  ~GpuJpegDecoder();
+
+  // Creates and intializes decoder asynchronously.
+  void Initialize();
+
+  // Returns true if Initialize is done and it's okay to call
+  // DecodeCapturedData.
+  bool ReadyToDecode() { return decoder_; }

[Pawel Osciak, 2015/06/22 08:51:23]

Should this DCHECK(CalledOnValidThread()) ?

[kcwu, 2015/06/23 13:58:25]

Done.

+
+  // Decodes a JPEG picture.
+  void DecodeCapturedData(
+      const uint8* data,

[Pawel Osciak, 2015/06/22 08:51:23]

s/uint8/uint8_t/ ?

[kcwu, 2015/06/23 13:58:25]

Done.

+      size_t in_buffer_size,
+      const media::VideoCaptureFormat& frame_format,
+      const base::TimeTicks& timestamp,
+      scoped_ptr<media::VideoCaptureDevice::Client::Buffer> out_buffer);
+
+  // JpegDecodeAccelerator::Client implementation.
+  // These will be called on IO thread.
+  virtual void VideoFrameReady(int32_t buffer_id) override;
+  virtual void NotifyError(int32_t buffer_id,
+                           media::JpegDecodeAccelerator::Error error) override;
+
+ private:
+  // Initialization helper, to establish GPU channel

[Pawel Osciak, 2015/06/22 08:51:23]

Nit: dot at the end of sentences please.

[kcwu, 2015/06/23 13:58:24]

Done.

+  static void EstablishGpuChannelOnUIThread(
+      const scoped_refptr<base::SingleThreadTaskRunner>& task_runner,
+      base::WeakPtr<GpuJpegDecoder> weak_this);
+
+  static void GpuChannelEstablishedOnUIThread(
+      const scoped_refptr<base::SingleThreadTaskRunner>& task_runner,
+      base::WeakPtr<GpuJpegDecoder> weak_this);
+
+  void InitializeDone(scoped_refptr<GpuChannelHost> gpu_channel_host);
+
+  // Returns true if the decoding of last frame is not finished yet.
+  bool IsDecoding_Locked();
+
+  scoped_refptr<GpuChannelHost> gpu_channel_host_;
+
+  // The underlying JPEG decode accelerator.
+  scoped_ptr<media::JpegDecodeAccelerator> decoder_;
+
+  // The callback to run when decode succeeds.
+  const DecodeDoneCB decode_done_cb_;
+
+  // Guards |decode_done_closure_| and |error_cb_|.
+  base::Lock lock_;

[Pawel Osciak, 2015/06/22 08:51:23]

Should we just trampoline NotifyError() and VideoFrameReady() to the main thread
this class runs on (same as Decode()) and eliminate the lock entirely?

[kcwu, 2015/06/22 09:18:48]

They cannot be on the device thread. The device thread often blocks on waiting
ioctl for next frame. If they are trampolined to the device thread, the capture
fps becomes half (15fps) because the ready event blocked by next incoming frame.

+
+  // The callback to run when an error occurs.
+  const ErrorCB error_cb_;
+
+  // The closure of |decode_done_cb_| with bound parameters.
+  base::Closure decode_done_closure_;
+
+  // Next id for |in_buffer_|.
+  int32 next_bitstream_buffer_id_;
+
+  // Shared memory to store JPEG stream buffer. |in_buffer_| is backed by this.
+  scoped_ptr<base::SharedMemory> in_shared_memory_;
+
+  // JPEG stream buffer as input to JpegDecodeAccelerator.
+  media::BitstreamBuffer in_buffer_;

[Pawel Osciak, 2015/06/22 08:51:23]

Should we say this is the in_buffer_ currently being decoded?

[kcwu, 2015/06/23 13:58:25]

Done. (applied to in_buffer_id_)

+
+  DISALLOW_COPY_AND_ASSIGN(GpuJpegDecoder);
+};
+
+}  // namespace content
+
+#endif  // CONTENT_BROWSER_RENDERER_HOST_GPU_JPEG_DECODER_H_