[webcodecs] Cleanups for VideoDecoder.

  - Changes initialize() to configure() and adds documentation to IDL.
  - Replaces VideoDecoderInitParams with EncodedVideoConfig.
  - Miscellaneous member renames and comments added.

Change-Id: Ie413e560c7dc5c250f964a0dc0856cbb73d363d8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2043001
Commit-Queue: Dan Sanders <sandersd@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Chrome Cunningham <chcunningham@chromium.org>
Cr-Commit-Position: refs/heads/master@{#742930}

third_party/blink/renderer/bindings/idl_in_modules.gni

@@ -707,8 +707,8 @@ static_idl_files_in_modules = get_path_info(
           "//third_party/blink/renderer/modules/webaudio/wave_shaper_node.idl",
           "//third_party/blink/renderer/modules/webaudio/wave_shaper_options.idl",
           "//third_party/blink/renderer/modules/webcodecs/encoded_video_chunk.idl",
+          "//third_party/blink/renderer/modules/webcodecs/encoded_video_config.idl",
           "//third_party/blink/renderer/modules/webcodecs/video_decoder.idl",
-          "//third_party/blink/renderer/modules/webcodecs/video_decoder_init_parameters.idl",
           "//third_party/blink/renderer/modules/webcodecs/video_frame.idl",
           "//third_party/blink/renderer/modules/webcodecs/video_track_reader.idl",
           "//third_party/blink/renderer/modules/webcodecs/video_track_writer.idl",

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

+// Copyright 2020 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.
+
+// https://github.com/WICG/web-codecs
+
+dictionary EncodedVideoConfig {
+  // Codec string, eg. "avc1.42001e" or "vp09.00.10.08".
+  // TODO(sandersd): Accept "avc1" if |description| is provided?
+  required DOMString codec;
+
+  // avcC, vpcC, or etc.
+  BufferSource description;
+
+  // If provided, these override in-band configuration.
+  double sampleAspect;
+  // TODO(sandersd): color space.
+
+  // TODO(sandersd): Constraints (sequential access) and requirements
+  // (imagebitmap, colorspace conversion).
+};

third_party/blink/renderer/modules/webcodecs/idls.gni

@@ -11,6 +11,14 @@ modules_idl_files = [
 ]
 
 modules_dictionary_idl_files = [
-  "video_decoder_init_parameters.idl",
+  "encoded_video_config.idl",
   "video_track_writer_parameters.idl",
 ]
+
+# IDL files that either define partial interfaces or target (right side of)
+# `includes`.
+modules_dependency_idl_files = []
+
+# Similar to |modules_dependency_idl_files| but limited to things that are
+# exposed only for testing.
+modules_testing_dependency_idl_files = []

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

@@ -16,8 +16,9 @@
 #include "third_party/blink/renderer/core/streams/underlying_source_base.h"
 #include "third_party/blink/renderer/core/streams/writable_stream.h"
 #include "third_party/blink/renderer/modules/webcodecs/encoded_video_chunk.h"
-#include "third_party/blink/renderer/modules/webcodecs/video_decoder_init_parameters.h"
+#include "third_party/blink/renderer/modules/webcodecs/encoded_video_config.h"
 #include "third_party/blink/renderer/modules/webcodecs/video_frame.h"
+#include "third_party/blink/renderer/platform/bindings/exception_code.h"
 #include "third_party/blink/renderer/platform/bindings/exception_state.h"
 #include "third_party/blink/renderer/platform/bindings/script_state.h"
 #include "third_party/blink/renderer/platform/wtf/functional.h"
@@ -29,11 +30,11 @@ namespace blink {
 namespace {
 
 // TODO(sandersd): Tune this number.
-// Desired number of chunks queued in |writable_sink_|.
+// Desired number of chunks queued in |underlying_sink_|.
 constexpr size_t kDesiredInputQueueSize = 4;
 
 // TODO(sandersd): Tune this number.
-// Desired number of pending decodes + chunks queued in |readable_source_|.
+// Desired number of pending decodes + chunks queued in |underlying_source_|.
 constexpr size_t kDesiredInternalQueueSize = 4;
 
 std::unique_ptr<media::VideoDecoder> CreateVideoDecoder(
@@ -43,9 +44,9 @@ std::unique_ptr<media::VideoDecoder> CreateVideoDecoder(
 
 }  // namespace
 
-class VideoDecoder::WritableSink final : public UnderlyingSinkBase {
+class VideoDecoder::UnderlyingSink final : public UnderlyingSinkBase {
  public:
-  explicit WritableSink(VideoDecoder* parent) : parent_(parent) {}
+  explicit UnderlyingSink(VideoDecoder* parent) : parent_(parent) {}
 
   // UnderlyingSinkBase overrides.
   ScriptPromise start(ScriptState*,
@@ -81,9 +82,9 @@ class VideoDecoder::WritableSink final : public UnderlyingSinkBase {
   Member<VideoDecoder> parent_;
 };
 
-class VideoDecoder::ReadableSource final : public UnderlyingSourceBase {
+class VideoDecoder::UnderlyingSource final : public UnderlyingSourceBase {
  public:
-  ReadableSource(ScriptState* script_state, VideoDecoder* parent)
+  UnderlyingSource(ScriptState* script_state, VideoDecoder* parent)
       : UnderlyingSourceBase(script_state), parent_(parent) {}
 
   // UnderlyingSourceBase overrides.
@@ -104,29 +105,21 @@ class VideoDecoder::ReadableSource final : public UnderlyingSourceBase {
 };
 
 // static
-VideoDecoder* VideoDecoder::Create(ScriptState* script_state,
-                                   ExceptionState& exception_state) {
-  return MakeGarbageCollected<VideoDecoder>(script_state, exception_state);
+VideoDecoder* VideoDecoder::Create(ScriptState* script_state) {
+  return MakeGarbageCollected<VideoDecoder>(script_state);
 }
 
-VideoDecoder::VideoDecoder(ScriptState* script_state,
-                           ExceptionState& exception_state)
+VideoDecoder::VideoDecoder(ScriptState* script_state)
     : script_state_(script_state), weak_factory_(this) {
   DVLOG(1) << __func__;
   weak_this_ = weak_factory_.GetWeakPtr();
-
-  WritableSink* writable_sink = MakeGarbageCollected<WritableSink>(this);
-  writable_ = WritableStream::CreateWithCountQueueingStrategy(
-      script_state, writable_sink, kDesiredInputQueueSize);
-
-  readable_source_ = MakeGarbageCollected<ReadableSource>(script_state_, this);
-  readable_ = ReadableStream::CreateWithCountQueueingStrategy(
-      script_state_, readable_source_, kDesiredInternalQueueSize);
 }
 
 VideoDecoder::~VideoDecoder() {
   DVLOG(1) << __func__;
-  // TODO(sandersd): Should we reject outstanding promises?
+  // TODO(sandersd): Assert no outstanding promises. We should have been kept
+  // alive.
+  // TODO(sandersd): GC tests.
 }
 
 ScriptPromise VideoDecoder::CreateWritePromise() {
@@ -147,9 +140,9 @@ void VideoDecoder::MaybeAcceptWrite() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK(!has_error_);
 
-  if (write_resolver_ && initialized_ &&
+  if (write_resolver_ && decoder_ && underlying_source_ &&
       pending_decodes_ < decoder_->GetMaxDecodeRequests() &&
-      pending_decodes_ < readable_source_->Controller()->DesiredSize()) {
+      pending_decodes_ < underlying_source_->Controller()->DesiredSize()) {
     write_resolver_.Release()->Resolve();
   }
 }
@@ -163,25 +156,14 @@ void VideoDecoder::HandleError() {
   // etc.
 }
 
-ReadableStream* VideoDecoder::readable() const {
-  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  return readable_;
-}
-
-WritableStream* VideoDecoder::writable() const {
-  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  return writable_;
-}
-
-ScriptPromise VideoDecoder::initialize(const VideoDecoderInitParameters* params,
-                                       ExceptionState& exception_state) {
+ScriptPromise VideoDecoder::configure(const EncodedVideoConfig* config,
+                                      ExceptionState& exception_state) {
   DVLOG(1) << __func__;
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
-  if (decoder_) {
-    // TODO(sandersd): Reinitialization.
+  if (configure_resolver_ || decoder_) {
     exception_state.ThrowDOMException(DOMExceptionCode::kNotSupportedError,
-                                      "Not implemented yet.");
+                                      "Reconfiguration is not implemented.");
     return ScriptPromise();
   }
 
@@ -193,14 +175,13 @@ ScriptPromise VideoDecoder::initialize(const VideoDecoderInitParameters* params,
   }
 
   // VideoDecoder::Initialize() may call OnInitializeDone() reentrantly, in
-  // which case |initialize_resolver_| will be nullptr.
-  DCHECK(!initialize_resolver_);
-  ScriptPromiseResolver* initialize_resolver =
+  // which case |configure_resolver_| will be nullptr.
+  DCHECK(!configure_resolver_);
+  ScriptPromiseResolver* configure_resolver =
       MakeGarbageCollected<ScriptPromiseResolver>(script_state_);
-  initialize_resolver_ = initialize_resolver;
+  configure_resolver_ = configure_resolver;
 
-  // TODO(sandersd): Convert |params| to VideoDecoderConfig.
-  // TODO(sandersd): Support |waiting_cb|.
+  // TODO(sandersd): Convert |config| to VideoDecoderConfig.
   decoder_->Initialize(
       media::VideoDecoderConfig(
           media::kCodecH264, media::H264PROFILE_BASELINE,
@@ -212,7 +193,17 @@ ScriptPromise VideoDecoder::initialize(const VideoDecoderInitParameters* params,
       WTF::BindRepeating(&VideoDecoder::OnOutput, weak_this_),
       base::RepeatingCallback<void(media::WaitingReason)>());
 
-  return initialize_resolver->Promise();
+  return configure_resolver->Promise();
+}
+
+ReadableStream* VideoDecoder::readable() const {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  return readable_;
+}
+
+WritableStream* VideoDecoder::writable() const {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  return writable_;
 }
 
 void VideoDecoder::OnInitializeDone(bool success) {
@@ -220,14 +211,22 @@ void VideoDecoder::OnInitializeDone(bool success) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
   if (!success) {
-    initialize_resolver_.Release()->Reject(MakeGarbageCollected<DOMException>(
-        DOMExceptionCode::kNotSupportedError, "Initialization failed."));
+    configure_resolver_.Release()->Reject(MakeGarbageCollected<DOMException>(
+        DOMExceptionCode::kNotSupportedError, "Codec initialization failed."));
     HandleError();
     return;
   }
 
-  initialized_ = true;
-  initialize_resolver_.Release()->Resolve();
+  underlying_sink_ = MakeGarbageCollected<UnderlyingSink>(this);
+  writable_ = WritableStream::CreateWithCountQueueingStrategy(
+      script_state_, underlying_sink_, kDesiredInputQueueSize);
+
+  underlying_source_ =
+      MakeGarbageCollected<UnderlyingSource>(script_state_, this);
+  readable_ = ReadableStream::CreateWithCountQueueingStrategy(
+      script_state_, underlying_source_, kDesiredInternalQueueSize);
+
+  configure_resolver_.Release()->Resolve();
   MaybeAcceptWrite();
 }
 
@@ -249,7 +248,7 @@ void VideoDecoder::OnOutput(scoped_refptr<media::VideoFrame> frame) {
   DVLOG(3) << __func__;
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
-  readable_source_->Controller()->Enqueue(ScriptValue::From(
+  underlying_source_->Controller()->Enqueue(ScriptValue::From(
       script_state_, MakeGarbageCollected<VideoFrame>(frame)));
 }
 
@@ -298,7 +297,8 @@ ScriptPromise VideoDecoder::Write(ScriptValue chunk,
 ScriptPromise VideoDecoder::Close(ExceptionState& exception_state) {
   DVLOG(2) << __func__;
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  // TODO(sandersd): Flush.
+
+  // TODO(sandersd): This probably should be an implicit flush.
   exception_state.ThrowDOMException(DOMExceptionCode::kNotSupportedError,
                                     "Not implemented yet.");
   return ScriptPromise();
@@ -307,7 +307,8 @@ ScriptPromise VideoDecoder::Close(ExceptionState& exception_state) {
 ScriptPromise VideoDecoder::Abort(ExceptionState& exception_state) {
   DVLOG(2) << __func__;
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  // TODO(sandersd): Reset.
+
+  // TODO(sandersd): Should this result in a reset?
   exception_state.ThrowDOMException(DOMExceptionCode::kNotSupportedError,
                                     "Not implemented yet.");
   return ScriptPromise();
@@ -317,7 +318,11 @@ ScriptPromise VideoDecoder::Pull() {
   DVLOG(2) << __func__;
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
+  // TODO(sandersd): Only potentially possible if |underlying_source_| was
+  // previously full.
   MaybeAcceptWrite();
+
+  // TODO(sandersd): No implementation holds a pull promise. Should we?
   return ScriptPromise::CastUndefined(script_state_);
 }
 
@@ -333,10 +338,11 @@ ScriptPromise VideoDecoder::Cancel() {
 
 void VideoDecoder::Trace(Visitor* visitor) {
   visitor->Trace(script_state_);
-  visitor->Trace(readable_source_);
-  visitor->Trace(readable_);
+  visitor->Trace(underlying_sink_);
   visitor->Trace(writable_);
-  visitor->Trace(initialize_resolver_);
+  visitor->Trace(underlying_source_);
+  visitor->Trace(readable_);
+  visitor->Trace(configure_resolver_);
   visitor->Trace(write_resolver_);
   ScriptWrappable::Trace(visitor);
 }

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

@@ -17,25 +17,24 @@
 
 namespace blink {
 
-class ExceptionState;
-class ScriptState;
+class EncodedVideoConfig;
 class ReadableStream;
-class VideoDecoderInitParameters;
 class WritableStream;
-class Visitor;
 
 class MODULES_EXPORT VideoDecoder final : public ScriptWrappable {
   DEFINE_WRAPPERTYPEINFO();
 
  public:
-  static VideoDecoder* Create(ScriptState*, ExceptionState&);
-  VideoDecoder(ScriptState*, ExceptionState&);
+  static VideoDecoder* Create(ScriptState*);
+
+  VideoDecoder(ScriptState*);
   ~VideoDecoder() override;
 
   // video_decoder.idl implementation.
+  ScriptPromise configure(const EncodedVideoConfig* config,
+                          ExceptionState& exception_state);
   ReadableStream* readable() const;
   WritableStream* writable() const;
-  ScriptPromise initialize(const VideoDecoderInitParameters*, ExceptionState&);
 
   // GarbageCollected override.
   void Trace(Visitor*) override;
@@ -43,9 +42,8 @@ class MODULES_EXPORT VideoDecoder final : public ScriptWrappable {
  private:
   SEQUENCE_CHECKER(sequence_checker_);
 
-  // Helper classes that manage the input and output streams.
-  class WritableSink;
-  class ReadableSource;
+  class UnderlyingSink;
+  class UnderlyingSource;
 
   // Creates a new |write_resolver_| and returns a promise from it.
   ScriptPromise CreateWritePromise();
@@ -61,30 +59,31 @@ class MODULES_EXPORT VideoDecoder final : public ScriptWrappable {
   void OnDecodeDone(media::DecodeStatus);
   void OnOutput(scoped_refptr<media::VideoFrame>);
 
-  // Called by WritableSink.
+  // Called by UnderlyingSink.
   ScriptPromise Start(ExceptionState&);
   ScriptPromise Write(ScriptValue chunk, ExceptionState&);
   ScriptPromise Close(ExceptionState&);
   ScriptPromise Abort(ExceptionState&);
 
-  // Called by ReadableSource.
+  // Called by UnderlyingSource.
   ScriptPromise Pull();
   ScriptPromise Cancel();
 
   Member<ScriptState> script_state_;
+
+  Member<UnderlyingSink> underlying_sink_;
   Member<WritableStream> writable_;
+  Member<UnderlyingSource> underlying_source_;
   Member<ReadableStream> readable_;
-  Member<ReadableSource> readable_source_;
 
-  // Signals completion of initialize().
-  Member<ScriptPromiseResolver> initialize_resolver_;
+  // Signals completion of configure().
+  Member<ScriptPromiseResolver> configure_resolver_;
 
   // Signals ability to accept an input chunk.
   Member<ScriptPromiseResolver> write_resolver_;
 
   std::unique_ptr<media::VideoDecoder> decoder_;
   bool has_error_ = false;
-  bool initialized_ = false;
   int pending_decodes_ = 0;
 
   base::WeakPtr<VideoDecoder> weak_this_;

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

@@ -2,12 +2,25 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-[
-    RuntimeEnabled=WebCodecs
-] interface VideoDecoder {
-    [CallWith=ScriptState, RaisesException] constructor();
-    [RaisesException] Promise<void> initialize(VideoDecoderInitParameters params);
-
-    readonly attribute WritableStream writable;  // of EncodedVideoChunk
-    readonly attribute ReadableStream readable;  // of VideoFrame
+// https://github.com/WICG/web-codecs
+
+[RuntimeEnabled=WebCodecs]
+interface VideoDecoder {
+  // TODO(sandersd): Configure properties of streams (type and high water mark).
+  [CallWith=ScriptState] constructor();
+
+  // Configure or reconfigure the decoder.
+  //
+  // |writable| and |readable| will be errored and set to null.
+  //
+  // If configuration is successful, |writable| and |readable| will be set to
+  // new streams.
+  [RaisesException] Promise<void> configure(EncodedVideoConfig config);
+
+  // Closing |writable| will flush the decoder, then close |readable|.
+  // Aborting |writable| will error |readable|.
+  readonly attribute WritableStream? writable;  // of EncodedVideoChunk
+
+  // Canceling |readable| will error |writable|.
+  readonly attribute ReadableStream? readable;  // of VideoFrame
 };

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

-// Copyright 2019 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.
-
-dictionary VideoDecoderInitParameters {
-  DOMString codec;
-  DOMString profile;
-
-  // These are the "coded size", not the "visible size"
-  unsigned long width;
-  unsigned long height;
-};
\ No newline at end of file

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

@@ -8653,8 +8653,8 @@ interface VideoDecoder
     attribute @@toStringTag
     getter readable
     getter writable
+    method configure
     method constructor
-    method initialize
 interface VideoFrame
     attribute @@toStringTag
     getter coded_height