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..b72b2a6283e71833f2b2bd7c94965a1589ba3adb 100644
--- a/media/cast/sender/external_video_encoder.cc
+++ b/media/cast/sender/external_video_encoder.cc
@@ -19,6 +19,7 @@
 #include "base/metrics/histogram.h"
 #include "base/strings/stringprintf.h"
 #include "build/build_config.h"
+#include "media/base/bind_to_current_loop.h"
 #include "media/base/video_frame.h"
 #include "media/base/video_types.h"
 #include "media/base/video_util.h"
@@ -33,8 +34,14 @@
 namespace {
 
 enum { MAX_H264_QUANTIZER = 51 };
+
+// Number of buffers for encoded bit stream.
 static const size_t kOutputBufferCount = 3;
 
+// Number of extra buffers for encoder input. The input buffers are used to copy
+// the video stream to match the required coded size.
+static const size_t kExtraInputBufferCount = 3;
+
 }  // namespace
 
 namespace media {
@@ -135,6 +142,15 @@ class ExternalVideoEncoder::VEAClientImpl
                                                                max_frame_rate_);
   }
 
+  // The destruction call back of the copied video frame to free its use of
+  // the input buffer.
+  void OnFinishEncodeFrame(int index) {

[miu, 2016/04/30 00:41:14]

naming nit: Since this is only used when we have to copy the VideoFrame, can we
name this something more-specific, like ReturnInputBufferToPool() or
MakeInputBufferAvailable().

[xjz, 2016/05/03 01:17:59]

Done.

+    DCHECK(task_runner_->RunsTasksOnCurrentThread());
+    DCHECK_GE(index, 0);
+    DCHECK_LT(index, static_cast<int>(input_buffers_.size()));
+    free_input_buffer_index_.push_back(index);
+  }
+
   void EncodeVideoFrame(
       const scoped_refptr<media::VideoFrame>& video_frame,
       const base::TimeTicks& reference_time,
@@ -145,14 +161,50 @@ class ExternalVideoEncoder::VEAClientImpl
     if (!encoder_active_)
       return;

[miu, 2016/04/30 00:41:14]

There are a bunch of early returns in this method. However,
|frame_encoded_callback| must *always* be run in order for VideoSender to
correctly account for the backlog in the encoder.

This early return here is kind of bad, but acceptable because the encoder is
permanently stopped (during shutdown). The rest that you've added will result in
the |frame_encoded_callback| never being run.

There's another issue here: VideoSender expects that, if the call to
ExternalVideoEncoder::EncodeVideoFrame() returns true, then it will always get
back an encoded frame, eventually.

So, OTOH, we need to do two things to support the extra failure possibilities:

1. For the two early returns in this method, add the following before the return
statement:

  std::unique_ptr<SenderEncodedFrame> no_result(nullptr);
  cast_environment_->PostTask(CastEnvironment::MAIN, FROM_HERE,
      base::Bind(in_progress_frame_encodes_.back().frame_encoded_callback,
                 base::Passed(&no_result)));
  in_progress_frame_encodes_.pop_back();
  return;

Since that's a bit of code to duplicate, you should merge the two failure points
into one, with:

  if (!frame || !media::I420CopyWithPadding(video_frame, frame)) {
    LOG(DFATAL) << "...";
    <post task with nullptr result here>
    return;
  }

2. In VideoSender::OnEncodedVideoFrame(), if it is called with nullptr for the
|encoded_frame|, it should return early (i.e., after it decrements the
|frames_in_encoder_| counter.

[xjz, 2016/05/03 01:17:59]

Done. 1. Put all the necessary early return operation into a private function,
since it needs to be called when the input buffer is not available too. 2. Also
add early return to SizeAdaptableVideoEncoderBase::OnEncodedVideoFrame() when
the input |encoded_frame| is nullptr.

 
+    DCHECK(!free_input_buffer_index_.empty());
+
     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_) {
+      DCHECK_GE(frame_coded_size_.width(), video_frame->visible_rect().width());
+      DCHECK_GE(frame_coded_size_.height(),
+                video_frame->visible_rect().height());
+
+      int index = free_input_buffer_index_.back();
+      base::SharedMemory* input_buffer = input_buffers_[index];
+      frame = VideoFrame::WrapExternalSharedMemory(
+          video_frame->format(), frame_coded_size_, video_frame->visible_rect(),
+          video_frame->visible_rect().size(),
+          static_cast<uint8_t*>(input_buffer->memory()),
+          input_buffer->mapped_size(), input_buffer->handle(), 0,
+          video_frame->timestamp());
+      if (!frame.get()) {
+        VLOG(1) << "Error: ExternalVideoEncoder: CreateFrame failed.";

[miu, 2016/04/30 00:41:14]

This should probably be LOG(DFATAL) since either there's a coding bug or an OOM.
For debug builds, we assume a coding bug; for release builds, an error log
message is suitable to report out-of-memory.

[xjz, 2016/05/03 01:17:59]

Done.

+        return;
+      }
+
+      // Copy the input frame to match the input requirements for the encoder.
+      if (!media::I420CopyWithPadding(video_frame, frame)) {
+        VLOG(1) << "ERROR: ExternalVideoEncoder: Copy failed.";

[miu, 2016/04/30 00:41:14]

ditto: LOG(DFATAL).

[xjz, 2016/05/03 01:17:59]

Done. Combined this check with the above check on WrapExternalSharedMemory().

+        return;
+      }
+
+      frame->AddDestructionObserver(media::BindToCurrentLoop(
+          base::Bind(&ExternalVideoEncoder::VEAClientImpl::OnFinishEncodeFrame,
+                     this, index)));
+      free_input_buffer_index_.pop_back();
+    }
+
     // 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);
   }
 
+  // Return true if there is available input buffer.
+  bool IsInputBufferReady() { return !free_input_buffer_index_.empty(); }
+
  protected:
   void NotifyError(VideoEncodeAccelerator::Error error) final {
     DCHECK(task_runner_->RunsTasksOnCurrentThread());
@@ -177,8 +229,15 @@ class ExternalVideoEncoder::VEAClientImpl
                                size_t output_buffer_size) final {
     DCHECK(task_runner_->RunsTasksOnCurrentThread());
 
-    // TODO(miu): Investigate why we are ignoring |input_count| (4) and instead
-    // using |kOutputBufferCount| (3) here.
+    frame_coded_size_ = input_coded_size;
+
+    for (size_t j = 0; j < input_count + kExtraInputBufferCount; ++j) {

[miu, 2016/04/30 00:41:14]

Instead of allocating these up-front, can we allocate on-demand, and only when
the coded_size is not a match? Also, instead of allocating all of them, we
should only allocate as many as are needed (i.e., one at a time, whenever the
|free_input_buffer_index_| list is empty). We want to be as memory-efficient as
possible. :)

[xjz, 2016/05/03 01:17:59]

Done.

+      create_video_encode_memory_cb_.Run(
+          media::VideoFrame::AllocationSize(media::PIXEL_FORMAT_I420,
+                                            input_coded_size),
+          base::Bind(&VEAClientImpl::OnCreateInputSharedMemory, this));
+    }
+
     for (size_t j = 0; j < kOutputBufferCount; ++j) {
       create_video_encode_memory_cb_.Run(
           output_buffer_size,
@@ -368,6 +427,12 @@ class ExternalVideoEncoder::VEAClientImpl
                                       base::Passed(&memory)));
   }
 
+  void OnCreateInputSharedMemory(scoped_ptr<base::SharedMemory> memory) {
+    task_runner_->PostTask(
+        FROM_HERE, base::Bind(&VEAClientImpl::OnReceivedInputSharedMemory, this,
+                              base::Passed(&memory)));
+  }
+
   void OnReceivedSharedMemory(scoped_ptr<base::SharedMemory> memory) {
     DCHECK(task_runner_->RunsTasksOnCurrentThread());
 
@@ -386,6 +451,13 @@ class ExternalVideoEncoder::VEAClientImpl
     }
   }
 
+  void OnReceivedInputSharedMemory(scoped_ptr<base::SharedMemory> memory) {
+    DCHECK(task_runner_->RunsTasksOnCurrentThread());
+
+    input_buffers_.push_back(std::move(memory));
+    free_input_buffer_index_.push_back(input_buffers_.size() - 1);
+  }
+
   // Parse H264 SPS, PPS, and Slice header, and return the averaged frame
   // quantizer in the range of [0, 51], or -1 on parse error.
   double GetH264FrameQuantizer(const uint8_t* encoded_data, off_t size) {
@@ -462,6 +534,12 @@ class ExternalVideoEncoder::VEAClientImpl
   // Shared memory buffers for output with the VideoAccelerator.
   ScopedVector<base::SharedMemory> output_buffers_;
 
+  // Shared memory buffers for input video frames with the VideoAccelerator.
+  ScopedVector<base::SharedMemory> input_buffers_;

[miu, 2016/04/30 00:41:14]

ScopedVector is deprecated. Please use
std::vector<std::unique_ptr<base::SharedMemory>> instead.

[xjz, 2016/05/03 01:17:59]

Done.

+
+  // Available input buffer index. These buffers are used in FILO order.
+  std::vector<int> free_input_buffer_index_;
+
   // FIFO list.
   std::list<InProgressFrameEncode> in_progress_frame_encodes_;
 
@@ -476,6 +554,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);
 };
 
@@ -535,6 +617,10 @@ bool ExternalVideoEncoder::EncodeVideoFrame(
   if (!client_ || video_frame->visible_rect().size() != frame_size_)
     return false;
 
+  // Drop frame until an input buffer is available.
+  if (!client_->IsInputBufferReady())

[miu, 2016/04/30 00:41:14]

See comments above. If input buffers are allocated on-demand, this isn't needed
anymore.

[xjz, 2016/05/03 01:17:59]

Removed. The purpose of using this was actually trying to early exit the
encoding when the buffer is not ready. This is not needed anymore since now the
encoding is exited by calling |ExitEncodingWithErrors()|.

+    return false;
+
   client_->task_runner()->PostTask(FROM_HERE,
                                    base::Bind(&VEAClientImpl::EncodeVideoFrame,
                                               client_,