Revert "[webcodecs] Changing encoder parameters without re-initialization"

This reverts commit ef52da93.

Reason for revert: Test fails flakily on multiple platforms: https://test-results.appspot.com/dashboards/flakiness_dashboard.html#testType=blink_web_tests&tests=webcodecs%2Freconfiguring_encooder.html


Original change's description:
> [webcodecs] Changing encoder parameters without re-initialization
>
> Bug: 1119892, 1146168
> Change-Id: Ibb548cd1684f22a0e095c0d0d51654bdd2ea59f2
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2534691
> Commit-Queue: Eugene Zemtsov <eugene@chromium.org>
> Reviewed-by: Dan Sanders <sandersd@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#827150}

TBR=sandersd@chromium.org,eugene@chromium.org

Change-Id: I20d68788883ac2f7d61cf53e9fa6381220d7aed7
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1119892
Bug: 1146168
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2537879Reviewed-by: Adithya Srinivasan <adithyas@chromium.org>
Commit-Queue: Adithya Srinivasan <adithyas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#827251}

media/video/openh264_video_encoder.cc

@@ -253,30 +253,8 @@ void OpenH264VideoEncoder::ChangeOptions(const Options& options,
     return;
   }
 
-  Status status;
-  SEncParamExt params = {};
-  if (int err = codec_->GetDefaultParams(&params)) {
-    status = Status(StatusCode::kEncoderInitializationError,
-                    "Failed to get default params.")
-                 .WithData("error", err);
-    std::move(done_cb).Run(status);
-    return;
-  }
+  // TODO(eugene): Not implemented yet.
 
-  status = SetUpOpenH264Params(options, &params);
-  if (!status.is_ok()) {
-    std::move(done_cb).Run(status);
-    return;
-  }
-
-  if (int err =
-          codec_->SetOption(ENCODER_OPTION_SVC_ENCODE_PARAM_EXT, &params)) {
-    status = Status(StatusCode::kEncoderInitializationError,
-                    "OpenH264 encoder failed to set new SEncParamExt.")
-                 .WithData("error", err);
-    std::move(done_cb).Run(status);
-    return;
-  }
   std::move(done_cb).Run(Status());
 }
 

media/video/video_encode_accelerator_adapter.cc

@@ -282,39 +282,7 @@ void VideoEncodeAcceleratorAdapter::EncodeOnAcceleratorThread(
 }
 
 void VideoEncodeAcceleratorAdapter::ChangeOptions(const Options& options,
-                                                  StatusCB done_cb) {
-  DCHECK(!accelerator_task_runner_->RunsTasksInCurrentSequence());
-  accelerator_task_runner_->PostTask(
-      FROM_HERE,
-      base::BindOnce(
-          &VideoEncodeAcceleratorAdapter::ChangeOptionsOnAcceleratorThread,
-          base::Unretained(this), options, WrapCallback(std::move(done_cb))));
-}
-
-void VideoEncodeAcceleratorAdapter::ChangeOptionsOnAcceleratorThread(
-    const Options options,
-    StatusCB done_cb) {
-  DCHECK_CALLED_ON_VALID_SEQUENCE(accelerator_sequence_checker_);
-
-  if (options.width != options_.width || options.height != options_.height) {
-    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 *
-                                        kVEADefaultBitratePerPixel),
-               uint64_t{std::numeric_limits<uint32_t>::max()});
-
-  uint32_t framerate = uint32_t{std::round(
-      options.framerate.value_or(VideoEncodeAccelerator::kDefaultFramerate))};
-
-  accelerator_->RequestEncodingParametersChange(bitrate, framerate);
-  options_ = options;
-  std::move(done_cb).Run(Status());
-}
+                                                  StatusCB done_cb) {}
 
 void VideoEncodeAcceleratorAdapter::Flush(StatusCB done_cb) {
   DCHECK(!accelerator_task_runner_->RunsTasksInCurrentSequence());

media/video/video_encode_accelerator_adapter.h

@@ -91,8 +91,6 @@ class MEDIA_EXPORT VideoEncodeAcceleratorAdapter
                                  bool key_frame,
                                  StatusCB done_cb);
   void FlushOnAcceleratorThread(StatusCB done_cb);
-  void ChangeOptionsOnAcceleratorThread(const Options options,
-                                        StatusCB done_cb);
 
   template <class T>
   T WrapCallback(T cb);

media/video/vpx_video_encoder.cc

@@ -307,26 +307,7 @@ void VpxVideoEncoder::ChangeOptions(const Options& options, StatusCB done_cb) {
 
   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) {
-    // 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)) {
-      status = Status(StatusCode::kEncoderInitializationError,
-                      "Invalid format or frame size.");
-      std::move(done_cb).Run(status);
-      return;
-    }
-    vpx_image_.bit_depth = bit_depth;
-  }
-
+  if (status.is_ok()) {
     auto vpx_error = vpx_codec_enc_config_set(codec_.get(), &new_config);
     if (vpx_error == VPX_CODEC_OK) {
       codec_config_ = new_config;
@@ -336,8 +317,10 @@ void VpxVideoEncoder::ChangeOptions(const Options& options, StatusCB done_cb) {
                       "Failed to set new VPX config")
                    .WithData("vpx_error", vpx_error);
     }
+  }
 
   std::move(done_cb).Run(std::move(status));
+  return;
 }
 
 base::TimeDelta VpxVideoEncoder::GetFrameDuration(const VideoFrame& frame) {

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

@@ -320,15 +320,6 @@ std::unique_ptr<media::VideoEncoder> VideoEncoder::CreateMediaVideoEncoder(
   }
 }
 
-bool VideoEncoder::CanReconfigure(ParsedConfig& original_config,
-                                  ParsedConfig& new_config) {
-  return original_config.codec == new_config.codec &&
-         original_config.profile == new_config.profile &&
-         original_config.level == new_config.level &&
-         original_config.color_space == new_config.color_space &&
-         original_config.acc_pref == new_config.acc_pref;
-}
-
 void VideoEncoder::configure(const VideoEncoderConfig* config,
                              ExceptionState& exception_state) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
@@ -337,6 +328,7 @@ void VideoEncoder::configure(const VideoEncoderConfig* config,
     return;
 
   auto parsed_config = ParseConfig(config, exception_state);
+
   if (!parsed_config) {
     DCHECK(exception_state.HadException());
     return;
@@ -347,16 +339,14 @@ void VideoEncoder::configure(const VideoEncoderConfig* config,
     return;
   }
 
+  // TODO(https://crbug.com/1119892): flush |media_encoder_| if it already
+  // exists, otherwise might could lose frames in flight.
+
+  state_ = V8CodecState(V8CodecState::Enum::kConfigured);
+
   Request* request = MakeGarbageCollected<Request>();
   request->reset_count = reset_count_;
-  if (media_encoder_ && active_config_ &&
-      state_.AsEnum() == V8CodecState::Enum::kConfigured &&
-      CanReconfigure(*active_config_, *parsed_config)) {
-    request->type = Request::Type::kReconfigure;
-  } else {
-    state_ = V8CodecState(V8CodecState::Enum::kConfigured);
   request->type = Request::Type::kConfigure;
-  }
   active_config_ = std::move(parsed_config);
   EnqueueRequest(request);
 }
@@ -445,8 +435,9 @@ void VideoEncoder::reset(ExceptionState& exception_state) {
   if (ThrowIfCodecStateClosed(state_, "reset", exception_state))
     return;
 
-  state_ = V8CodecState(V8CodecState::Enum::kUnconfigured);
   ResetInternal();
+
+  state_ = V8CodecState(V8CodecState::Enum::kUnconfigured);
 }
 
 void VideoEncoder::ResetInternal() {
@@ -455,8 +446,11 @@ void VideoEncoder::ResetInternal() {
   while (!requests_.empty()) {
     Request* pending_req = requests_.TakeFirst();
     DCHECK(pending_req);
-    if (pending_req->resolver)
-      pending_req->resolver.Release()->Reject();
+    if (pending_req->resolver) {
+      auto* ex = MakeGarbageCollected<DOMException>(
+          DOMExceptionCode::kOperationError, "reset() was called.");
+      pending_req->resolver.Release()->Reject(ex);
+    }
   }
   stall_request_processing_ = false;
 }
@@ -499,9 +493,6 @@ void VideoEncoder::ProcessRequests() {
       case Request::Type::kConfigure:
         ProcessConfigure(request);
         break;
-      case Request::Type::kReconfigure:
-        ProcessReconfigure(request);
-        break;
       case Request::Type::kEncode:
         ProcessEncode(request);
         break;
@@ -594,57 +585,6 @@ void VideoEncoder::ProcessConfigure(Request* request) {
                                        WrapPersistent(request)));
 }
 
-void VideoEncoder::ProcessReconfigure(Request* request) {
-  DCHECK_EQ(state_.AsEnum(), V8CodecState::Enum::kConfigured);
-  DCHECK_EQ(request->type, Request::Type::kReconfigure);
-  DCHECK(active_config_);
-  DCHECK(media_encoder_);
-  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-
-  auto reconf_done_callback = [](VideoEncoder* self, Request* req,
-                                 media::Status status) {
-    if (!self || self->reset_count_ != req->reset_count)
-      return;
-    DCHECK_CALLED_ON_VALID_SEQUENCE(self->sequence_checker_);
-    DCHECK(self->active_config_);
-
-    if (status.is_ok()) {
-      self->stall_request_processing_ = false;
-      self->ProcessRequests();
-    } else {
-      // Reconfiguration failed. Either encoder doesn't support changing options
-      // or it didn't like this partecular change. Let's try to configure it
-      // from scratch.
-      req->type = Request::Type::kConfigure;
-      self->ProcessConfigure(req);
-    }
-  };
-
-  auto flush_done_callback = [](VideoEncoder* self, Request* req,
-                                decltype(reconf_done_callback) reconf_callback,
-                                media::Status status) {
-    if (!self || self->reset_count_ != req->reset_count)
-      return;
-    DCHECK_CALLED_ON_VALID_SEQUENCE(self->sequence_checker_);
-    if (!status.is_ok()) {
-      std::string msg = "Encoder reconfiguration error: " + status.message();
-      self->HandleError(DOMExceptionCode::kOperationError, msg.c_str());
-      self->stall_request_processing_ = false;
-      return;
-    }
-
-    self->media_encoder_->ChangeOptions(
-        self->active_config_->options,
-        WTF::Bind(reconf_callback, WrapWeakPersistent(self),
-                  WrapPersistent(req)));
-  };
-
-  stall_request_processing_ = true;
-  media_encoder_->Flush(WTF::Bind(flush_done_callback, WrapWeakPersistent(this),
-                                  WrapPersistent(request),
-                                  std::move(reconf_done_callback)));
-}
-
 void VideoEncoder::ProcessFlush(Request* request) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK_EQ(state_, V8CodecState::Enum::kConfigured);
@@ -672,7 +612,7 @@ void VideoEncoder::ProcessFlush(Request* request) {
 
   stall_request_processing_ = true;
   media_encoder_->Flush(WTF::Bind(done_callback, WrapWeakPersistent(this),
-                                  WrapPersistent(request)));
+                                  WrapPersistentIfNeeded(request)));
 }
 
 void VideoEncoder::CallOutputCallback(

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

@@ -82,10 +82,7 @@ class MODULES_EXPORT VideoEncoder final : public ScriptWrappable {
 
   struct Request final : public GarbageCollected<Request> {
     enum class Type {
-      // Configure an encoder from scratch, possibly replacing the existing one.
       kConfigure,
-      // Adjust options in the already configured encoder.
-      kReconfigure,
       kEncode,
       kFlush,
     };
@@ -110,7 +107,6 @@ class MODULES_EXPORT VideoEncoder final : public ScriptWrappable {
   void ProcessRequests();
   void ProcessEncode(Request* request);
   void ProcessConfigure(Request* request);
-  void ProcessReconfigure(Request* request);
   void ProcessFlush(Request* request);
 
   void ResetInternal();
@@ -120,7 +116,6 @@ class MODULES_EXPORT VideoEncoder final : public ScriptWrappable {
   bool VerifyCodecSupport(ParsedConfig*, ExceptionState&);
   std::unique_ptr<media::VideoEncoder> CreateMediaVideoEncoder(
       const ParsedConfig& config);
-  bool CanReconfigure(ParsedConfig& original_config, ParsedConfig& new_config);
 
   std::unique_ptr<ParsedConfig> active_config_;
   std::unique_ptr<media::VideoEncoder> media_encoder_;

third_party/blink/web_tests/webcodecs/reconfiguring_encooder.html

-<!DOCTYPE html>
-<html>
-
-<head>
-  <script src="../resources/testharness.js"></script>
-  <script src="../resources/testharnessreport.js"></script>
-</head>
-
-<body>
-  <img id='frame_image' style='display: none;' src="pattern.png">
-  <script>
-
-    'use strict';
-
-    async function waitTillLoaded(img) {
-      if (img.complete)
-        return Promise.resolve();
-      return new Promise((resolve, reject) => {
-        img.onload = () => resolve()
-      });
-    }
-
-    async function generateBitmap(width, height, text) {
-      let img = document.getElementById('frame_image');
-      await waitTillLoaded(img);
-      let cnv = document.createElement("canvas");
-      cnv.height = height;
-      cnv.width = width;
-      var ctx = cnv.getContext('2d');
-      ctx.drawImage(img, 0, 0, width, height);
-      ctx.font = '30px fantasy';
-      ctx.fillText(text, 5, 40);
-      return createImageBitmap(cnv);
-    }
-
-    async function createFrame(width, height, ts) {
-      let imageBitmap = await generateBitmap(width, height, ts.toString());
-      let frame = new VideoFrame(imageBitmap, { timestamp: ts });
-      imageBitmap.close();
-      return frame;
-    }
-
-    function delay(time_ms) {
-      return new Promise((resolve, reject) => {
-        setTimeout(resolve, time_ms);
-      });
-    };
-
-    async function change_encoding_params_test(codec, acc) {
-      let original_w = 800;
-      let original_h = 600;
-      let original_bitrate = 5_000_000;
-
-      let new_w = 640;
-      let new_h = 480;
-      let new_bitrate = 3_000_000;
-
-      let next_ts = 0
-      let reconf_ts = 0;
-      let frames_to_encode = 16;
-      let before_reconf_frames = 0;
-      let after_reconf_frames = 0;
-      let errors = 0;
-
-      let process_video_chunk = function (chunk, config) {
-        var data = new Uint8Array(chunk.data);
-        assert_greater_than_equal(data.length, 0);
-        let after_reconf = (reconf_ts != 0) && (chunk.timestamp >= reconf_ts);
-        if (after_reconf) {
-          after_reconf_frames++;
-          if (config) {
-            assert_equals(config.codedWidth, new_w);
-            assert_equals(config.codedHeight, new_h);
-          }
-        } else {
-          before_reconf_frames++;
-          if (config) {
-            assert_equals(config.codedWidth, original_w);
-            assert_equals(config.codedHeight, original_h);
-          }
-        }
-      };
-
-      const init = {
-        output: process_video_chunk,
-        error: (e) => {
-          errors++;
-          console.log(e.message);
-        },
-      };
-      const params = {
-        codec: codec,
-        acceleration: acc,
-        width: original_w,
-        height: original_h,
-        bitrate: original_bitrate,
-        framerate: 30,
-      };
-      let encoder = new VideoEncoder(init);
-      encoder.configure(params);
-
-      for (let i = 0; i < frames_to_encode; i++) {
-        var frame = await createFrame(original_w, original_h, next_ts++);
-        encoder.encode(frame, {});
-        await delay(1);
-      }
-
-      params.width = new_w;
-      params.height = new_h;
-      params.bitrate = new_bitrate;
-
-      encoder.configure(params);
-      reconf_ts = next_ts;
-
-      for (let i = 0; i < frames_to_encode; i++) {
-        var frame = await createFrame(new_w, new_h, next_ts++);
-        encoder.encode(frame, {});
-        await delay(1);
-      }
-
-      await encoder.flush();
-      encoder.close();
-      assert_equals(before_reconf_frames, frames_to_encode);
-      assert_equals(after_reconf_frames, frames_to_encode);
-      assert_equals(errors, 0);
-    }
-
-    promise_test(change_encoding_params_test.bind(null, "vp8", "allow"),
-      "reconfiguring vp8");
-
-    promise_test(change_encoding_params_test.bind(null, "vp09.00.10.08", "allow"),
-      "reconfiguring vp9 profile0");
-
-    promise_test(change_encoding_params_test.bind(null, "vp09.02.10.10", "allow"),
-      "reconfiguring vp9 profile2");
-
-    promise_test(change_encoding_params_test.bind(null, "avc1.42001E", "allow"),
-      "reconfiguring avc1.42001E");
-
-    /* Uncomment this for manual testing, before we have GPU tests for that */
-    //promise_test(change_encoding_params_test.bind(null, "avc1.42001E", "require"),
-    //  "reconfiguring avc1.42001E hw");
-
-
-  </script>
-</body>
-
-</html>
\ No newline at end of file