Refuse VPX encoder options change when frame area increases

Bug: 1151684
Change-Id: I4a25e6fdea3cba242df439315e61a6d1e36ccad4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2554033
Commit-Queue: Eugene Zemtsov <eugene@chromium.org>
Reviewed-by: Chrome Cunningham <chcunningham@chromium.org>
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#830417}

ash/services/recording/recording_service.cc

@@ -255,8 +255,7 @@ void RecordingService::StartNewRecording(
   media::VideoEncoder::Options video_encoder_options;
   video_encoder_options.bitrate = CalculateVpxEncoderBitrate(capture_size);
   video_encoder_options.framerate = kMaxFrameRate;
-  video_encoder_options.width = capture_size.width();
-  video_encoder_options.height = capture_size.height();
+  video_encoder_options.frame_size = capture_size;
   // This value, expressed as a number of frames, forces the encoder to code
   // a keyframe if one has not been coded in the last keyframe_interval frames.
   video_encoder_options.keyframe_interval = 100;

media/base/video_encoder.h

@@ -41,8 +41,7 @@ class MEDIA_EXPORT VideoEncoder {
     base::Optional<uint64_t> bitrate;
     base::Optional<double> framerate;
 
-    int width = 0;
-    int height = 0;
+    gfx::Size frame_size;
 
     base::Optional<int> keyframe_interval = 10000;
   };

media/video/openh264_video_encoder.cc

@@ -29,8 +29,8 @@ Status SetUpOpenH264Params(const VideoEncoder::Options& options,
   params->iMultipleThreadIdc = 1;
   if (options.framerate.has_value())
     params->fMaxFrameRate = options.framerate.value();
-  params->iPicHeight = options.height;
-  params->iPicWidth = options.width;
+  params->iPicHeight = options.frame_size.height();
+  params->iPicWidth = options.frame_size.width();
 
   if (options.keyframe_interval.has_value())
     params->uiIntraPeriod = options.keyframe_interval.value();

media/video/video_encode_accelerator_adapter.cc

@@ -36,8 +36,8 @@ VideoEncodeAccelerator::Config SetUpVeaConfig(
     VideoPixelFormat format,
     VideoFrame::StorageType storage_type) {
   auto config = VideoEncodeAccelerator::Config(
-      format, gfx::Size(opts.width, opts.height), profile,
-      opts.bitrate.value_or(opts.width * opts.height *
+      format, opts.frame_size, profile,
+      opts.bitrate.value_or(opts.frame_size.width() * opts.frame_size.height() *
                             kVEADefaultBitratePerPixel));
 
 #if defined(OS_LINUX) || defined(OS_CHROMEOS)
@@ -180,9 +180,16 @@ void VideoEncodeAcceleratorAdapter::InitializeOnAcceleratorThread(
     return;
   }
 
-  if (options.width <= 0 || options.height <= 0) {
+  if (options.frame_size.width() <= 0 || options.frame_size.height() <= 0) {
     auto status = Status(StatusCode::kEncoderUnsupportedConfig,
-                         "Negative width or height values");
+                         "Negative width or height values.");
+    std::move(done_cb).Run(status);
+    return;
+  }
+
+  if (!options.frame_size.GetCheckedArea().IsValid()) {
+    auto status =
+        Status(StatusCode::kEncoderUnsupportedConfig, "Frame is too large.");
     std::move(done_cb).Run(status);
     return;
   }
@@ -367,15 +374,16 @@ void VideoEncodeAcceleratorAdapter::ChangeOptionsOnAcceleratorThread(
   DCHECK(pending_encodes_.empty());
   DCHECK_EQ(state_, State::kReadyToEncode);
 
-  if (options.width != options_.width || options.height != options_.height) {
+  if (options.frame_size != options_.frame_size) {
     auto status = Status(StatusCode::kEncoderInitializationError,
                          "Resolution change is not supported.");
     std::move(done_cb).Run(status);
     return;
   }
 
   uint32_t bitrate =
-      std::min(options.bitrate.value_or(options.width * options.height *
+      std::min(options.bitrate.value_or(options.frame_size.width() *
+                                        options.frame_size.height() *
                                         kVEADefaultBitratePerPixel),
                uint64_t{std::numeric_limits<uint32_t>::max()});
 

media/video/vpx_video_encoder.cc

@@ -4,6 +4,7 @@
 
 #include "media/video/vpx_video_encoder.h"
 
+#include "base/numerics/checked_math.h"
 #include "base/numerics/ranges.h"
 #include "base/strings/stringprintf.h"
 #include "base/system/sys_info.h"
@@ -40,9 +41,12 @@ int GetNumberOfThreads(int width) {
 
 Status SetUpVpxConfig(const VideoEncoder::Options& opts,
                       vpx_codec_enc_cfg_t* config) {
-  if (opts.width <= 0 || opts.height <= 0)
+  if (opts.frame_size.width() <= 0 || opts.frame_size.height() <= 0)
     return Status(StatusCode::kEncoderUnsupportedConfig,
-                  "Negative width or height values");
+                  "Negative width or height values.");
+
+  if (!opts.frame_size.GetCheckedArea().IsValid())
+    return Status(StatusCode::kEncoderUnsupportedConfig, "Frame is too large.");
 
   config->g_pass = VPX_RC_ONE_PASS;
   config->g_lag_in_frames = 0;
@@ -52,7 +56,7 @@ Status SetUpVpxConfig(const VideoEncoder::Options& opts,
   config->g_timebase.den = base::Time::kMicrosecondsPerSecond;
 
   // Set the number of threads based on the image width and num of cores.
-  config->g_threads = GetNumberOfThreads(opts.width);
+  config->g_threads = GetNumberOfThreads(opts.frame_size.width());
 
   // Insert keyframes at will with a given max interval
   if (opts.keyframe_interval.has_value()) {
@@ -66,13 +70,13 @@ Status SetUpVpxConfig(const VideoEncoder::Options& opts,
     config->rc_target_bitrate = opts.bitrate.value() / 1000;
   } else {
     config->rc_end_usage = VPX_VBR;
-    config->rc_target_bitrate = double{opts.width} * double{opts.height} /
-                                config->g_w / config->g_h *
-                                config->rc_target_bitrate;
+    config->rc_target_bitrate =
+        double{opts.frame_size.GetCheckedArea().ValueOrDie()} / config->g_w /
+        config->g_h * config->rc_target_bitrate;
   }
 
-  config->g_w = opts.width;
-  config->g_h = opts.height;
+  config->g_w = opts.frame_size.width();
+  config->g_h = opts.frame_size.height();
 
   return Status();
 }
@@ -185,8 +189,9 @@ void VpxVideoEncoder::Initialize(VideoCodecProfile profile,
     return;
   }
 
-  if (&vpx_image_ != vpx_img_wrap(&vpx_image_, img_fmt, options.width,
-                                  options.height, 1, nullptr)) {
+  if (&vpx_image_ != vpx_img_wrap(&vpx_image_, img_fmt,
+                                  options.frame_size.width(),
+                                  options.frame_size.height(), 1, nullptr)) {
     status = Status(StatusCode::kEncoderInitializationError,
                     "Invalid format or frame size.");
     std::move(done_cb).Run(status);
@@ -303,20 +308,35 @@ void VpxVideoEncoder::ChangeOptions(const Options& options,
     return;
   }
 
+  auto old_area = options_.frame_size.GetCheckedArea();
+  auto new_area = options.frame_size.GetCheckedArea();
+  DCHECK(old_area.IsValid());
+
+  // Libvpx doesn't support reconfiguring in a way that enlarges frame area.
+  // https://bugs.chromium.org/p/webm/issues/detail?id=1642
+  if (!new_area.IsValid() || new_area.ValueOrDie() > old_area.ValueOrDie()) {
+    auto status =
+        Status(StatusCode::kEncoderUnsupportedConfig,
+               "libvpx doesn't support dynamically increasing frame area");
+    std::move(done_cb).Run(std::move(status));
+    return;
+  }
+
   vpx_codec_enc_cfg_t new_config = codec_config_;
   auto status = SetUpVpxConfig(options, &new_config);
   if (!status.is_ok()) {
     std::move(done_cb).Run(status);
     return;
   }
 
-  if (options_.width != options.width || options_.height != options.height) {
+  if (options_.frame_size != options.frame_size) {
     // Need to re-allocate |vpx_image_| because the size has changed.
     auto img_fmt = vpx_image_.fmt;
     auto bit_depth = vpx_image_.bit_depth;
     vpx_img_free(&vpx_image_);
-    if (&vpx_image_ != vpx_img_wrap(&vpx_image_, img_fmt, options.width,
-                                    options.height, 1, nullptr)) {
+    if (&vpx_image_ != vpx_img_wrap(&vpx_image_, img_fmt,
+                                    options.frame_size.width(),
+                                    options.frame_size.height(), 1, nullptr)) {
       status = Status(StatusCode::kEncoderInitializationError,
                       "Invalid format or frame size.");
       std::move(done_cb).Run(status);

third_party/blink/renderer/modules/webcodecs/video_encoder.cc

@@ -70,13 +70,15 @@ std::unique_ptr<media::VideoEncoder> CreateAcceleratedVideoEncoder(
     if (supported_profile.profile != profile)
       continue;
 
-    if (supported_profile.min_resolution.width() > options.width ||
-        supported_profile.min_resolution.height() > options.height) {
+    if (supported_profile.min_resolution.width() > options.frame_size.width() ||
+        supported_profile.min_resolution.height() >
+            options.frame_size.height()) {
       continue;
     }
 
-    if (supported_profile.max_resolution.width() < options.width ||
-        supported_profile.max_resolution.height() < options.height) {
+    if (supported_profile.max_resolution.width() < options.frame_size.width() ||
+        supported_profile.max_resolution.height() <
+            options.frame_size.height()) {
       continue;
     }
 
@@ -207,16 +209,16 @@ VideoEncoder::ParsedConfig* VideoEncoder::ParseConfig(
   constexpr int kMaxSupportedFrameSize = 8000;
   auto* parsed = MakeGarbageCollected<ParsedConfig>();
 
-  parsed->options.height = config->height();
-  if (parsed->options.height == 0 ||
-      parsed->options.height > kMaxSupportedFrameSize) {
+  parsed->options.frame_size.set_height(config->height());
+  if (parsed->options.frame_size.height() == 0 ||
+      parsed->options.frame_size.height() > kMaxSupportedFrameSize) {
     exception_state.ThrowTypeError("Invalid height.");
     return nullptr;
   }
 
-  parsed->options.width = config->width();
-  if (parsed->options.width == 0 ||
-      parsed->options.width > kMaxSupportedFrameSize) {
+  parsed->options.frame_size.set_width(config->width());
+  if (parsed->options.frame_size.width() == 0 ||
+      parsed->options.frame_size.width() > kMaxSupportedFrameSize) {
     exception_state.ThrowTypeError("Invalid width.");
     return nullptr;
   }
@@ -472,9 +474,8 @@ void VideoEncoder::encode(VideoFrame* frame,
   }
 
   DCHECK(active_config_);
-  if (internal_frame->cropWidth() != uint32_t{active_config_->options.width} ||
-      internal_frame->cropHeight() !=
-          uint32_t{active_config_->options.height}) {
+  if (internal_frame->frame()->coded_size() !=
+      active_config_->options.frame_size) {
     exception_state.ThrowDOMException(
         DOMExceptionCode::kOperationError,
         "Frame size doesn't match initial encoder parameters.");
@@ -790,8 +791,8 @@ void VideoEncoder::CallOutputCallback(
   VideoDecoderConfig* decoder_config =
       MakeGarbageCollected<VideoDecoderConfig>();
   decoder_config->setCodec(active_config->codec_string);
-  decoder_config->setCodedHeight(active_config->options.height);
-  decoder_config->setCodedWidth(active_config->options.width);
+  decoder_config->setCodedHeight(active_config->options.frame_size.height());
+  decoder_config->setCodedWidth(active_config->options.frame_size.width());
   if (codec_desc.has_value()) {
     auto* desc_array_buf = DOMArrayBuffer::Create(codec_desc.value().data(),
                                                   codec_desc.value().size());

third_party/blink/web_tests/webcodecs/reconfiguring_encooder.html

@@ -99,6 +99,7 @@
       let encoder = new VideoEncoder(init);
       encoder.configure(params);
 
+      // Encode |frames_to_encode| frames with original settings
       for (let i = 0; i < frames_to_encode; i++) {
         var frame = await createFrame(original_w, original_h, next_ts++);
         encoder.encode(frame, {});
@@ -109,6 +110,7 @@
       params.height = new_h;
       params.bitrate = new_bitrate;
 
+      // Reconfigure encoder and run |frames_to_encode| frames with new settings
       encoder.configure(params);
       reconf_ts = next_ts;
 
@@ -119,6 +121,14 @@
       }
 
       await encoder.flush();
+
+      // Configure back to original config
+      params.width = original_w;
+      params.height = original_h;
+      params.bitrate = original_bitrate;
+      encoder.configure(params);
+      await encoder.flush();
+
       encoder.close();
       assert_equals(before_reconf_frames, frames_to_encode);
       assert_equals(after_reconf_frames, frames_to_encode);