Skip to content

Commit

Permalink
[M88] Add VideoEncoder logs to DevTools
Browse files Browse the repository at this point in the history 
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.

(cherry picked from commit 77551b4)

TBR: eugene@chromium.org
Bug: 1148576
Change-Id: Ib9b0188421b08071672f49aead4cc87bd7d55d69
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2535561
Reviewed-by: Eugene Zemtsov <eugene@chromium.org>
Commit-Queue: Thomas Guilbert <tguilbert@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#827508}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2541548
Reviewed-by: Thomas Guilbert <tguilbert@chromium.org>
Cr-Commit-Position: refs/branch-heads/4324@{chromium#43}
Cr-Branched-From: c73b5a6-refs/heads/master@{#827102}
  • Loading branch information
@tguilbert-google
tguilbert-google authored and Commit Bot committed Nov 17, 2020
1 parent 3ea729c commit 599e6db
Show file tree
Hide file tree
Showing 2 changed files with 93 additions and 19 deletions.
89 changes: 72 additions & 17 deletions third_party/blink/renderer/modules/webcodecs/video_encoder.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -284,27 +308,45 @@ 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;
switch (config.codec) {
case media::kCodecVP8:
case media::kCodecVP9:
result = CreateVpxVideoEncoder();
UpdateEncoderLog("VpxVideoEncoder", false);
break;
case media::kCodecH264:
result = CreateOpenH264VideoEncoder();
UpdateEncoderLog("OpenH264VideoEncoder", false);
break;
default:
return nullptr;
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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();
};
Expand All @@ -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;
}
}
Expand All @@ -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;
}

Expand All @@ -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;
Expand Down Expand Up @@ -600,17 +649,18 @@ 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;
self->ProcessRequests();
};

stall_request_processing_ = true;

media_encoder_->Flush(WTF::Bind(done_callback, WrapWeakPersistent(this),
WrapPersistentIfNeeded(request)));
}
Expand Down Expand Up @@ -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 {
Expand Down
23 changes: 21 additions & 2 deletions third_party/blink/renderer/modules/webcodecs/video_encoder.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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;
Expand All @@ -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:
Expand All @@ -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;

Expand Down Expand Up @@ -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*,
Expand All @@ -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_;
Expand Down

0 comments on commit 599e6db

Please sign in to comment.