H264 HW encode using MediaFoundation

media/gpu/media_foundation_video_encode_accelerator_win.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 CONTENT_COMMON_GPU_MEDIA_MEDIA_FOUNDATION_VIDEO_ENCODE_ACCELERATOR_H_

[grt (UTC plus 2), 2016/07/14 20:07:25]

CONTENT_COMMON -> MEDIA
H_ -> WIN_H_

[emircan, 2016/07/15 04:58:31]

Done. I must have missed it in one of the rebases.

+#define CONTENT_COMMON_GPU_MEDIA_MEDIA_FOUNDATION_VIDEO_ENCODE_ACCELERATOR_H_
+
+#include <mfapi.h>
+#include <mfidl.h>
+#include <strmif.h>
+
+#include "base/threading/thread_checker.h"
+#include "base/win/scoped_comptr.h"
+#include "media/gpu/media_gpu_export.h"
+#include "media/video/video_encode_accelerator.h"
+
+namespace media {
+
+// Media Foundation implementation of the VideoEncodeAccelerator interface for
+// Windows.
+// This class is pinned to the thread on which it is constructed and DCHECKs
+// that it is never accessed from any other. It starts an encoder thread on
+// which media::VideoEncodeAccelerator implementation tasks are posted.
+class MEDIA_GPU_EXPORT MediaFoundationVideoEncodeAccelerator
+    : public media::VideoEncodeAccelerator {

[grt (UTC plus 2), 2016/07/14 20:07:25]

nit: omit "media::" here and elsewhere since you're in the namespace.

[emircan, 2016/07/15 04:58:31]

Done.

+ public:
+  MediaFoundationVideoEncodeAccelerator();
+  ~MediaFoundationVideoEncodeAccelerator() override;
+
+  // media::VideoEncodeAccelerator implementation.
+  media::VideoEncodeAccelerator::SupportedProfiles GetSupportedProfiles()
+      override;
+  bool Initialize(media::VideoPixelFormat input_format,
+                  const gfx::Size& input_visible_size,
+                  media::VideoCodecProfile output_profile,
+                  uint32_t initial_bitrate,
+                  Client* client) override;
+  void Encode(const scoped_refptr<media::VideoFrame>& frame,
+              bool force_keyframe) override;
+  void UseOutputBitstreamBuffer(const media::BitstreamBuffer& buffer) override;
+  void RequestEncodingParametersChange(uint32_t bitrate,
+                                       uint32_t framerate) override;
+  void Destroy() override;
+
+  // Preload dlls required for decoding.
+  static void PreSandboxInitialization();
+
+ private:
+  // Holds output buffers coming from the client ready to be filled.
+  struct BitstreamBufferRef;
+
+  // Holds output buffers coming from the encoder.
+  class EncodeOutput;
+
+  // Initializes and allocates memory for input and output samples.
+  bool InitializeInputOutputSamples();
+
+  // Initializes encoder parameters for real-time use.
+  bool SetEncoderModes();
+
+  // Encoding tasks to be run on |encoder_thread_|.
+  void EncodeTask(const scoped_refptr<media::VideoFrame>& frame,
+                  bool force_keyframe);
+
+  // Checks for and copies encoded output on |encoder_thread_|.
+  void ProcessOutput();
+
+  // Inserts the output buffers for reuse on |encoder_thread_|.
+  void UseOutputBitstreamBufferTask(
+      std::unique_ptr<BitstreamBufferRef> buffer_ref);

[grt (UTC plus 2), 2016/07/14 20:07:25]

#include <memory>
i see many other missing #includes. please review everything in this file for
iwyu

[emircan, 2016/07/15 04:58:31]

Done.

+
+  // Copies EncodeOutput into a BitstreamBuffer and returns it to the |client_|.
+  void ReturnBitstreamBuffer(
+      std::unique_ptr<EncodeOutput> encode_output,
+      std::unique_ptr<MediaFoundationVideoEncodeAccelerator::BitstreamBufferRef>
+          buffer_ref);
+
+  // Changes encode parameters on |encoder_thread_|.
+  void RequestEncodingParametersChangeTask(uint32_t bitrate,

[grt (UTC plus 2), 2016/07/14 20:07:25]

#include <stdint.h> for uint32_t

[emircan, 2016/07/15 04:58:31]

Done.

+                                           uint32_t framerate);
+
+  // Destroys encode session on |encoder_thread_|.
+  void DestroyTask();
+
+  // Bitstream buffers ready to be used to return encoded output as a FIFO.
+  std::deque<std::unique_ptr<BitstreamBufferRef>> bitstream_buffer_queue_;

[grt (UTC plus 2), 2016/07/14 20:07:25]

#include <deque>

[emircan, 2016/07/15 04:58:31]

Done.

+
+  // EncodeOutput needs to be copied into a BitstreamBufferRef as a FIFO.
+  std::deque<std::unique_ptr<EncodeOutput>> encoder_output_queue_;
+
+  gfx::Size input_visible_size_;
+  size_t bitstream_buffer_size_;
+  int32_t frame_rate_;
+  int32_t target_bitrate_;
+  size_t u_plane_offset_;
+  size_t v_plane_offset_;
+
+  // Our original calling task runner for the child thread.
+  const scoped_refptr<base::SingleThreadTaskRunner> client_task_runner_;

[grt (UTC plus 2), 2016/07/14 20:07:25]

do you really require that the caller be bound to a specific thread rather than
to a sequence? consider making this SequencedTaskRunner and use
SequencedTaskRunnerHandle.

[emircan, 2016/07/15 04:58:31]

It does not specify in the VideoEncodeAccelerator interface, and all the
implementations have checks to be pinned to the single thread [0][1][2]. I guess
that is fine, since only calls come from GpuVideoEncodeAccelerator on GPU
process. [3]

[0]
https://cs.chromium.org/chromium/src/media/gpu/v4l2_video_encode_accelerator....
[1]
https://cs.chromium.org/chromium/src/media/gpu/vt_video_encode_accelerator_ma...
[2]
https://cs.chromium.org/chromium/src/media/gpu/vaapi_video_encode_accelerator...
[3]
https://cs.chromium.org/chromium/src/media/gpu/ipc/service/gpu_video_encode_a...

However, looking at the interface, I think there is no particular reason to be
single threaded. I can discuss with other owners and add a TODO to change all
implementations to SequencedTaskRunner as you suggest. WDYT?

[grt (UTC plus 2), 2016/07/15 07:59:03]

Whether or not other parts of the video pipeline require thread affinity doesn't
need to come into play here. A SingleThreadTaskRunner is a kind of
SequencedTaskRunner. If this class here doesn't need to know that the caller is
single threaded, it can take the client runner as a SequencedTaskRunner and all
is well.

[emircan, 2016/07/16 06:56:37]

Sounds good. I am going to change this to SequencedTaskRunner and
|thread_checker_| to SequenceChecker. Also, updating the comments about
threading of this class.

+
+  // To expose client callbacks from VideoEncodeAccelerator.
+  // NOTE: all calls to this object *MUST* be executed on
+  // |client_task_runner_|.
+  base::WeakPtr<Client> client_;
+  std::unique_ptr<base::WeakPtrFactory<Client>> client_ptr_factory_;

[grt (UTC plus 2), 2016/07/14 20:07:25]

this seems backward. does base::CancelableCallback suit your needs?

[emircan, 2016/07/15 04:58:31]

We receive client as a naked ptr in VideoEncodeAccelerator::Initialize(). We
cast it to be a base::WeakPtrFactory to have WeakPtrs for posting tasks. There
are multiple callbacks on client as well, so a single callback wouldn't be
enough here. 

+
+  base::win::ScopedComPtr<IMFTransform> encoder_;
+  base::win::ScopedComPtr<ICodecAPI> codec_api_;
+
+  DWORD input_stream_count_min_;
+  DWORD input_stream_count_max_;
+  DWORD output_stream_count_min_;
+  DWORD output_stream_count_max_;
+
+  base::win::ScopedComPtr<IMFSample> input_sample_;
+  base::win::ScopedComPtr<IMFSample> output_sample_;
+
+  // Thread checker to enforce that this object is used on a specific thread.
+  // It is pinned on |client_task_runner_| thread.
+  base::ThreadChecker thread_checker_;
+
+  // This thread services tasks posted from the VEA API entry points by the
+  // GPU child thread and CompressionCallback() posted from device thread.
+  base::Thread encoder_thread_;
+  scoped_refptr<base::SingleThreadTaskRunner> encoder_thread_task_runner_;
+
+  // Declared last to ensure that all weak pointers are invalidated before
+  // other destructors run.
+  base::WeakPtr<MediaFoundationVideoEncodeAccelerator> encoder_weak_ptr_;
+  base::WeakPtrFactory<MediaFoundationVideoEncodeAccelerator>
+      encoder_task_weak_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(MediaFoundationVideoEncodeAccelerator);
+};

[grt (UTC plus 2), 2016/07/14 20:07:25]

nit: blank line after this

[emircan, 2016/07/15 04:58:31]

Done.

+}  // namespace content

[grt (UTC plus 2), 2016/07/14 20:07:25]

media

[emircan, 2016/07/15 04:58:31]

Done.

+
+#endif  // CONTENT_COMMON_GPU_MEDIA_MEDIA_FOUNDATION_VIDEO_ENCODE_ACCELERATOR_H_