Memory copy the VideoFrame to match the requirement for HW encoders.

media/cast/sender/external_video_encoder.cc

Index: media/cast/sender/external_video_encoder.cc
diff --git a/media/cast/sender/external_video_encoder.cc b/media/cast/sender/external_video_encoder.cc
index 3eb7740a3c5ba7621b48054c4c7bd1a171a7ef8e..c2fe2fdc602b5c28570b7f2dbd1e3f6c330507d4 100644
--- a/media/cast/sender/external_video_encoder.cc
+++ b/media/cast/sender/external_video_encoder.cc
@@ -35,6 +35,72 @@ namespace {
 enum { MAX_H264_QUANTIZER = 51 };
 static const size_t kOutputBufferCount = 3;
 
+// Copy buffer to match the required coded size.
+// Cropping or padding is done as required.
+bool StridedBufferCopy(const uint8_t* src,
+                       const gfx::Size& visible_rect,
+                       int src_stride,
+                       uint8_t* dst,
+                       const gfx::Size& dst_coded_size) {
+  if (!src || !dst)
+    return false;
+
+  const int src_width = visible_rect.width();
+  const int src_height = visible_rect.height();
+  const int dst_stride = dst_coded_size.width();
+  const int dst_height = dst_coded_size.height();
+  const int copy_width = src_width < dst_stride ? src_width : dst_stride;
+  const int copy_height = src_height < dst_height ? src_height : dst_height;
+
+  for (int i = 0; i < copy_height; ++i) {
+    std::memcpy(dst, src, copy_width);
+    // Padding the remaining columns by repeating the last column of the source.
+    if (copy_width < dst_stride)
+      std::memset(dst + copy_width, src[copy_width - 1],
+                  dst_stride - copy_width);
+    src += src_stride;
+    dst += dst_stride;
+  }
+
+  // Padding the remaining rows by repeating the last row of the source.
+  src = dst - dst_stride;
+  for (int i = copy_height; i < dst_height; ++i) {
+    memcpy(dst, src, dst_stride);
+    dst += dst_stride;
+  }
+
+  return true;
+}
+
+// Copy video frame to match the required coded size.
+bool StridedFrameCopy(const scoped_refptr<media::VideoFrame> src_frame,

[miu, 2016/04/25 23:39:39]

Oh!  Oops, I forgot to mention these routines already exist.  Sorry I didn't
mention this before you wrote them.

Suggestion:

  #include "third_party/libyuv/include/libyuv/convert.h"

  ...

  libyuv::I420Copy(...)

Reference:
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/libyuv...

[xjz, 2016/04/26 17:42:30]

Done. Added dependency on libyuv. This copy does not do any padding. So I now
initialize the new frame with 0 when it is created.

[miu, 2016/04/26 19:20:17]

Oh.  IMO, the padding was a nice thing to have.  Also, it would be useful to
place this function (I420 copy + padding) in src/media/base/video_util.cc so
others can use it.

[xjz, 2016/04/27 22:13:54]

Done. Padding is nice to improve coding efficiency. However I am not sure if it
might cause performance issue, as it increases encoding time and may result in
lower quality (by zero configuring). 

+                      scoped_refptr<media::VideoFrame> dst_frame) {
+  if (!StridedBufferCopy(src_frame->data(media::VideoFrame::kYPlane),
+                         src_frame->visible_rect().size(),
+                         src_frame->stride(media::VideoFrame::kYPlane),
+                         dst_frame->data(media::VideoFrame::kYPlane),
+                         dst_frame->coded_size()))
+    return false;
+
+  const gfx::Size uv_visible_size(src_frame->visible_rect().width() / 2,
+                                  src_frame->visible_rect().height() / 2);
+  const gfx::Size uv_coded_size(dst_frame->coded_size().width() / 2,
+                                dst_frame->coded_size().height() / 2);
+  if (!StridedBufferCopy(
+          src_frame->data(media::VideoFrame::kUPlane), uv_visible_size,
+          src_frame->stride(media::VideoFrame::kUPlane),
+          dst_frame->data(media::VideoFrame::kUPlane), uv_coded_size))
+    return false;
+
+  if (!StridedBufferCopy(
+          src_frame->data(media::VideoFrame::kVPlane), uv_visible_size,
+          src_frame->stride(media::VideoFrame::kVPlane),
+          dst_frame->data(media::VideoFrame::kVPlane), uv_coded_size))
+    return false;
+
+  return true;
+}
+
 }  // namespace
 
 namespace media {
@@ -149,8 +215,29 @@ class ExternalVideoEncoder::VEAClientImpl
         video_frame, reference_time, frame_encoded_callback,
         requested_bit_rate_));
 
+    scoped_refptr<media::VideoFrame> frame = video_frame;
+    if (video_frame->coded_size() != frame_coded_size_) {
+      VLOG(1) << "ExternalVideoEncoder copy required. "

[miu, 2016/04/25 23:39:39]

nit: Maybe make these logging statements VLOG(2) since they can occur at high
rates (>1 log lines per second)?  Also, perhaps these should be DVLOG(2) (i.e.,
only built into debug builds)?

[xjz, 2016/04/26 17:42:30]

Removed this log.

+              << "Source coded size: " << video_frame->coded_size().ToString()
+              << " Required coded size: " << frame_coded_size_.ToString();
+
+      frame = VideoFrame::CreateFrame(
+          video_frame->format(), frame_coded_size_, video_frame->visible_rect(),
+          video_frame->natural_size(), video_frame->timestamp());
+      if (!frame.get()) {
+        VLOG(1) << "Error: ExternalVideoEncoder: CreateFrame failed.";
+        return;
+      }
+
+      // Copy the input frame to match the input requirements for the encoder.
+      if (!StridedFrameCopy(video_frame, frame)) {
+        VLOG(1) << "ERROR: ExternalVideoEncoder: Copy failed.";
+        return;
+      }
+    }
+
     // BitstreamBufferReady will be called once the encoder is done.
-    video_encode_accelerator_->Encode(video_frame, key_frame_requested);
+    video_encode_accelerator_->Encode(frame, key_frame_requested);
   }
 
  protected:
@@ -177,6 +264,8 @@ class ExternalVideoEncoder::VEAClientImpl
                                size_t output_buffer_size) final {
     DCHECK(task_runner_->RunsTasksOnCurrentThread());
 
+    frame_coded_size_ = input_coded_size;
+
     // TODO(miu): Investigate why we are ignoring |input_count| (4) and instead
     // using |kOutputBufferCount| (3) here.
     for (size_t j = 0; j < kOutputBufferCount; ++j) {

[miu, 2016/04/25 23:39:39]

BTW--In a follow-up change, perhaps you could take a look at this?  My gut says
we should be respecting |input_count| instead of using our own hard-coded one.

[xjz, 2016/04/26 17:42:30]

|input_count| is how many frames the encoder will hold. It is for input frames.
We ignore this value here as we don't use fixed number of buffers for input
frames.|kOutputBufferCount| is how many output buffers to be allocated. This is
for the encoder output. Removed this TODO comment.

@@ -476,6 +565,10 @@ class ExternalVideoEncoder::VEAClientImpl
   // TODO(miu): Remove after discovering cause.  http://crbug.com/519022
   bool has_seen_zero_length_encoded_frame_;
 
+  // The coded size of the video frame required by Encoder. This size is
+  // obtained from VEA through |RequireBitstreamBuffers()|.
+  gfx::Size frame_coded_size_;
+
   DISALLOW_COPY_AND_ASSIGN(VEAClientImpl);
 };