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

media/cast/sender/external_video_encoder.cc

 // Copyright 2014 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 "media/cast/sender/external_video_encoder.h"
 
 #include <cmath>
 #include <utility>
 
 #include "base/bind.h"
@@ skipping 17 matching lines @@
 #include "media/cast/logging/logging_defines.h"
 #include "media/cast/net/cast_transport_config.h"
 #include "media/cast/sender/vp8_quantizer_parser.h"
 #include "media/filters/h264_parser.h"
 
 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 {
 namespace cast {
 
 // Container for the associated data of a video frame being processed.
 struct InProgressFrameEncode {
   // The source content to encode.
   const scoped_refptr<VideoFrame> video_frame;
 
@@ skipping 94 matching lines @@
       const VideoEncoder::FrameEncodedCallback& frame_encoded_callback) {
     DCHECK(task_runner_->RunsTasksOnCurrentThread());
 
     if (!encoder_active_)
       return;
 
     in_progress_frame_encodes_.push_back(InProgressFrameEncode(
         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:
   void NotifyError(VideoEncodeAccelerator::Error error) final {
     DCHECK(task_runner_->RunsTasksOnCurrentThread());
 
     DCHECK(error != VideoEncodeAccelerator::kInvalidArgumentError &&
            error != VideoEncodeAccelerator::kIllegalStateError);
 
     encoder_active_ = false;
 
     cast_environment_->PostTask(
         CastEnvironment::MAIN,
         FROM_HERE,
         base::Bind(status_change_cb_, STATUS_CODEC_RUNTIME_ERROR));
 
     // TODO(miu): Force-flush all |in_progress_frame_encodes_| immediately so
     // pending frames do not become stuck, freezing VideoSender.
   }
 
   // Called to allocate the input and output buffers.
   void RequireBitstreamBuffers(unsigned int input_count,
                                const gfx::Size& input_coded_size,
                                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.

       create_video_encode_memory_cb_.Run(
           output_buffer_size,
           base::Bind(&VEAClientImpl::OnCreateSharedMemory, this));
     }
   }
 
   // Encoder has encoded a frame and it's available in one of the output
   // buffers.  Package the result in a media::cast::EncodedFrame and post it
   // to the Cast MAIN thread via the supplied callback.
   void BitstreamBufferReady(int32_t bitstream_buffer_id,
@@ skipping 276 matching lines @@
   int requested_bit_rate_;
 
   // Used to compute utilization metrics for each frame.
   QuantizerEstimator quantizer_estimator_;
 
   // Set to true once a frame with zero-length encoded data has been
   // encountered.
   // 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);
 };
 
 // static
 bool ExternalVideoEncoder::IsSupported(const VideoSenderConfig& video_config) {
   if (video_config.codec != CODEC_VIDEO_VP8 &&
       video_config.codec != CODEC_VIDEO_H264)
     return false;
 
   // TODO(miu): "Layering hooks" are needed to be able to query outside of
@@ skipping 298 matching lines @@
   const double kEntropyAtMaxQuantizer = 7.5;
   const double slope =
       (MAX_VP8_QUANTIZER - MIN_VP8_QUANTIZER) / kEntropyAtMaxQuantizer;
   const double quantizer = std::min<double>(
       MAX_VP8_QUANTIZER, MIN_VP8_QUANTIZER + slope * shannon_entropy);
   return quantizer;
 }
 
 }  //  namespace cast
 }  //  namespace media