content: pepper: VideoEncoder: add software encoder support

content/renderer/pepper/video_encoder_shim.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 "content/renderer/pepper/video_encoder_shim.h"
+
+#include <deque>
+
+#include "base/bind.h"
+#include "base/bind_helpers.h"
+#include "base/memory/scoped_vector.h"
+#include "base/memory/shared_memory.h"
+#include "base/message_loop/message_loop.h"
+#include "content/renderer/pepper/pepper_video_encoder_host.h"
+#include "content/renderer/render_thread_impl.h"
+#include "media/cast/cast_config.h"
+#include "media/cast/sender/vp8_encoder.h"
+#include "ui/gfx/geometry/size.h"
+
+namespace content {
+
+const int32_t kMaxWidth = 1920;
+const int32_t kMaxHeight = 1080;

[bbudge, 2015/03/02 20:01:06]

Needs a comment explaining these constants.

[llandwerlin-old, 2015/03/03 15:23:28]

Done.

+
+class VideoEncoderShim::EncoderImpl {
+ public:
+  explicit EncoderImpl(const base::WeakPtr<VideoEncoderShim>& proxy);
+  ~EncoderImpl();
+
+  void Initialize(media::VideoFrame::Format input_format,
+                  const gfx::Size& input_visible_size,
+                  media::VideoCodecProfile output_profile,
+                  uint32 initial_bitrate);
+  void Encode(const scoped_refptr<media::VideoFrame>& frame,
+              bool force_keyframe);
+  void UseOutputBitstreamBuffer(const media::BitstreamBuffer& buffer,
+                                uint8_t* mem);
+  void RequestEncodingParametersChange(uint32 bitrate, uint32 framerate);
+  void Stop();
+
+ private:
+  struct Frame {

[bbudge, 2015/03/02 20:01:06]

Suggestion: s/Frame/PendingEncode
So you don't have frame.frame below.

[llandwerlin-old, 2015/03/03 15:23:28]

Done.

+    Frame(const scoped_refptr<media::VideoFrame>& frame, bool force_keyframe)
+        : frame(frame), force_keyframe(force_keyframe) {}
+    ~Frame() {}
+
+    scoped_refptr<media::VideoFrame> frame;
+    bool force_keyframe;
+  };
+
+  struct BitstreamBuffer {
+    BitstreamBuffer(const media::BitstreamBuffer buffer, uint8_t* mem)
+        : buffer(buffer), mem(mem) {}
+    ~BitstreamBuffer() {}
+
+    media::BitstreamBuffer buffer;
+    uint8_t* mem;
+  };
+
+  void DoEncode();
+
+  base::WeakPtr<VideoEncoderShim> shim_;
+  scoped_refptr<base::SingleThreadTaskRunner> shim_task_runner_;

[bbudge, 2015/03/02 20:01:06]

Name suggestion: task_runner_ or media_task_runner_. This isn't really specific
to the shim.

[llandwerlin-old, 2015/03/03 15:23:28]

Done.

+
+  scoped_ptr<media::cast::SoftwareVideoEncoder> encoder_;
+  std::deque<Frame> frames_;
+  std::deque<BitstreamBuffer> buffers_;
+};
+
+VideoEncoderShim::EncoderImpl::EncoderImpl(
+    const base::WeakPtr<VideoEncoderShim>& proxy)

[bbudge, 2015/03/02 20:01:06]

s/proxy/shim
And above in class decl.

[llandwerlin-old, 2015/03/03 15:23:28]

Done.

+    : shim_(proxy), shim_task_runner_(base::MessageLoopProxy::current()) {
+}
+
+VideoEncoderShim::EncoderImpl::~EncoderImpl() {
+}
+
+void VideoEncoderShim::EncoderImpl::Initialize(
+    media::VideoFrame::Format input_format,
+    const gfx::Size& input_visible_size,
+    media::VideoCodecProfile output_profile,
+    uint32 initial_bitrate) {
+  media::cast::VideoSenderConfig config;
+
+  config.max_number_of_video_buffers_used =
+      3;  // media::cast::Vp8Encoder::kNumberOfVp8VideoBuffers;

[bbudge, 2015/03/02 20:01:06]

This comment confused me, since it refers to a private enum value. It would be
better to refer the reader to the comments on the media::cast::VideoSenderConfig
struct. Looking at the actual source in vp8_encoder.cc, I don't think it holds
more than 1 video frame at a time, so you could return 1 to the shim below.
Perhaps defining some constants at the top of this file and commenting them
would help here.

[llandwerlin-old, 2015/03/03 15:23:28]

Done.

+  config.number_of_encode_threads = 1;
+  encoder_.reset(new media::cast::Vp8Encoder(config));
+
+  encoder_->UpdateRates(initial_bitrate);
+
+  shim_task_runner_->PostTask(
+      FROM_HERE, base::Bind(&VideoEncoderShim::OnRequireBitstreamBuffers, shim_,
+                            3, input_visible_size, 4 * 1000000));

[bbudge, 2015/03/02 20:01:06]

4 * 1000000 should be a constant with a comment.

[llandwerlin-old, 2015/03/03 15:23:28]

Done.

+}
+
+void VideoEncoderShim::EncoderImpl::Encode(
+    const scoped_refptr<media::VideoFrame>& frame,
+    bool force_keyframe) {
+  frames_.push_back(Frame(frame, force_keyframe));
+  DoEncode();
+}
+
+void VideoEncoderShim::EncoderImpl::UseOutputBitstreamBuffer(
+    const media::BitstreamBuffer& buffer,
+    uint8_t* mem) {
+  buffers_.push_back(BitstreamBuffer(buffer, mem));
+  DoEncode();
+}
+
+void VideoEncoderShim::EncoderImpl::RequestEncodingParametersChange(
+    uint32 bitrate,
+    uint32 framerate) {
+  encoder_->UpdateRates(bitrate);
+}
+
+void VideoEncoderShim::EncoderImpl::Stop() {
+  frames_.clear();
+  buffers_.clear();
+  encoder_.reset();
+}
+
+void VideoEncoderShim::EncoderImpl::DoEncode() {
+  if (frames_.empty() || buffers_.empty())
+    return;

[bbudge, 2015/03/02 20:01:06]

I don't think you need this 'if' - the 'while' handles this case too.

[llandwerlin-old, 2015/03/03 15:23:28]

Done.

+
+  while (!frames_.empty() && !buffers_.empty()) {
+    Frame frame = frames_.front();
+    frames_.pop_front();
+
+    if (frame.force_keyframe)
+      encoder_->GenerateKeyFrame();
+
+    scoped_ptr<media::cast::EncodedFrame> encoded_frame(
+        new media::cast::EncodedFrame());
+    encoder_->Encode(frame.frame, base::TimeTicks::Now(), encoded_frame.get());

[bbudge, 2015/03/02 20:01:06]

nit: .get() isn't necessary.

[llandwerlin-old, 2015/03/03 15:23:27]

That doesn't compile on GCC : no known conversion for argument 3 from
'scoped_ptr<media::cast::EncodedFrame>' to 'media::cast::EncodedFrame*

+
+    BitstreamBuffer buffer = buffers_.front();
+    buffers_.pop_front();
+
+    CHECK(buffer.buffer.size() >= encoded_frame->data.size());
+    memcpy(buffer.mem, encoded_frame->bytes(), encoded_frame->data.size());
+
+    shim_task_runner_->PostTask(
+        FROM_HERE,
+        base::Bind(
+            &VideoEncoderShim::OnBitstreamBufferReady, shim_,
+            // Pass the media::VideoFrame back
+            // to the renderer thread so it's
+            // freed on the right thread.

[bbudge, 2015/03/02 20:01:06]

comment formatting is a little weird. Maybe put it above the PostTask call?

[llandwerlin-old, 2015/03/03 15:23:27]

Done.

+            frame.frame, buffer.buffer.id(), encoded_frame->data.size(),
+            encoded_frame->dependency == media::cast::EncodedFrame::KEY));
+  }
+}
+
+// VideoEncoderShim :

[bbudge, 2015/03/02 20:01:06]

nit: remove

[llandwerlin-old, 2015/03/03 15:23:27]

Done.

+
+VideoEncoderShim::VideoEncoderShim(PepperVideoEncoderHost* host)
+    : host_(host),
+      media_task_runner_(
+          RenderThreadImpl::current()->GetMediaThreadTaskRunner()),
+      weak_ptr_factory_(this) {
+  encoder_impl_.reset(new EncoderImpl(weak_ptr_factory_.GetWeakPtr()));
+}
+
+VideoEncoderShim::~VideoEncoderShim() {
+  DCHECK(RenderThreadImpl::current());
+
+  media_task_runner_->PostTask(
+      FROM_HERE, base::Bind(&VideoEncoderShim::EncoderImpl::Stop,
+                            base::Owned(encoder_impl_.release())));
+}
+
+std::vector<media::VideoEncodeAccelerator::SupportedProfile>
+VideoEncoderShim::GetSupportedProfiles() {
+  media::VideoEncodeAccelerator::SupportedProfile profile = {
+      media::VP8PROFILE_ANY,
+      // TODO(llandwerlin): find correct values from libvpx.

[bbudge, 2015/03/02 20:01:06]

Move TODO to definitions at top of file.

[llandwerlin-old, 2015/03/03 15:23:28]

Done.

+      gfx::Size(kMaxWidth, kMaxHeight),
+      media::cast::kDefaultMaxFrameRate,
+      1};
+  std::vector<media::VideoEncodeAccelerator::SupportedProfile> profiles;
+  profiles.push_back(profile);
+  return profiles;
+}
+
+bool VideoEncoderShim::Initialize(
+    media::VideoFrame::Format input_format,
+    const gfx::Size& input_visible_size,
+    media::VideoCodecProfile output_profile,
+    uint32 initial_bitrate,
+    media::VideoEncodeAccelerator::Client* client) {
+  DCHECK(RenderThreadImpl::current());
+  DCHECK_EQ(client, host_);
+
+  if (input_format != media::VideoFrame::I420)
+    return false;

[bbudge, 2015/03/02 20:01:06]

Is it possible that other formats could work? I see that vp8_encoder.cc sets
this format.

[llandwerlin-old, 2015/03/03 15:23:28]

Looking at the VPX code, it depends on the codec and profile.
VP8 only accepts I420 and YV12.
VP9 always accepts I420 and YV12 and for some profiles, I422/I444/I440.

I put I420 as a restriction because it seems to be the norm all over the
chromium code base (from the capture to the encoders).

I avoided using libvpx directly because it's been used in several other places
through the chromium code base. Do you think it would make sense to drop the
usage of media/cast/sender/ and directly rely on libvpx?
I think this would give us VP9 support.

[bbudge, 2015/03/05 01:43:59]

It would be nice to have more profiles. I don't know much about the video stack,
so I'm going to ask those folks what approach they recommend.

+  if (output_profile != media::VP8PROFILE_ANY)
+    return false;
+  if (input_visible_size.width() > kMaxWidth ||
+      input_visible_size.height() > kMaxHeight)
+    return false;

[bbudge, 2015/03/02 20:01:06]

Is this input validation needed? Doesn't the host check against invalid
profiles?
(Except for the I420 check.)

[llandwerlin-old, 2015/03/03 15:23:27]

Fair, removing.

+
+  media_task_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(&VideoEncoderShim::EncoderImpl::Initialize,
+                 base::Unretained(encoder_impl_.get()), input_format,
+                 input_visible_size, output_profile, initial_bitrate));
+
+  return true;
+}
+
+void VideoEncoderShim::Encode(const scoped_refptr<media::VideoFrame>& frame,
+                              bool force_keyframe) {
+  DCHECK(RenderThreadImpl::current());
+
+  media_task_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(&VideoEncoderShim::EncoderImpl::Encode,
+                 base::Unretained(encoder_impl_.get()), frame, force_keyframe));
+}
+
+void VideoEncoderShim::UseOutputBitstreamBuffer(
+    const media::BitstreamBuffer& buffer) {
+  DCHECK(RenderThreadImpl::current());
+
+  media_task_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(&VideoEncoderShim::EncoderImpl::UseOutputBitstreamBuffer,
+                 base::Unretained(encoder_impl_.get()), buffer,
+                 host_->ShmHandleToAddress(buffer.id())));
+}
+
+void VideoEncoderShim::RequestEncodingParametersChange(uint32 bitrate,
+                                                       uint32 framerate) {
+  DCHECK(RenderThreadImpl::current());
+
+  media_task_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(
+          &VideoEncoderShim::EncoderImpl::RequestEncodingParametersChange,
+          base::Unretained(encoder_impl_.get()), bitrate, framerate));
+}
+
+void VideoEncoderShim::Destroy() {
+  DCHECK(RenderThreadImpl::current());
+
+  delete this;
+}
+
+void VideoEncoderShim::OnRequireBitstreamBuffers(
+    unsigned int input_count,
+    const gfx::Size& input_coded_size,
+    size_t output_buffer_size) {
+  DCHECK(RenderThreadImpl::current());
+
+  host_->RequireBitstreamBuffers(input_count, input_coded_size,
+                                 output_buffer_size);
+}
+
+void VideoEncoderShim::OnBitstreamBufferReady(
+    scoped_refptr<media::VideoFrame> frame,
+    int32 bitstream_buffer_id,
+    size_t payload_size,
+    bool key_frame) {
+  DCHECK(RenderThreadImpl::current());
+
+  host_->BitstreamBufferReady(bitstream_buffer_id, payload_size, key_frame);
+}
+
+void VideoEncoderShim::OnNotifyError(
+    media::VideoEncodeAccelerator::Error error) {
+  DCHECK(RenderThreadImpl::current());
+
+  host_->NotifyError(error);
+}
+
+}  // namespace content