Misc bug fixes/features for webcodecs/VideoEncoder

- don't clear the error callback prior to calling it
- implement encodeQueueSize to signal backpressure
- add extra error logging for VpxVideoEncoder
- disable vp9 profiles 1 and 3 (not yet implemented)

And opportunistically mark a number of inputs
in IDL as required
- timestamp (its required for creation of VideoFrame internally)
- error and output callbacks (duh).

Remove ? from duration input dictionary. Its not required,
no need to null.

Bug: 1094166, 1116617

Change-Id: I9577bc42d1f24c2ec14885401e87337ba0937e43
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2355276
Commit-Queue: Chrome Cunningham <chcunningham@chromium.org>
Reviewed-by: Thomas Guilbert <tguilbert@chromium.org>
Cr-Commit-Position: refs/heads/master@{#798473}

media/video/vpx_video_encoder.cc

@@ -65,8 +65,9 @@ void VpxVideoEncoder::Initialize(VideoCodecProfile profile,
   vpx_codec_iface_t* iface = nullptr;
   if (profile == media::VP8PROFILE_ANY) {
     iface = vpx_codec_vp8_cx();
-  } else if (profile >= media::VP9PROFILE_PROFILE0 &&
-             profile <= media::VP9PROFILE_PROFILE3) {
+  } else if (profile == media::VP9PROFILE_PROFILE0 ||
+             profile == media::VP9PROFILE_PROFILE2) {
+    // TODO(https://crbug.com/1116617): Consider support for profiles 1 and 3.
     iface = vpx_codec_vp9_cx();
   } else {
     auto status = Status(StatusCode::kEncoderUnsupportedProfile)
@@ -123,8 +124,9 @@ void VpxVideoEncoder::Initialize(VideoCodecProfile profile,
       codec_, iface, &codec_config_,
       codec_config_.g_bit_depth == VPX_BITS_8 ? 0 : VPX_CODEC_USE_HIGHBITDEPTH);
   if (vpx_error != VPX_CODEC_OK) {
-    std::string msg = base::StringPrintf("VPX encoder initialization error: %s",
-                                         vpx_codec_err_to_string(vpx_error));
+    std::string msg = base::StringPrintf(
+        "VPX encoder initialization error: %s %s",
+        vpx_codec_err_to_string(vpx_error), codec_->err_detail);
 
     status = Status(StatusCode::kEncoderInitializationError, msg);
     std::move(done_cb).Run(status);

third_party/blink/renderer/modules/webcodecs/audio_decoder_init.idl

@@ -5,6 +5,6 @@
 // https://github.com/WICG/web-codecs
 
 dictionary AudioDecoderInit {
-  AudioFrameOutputCallback output;
-  WebCodecsErrorCallback error;
+  required AudioFrameOutputCallback output;
+  required WebCodecsErrorCallback error;
 };

third_party/blink/renderer/modules/webcodecs/video_decoder_init.idl

@@ -5,6 +5,6 @@
 // https://github.com/WICG/web-codecs
 
 dictionary VideoDecoderInit {
-  VideoFrameOutputCallback output;
-  WebCodecsErrorCallback error;
+  required VideoFrameOutputCallback output;
+  required WebCodecsErrorCallback error;
 };

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

@@ -83,6 +83,10 @@ VideoEncoder::~VideoEncoder() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 }
 
+int32_t VideoEncoder::encodeQueueSize() {
+  return requested_encodes_;
+}
+
 void VideoEncoder::configure(const VideoEncoderConfig* config,
                              ExceptionState& exception_state) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
@@ -126,6 +130,7 @@ void VideoEncoder::encode(VideoFrame* frame,
   request->type = Request::Type::kEncode;
   request->frame = frame;
   request->encodeOpts = opts;
+  ++requested_encodes_;
   return EnqueueRequest(request);
 }
 
@@ -178,17 +183,25 @@ void VideoEncoder::CallOutputCallback(EncodedVideoChunk* chunk) {
   output_callback_->InvokeAndReportException(nullptr, chunk);
 }
 
-void VideoEncoder::CallErrorCallback(DOMException* ex) {
-  if (!script_state_->ContextIsValid() || !error_callback_)
+void VideoEncoder::HandleError(DOMException* ex) {
+  // Save a temp before we clear the callback.
+  V8WebCodecsErrorCallback* error_callback = error_callback_.Get();
+
+  // Errors are permanent. Shut everything down.
+  error_callback_.Clear();
+  media_encoder_.reset();
+  output_callback_.Clear();
+
+  if (!script_state_->ContextIsValid() || !error_callback)
     return;
+
   ScriptState::Scope scope(script_state_);
-  error_callback_->InvokeAndReportException(nullptr, ex);
+  error_callback->InvokeAndReportException(nullptr, ex);
 }
 
-void VideoEncoder::CallErrorCallback(DOMExceptionCode code,
-                                     const String& message) {
+void VideoEncoder::HandleError(DOMExceptionCode code, const String& message) {
   auto* ex = MakeGarbageCollected<DOMException>(code, message);
-  CallErrorCallback(ex);
+  HandleError(ex);
 }
 
 void VideoEncoder::EnqueueRequest(Request* request) {
@@ -219,6 +232,7 @@ void VideoEncoder::ProcessRequests() {
 void VideoEncoder::ProcessEncode(Request* request) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK_EQ(request->type, Request::Type::kEncode);
+  DCHECK_GT(requested_encodes_, 0);
 
   auto done_callback = [](VideoEncoder* self, Request* req,
                           media::Status status) {
@@ -227,19 +241,19 @@ void VideoEncoder::ProcessEncode(Request* request) {
     DCHECK_CALLED_ON_VALID_SEQUENCE(self->sequence_checker_);
     if (!status.is_ok()) {
       std::string msg = "Encoding error: " + status.message();
-      self->CallErrorCallback(DOMExceptionCode::kOperationError, msg.c_str());
+      self->HandleError(DOMExceptionCode::kOperationError, msg.c_str());
     }
     self->ProcessRequests();
   };
 
   if (!media_encoder_) {
-    CallErrorCallback(DOMExceptionCode::kOperationError,
-                      "Encoder is not configured");
+    HandleError(DOMExceptionCode::kOperationError, "Encoder is not configured");
     return;
   }
 
   bool keyframe = request->encodeOpts->hasKeyFrameNonNull() &&
                   request->encodeOpts->keyFrameNonNull();
+  --requested_encodes_;
   media_encoder_->Encode(request->frame->frame(), keyframe,
                          WTF::Bind(done_callback, WrapWeakPersistent(this),
                                    WrapPersistentIfNeeded(request)));
@@ -252,9 +266,10 @@ void VideoEncoder::ProcessConfigure(Request* request) {
   auto* config = request->config.Get();
   AccelerationPreference acc_pref = AccelerationPreference::kAllow;
 
+  // TODO(https://crbug.com/1116783): Delete this, allow reconfiguration.
   if (media_encoder_) {
-    CallErrorCallback(DOMExceptionCode::kOperationError,
-                      "Encoder has already been congfigured");
+    HandleError(DOMExceptionCode::kOperationError,
+                "Encoder has already been congfigured");
     return;
   }
 
@@ -267,8 +282,8 @@ void VideoEncoder::ProcessConfigure(Request* request) {
     } else if (preference == "allow") {
       acc_pref = AccelerationPreference::kAllow;
     } else {
-      CallErrorCallback(DOMExceptionCode::kNotFoundError,
-                        "Unknown acceleration type");
+      HandleError(DOMExceptionCode::kNotFoundError,
+                  "Unknown acceleration type");
       return;
     }
   }
@@ -277,7 +292,7 @@ void VideoEncoder::ProcessConfigure(Request* request) {
   std::string profile_str = config->profile().Utf8();
   auto codec_type = media::StringToVideoCodec(codec_str);
   if (codec_type == media::kUnknownVideoCodec) {
-    CallErrorCallback(DOMExceptionCode::kNotFoundError, "Unknown codec type");
+    HandleError(DOMExceptionCode::kNotFoundError, "Unknown codec type");
     return;
   }
 
@@ -286,8 +301,8 @@ void VideoEncoder::ProcessConfigure(Request* request) {
   media::VideoCodecProfile profile = media::VIDEO_CODEC_PROFILE_UNKNOWN;
   if (codec_type == media::kCodecVP8) {
     if (acc_pref == AccelerationPreference::kRequire) {
-      CallErrorCallback(DOMExceptionCode::kNotFoundError,
-                        "Accelerated vp8 is not supported");
+      HandleError(DOMExceptionCode::kNotFoundError,
+                  "Accelerated vp8 is not supported");
       return;
     }
     media_encoder_ = CreateVpxVideoEncoder();
@@ -296,13 +311,12 @@ void VideoEncoder::ProcessConfigure(Request* request) {
     uint8_t level = 0;
     media::VideoColorSpace color_space;
     if (!ParseNewStyleVp9CodecID(profile_str, &profile, &level, &color_space)) {
-      CallErrorCallback(DOMExceptionCode::kNotFoundError,
-                        "Invalid vp9 profile");
+      HandleError(DOMExceptionCode::kNotFoundError, "Invalid vp9 codec string");
       return;
     }
     if (acc_pref == AccelerationPreference::kRequire) {
-      CallErrorCallback(DOMExceptionCode::kNotFoundError,
-                        "Accelerated vp9 is not supported");
+      HandleError(DOMExceptionCode::kNotFoundError,
+                  "Accelerated vp9 is not supported");
       return;
     }
     media_encoder_ = CreateVpxVideoEncoder();
@@ -310,21 +324,19 @@ void VideoEncoder::ProcessConfigure(Request* request) {
     codec_type = media::kCodecH264;
     uint8_t level = 0;
     if (!ParseAVCCodecId(profile_str, &profile, &level)) {
-      CallErrorCallback(DOMExceptionCode::kNotFoundError,
-                        "Invalid AVC profile");
+      HandleError(DOMExceptionCode::kNotFoundError, "Invalid AVC profile");
       return;
     }
     if (acc_pref == AccelerationPreference::kDeny) {
-      CallErrorCallback(DOMExceptionCode::kNotFoundError,
-                        "Software h264 is not supported yet");
+      HandleError(DOMExceptionCode::kNotFoundError,
+                  "Software h264 is not supported yet");
       return;
     }
     media_encoder_ = CreateAcceleratedVideoEncoder();
   }
 
   if (!media_encoder_) {
-    CallErrorCallback(DOMExceptionCode::kNotFoundError,
-                      "Unsupported codec type");
+    HandleError(DOMExceptionCode::kNotFoundError, "Unsupported codec type");
     return;
   }
 
@@ -339,22 +351,28 @@ void VideoEncoder::ProcessConfigure(Request* request) {
       return;
     DCHECK_CALLED_ON_VALID_SEQUENCE(self->sequence_checker_);
     if (!status.is_ok()) {
-      self->media_encoder_.reset();
-      self->output_callback_.Clear();
-      self->error_callback_.Clear();
       std::string msg = "Encoder initialization error: " + status.message();
-      self->CallErrorCallback(DOMExceptionCode::kOperationError, msg.c_str());
+      self->HandleError(DOMExceptionCode::kOperationError, msg.c_str());
     }
     self->stall_request_processing_ = false;
     self->ProcessRequests();
   };
 
   media::VideoEncoder::Options options;
-  options.bitrate = config->bitrate();
+
+  // Required configuration.
   options.height = frame_size_.height();
   options.width = frame_size_.width();
   options.framerate = config->framerate();
+
+  // Optional configuration.
+  if (config->hasBitrate())
+    options.bitrate = config->bitrate();
+
+  // TODO(https://crbug.com/1116771): Let the encoder figure out its thread
+  // count (it knows better).
   options.threads = 1;
+
   stall_request_processing_ = true;
   media_encoder_->Initialize(profile, options, output_cb,
                              WTF::Bind(done_callback, WrapWeakPersistent(this),
@@ -377,7 +395,7 @@ void VideoEncoder::ProcessFlush(Request* request) {
       std::string msg = "Flushing error: " + status.message();
       auto* ex = MakeGarbageCollected<DOMException>(
           DOMExceptionCode::kOperationError, msg.c_str());
-      self->CallErrorCallback(ex);
+      self->HandleError(ex);
       req->resolver.Release()->Reject(ex);
     }
     self->stall_request_processing_ = false;
@@ -387,7 +405,7 @@ void VideoEncoder::ProcessFlush(Request* request) {
   if (!media_encoder_) {
     auto* ex = MakeGarbageCollected<DOMException>(
         DOMExceptionCode::kOperationError, "Encoder is not configured");
-    CallErrorCallback(ex);
+    HandleError(ex);
     request->resolver.Release()->Reject(ex);
     return;
   }

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

@@ -41,6 +41,8 @@ class MODULES_EXPORT VideoEncoder final : public ScriptWrappable {
   ~VideoEncoder() override;
 
   // video_encoder.idl implementation.
+  int32_t encodeQueueSize();
+
   void encode(VideoFrame* frame,
               const VideoEncoderEncodeOptions*,
               ExceptionState&);
@@ -76,8 +78,8 @@ class MODULES_EXPORT VideoEncoder final : public ScriptWrappable {
   enum class AccelerationPreference { kAllow, kDeny, kRequire };
 
   void CallOutputCallback(EncodedVideoChunk* chunk);
-  void CallErrorCallback(DOMException* ex);
-  void CallErrorCallback(DOMExceptionCode code, const String& message);
+  void HandleError(DOMException* ex);
+  void HandleError(DOMExceptionCode code, const String& message);
   void EnqueueRequest(Request* request);
   void ProcessRequests();
   void ProcessEncode(Request* request);
@@ -93,6 +95,7 @@ class MODULES_EXPORT VideoEncoder final : public ScriptWrappable {
   Member<V8VideoEncoderOutputCallback> output_callback_;
   Member<V8WebCodecsErrorCallback> error_callback_;
   HeapDeque<Member<Request>> requests_;
+  int32_t requested_encodes_ = 0;
 
   // Some kConfigure and kFlush requests can't be executed in parallel with
   // kEncode. This flag stops processing of new requests in the requests_ queue

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

@@ -11,6 +11,13 @@
     [CallWith=ScriptState, RaisesException]
     constructor(VideoEncoderInit init);
 
+    // The number of pending decode requests. This does not include requests that
+    // have been sent to the underlying codec.
+    //
+    // Applications can minimize underflow by enqueueing encode requests until
+    // |encodeQueueSize| is greater than a constant.
+    readonly attribute long encodeQueueSize;
+
     // Performs original configuration of the encoder.
     // Resolved after configuration is done. It should be called only
     // once per encoder instance, before calling any other methods.

third_party/blink/renderer/modules/webcodecs/video_encoder_config.idl

@@ -14,11 +14,11 @@ enum VideoEncoderAccelerationPreference {
 dictionary VideoEncoderConfig {
   required DOMString codec;
   DOMString profile;
-  VideoEncoderAccelerationPreference acceleration;
+  VideoEncoderAccelerationPreference acceleration = "allow";
 
   unsigned long long bitrate;
 
-  double framerate;
+  required double framerate;
 
   required unsigned long width;
   required unsigned long height;

third_party/blink/renderer/modules/webcodecs/video_encoder_init.idl

@@ -9,5 +9,5 @@ dictionary VideoEncoderInit {
   required VideoEncoderOutputCallback output;
 
   // Called when there is a decoding error.
-  WebCodecsErrorCallback error;
+  required WebCodecsErrorCallback error;
 };
\ No newline at end of file

third_party/blink/renderer/modules/webcodecs/video_frame_init.idl

@@ -5,6 +5,6 @@
 // https://github.com/WICG/web-codecs
 
 dictionary VideoFrameInit {
-  unsigned long long timestamp;  // microseconds
-  unsigned long long? duration;  // microseconds
+  required unsigned long long timestamp;  // microseconds
+  unsigned long long duration;  // microseconds
 };
\ No newline at end of file

third_party/blink/web_tests/external/wpt/webcodecs/video-encoder.html

+<!DOCTYPE html>
+<html>
+<title>Test the VideoTrackReader API.</title>
+<body>
+  <img id='frame_image' src="pattern.png">
+</body>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="/common/media.js"></script>
+<script>
+
+async function generateBitmap(width, height) {
+  return createImageBitmap(document.getElementById('frame_image'),
+                                  { resizeWidth: width,
+                                    resizeHeight: height });
+}
+
+async function createVideoFrame(width, height, timestamp) {
+  let bitmap = await generateBitmap(width, height);
+  return new VideoFrame(bitmap, { timestamp: timestamp });
+}
+
+// Calls done after giving async output/error callbacks a final chance to run.
+async function asyncDone(test) {
+  test.step_timeout(test.done.bind(test), 0);
+}
+
+async_test(async (t) => {
+  // VideoEncoderInit lacks required fields.
+  assert_throws_js(TypeError, () => { new VideoEncoder({}); });
+
+  // VideoEncoderInit has required fields.
+  let encoder = new VideoEncoder({
+    output(chunk) { assert_unreached("Unexpected output");  },
+    error(error) { assert_unreached("Unexpected error:" + error); },
+  });
+  encoder.close();
+
+  asyncDone(t);
+}, 'Test VideoEncoder construction');
+
+async_test(async (t) => {
+  let encoder = new VideoEncoder({
+    output(chunk) { assert_unreached("Unexpected output");  },
+    error(error) { assert_unreached("Unexpected error:" + error); },
+  });
+
+  const requiredConfigPairs = {
+    codec: 'vp8',
+    framerate: 25,
+    width: 640,
+    height: 480
+  };
+  let incrementalConfig = {};
+
+  for (let key in requiredConfigPairs) {
+    // Configure  should fail while required keys are missing.
+    assert_throws_js(TypeError, () => { encoder.configure(incrementalConfig); });
+    incrementalConfig[key] = requiredConfigPairs[key];
+  }
+
+  // Configure should pass once incrementalConfig meets all requirements.
+  encoder.configure(incrementalConfig);
+
+  encoder.configure(incrementalConfig);
+
+  encoder.close();
+
+  asyncDone(t);
+}, 'Test VideoEncoder.configure()');
+
+async_test(async (t) => {
+  let encoder = new VideoEncoder({
+    output(chunk) { assert_unreached("Unexpected output"); },
+    error(error) { assert_unreached("Unexpected error:" + error); },
+  });
+
+  let videoFrame = await createVideoFrame(640, 480, 0);
+
+  assert_throws_dom('InvalidStateError',
+                    () => { encoder.encode(videoFrame); },
+                    'first encode');
+
+  // Once more for good measure.
+  assert_throws_dom('InvalidStateError',
+                    () => { encoder.encode(videoFrame); },
+                    'second encode');
+
+  encoder.close();
+
+  asyncDone(t);
+}, 'Test encode() before configure() throws InvalidStateError.');
+
+async_test(async (t) => {
+  let output_chunks = [];
+  let encoder = new VideoEncoder({
+    output(chunk) { output_chunks.push(chunk); },
+    error(error) { assert_unreached("Unexpected error:" + error); },
+  });
+
+  // No encodes yet.
+  assert_equals(encoder.encodeQueueSize, 0);
+
+  const config = {
+    codec: 'vp8',
+    framerate: 25,
+    width: 640,
+    height: 480
+  };
+  encoder.configure(config);
+
+  // Still no encodes.
+  assert_equals(encoder.encodeQueueSize, 0);
+
+  let frame1 = await createVideoFrame(640, 480, 0);
+  let frame2 = await createVideoFrame(640, 480, 33333);
+
+  encoder.encode(frame1);
+  encoder.encode(frame2);
+
+  // Could be 0, 1, or 2. We can't guarantee this check runs before the UA has
+  // processed the encodes.
+  assert_true(encoder.encodeQueueSize >= 0 && encoder.encodeQueueSize <= 2)
+
+  await encoder.flush();
+
+  // We can guarantee that all encodes are processed after a flush.
+  assert_equals(encoder.encodeQueueSize, 0);
+
+  assert_equals(output_chunks.length, 2);
+  assert_equals(output_chunks[0].timestamp, frame1.timestamp);
+  assert_equals(output_chunks[1].timestamp, frame2.timestamp);
+
+  encoder.close();
+
+  asyncDone(t);
+}, 'Test successful configure(), encode(), and flush()');
+</script>
+</html>

third_party/blink/web_tests/webexposed/global-interface-listing-expected.txt

@@ -8787,6 +8787,7 @@ interface VideoDecoder
     method reset
 interface VideoEncoder
     attribute @@toStringTag
+    getter encodeQueueSize
     method close
     method configure
     method constructor