Add VideoEncoder logs to DevTools

This CL adds a MediaLog to VideoEncoder and uses it to surface basic
properties. For now, it uses the VideoDecoder MediaLog properties to
surface encoder name, because changes to the Devtools code would and
the creation of new properties would be required to surface it as
an encoder.

Bug: 1148576
Change-Id: Ib9b0188421b08071672f49aead4cc87bd7d55d69
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2535561Reviewed-by: Eugene Zemtsov <eugene@chromium.org>
Commit-Queue: Thomas Guilbert <tguilbert@chromium.org>
Cr-Commit-Position: refs/heads/master@{#827508}

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

@@ -12,6 +12,7 @@
 #include "base/macros.h"
 #include "build/build_config.h"
 #include "media/base/async_destroy_video_encoder.h"
+#include "media/base/media_util.h"
 #include "media/base/mime_util.h"
 #include "media/base/offloading_video_encoder.h"
 #include "media/base/video_codecs.h"
@@ -36,6 +37,7 @@
 #include "third_party/blink/renderer/bindings/modules/v8/v8_video_encoder_encode_options.h"
 #include "third_party/blink/renderer/bindings/modules/v8/v8_video_encoder_init.h"
 #include "third_party/blink/renderer/core/dom/dom_exception.h"
+#include "third_party/blink/renderer/core/inspector/inspector_media_context_impl.h"
 #include "third_party/blink/renderer/core/streams/readable_stream.h"
 #include "third_party/blink/renderer/core/streams/writable_stream.h"
 #include "third_party/blink/renderer/modules/webcodecs/codec_state_helper.h"
@@ -162,10 +164,32 @@ VideoEncoder* VideoEncoder::Create(ScriptState* script_state,
 VideoEncoder::VideoEncoder(ScriptState* script_state,
                            const VideoEncoderInit* init,
                            ExceptionState& exception_state)
-    : state_(V8CodecState::Enum::kUnconfigured), script_state_(script_state) {
+    : ExecutionContextLifecycleObserver(ExecutionContext::From(script_state)),
+      state_(V8CodecState::Enum::kUnconfigured),
+      script_state_(script_state) {
   UseCounter::Count(ExecutionContext::From(script_state),
                     WebFeature::kWebCodecs);
 
+  ExecutionContext* context = GetExecutionContext();
+
+  DCHECK(context);
+
+  parent_media_log_ = Platform::Current()->GetMediaLog(
+      MediaInspectorContextImpl::From(*context),
+      Thread::MainThread()->GetTaskRunner());
+
+  if (!parent_media_log_)
+    parent_media_log_ = std::make_unique<media::NullMediaLog>();
+
+  // This allows us to destroy |parent_media_log_| and stop logging,
+  // without causing problems to |media_log_| users.
+  media_log_ = parent_media_log_->Clone();
+
+  media_log_->SetProperty<media::MediaLogProperty::kFrameTitle>(
+      std::string("VideoEncoder(WebCodecs)"));
+  media_log_->SetProperty<media::MediaLogProperty::kFrameUrl>(
+      GetExecutionContext()->Url().GetString().Ascii());
+
   output_callback_ = init->output();
   if (init->hasError())
     error_callback_ = init->error();
@@ -284,17 +308,33 @@ bool VideoEncoder::VerifyCodecSupport(ParsedConfig* config,
   return true;
 }
 
+void VideoEncoder::UpdateEncoderLog(std::string encoder_name,
+                                    bool is_hw_accelerated) {
+  // TODO(https://crbug.com/1139089) : Add encoder properties.
+  media_log_->SetProperty<media::MediaLogProperty::kVideoDecoderName>(
+      encoder_name);
+  media_log_->SetProperty<media::MediaLogProperty::kIsPlatformVideoDecoder>(
+      is_hw_accelerated);
+}
+
 std::unique_ptr<media::VideoEncoder> VideoEncoder::CreateMediaVideoEncoder(
     const ParsedConfig& config) {
   // TODO(https://crbug.com/1119636): Implement / call a proper method for
   // detecting support of encoder configs.
   switch (config.acc_pref) {
-    case AccelerationPreference::kRequire:
-      return CreateAcceleratedVideoEncoder(config.profile, config.options);
+    case AccelerationPreference::kRequire: {
+      auto result =
+          CreateAcceleratedVideoEncoder(config.profile, config.options);
+      if (result)
+        UpdateEncoderLog("AcceleratedVideoEncoder", true);
+      return result;
+    }
     case AccelerationPreference::kAllow:
       if (auto result =
-              CreateAcceleratedVideoEncoder(config.profile, config.options))
+              CreateAcceleratedVideoEncoder(config.profile, config.options)) {
+        UpdateEncoderLog("AcceleratedVideoEncoder", true);
         return result;
+      }
       FALLTHROUGH;
     case AccelerationPreference::kDeny: {
       std::unique_ptr<media::VideoEncoder> result;
@@ -302,9 +342,11 @@ std::unique_ptr<media::VideoEncoder> VideoEncoder::CreateMediaVideoEncoder(
         case media::kCodecVP8:
         case media::kCodecVP9:
           result = CreateVpxVideoEncoder();
+          UpdateEncoderLog("VpxVideoEncoder", false);
           break;
         case media::kCodecH264:
           result = CreateOpenH264VideoEncoder();
+          UpdateEncoderLog("OpenH264VideoEncoder", false);
           break;
         default:
           return nullptr;
@@ -475,8 +517,13 @@ void VideoEncoder::HandleError(DOMException* ex) {
   error_callback->InvokeAndReportException(nullptr, ex);
 }
 
-void VideoEncoder::HandleError(DOMExceptionCode code, const String& message) {
-  auto* ex = MakeGarbageCollected<DOMException>(code, message);
+void VideoEncoder::HandleError(std::string error_message,
+                               media::Status status) {
+  media_log_->NotifyError(status);
+
+  // For now, the only uses of this method correspond to kOperationErrors.
+  auto* ex = MakeGarbageCollected<DOMException>(
+      DOMExceptionCode::kOperationError, error_message.c_str());
   HandleError(ex);
 }
 
@@ -518,8 +565,7 @@ void VideoEncoder::ProcessEncode(Request* request) {
       return;
     DCHECK_CALLED_ON_VALID_SEQUENCE(self->sequence_checker_);
     if (!status.is_ok()) {
-      std::string msg = "Encoding error: " + status.message();
-      self->HandleError(DOMExceptionCode::kOperationError, msg.c_str());
+      self->HandleError("Encoding error.", status);
     }
     self->ProcessRequests();
   };
@@ -528,7 +574,9 @@ void VideoEncoder::ProcessEncode(Request* request) {
   if (frame->storage_type() == media::VideoFrame::STORAGE_GPU_MEMORY_BUFFER) {
     frame = ConvertToI420Frame(frame);
     if (!frame) {
-      HandleError(DOMExceptionCode::kOperationError, "Unexpected frame format");
+      HandleError("Unexpected frame format.",
+                  media::Status(media::StatusCode::kEncoderFailedEncode,
+                                "Unexpected frame format"));
       return;
     }
   }
@@ -553,9 +601,11 @@ void VideoEncoder::ProcessConfigure(Request* request) {
 
   media_encoder_ = CreateMediaVideoEncoder(*active_config_);
   if (!media_encoder_) {
-    HandleError(DOMExceptionCode::kOperationError,
-                "Encoder creation error. Most likely unsupported "
-                "codec/acceleration requirement combination.");
+    HandleError(
+        "Encoder creation error.",
+        media::Status(media::StatusCode::kEncoderInitializationError,
+                      "Unable to create encoder (most likely unsupported "
+                      "codec/acceleration requirement combination)"));
     return;
   }
 
@@ -570,8 +620,7 @@ void VideoEncoder::ProcessConfigure(Request* request) {
     DCHECK(self->active_config_);
 
     if (!status.is_ok()) {
-      std::string msg = "Encoder initialization error: " + status.message();
-      self->HandleError(DOMExceptionCode::kOperationError, msg.c_str());
+      self->HandleError("Encoder initialization error.", status);
     }
 
     self->stall_request_processing_ = false;
@@ -600,10 +649,10 @@ void VideoEncoder::ProcessFlush(Request* request) {
     if (status.is_ok()) {
       req->resolver.Release()->Resolve();
     } else {
-      std::string msg = "Flushing error: " + status.message();
+      std::string error_msg = "Flushing error.";
+      self->HandleError(error_msg, status);
       auto* ex = MakeGarbageCollected<DOMException>(
-          DOMExceptionCode::kOperationError, msg.c_str());
-      self->HandleError(ex);
+          DOMExceptionCode::kOperationError, error_msg.c_str());
       req->resolver.Release()->Reject(ex);
     }
     self->stall_request_processing_ = false;
@@ -611,6 +660,7 @@ void VideoEncoder::ProcessFlush(Request* request) {
   };
 
   stall_request_processing_ = true;
+
   media_encoder_->Flush(WTF::Bind(done_callback, WrapWeakPersistent(this),
                                   WrapPersistentIfNeeded(request)));
 }
@@ -650,12 +700,17 @@ void VideoEncoder::CallOutputCallback(
   output_callback_->InvokeAndReportException(nullptr, chunk, decoder_config);
 }
 
+void VideoEncoder::ContextDestroyed() {
+  parent_media_log_ = nullptr;
+}
+
 void VideoEncoder::Trace(Visitor* visitor) const {
   visitor->Trace(script_state_);
   visitor->Trace(output_callback_);
   visitor->Trace(error_callback_);
   visitor->Trace(requests_);
   ScriptWrappable::Trace(visitor);
+  ExecutionContextLifecycleObserver::Trace(visitor);
 }
 
 void VideoEncoder::Request::Trace(Visitor* visitor) const {

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

@@ -8,6 +8,7 @@
 #include <memory>
 
 #include "base/optional.h"
+#include "media/base/media_log.h"
 #include "media/base/status.h"
 #include "media/base/video_codecs.h"
 #include "media/base/video_color_space.h"
@@ -19,6 +20,7 @@
 #include "third_party/blink/renderer/modules/modules_export.h"
 #include "third_party/blink/renderer/modules/webcodecs/video_frame.h"
 #include "third_party/blink/renderer/platform/bindings/script_wrappable.h"
+#include "third_party/blink/renderer/platform/context_lifecycle_observer.h"
 
 namespace media {
 class VideoEncoder;
@@ -34,7 +36,9 @@ class VideoEncoderInit;
 class VideoEncoderEncodeOptions;
 class Visitor;
 
-class MODULES_EXPORT VideoEncoder final : public ScriptWrappable {
+class MODULES_EXPORT VideoEncoder final
+    : public ScriptWrappable,
+      public ExecutionContextLifecycleObserver {
   DEFINE_WRAPPERTYPEINFO();
 
  public:
@@ -61,6 +65,9 @@ class MODULES_EXPORT VideoEncoder final : public ScriptWrappable {
 
   String state() { return state_; }
 
+  // ExecutionContextLifecycleObserver override.
+  void ContextDestroyed() override;
+
   // GarbageCollected override.
   void Trace(Visitor*) const override;
 
@@ -102,13 +109,15 @@ class MODULES_EXPORT VideoEncoder final : public ScriptWrappable {
       media::VideoEncoderOutput output,
       base::Optional<media::VideoEncoder::CodecDescription> codec_desc);
   void HandleError(DOMException* ex);
-  void HandleError(DOMExceptionCode code, const String& message);
+  void HandleError(std::string context, media::Status);
   void EnqueueRequest(Request* request);
   void ProcessRequests();
   void ProcessEncode(Request* request);
   void ProcessConfigure(Request* request);
   void ProcessFlush(Request* request);
 
+  void UpdateEncoderLog(std::string encoder_name, bool is_hw_accelerated);
+
   void ResetInternal();
 
   std::unique_ptr<ParsedConfig> ParseConfig(const VideoEncoderConfig*,
@@ -120,6 +129,16 @@ class MODULES_EXPORT VideoEncoder final : public ScriptWrappable {
   std::unique_ptr<ParsedConfig> active_config_;
   std::unique_ptr<media::VideoEncoder> media_encoder_;
 
+  // |parent_media_log_| must be destroyed if ever the ExecutionContext is
+  // destroyed, since the blink::MediaInspectorContext* pointer given to
+  // InspectorMediaEventHandler might no longer be valid.
+  // |parent_media_log_| should not be used directly. Use |media_log_| instead.
+  std::unique_ptr<media::MediaLog> parent_media_log_;
+
+  // We might destroy |parent_media_log_| at any point, so keep a clone which
+  // can be safely accessed, and whose raw pointer can be given callbacks.
+  std::unique_ptr<media::MediaLog> media_log_;
+
   V8CodecState state_;
 
   Member<ScriptState> script_state_;