Pepper: PPB_VideoEncoder implementation

content/renderer/pepper/pepper_video_encoder_host.cc

+// 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.
+
+#include "base/bind.h"
+#include "base/memory/shared_memory.h"
+#include "base/numerics/safe_math.h"
+#include "content/common/gpu/client/command_buffer_proxy_impl.h"
+#include "content/public/renderer/renderer_ppapi_host.h"
+#include "content/renderer/pepper/gfx_conversion.h"
+#include "content/renderer/pepper/host_globals.h"
+#include "content/renderer/pepper/pepper_video_encoder_host.h"
+#include "content/renderer/render_thread_impl.h"
+#include "media/base/bind_to_current_loop.h"
+#include "media/base/video_frame.h"
+#include "media/filters/gpu_video_accelerator_factories.h"
+#include "media/video/video_encode_accelerator.h"
+#include "ppapi/c/pp_codecs.h"
+#include "ppapi/c/pp_errors.h"
+#include "ppapi/c/pp_graphics_3d.h"
+#include "ppapi/host/dispatch_host_message.h"
+#include "ppapi/host/ppapi_host.h"
+#include "ppapi/proxy/ppapi_messages.h"
+#include "ppapi/shared_impl/media_stream_buffer.h"
+
+using ppapi::proxy::SerializedHandle;
+
+namespace content {
+
+namespace {
+
+const int32_t kDefaultNumberOfBitstreamBuffers = 4;
+
+base::PlatformFile ConvertSharedMemoryHandle(
+    const base::SharedMemory& shared_memory) {
+#if defined(OS_POSIX)
+  return shared_memory.handle().fd;
+#elif defined(OS_WIN)
+  return shared_memory.handle();
+#else
+#error "Platform not supported."
+#endif
+}
+
+int32_t PP_FromMediaEncodeAcceleratorError(
+    media::VideoEncodeAccelerator::Error error) {
+  switch (error) {
+    case media::VideoEncodeAccelerator::kInvalidArgumentError:
+      return PP_ERROR_MALFORMED_INPUT;
+    case media::VideoEncodeAccelerator::kIllegalStateError:
+    case media::VideoEncodeAccelerator::kPlatformFailureError:
+      return PP_ERROR_RESOURCE_FAILED;
+      // No default case, to catch unhandled enum values.

[bbudge, 2015/02/10 22:47:37]

Indentation should match 'case', here and below.

[llandwerlin-old, 2015/02/11 18:42:23]

Done.

+  }
+  return PP_ERROR_FAILED;
+}
+
+}  // namespace
+
+// TODO(llandwerlin): move following to media_conversion.cc/h?
+media::VideoCodecProfile PepperToMediaVideoProfile(PP_VideoProfile profile) {
+  switch (profile) {
+    case PP_VIDEOPROFILE_H264BASELINE:
+      return media::H264PROFILE_BASELINE;
+    case PP_VIDEOPROFILE_H264MAIN:
+      return media::H264PROFILE_MAIN;
+    case PP_VIDEOPROFILE_H264EXTENDED:
+      return media::H264PROFILE_EXTENDED;
+    case PP_VIDEOPROFILE_H264HIGH:
+      return media::H264PROFILE_HIGH;
+    case PP_VIDEOPROFILE_H264HIGH10PROFILE:
+      return media::H264PROFILE_HIGH10PROFILE;
+    case PP_VIDEOPROFILE_H264HIGH422PROFILE:
+      return media::H264PROFILE_HIGH422PROFILE;
+    case PP_VIDEOPROFILE_H264HIGH444PREDICTIVEPROFILE:
+      return media::H264PROFILE_HIGH444PREDICTIVEPROFILE;
+    case PP_VIDEOPROFILE_H264SCALABLEBASELINE:
+      return media::H264PROFILE_SCALABLEBASELINE;
+    case PP_VIDEOPROFILE_H264SCALABLEHIGH:
+      return media::H264PROFILE_SCALABLEHIGH;
+    case PP_VIDEOPROFILE_H264STEREOHIGH:
+      return media::H264PROFILE_STEREOHIGH;
+    case PP_VIDEOPROFILE_H264MULTIVIEWHIGH:
+      return media::H264PROFILE_MULTIVIEWHIGH;
+    case PP_VIDEOPROFILE_VP8_ANY:
+      return media::VP8PROFILE_ANY;
+    case PP_VIDEOPROFILE_VP9_ANY:
+      return media::VP9PROFILE_ANY;
+      // No default case, to catch unhandled PP_VideoProfile values.
+  }
+
+  return media::VIDEO_CODEC_PROFILE_UNKNOWN;
+}
+
+PP_VideoProfile MediaToPepperVideoProfile(media::VideoCodecProfile profile) {
+  switch (profile) {
+    case media::H264PROFILE_BASELINE:
+      return PP_VIDEOPROFILE_H264BASELINE;
+    case media::H264PROFILE_MAIN:
+      return PP_VIDEOPROFILE_H264MAIN;
+    case media::H264PROFILE_EXTENDED:
+      return PP_VIDEOPROFILE_H264EXTENDED;
+    case media::H264PROFILE_HIGH:
+      return PP_VIDEOPROFILE_H264HIGH;
+    case media::H264PROFILE_HIGH10PROFILE:
+      return PP_VIDEOPROFILE_H264HIGH10PROFILE;
+    case media::H264PROFILE_HIGH422PROFILE:
+      return PP_VIDEOPROFILE_H264HIGH422PROFILE;
+    case media::H264PROFILE_HIGH444PREDICTIVEPROFILE:
+      return PP_VIDEOPROFILE_H264HIGH444PREDICTIVEPROFILE;
+    case media::H264PROFILE_SCALABLEBASELINE:
+      return PP_VIDEOPROFILE_H264SCALABLEBASELINE;
+    case media::H264PROFILE_SCALABLEHIGH:
+      return PP_VIDEOPROFILE_H264SCALABLEHIGH;
+    case media::H264PROFILE_STEREOHIGH:
+      return PP_VIDEOPROFILE_H264STEREOHIGH;
+    case media::H264PROFILE_MULTIVIEWHIGH:
+      return PP_VIDEOPROFILE_H264MULTIVIEWHIGH;
+    case media::VP8PROFILE_ANY:
+      return PP_VIDEOPROFILE_VP8_ANY;
+    case media::VP9PROFILE_ANY:
+      return PP_VIDEOPROFILE_VP9_ANY;
+    default:
+      // TODO(llandwerlin): This is wrong, but there is no PP_VIDEOPROFILE_NONE.
+      return PP_VIDEOPROFILE_VP9_ANY;

[bbudge, 2015/02/10 22:47:37]

add a NOTREACHED and return -1, e.g.
https://code.google.com/p/chromium/codesearch#chromium/src/net/spdy/spdy_sess...

[llandwerlin-old, 2015/02/11 18:42:22]

Done.

+  }
+}
+
+media::VideoFrame::Format PepperToMediaVideoFormat(

[bbudge, 2015/02/10 22:47:36]

s/MediaVideo/VideoFrame or MediaVideoFrame

[llandwerlin-old, 2015/02/11 18:42:23]

Renamed all these functions to match the PP_From*/PP_To* pattern.

+    PP_VideoFrame_Format format) {
+  switch (format) {
+    case PP_VIDEOFRAME_FORMAT_UNKNOWN:
+      return media::VideoFrame::UNKNOWN;
+    case PP_VIDEOFRAME_FORMAT_YV12:
+      return media::VideoFrame::YV12;
+    case PP_VIDEOFRAME_FORMAT_I420:
+      return media::VideoFrame::I420;
+    case PP_VIDEOFRAME_FORMAT_BGRA:
+      return media::VideoFrame::UNKNOWN;
+      // No default case, to catch unhandled PP_VideoFrame_Format values.
+  }
+
+  return media::VideoFrame::UNKNOWN;
+}
+
+PP_VideoFrame_Format MediaToPepperVideoFormat(
+    media::VideoFrame::Format format) {
+  switch (format) {
+    case media::VideoFrame::UNKNOWN:
+      return PP_VIDEOFRAME_FORMAT_UNKNOWN;
+    case media::VideoFrame::YV12:
+      return PP_VIDEOFRAME_FORMAT_YV12;
+    case media::VideoFrame::I420:
+      return PP_VIDEOFRAME_FORMAT_I420;
+    default:
+      return PP_VIDEOFRAME_FORMAT_UNKNOWN;
+  }
+}
+
+////////////////////////////////////////////////////////////////////////////////
+//
+// PepperVideoEncoderHost
+//
+////////////////////////////////////////////////////////////////////////////////

[bbudge, 2015/02/10 22:47:37]

Chrome doesn't really use this commenting convention

[llandwerlin-old, 2015/02/11 18:42:24]

Done.

+
+PepperVideoEncoderHost::ShmBuffer::ShmBuffer(
+    int32_t id,
+    scoped_ptr<base::SharedMemory> memory,
+    size_t size)
+    : id(id), shm(memory.Pass()), in_use(true) {
+  if (shm)
+    shm->Map(size);
+}
+
+PepperVideoEncoderHost::ShmBuffer::~ShmBuffer() {
+}
+
+media::BitstreamBuffer PepperVideoEncoderHost::ShmBuffer::toBitstreamBuffer() {
+  return media::BitstreamBuffer(id, shm->handle(), shm->mapped_size());
+}
+
+PepperVideoEncoderHost::PepperVideoEncoderHost(RendererPpapiHost* host,
+                                               PP_Instance instance,
+                                               PP_Resource resource)
+    : ResourceHost(host->GetPpapiHost(), instance, resource),
+      renderer_ppapi_host_(host),
+      buffer_manager_(this),
+      initialized_(false),
+      encoder_last_error_(PP_ERROR_FAILED),

[bbudge, 2015/02/10 22:47:36]

initialize input_coded_size_, frame_count_, and media_format_.

[llandwerlin-old, 2015/02/11 18:42:23]

Sure, can I leave the input_coded_size_ initialized with the default constructor
though?

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

Oh, right, that's fine.

+      command_buffer_(nullptr),
+      weak_ptr_factory_(this) {
+}
+
+PepperVideoEncoderHost::~PepperVideoEncoderHost() {
+  if (command_buffer_) {
+    DCHECK(channel_.get());
+    channel_->DestroyCommandBuffer(command_buffer_);
+    command_buffer_ = NULL;
+  }

[bbudge, 2015/02/10 22:47:37]

Shouldn't we Destroy the encoder, here or where we handle Close()?

[llandwerlin-old, 2015/02/11 18:42:23]

I guess it should be destroyed before the command buffer is destroyed. Thanks.

+
+  channel_ = NULL;
+}
+
+int32_t PepperVideoEncoderHost::OnResourceMessageReceived(
+    const IPC::Message& msg,
+    ppapi::host::HostMessageContext* context) {
+  PPAPI_BEGIN_MESSAGE_MAP(PepperVideoEncoderHost, msg)
+  PPAPI_DISPATCH_HOST_RESOURCE_CALL_0(
+      PpapiHostMsg_VideoEncoder_GetSupportedProfiles,
+      OnHostMsgGetSupportedProfiles)
+  PPAPI_DISPATCH_HOST_RESOURCE_CALL(PpapiHostMsg_VideoEncoder_Initialize,
+                                    OnHostMsgInitialize)
+  PPAPI_DISPATCH_HOST_RESOURCE_CALL_0(PpapiHostMsg_VideoEncoder_GetVideoFrames,
+                                      OnHostMsgGetVideoFrames)
+  PPAPI_DISPATCH_HOST_RESOURCE_CALL(PpapiHostMsg_VideoEncoder_Encode,
+                                    OnHostMsgEncode)
+  PPAPI_DISPATCH_HOST_RESOURCE_CALL(
+      PpapiHostMsg_VideoEncoder_RecycleBitstreamBuffer,
+      OnHostMsgRecycleBitstreamBuffer)
+  PPAPI_DISPATCH_HOST_RESOURCE_CALL(
+      PpapiHostMsg_VideoEncoder_RequestEncodingParametersChange,
+      OnHostMsgRequestEncodingParametersChange)
+  PPAPI_DISPATCH_HOST_RESOURCE_CALL_0(PpapiHostMsg_VideoEncoder_Close,
+                                      OnHostMsgClose)
+  PPAPI_END_MESSAGE_MAP()
+  return PP_ERROR_FAILED;
+}
+
+int32_t PepperVideoEncoderHost::OnHostMsgGetSupportedProfiles(
+    ppapi::host::HostMessageContext* context) {
+  std::vector<media::VideoEncodeAccelerator::SupportedProfile> profiles =
+      RenderThreadImpl::current()
+          ->GetGpuFactories()
+          ->GetVideoEncodeAcceleratorSupportedProfiles();
+  // TODO(llandwerlin): merge software supported profiles.
+
+  std::vector<PP_VideoProfileDescription> pp_profiles;
+  for (media::VideoEncodeAccelerator::SupportedProfile profile : profiles) {
+    PP_VideoProfileDescription pp_profile;
+    pp_profile.profile = MediaToPepperVideoProfile(profile.profile);
+    pp_profile.max_resolution = PP_FromGfxSize(profile.max_resolution);
+    pp_profile.max_framerate_numerator = profile.max_framerate_numerator;
+    pp_profile.max_framerate_denominator = profile.max_framerate_denominator;
+    pp_profile.acceleration = PP_HARDWAREACCELERATION_ONLY;
+    pp_profiles.push_back(pp_profile);
+  }
+
+  host()->SendReply(
+      context->MakeReplyMessageContext(),
+      PpapiPluginMsg_VideoEncoder_GetSupportedProfilesReply(pp_profiles));
+
+  return PP_OK_COMPLETIONPENDING;
+}
+
+int32_t PepperVideoEncoderHost::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) {

[bbudge, 2015/02/10 22:47:36]

You should use this parameter to fail when it's not hardware for now.

[llandwerlin-old, 2015/02/11 18:42:23]

Done.

+  scoped_ptr<media::VideoEncodeAccelerator> encoder;

[bbudge, 2015/02/10 22:47:37]

Unused.

[llandwerlin-old, 2015/02/11 18:42:22]

Done.

+  if (initialized_)
+    return PP_ERROR_FAILED;
+
+  media_format_ = PepperToMediaVideoFormat(input_format);

[bbudge, 2015/02/10 22:47:37]

Could you make the locals that are translated from input params follow the name
so it's easier to follow? In this case,
s/media_format_/media_input_format_
And below.

[llandwerlin-old, 2015/02/11 18:42:23]

Done.

+  if (media_format_ == media::VideoFrame::UNKNOWN)
+    return PP_ERROR_NOTSUPPORTED;

[bbudge, 2015/02/10 22:47:36]

PP_ERROR_BADARGUMENT

[llandwerlin-old, 2015/02/11 18:42:22]

Done.

+
+  media::VideoCodecProfile media_profile =
+      PepperToMediaVideoProfile(output_profile);
+  gfx::Size input_size(input_visible_size.width, input_visible_size.height);
+
+  initialize_reply_context_ = context->MakeReplyMessageContext();
+
+  // There is no garantee that we have a 3D context to work with. So
+  // we create a dump command buffer to communicate with the gpu

[bbudge, 2015/02/10 22:47:37]

s/garantee/guarantee
s/dump/dummy ?

[llandwerlin-old, 2015/02/11 18:42:24]

Done.

+  // process.
+  channel_ = RenderThreadImpl::current()->EstablishGpuChannelSync(
+      CAUSE_FOR_GPU_LAUNCH_PEPPERVIDEOENCODERACCELERATOR_INITIALIZE);
+  if (!channel_.get())
+    return PP_ERROR_FAILED;
+
+  std::vector<int32> attribs;
+  attribs.push_back(PP_GRAPHICS3DATTRIB_NONE);
+
+  command_buffer_ = channel_->CreateOffscreenCommandBuffer(
+      gfx::Size(), nullptr, attribs, GURL::EmptyGURL(),
+      gfx::PreferIntegratedGpu);
+  if (!command_buffer_)
+    return PP_ERROR_FAILED;
+
+  command_buffer_->SetChannelErrorCallback(media::BindToCurrentLoop(base::Bind(
+      &PepperVideoEncoderHost::NotifyPepperError,
+      weak_ptr_factory_.GetWeakPtr(),
+      PP_ERROR_FAILED)));
+
+  if (!command_buffer_->Initialize())
+    return PP_ERROR_FAILED;
+
+  encoder_ = command_buffer_->CreateVideoEncoder();
+
+  if (encoder_.get() &&

[bbudge, 2015/02/10 22:47:36]

I don't think you need the .get()

[llandwerlin-old, 2015/02/11 18:42:24]

Done.

+      encoder_->Initialize(media_format_, input_size, media_profile,
+                           initial_bitrate, this)) {
+    return PP_OK_COMPLETIONPENDING;
+  }
+
+  // TODO(llandwerlin): Software encoder.
+  return PP_ERROR_NOTSUPPORTED;
+}
+
+int32_t PepperVideoEncoderHost::OnHostMsgGetVideoFrames(
+    ppapi::host::HostMessageContext* context) {
+  if (encoder_last_error_)
+    return encoder_last_error_;
+
+  uint32_t frame_length =
+      media::VideoFrame::AllocationSize(media_format_, input_coded_size_);

[bbudge, 2015/02/10 22:47:37]

Since this returns size_t and we don't range limit input_coded_size_, a
malicious plugin could force an overflow. Use size_t here and below. I think
it's still possible to overflow, so you should also limit input_coded_size_ in
OnHostMsgInitialize above (i.e. the product of width * height should be < some
number so size_t can handle all of this arithmetic without overflow.

[llandwerlin-old, 2015/02/11 18:42:23]

Added a method to check against supported format instead.

+  uint32_t buffer_size = frame_length +
+                         sizeof(ppapi::MediaStreamBuffer::Video) -
+                         sizeof(ppapi::MediaStreamBuffer::Video::data);
+
+  // Make each buffer 4 byte aligned.
+  base::CheckedNumeric<int32_t> buffer_size_aligned = buffer_size;
+  buffer_size_aligned += (4 - buffer_size % 4);
+
+  base::CheckedNumeric<int32_t> total_size = frame_count_ * buffer_size_aligned;
+  if (!total_size.IsValid())
+    return PP_ERROR_FAILED;
+
+  // TODO(llandwerlin): HostAllocateSharedMemoryBuffer() is apparently
+  // synchronous and should be avoided.

[bbudge, 2015/02/10 22:47:36]

I don't think this TODO will ever get todone. I'd remove it.

[llandwerlin-old, 2015/02/11 18:42:23]

Done.

+  scoped_ptr<base::SharedMemory> shm(
+      RenderThreadImpl::current()
+          ->HostAllocateSharedMemoryBuffer(total_size.ValueOrDie())
+          .Pass());
+  if (!shm)
+    return PP_ERROR_FAILED;
+
+  VLOG(4) << " frame_count=" << frame_count_ << " frame_length=" << frame_length
+          << " buffer_size=" << buffer_size_aligned.ValueOrDie();
+
+  if (!buffer_manager_.SetBuffers(
+          frame_count_, buffer_size_aligned.ValueOrDie(), shm.Pass(), true))
+    return PP_ERROR_FAILED;
+
+  for (int32_t i = 0; i < buffer_manager_.number_of_buffers(); ++i) {
+    ppapi::MediaStreamBuffer::Video* buffer =
+        &(buffer_manager_.GetBufferPointer(i)->video);
+    buffer->header.size = buffer_manager_.buffer_size();
+    buffer->header.type = ppapi::MediaStreamBuffer::TYPE_VIDEO;
+    buffer->format = MediaToPepperVideoFormat(media_format_);
+    buffer->size.width = input_coded_size_.width();
+    buffer->size.height = input_coded_size_.height();
+    buffer->data_size = frame_length;
+  }
+
+  ppapi::host::ReplyMessageContext reply_context =
+      context->MakeReplyMessageContext();
+  reply_context.params.AppendHandle(SerializedHandle(
+      renderer_ppapi_host_->ShareHandleWithRemote(
+          ConvertSharedMemoryHandle(*buffer_manager_.shm()), false),
+      total_size.ValueOrDie()));
+
+  host()->SendReply(reply_context,
+                    PpapiPluginMsg_VideoEncoder_GetVideoFramesReply(
+                        frame_count_, buffer_size_aligned.ValueOrDie(),
+                        PP_FromGfxSize(input_coded_size_)));
+
+  return PP_OK_COMPLETIONPENDING;
+}
+
+int32_t PepperVideoEncoderHost::OnHostMsgEncode(
+    ppapi::host::HostMessageContext* context,
+    uint32_t frame_id,
+    bool force_keyframe) {
+  if (encoder_last_error_)
+    return encoder_last_error_;
+

[bbudge, 2015/02/10 22:47:36]

Range check 'frame_id' here. This is an attack surface.

[llandwerlin-old, 2015/02/11 18:42:23]

Done.

+  encoder_->Encode(
+      CreateVideoFrame(frame_id, context->MakeReplyMessageContext()),
+      force_keyframe);
+
+  return PP_OK_COMPLETIONPENDING;
+}
+
+int32_t PepperVideoEncoderHost::OnHostMsgRecycleBitstreamBuffer(
+    ppapi::host::HostMessageContext* context,
+    uint32_t buffer_id) {
+  if (encoder_last_error_)
+    return encoder_last_error_;
+
+  if (buffer_id < 0 || buffer_id >= buffers_.size() ||
+      buffers_[buffer_id]->in_use)
+    return PP_ERROR_BADARGUMENT;

[bbudge, 2015/02/10 22:47:37]

use brackets. Also PP_ERROR_FAILED would be better, as this could only happen if
there's an internal error or it's a malicious plugin.

[llandwerlin-old, 2015/02/11 18:42:24]

Done.
Not sure where you would like me to put brackets? Around the statement or around
the conditions?

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

http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Conditionals

+
+  buffers_[buffer_id]->in_use = true;
+  encoder_->UseOutputBitstreamBuffer(buffers_[buffer_id]->toBitstreamBuffer());
+
+  return PP_OK;
+}
+
+int32_t PepperVideoEncoderHost::OnHostMsgRequestEncodingParametersChange(
+    ppapi::host::HostMessageContext* context,
+    uint32_t bitrate,
+    uint32_t framerate) {
+  if (encoder_last_error_)
+    return encoder_last_error_;
+
+  encoder_->RequestEncodingParametersChange(bitrate, framerate);
+
+  return PP_OK;
+}
+
+int32_t PepperVideoEncoderHost::OnHostMsgClose(
+    ppapi::host::HostMessageContext* context) {
+  NotifyPepperError(PP_ERROR_ABORTED);

[bbudge, 2015/02/10 22:47:37]

I think you should just call encoder_->Destroy() here, after checking that
encoder_ != null.
Let's reserve NotifyError for the special class of errors that encoders report.

[llandwerlin-old, 2015/02/11 18:42:23]

Ok, I'll keep setting the encoder_last_error_ variable to aborted though.
I need to know whether an error should be reported in EncodeVideoFrameDone().

+
+  return PP_OK;
+}
+
+void PepperVideoEncoderHost::RequireBitstreamBuffers(
+    unsigned int frame_count,
+    const gfx::Size& input_coded_size,
+    size_t output_buffer_size) {
+  DCHECK(RenderThreadImpl::current());
+
+  buffers_.clear();

[bbudge, 2015/02/10 22:47:36]

I think I asked before, but is it possible to receive multiple
RequireBitstreamBuffers calls from the encoder? If not, I'd remove this, or at
most have a DCHECK.

[llandwerlin-old, 2015/02/11 18:42:23]

No, not with the current implementations.
That could change though, it seems changing the bitrate is not really well
tested. I wonder if that could require bigger bitstream buffers at some point.

Removing this for now.

+  for (int32_t i = 0; i < kDefaultNumberOfBitstreamBuffers; ++i) {
+    scoped_ptr<base::SharedMemory> shm(
+        RenderThread::Get()
+            ->HostAllocateSharedMemoryBuffer(output_buffer_size)
+            .Pass());
+
+    if (!shm || !shm->Map(output_buffer_size)) {
+      buffers_.clear();
+      break;
+    }
+
+    buffers_.push_back(new ShmBuffer(i, shm.Pass(), output_buffer_size));
+
+    initialize_reply_context_.params.AppendHandle(
+        ppapi::proxy::SerializedHandle(
+            renderer_ppapi_host_->ShareHandleWithRemote(
+                ConvertSharedMemoryHandle(*buffers_.back()->shm), false),
+            output_buffer_size));
+  }
+
+  if (buffers_.empty()) {
+    NotifyPepperError(PP_ERROR_FAILED);

[bbudge, 2015/02/10 22:47:37]

Don't use NotifyPepperError as a generic error reporter. Just return the error
code as a normal initialize reply.

[llandwerlin-old, 2015/02/11 18:42:23]

Done.

+    return;
+  }
+
+  // Feed buffers to the encoder.
+  for (size_t i = 0; i < buffers_.size(); ++i) {
+    encoder_->UseOutputBitstreamBuffer(buffers_[i]->toBitstreamBuffer());
+  }
+
+  // Notify the plugins of the buffers and the input size.

[bbudge, 2015/02/10 22:47:36]

s/plugins/plugin

[llandwerlin-old, 2015/02/11 18:42:24]

Done.

+  PP_Size size;

[bbudge, 2015/02/10 22:47:37]

PP_MakeSize would be more concise.

[llandwerlin-old, 2015/02/11 18:42:22]

Done.

+  size.width = input_coded_size.width();
+  size.height = input_coded_size.height();
+  input_coded_size_ = input_coded_size;
+  frame_count_ = frame_count;
+  initialized_ = true;
+  encoder_last_error_ = PP_OK;
+
+  host()->SendReply(
+      initialize_reply_context_,
+      PpapiPluginMsg_VideoEncoder_InitializeReply(
+          output_buffer_size, frame_count, size));
+}
+
+void PepperVideoEncoderHost::BitstreamBufferReady(int32 buffer_id,
+                                                  size_t payload_size,
+                                                  bool key_frame) {
+  DCHECK(RenderThreadImpl::current());
+  DCHECK(buffers_[buffer_id]->in_use);
+
+  buffers_[buffer_id]->in_use = false;
+  host()->SendUnsolicitedReply(pp_resource(),
+                               PpapiPluginMsg_VideoEncoder_BitstreamBufferReady(
+                                   buffer_id, payload_size, key_frame));
+}
+
+void PepperVideoEncoderHost::NotifyError(
+    media::VideoEncodeAccelerator::Error error) {
+  DCHECK(RenderThreadImpl::current());
+  NotifyPepperError(PP_FromMediaEncodeAcceleratorError(error));
+}
+
+scoped_refptr<media::VideoFrame> PepperVideoEncoderHost::CreateVideoFrame(
+    uint32_t frame_id,
+    const ppapi::host::ReplyMessageContext& reply_context) {

[bbudge, 2015/02/10 22:47:37]

You might want to DCHECK that frame_id is in range.

[llandwerlin-old, 2015/02/11 18:42:23]

Done.

+  ppapi::MediaStreamBuffer* buffer =
+      buffer_manager_->GetBufferPointer(frame_id);
+
+  uint32_t shm_offset = frame_id * buffer_manager_->buffer_size() +
+                        sizeof(ppapi::MediaStreamBuffer::Video) -
+                        sizeof(ppapi::MediaStreamBuffer::Video::data);
+
+  return media::VideoFrame::WrapExternalPackedMemory(
+      media_format_, input_coded_size, gfx::Rect(input_coded_size_),
+      input_coded_size_, static_cast<uint8*>(buffer->video.data),
+      buffer->video.data_size, buffer_manager_->shm()->handle(), shm_offset,
+      base::TimeDelta(), base::Bind(
+          &PepperVideoEncoderHost::EncodeVideoFrameDone,
+          weak_ptr_factory_.GetWeakPtr(), base::Passed(shm.Pass()),
+          reply_context));
+}
+
+void PepperVideoEncoderHost::EncodeVideoFrameDone(
+    const ppapi::host::ReplyMessageContext& reply_context,
+    uint32_t frame_id) {
+  DCHECK(RenderThreadImpl::current());
+
+  ppapi::host::ReplyMessageContext context = reply_context;
+  context.params.set_result(encoder_last_error_);
+  host()->SendReply(reply_context,
+                    PpapiPluginMsg_VideoEncoder_EncodeReply(frame_id));
+}
+
+void PepperVideoEncoderHost::NotifyPepperError(int32_t error) {
+  DCHECK(RenderThreadImpl::current());
+
+  encoder_last_error_ = error;
+  encoder_->Destroy();
+  base::Unretained(encoder_.release());
+  host()->SendUnsolicitedReply(
+      pp_resource(),
+      PpapiPluginMsg_VideoEncoder_NotifyError(encoder_last_error_));
+}
+
+}  // namespace content