Remove MediaCodecBridge::GetOutputBuffersCount() and GetOutputBuffersCapacity()

content/common/gpu/media/android_video_encode_accelerator.cc

 // Copyright 2013 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/common/gpu/media/android_video_encode_accelerator.h"
 
 #include <set>
 
 #include "base/bind.h"
 #include "base/command_line.h"
@@ skipping 77 matching lines @@
   else if (formats.count(COLOR_FORMAT_YUV420_PLANAR) > 0)
     *pixel_format = COLOR_FORMAT_YUV420_PLANAR;
   else
     return false;
 
   return true;
 }
 
 AndroidVideoEncodeAccelerator::AndroidVideoEncodeAccelerator()
     : num_buffers_at_codec_(0),
-      num_output_buffers_(-1),
-      output_buffers_capacity_(0),
       last_set_bitrate_(0) {}
 
 AndroidVideoEncodeAccelerator::~AndroidVideoEncodeAccelerator() {
   DCHECK(thread_checker_.CalledOnValidThread());
 }
 
 media::VideoEncodeAccelerator::SupportedProfiles
 AndroidVideoEncodeAccelerator::GetSupportedProfiles() {
   SupportedProfiles profiles;
 
@@ skipping 47 matching lines @@
   client_ptr_factory_.reset(new base::WeakPtrFactory<Client>(client));
 
   if (!(media::MediaCodecUtil::SupportsSetParameters() &&
         format == media::PIXEL_FORMAT_I420)) {
     DLOG(ERROR) << "Unexpected combo: " << format << ", " << output_profile;
     return false;
   }
 
   std::string mime_type;
   media::VideoCodec codec;
+  // The client should be prepared to feed at least this many frames into the
+  // encoder before being returned any output frames, since the encoder may
+  // need to hold onto some subset of inputs as reference pictures.
+  unsigned int frame_input_count;

[DaleCurtis, 2016/02/10 17:52:48]

We don't use unsigned in Chromium code, this should be uint32_t, size_t, or just
plain int if you don't need an unsigned value.

[magjed_chromium, 2016/02/10 20:25:12]

Done, changed to uint32_t. The reason I used 'unsigned int' was to be consistent
with the declaration of
VideoEncodeAccelerator::Client::RequireBitstreamBuffers()
https://code.google.com/p/chromium/codesearch#chromium/src/media/video/video_...
where |frame_input_count| is used.

   if (output_profile == media::VP8PROFILE_ANY) {
     codec = media::kCodecVP8;
     mime_type = "video/x-vnd.on2.vp8";
+    frame_input_count = 1;
   } else if (output_profile == media::H264PROFILE_BASELINE ||
              output_profile == media::H264PROFILE_MAIN) {
     codec = media::kCodecH264;
     mime_type = "video/avc";
+    frame_input_count = 30;
   } else {
     return false;
   }
 
+  frame_size_ = input_visible_size;
   last_set_bitrate_ = initial_bitrate;
 
   // Only consider using MediaCodec if it's likely backed by hardware.
   if (media::VideoCodecBridge::IsKnownUnaccelerated(
           codec, media::MEDIA_CODEC_ENCODER)) {
     DLOG(ERROR) << "No HW support";
     return false;
   }
 
   PixelFormat pixel_format = COLOR_FORMAT_YUV420_SEMIPLANAR;
   if (!GetSupportedColorFormatForMime(mime_type, &pixel_format)) {
     DLOG(ERROR) << "No color format support.";
     return false;
   }
   media_codec_.reset(media::VideoCodecBridge::CreateEncoder(codec,
                                                             input_visible_size,
                                                             initial_bitrate,
                                                             INITIAL_FRAMERATE,
                                                             IFRAME_INTERVAL,
                                                             pixel_format));
 
   if (!media_codec_) {
     DLOG(ERROR) << "Failed to create/start the codec: "
                 << input_visible_size.ToString();
     return false;
   }
 
-  num_output_buffers_ = media_codec_->GetOutputBuffersCount();
-  output_buffers_capacity_ = media_codec_->GetOutputBuffersCapacity();
+  // Conservative upper bound for output buffer size: decoded size + 2KB.
+  const size_t output_buffer_capacity =
+      VideoFrame::AllocationSize(format, input_visible_size) + 2048;
   base::MessageLoop::current()->PostTask(
       FROM_HERE,
       base::Bind(&VideoEncodeAccelerator::Client::RequireBitstreamBuffers,
                  client_ptr_factory_->GetWeakPtr(),
-                 num_output_buffers_,
+                 frame_input_count,
                  input_visible_size,
-                 output_buffers_capacity_));
+                 output_buffer_capacity));
   return true;
 }
 
 void AndroidVideoEncodeAccelerator::MaybeStartIOTimer() {
   if (!io_timer_.IsRunning() &&
       (num_buffers_at_codec_ > 0 || !pending_frames_.empty())) {
     io_timer_.Start(FROM_HERE,
                     EncodePollDelay(),
                     this,
                     &AndroidVideoEncodeAccelerator::DoIOTask);
   }
 }
 
 void AndroidVideoEncodeAccelerator::MaybeStopIOTimer() {
   if (io_timer_.IsRunning() &&
       (num_buffers_at_codec_ == 0 && pending_frames_.empty())) {
     io_timer_.Stop();
   }
 }
 
 void AndroidVideoEncodeAccelerator::Encode(
     const scoped_refptr<VideoFrame>& frame,
     bool force_keyframe) {
   DVLOG(3) << __PRETTY_FUNCTION__ << ": " << force_keyframe;
   DCHECK(thread_checker_.CalledOnValidThread());
   RETURN_ON_FAILURE(frame->format() == media::PIXEL_FORMAT_I420,
                     "Unexpected format", kInvalidArgumentError);
-
+  RETURN_ON_FAILURE(frame->visible_rect().size() == frame_size_,
+                    "Unexpected resolution", kInvalidArgumentError);
   // MediaCodec doesn't have a way to specify stride for non-Packed formats, so
   // we insist on being called with packed frames and no cropping :(
   RETURN_ON_FAILURE(frame->row_bytes(VideoFrame::kYPlane) ==
                             frame->stride(VideoFrame::kYPlane) &&
                         frame->row_bytes(VideoFrame::kUPlane) ==
                             frame->stride(VideoFrame::kUPlane) &&
                         frame->row_bytes(VideoFrame::kVPlane) ==
                             frame->stride(VideoFrame::kVPlane) &&
                         frame->coded_size() == frame->visible_rect().size(),
                     "Non-packed frame, or visible_rect != coded_size",
                     kInvalidArgumentError);
 
   pending_frames_.push(
       base::MakeTuple(frame, force_keyframe, base::Time::Now()));
   DoIOTask();
 }
 
 void AndroidVideoEncodeAccelerator::UseOutputBitstreamBuffer(
     const media::BitstreamBuffer& buffer) {
   DVLOG(3) << __PRETTY_FUNCTION__ << ": bitstream_buffer_id=" << buffer.id();
   DCHECK(thread_checker_.CalledOnValidThread());
-  RETURN_ON_FAILURE(buffer.size() >= media_codec_->GetOutputBuffersCapacity(),
-                    "Output buffers too small!",
-                    kInvalidArgumentError);
   available_bitstream_buffers_.push_back(buffer);
   DoIOTask();
 }
 
 void AndroidVideoEncodeAccelerator::RequestEncodingParametersChange(
     uint32_t bitrate,
     uint32_t framerate) {
   DVLOG(3) << __PRETTY_FUNCTION__ << ": bitrate: " << bitrate
            << ", framerate: " << framerate;
   DCHECK(thread_checker_.CalledOnValidThread());
@@ skipping 90 matching lines @@
       input_buf_index, NULL, queued_size, fake_input_timestamp_);
   UMA_HISTOGRAM_TIMES("Media.AVEA.InputQueueTime",
                       base::Time::Now() - base::get<2>(input));
   RETURN_ON_FAILURE(status == media::MEDIA_CODEC_OK,
                     "Failed to QueueInputBuffer: " << status,
                     kPlatformFailureError);
   ++num_buffers_at_codec_;
   pending_frames_.pop();
 }
 
-bool AndroidVideoEncodeAccelerator::DoOutputBuffersSuffice() {
-  // If this returns false ever, then the VEA::Client interface will need to
-  // grow a DismissBitstreamBuffer() call, and VEA::Client impls will have to be
-  // prepared to field multiple requests to RequireBitstreamBuffers().
-  int count = media_codec_->GetOutputBuffersCount();
-  size_t capacity = media_codec_->GetOutputBuffersCapacity();
-  bool ret = count <= num_output_buffers_ &&
-      capacity <= output_buffers_capacity_;
-  LOG_IF(ERROR, !ret) << "Need more/bigger buffers; before: "
-                      << num_output_buffers_ << "x" << output_buffers_capacity_
-                      << ", now: " << count << "x" << capacity;
-  UMA_HISTOGRAM_BOOLEAN("Media.AVEA.OutputBuffersSuffice", ret);
-  return ret;
-}
-
 void AndroidVideoEncodeAccelerator::DequeueOutput() {
   if (!client_ptr_factory_->GetWeakPtr() ||
       available_bitstream_buffers_.empty() || num_buffers_at_codec_ == 0) {
     return;
   }
 
   int32_t buf_index = 0;
   size_t offset = 0;
   size_t size = 0;
   bool key_frame = false;
   do {
     media::MediaCodecStatus status = media_codec_->DequeueOutputBuffer(
         NoWaitTimeOut(), &buf_index, &offset, &size, NULL, NULL, &key_frame);
     switch (status) {
       case media::MEDIA_CODEC_DEQUEUE_OUTPUT_AGAIN_LATER:
         return;
 
       case media::MEDIA_CODEC_ERROR:
         RETURN_ON_FAILURE(false, "Codec error", kPlatformFailureError);
         // Unreachable because of previous statement, but included for clarity.
         return;
 
-      case media::MEDIA_CODEC_OUTPUT_FORMAT_CHANGED:  // Fall-through.
+      case media::MEDIA_CODEC_OUTPUT_FORMAT_CHANGED:
+        int width;
+        int height;
+        media_codec_->GetOutputFormat(&width, &height);

[liberato (no reviews please), 2016/02/10 18:40:09]

i don't understand the goal of asking for output size during encoding.  can you
provide some background on why it does this?  checking in Encode (as you've
already done) seems like the right place to do it.

admittedly, i'm not very familiar with how MC behaves when encoding.  is there
something subtle here?

[magjed_chromium, 2016/02/10 20:25:12]

It's just a sanity check to make sure the resolution of the encoded video hasn't
changed. I tried to make it similar to the previous DoOutputBuffersSuffice().
I'm pretty sure MEDIA_CODEC_OUTPUT_FORMAT_CHANGED should never be called at all
for video encoder so I replaced this with a simple RETURN_ON_FAILURE() instead.

+        RETURN_ON_FAILURE(
+            width == frame_size_.width() && height == frame_size_.height(),
+            "Unexpected resolution change. input: "
+                << frame_size_.width() << "x" << frame_size_.height()
+                << ", output: " << width << "x" << height,
+            kPlatformFailureError);
+        break;
+
       case media::MEDIA_CODEC_OUTPUT_BUFFERS_CHANGED:
-        RETURN_ON_FAILURE(DoOutputBuffersSuffice(),
-                          "Bitstream now requires more/larger buffers",
-                          kPlatformFailureError);
         break;
 
       case media::MEDIA_CODEC_OK:
         DCHECK_GE(buf_index, 0);
         break;
 
       default:
         NOTREACHED();
         break;
     }
@@ skipping 19 matching lines @@
   base::MessageLoop::current()->PostTask(
       FROM_HERE,
       base::Bind(&VideoEncodeAccelerator::Client::BitstreamBufferReady,
                  client_ptr_factory_->GetWeakPtr(),
                  bitstream_buffer.id(),
                  size,
                  key_frame));
 }
 
 }  // namespace content