Pepper: PPB_VideoEncoder implementation

content/renderer/pepper/pepper_video_encoder_host.h

+// 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_RENDERER_PEPPER_PEPPER_VIDEO_ENCODER_HOST_H_
+#define CONTENT_RENDERER_PEPPER_PEPPER_VIDEO_ENCODER_HOST_H_
+
+#include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/memory/scoped_vector.h"
+#include "content/common/content_export.h"
+#include "gpu/command_buffer/common/mailbox_holder.h"
+#include "media/video/video_encode_accelerator.h"
+#include "ppapi/c/pp_codecs.h"
+#include "ppapi/c/ppb_video_frame.h"
+#include "ppapi/host/host_message_context.h"
+#include "ppapi/host/resource_host.h"
+#include "ppapi/proxy/resource_message_params.h"
+#include "ppapi/shared_impl/media_stream_buffer_manager.h"
+
+namespace media {
+class GpuVideoAcceleratorFactories;
+}
+
+namespace content {
+
+class CommandBufferProxyImpl;
+class GpuChannelHost;
+class RendererPpapiHost;
+class RenderViewImpl;
+
+class CONTENT_EXPORT PepperVideoEncoderHost
+    : public ppapi::host::ResourceHost,
+      public media::VideoEncodeAccelerator::Client,
+      public ppapi::MediaStreamBufferManager::Delegate {
+ public:
+  PepperVideoEncoderHost(RendererPpapiHost* host,
+                         PP_Instance instance,
+                         PP_Resource resource);
+  ~PepperVideoEncoderHost() override;
+
+ private:
+  // media::VideoEncodeAccelerator implementation.
+  virtual void RequireBitstreamBuffers(unsigned int input_count,
+                                       const gfx::Size& input_coded_size,
+                                       size_t output_buffer_size) override;
+  virtual void BitstreamBufferReady(int32 bitstream_buffer_id,
+                                    size_t payload_size,
+                                    bool key_frame) override;
+  virtual void NotifyError(media::VideoEncodeAccelerator::Error error) override;
+
+  // ResourceHost implementation.
+  virtual int32_t OnResourceMessageReceived(
+      const IPC::Message& msg,
+      ppapi::host::HostMessageContext* context) override;
+
+  // Plugin's IPC calls.

[bbudge, 2015/02/11 23:27:46]

nit: These are message handlers, but I don't think a comment adds much here.

[llandwerlin-old, 2015/02/12 15:59:40]

Done.

+  int32_t OnHostMsgGetSupportedProfiles(
+      ppapi::host::HostMessageContext* context);
+  int32_t OnHostMsgInitialize(ppapi::host::HostMessageContext* context,
+                              PP_VideoFrame_Format input_format,
+                              const PP_Size& input_visible_size,
+                              PP_VideoProfile output_profile,
+                              uint32_t initial_bitrate,
+                              PP_HardwareAcceleration acceleration);
+  int32_t OnHostMsgGetVideoFrames(ppapi::host::HostMessageContext* context);
+  int32_t OnHostMsgEncode(ppapi::host::HostMessageContext* context,
+                          uint32_t frame_id,
+                          bool force_keyframe);
+  int32_t OnHostMsgRecycleBitstreamBuffer(
+      ppapi::host::HostMessageContext* context,
+      uint32_t buffer_id);
+  int32_t OnHostMsgRequestEncodingParametersChange(
+      ppapi::host::HostMessageContext* context,
+      uint32_t bitrate,
+      uint32_t framerate);
+  int32_t OnHostMsgClose(ppapi::host::HostMessageContext* context);
+
+  // Internal methods.
+  void GetSupportedProfiles(
+      std::vector<PP_VideoProfileDescription>* pp_profiles);
+  bool IsInitializationValid(const PP_Size& input_size,
+                             PP_VideoProfile ouput_profile,
+                             PP_HardwareAcceleration acceleration);
+  scoped_refptr<media::VideoFrame> CreateVideoFrame(
+      uint32_t frame_id,
+      const ppapi::host::ReplyMessageContext& reply_context);
+  void EncodeDone(const ppapi::host::ReplyMessageContext& reply_context,
+                  uint32_t frame_id);
+  void NotifyPepperError(int32_t error);
+
+  // Non-owning pointer.
+  RendererPpapiHost* renderer_ppapi_host_;
+
+  ppapi::host::ReplyMessageContext initialize_reply_context_;
+
+  // Shared memory buffers.

[bbudge, 2015/02/11 23:27:45]

Child type definitions should be the first private definitions. Move this up to
line 43.

[llandwerlin-old, 2015/02/12 15:59:40]

Done.

+  struct ShmBuffer {
+    ShmBuffer(int32_t id, scoped_ptr<base::SharedMemory> memory, size_t size);
+    ~ShmBuffer();
+
+    media::BitstreamBuffer toBitstreamBuffer();
+
+    int32_t id;
+    scoped_ptr<base::SharedMemory> shm;
+    bool in_use;
+  };
+
+  // Bitstream buffers.

[bbudge, 2015/02/11 23:27:45]

Thanks for adding comments, but it isn't necessary to comment each field. In
general, add a comment when there are things future maintainers should know
about the field. For an example:
https://code.google.com/p/chromium/codesearch#chromium/src/content/renderer/p...

[llandwerlin-old, 2015/02/12 15:59:40]

Done.

[llandwerlin-old, 2015/02/12 15:59:40]

Acknowledged.

+  ScopedVector<ShmBuffer> shm_buffers_;
+
+  // Video frames.

[bbudge, 2015/02/11 23:27:46]

nit: // Buffer manager for shared memory that holds video frames.

[llandwerlin-old, 2015/02/12 15:59:40]

Done.

[llandwerlin-old, 2015/02/12 15:59:40]

Done.

+  ppapi::MediaStreamBufferManager buffer_manager_;
+
+  // Gpu process communication.

[bbudge, 2015/02/11 23:27:45]

nit: I think you could omit the comment here.

[llandwerlin-old, 2015/02/12 15:59:40]

Done.

+  scoped_refptr<GpuChannelHost> channel_;
+  CommandBufferProxyImpl* command_buffer_;
+
+  scoped_ptr<media::VideoEncodeAccelerator> encoder_;
+
+  // Whether the encoder has been successfully initialized.
+  bool initialized_;
+
+  // Last error encountered.

[bbudge, 2015/02/11 23:27:46]

This is too vague given the complexity of this.

[llandwerlin-old, 2015/02/12 15:59:40]

Done.

+  int32_t encoder_last_error_;
+
+  // Size of the frames allocated for the encoder (matching hardware
+  // constraints).
+  gfx::Size input_coded_size_;
+
+  // Number of frames the encoder needs.
+  int32_t frame_count_;
+
+  // Format of the frames to give to the encoder.
+  media::VideoFrame::Format media_input_format_;
+
+  base::WeakPtrFactory<PepperVideoEncoderHost> weak_ptr_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(PepperVideoEncoderHost);
+};
+
+}  // namespace content
+
+#endif  // CONTENT_RENDERER_PEPPER_PEPPER_VIDEO_ENCODER_HOST_H_