Commit 23fdf4c2 authored by Thomas Guilbert's avatar Thomas Guilbert Committed by Commit Bot

Add CodecLogger

VideoEncoder and DecoderTemplate share some common logic around the
lifetime management of MediaLogs. This CL abstracts away the logic into
a CodecLogger, and updates MediaLog uses through out the code.

This CL also makes sure not to report the media::Status messages in JS
exceptions, to avoid leaking security sensitive information.

Bug: 1148541
Change-Id: I483ee7be7aa52a7996c91cef7d781ba80b516855
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2538964Reviewed-by: default avatarEugene Zemtsov <eugene@chromium.org>
Commit-Queue: Thomas Guilbert <tguilbert@chromium.org>
Cr-Commit-Position: refs/heads/master@{#831219}
parent ce16d8e5
......@@ -15,6 +15,8 @@ blink_modules_sources("webcodecs") {
"audio_frame.cc",
"audio_frame.h",
"codec_config_eval.h",
"codec_logger.cc",
"codec_logger.h",
"codec_state_helper.cc",
"codec_state_helper.h",
"decoder_selector.cc",
......
// 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.
#include "third_party/blink/renderer/modules/webcodecs/codec_logger.h"
#include <string>
#include "media/base/media_util.h"
#include "third_party/blink/public/platform/platform.h"
#include "third_party/blink/renderer/core/execution_context/execution_context.h"
#include "third_party/blink/renderer/core/inspector/inspector_media_context_impl.h"
namespace blink {
CodecLogger::CodecLogger(
ExecutionContext* context,
scoped_refptr<base::SingleThreadTaskRunner> task_runner) {
DCHECK(context);
parent_media_log_ = Platform::Current()->GetMediaLog(
MediaInspectorContextImpl::From(*context), task_runner);
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();
}
CodecLogger::CodecLogger()
: parent_media_log_(std::make_unique<media::NullMediaLog>()),
media_log_(parent_media_log_->Clone()) {}
DOMException* CodecLogger::MakeException(std::string error_msg,
media::Status status) {
media_log_->NotifyError(status);
return MakeGarbageCollected<DOMException>(DOMExceptionCode::kOperationError,
error_msg.c_str());
}
DOMException* CodecLogger::MakeException(std::string error_msg,
media::StatusCode code,
const base::Location& location) {
return MakeException(error_msg, media::Status(code, error_msg, location));
}
CodecLogger::~CodecLogger() {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
}
void CodecLogger::Neuter() {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
parent_media_log_ = nullptr;
}
} // namespace blink
// 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.
#ifndef THIRD_PARTY_BLINK_RENDERER_MODULES_WEBCODECS_CODEC_LOGGER_H_
#define THIRD_PARTY_BLINK_RENDERER_MODULES_WEBCODECS_CODEC_LOGGER_H_
#include <memory>
#include "media/base/media_log.h"
#include "media/base/status.h"
#include "third_party/blink/renderer/core/dom/dom_exception.h"
#include "third_party/blink/renderer/modules/modules_export.h"
namespace blink {
class ExecutionContext;
// Simple wrapper around MediaLog instances, to manage the lifetime safety of
// said MediaLogs. |parent_media_log_| must be destroyed and created on the
// main thread (or the worker thread if we are in a worker context).
// |media_log_| is a clone of |parent_media_log_| which can be safely passed to
// any thread. If the parent log is destroyed, |media_log_| will safely and
// silently stop logging.
// Note: Owners of this class should be ExecutionLifeCycleObservers, and should
// call Neuter() if the ExecutionContext passed to the constructor is destroyed.
class MODULES_EXPORT CodecLogger final {
public:
// Creates a CodecLogger backed by a NullMediaLog, which does nothing.
CodecLogger();
// Attempts to create CodecLogger backed by a BatchingMediaLog. Falls back to
// a NullMediaLog on failure.
CodecLogger(ExecutionContext*,
scoped_refptr<base::SingleThreadTaskRunner> task_runner);
~CodecLogger();
// Creates an OperationError DOMException with the given |error_msg|, and logs
// the given |status| in |media_log_|.
// Since |status| can come from platform codecs, its contents won't be
// surfaced to JS, since we could leak important information.
DOMException* MakeException(std::string error_msg, media::Status status);
// Convenience wrapper for MakeException(), where |error_msg| is shared for
// both the exception message and the status message.
DOMException* MakeException(
std::string error_msg,
media::StatusCode code,
const base::Location& location = base::Location::Current());
// Safe to use on any thread. |this| should still outlive users of log().
media::MediaLog* log() { return media_log_.get(); }
// Destroys |parent_media_log_|, which makes |media_log_| silently stop
// logging in a thread safe way.
// Must be called if the ExecutionContext passed into the constructor is
// destroyed.
void Neuter();
private:
// |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_;
SEQUENCE_CHECKER(sequence_checker_);
};
} // namespace blink
#endif // THIRD_PARTY_BLINK_RENDERER_MODULES_WEBCODECS_CODEC_LOGGER_H_
......@@ -11,7 +11,6 @@
#include "base/logging.h"
#include "base/memory/scoped_refptr.h"
#include "base/time/time.h"
#include "media/base/media_util.h"
#include "media/media_buildflags.h"
#include "media/video/gpu_video_accelerator_factories.h"
#include "third_party/blink/public/platform/platform.h"
......@@ -25,7 +24,6 @@
#include "third_party/blink/renderer/bindings/modules/v8/v8_video_decoder_init.h"
#include "third_party/blink/renderer/core/dom/dom_exception.h"
#include "third_party/blink/renderer/core/execution_context/execution_context.h"
#include "third_party/blink/renderer/core/inspector/inspector_media_context_impl.h"
#include "third_party/blink/renderer/modules/modules_export.h"
#include "third_party/blink/renderer/modules/webcodecs/audio_decoder.h"
#include "third_party/blink/renderer/modules/webcodecs/audio_frame.h"
......@@ -74,20 +72,15 @@ DecoderTemplate<Traits>::DecoderTemplate(ScriptState* script_state,
// TODO(crbug.com/1151005): Use a real MediaLog in worker contexts too.
if (IsMainThread()) {
parent_media_log_ = Platform::Current()->GetMediaLog(
MediaInspectorContextImpl::From(*context),
context->GetTaskRunner(TaskType::kInternalMedia));
logger_ = std::make_unique<CodecLogger>(
context, context->GetTaskRunner(TaskType::kInternalMedia));
} else {
// This will create a logger backed by a NullMediaLog, which does nothing.
logger_ = std::make_unique<CodecLogger>();
}
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::kFrameUrl>(
GetExecutionContext()->Url().GetString().Ascii());
logger_->log()->SetProperty<media::MediaLogProperty::kFrameUrl>(
context->Url().GetString().Ascii());
output_cb_ = init->output();
error_cb_ = init->error();
......@@ -274,11 +267,11 @@ bool DecoderTemplate<Traits>::ProcessConfigureRequest(Request* request) {
if (!decoder_) {
decoder_ = Traits::CreateDecoder(*ExecutionContext::From(script_state_),
gpu_factories_, media_log_.get());
gpu_factories_, logger_->log());
if (!decoder_) {
HandleError("Configuration error",
media::Status(media::StatusCode::kDecoderCreationFailed,
"Could not create decoder."));
Shutdown(logger_->MakeException(
"Configuration error: Could not create decoder.",
media::StatusCode::kDecoderCreationFailed));
return false;
}
......@@ -319,10 +312,9 @@ bool DecoderTemplate<Traits>::ProcessDecodeRequest(Request* request) {
DCHECK_GT(num_pending_decodes_, 0);
if (!decoder_) {
HandleError(
"Decoding error",
media::Status(media::StatusCode::kDecoderInitializeNeverCompleted,
"No decoder found."));
Shutdown(logger_->MakeException(
"Decoding error: no decoder found.",
media::StatusCode::kDecoderInitializeNeverCompleted));
return false;
}
......@@ -334,13 +326,13 @@ bool DecoderTemplate<Traits>::ProcessDecodeRequest(Request* request) {
// The request may be invalid, if so report that now.
if (!request->decoder_buffer || request->decoder_buffer->data_size() == 0) {
media::Status error =
!request->status.is_ok()
? request->status
: media::Status(media::StatusCode::kDecoderFailedDecode,
"Null or empty decoder buffer.");
if (request->status.is_ok()) {
Shutdown(logger_->MakeException("Null or empty decoder buffer.",
media::StatusCode::kDecoderFailedDecode));
} else {
Shutdown(logger_->MakeException("Decoder error.", request->status));
}
HandleError("Decoding error", error);
return false;
}
......@@ -403,29 +395,11 @@ bool DecoderTemplate<Traits>::ProcessResetRequest(Request* request) {
return true;
}
template <typename Traits>
void DecoderTemplate<Traits>::HandleError(std::string context,
media::Status status) {
DVLOG(1) << __func__;
if (IsClosed())
return;
media_log_->NotifyError(status);
std::string message =
context + (status.message().empty() ? "." : ": " + status.message());
// We could have different DOMExceptionCodes, but for the moment, all of our
// exceptions seem appropriate as operation errors.
auto* ex = MakeGarbageCollected<DOMException>(
DOMExceptionCode::kOperationError, message.c_str());
Shutdown(ex);
}
template <typename Traits>
void DecoderTemplate<Traits>::Shutdown(DOMException* exception) {
DVLOG(3) << __func__;
DCHECK(!IsClosed());
if (IsClosed())
return;
// Abort pending work (otherwise it will never complete)
if (pending_request_) {
......@@ -449,6 +423,9 @@ void DecoderTemplate<Traits>::Shutdown(DOMException* exception) {
output_cb_.Release();
error_cb_.Release();
// Prevent any further logging from being reported.
logger_->Neuter();
// Clear decoding and JS-visible queue state.
decoder_.reset();
pending_decodes_.clear();
......@@ -491,7 +468,7 @@ void DecoderTemplate<Traits>::OnConfigureFlushDone(media::Status status) {
DCHECK_EQ(pending_request_->type, Request::Type::kConfigure);
if (!status.is_ok()) {
HandleError("Configuration error", status);
Shutdown(logger_->MakeException("Configuration error.", status));
return;
}
......@@ -513,12 +490,12 @@ void DecoderTemplate<Traits>::OnInitializeDone(media::Status status) {
DCHECK_EQ(pending_request_->type, Request::Type::kConfigure);
if (!status.is_ok()) {
HandleError("Decoder initialization error", status);
Shutdown(logger_->MakeException("Decoder initialization error.", status));
return;
}
Traits::UpdateDecoderLog(*decoder_, *pending_request_->media_config,
media_log_.get());
logger_->log());
pending_request_.Release();
......@@ -533,7 +510,7 @@ void DecoderTemplate<Traits>::OnDecodeDone(uint32_t id, media::Status status) {
return;
if (!status.is_ok() && status.code() != media::StatusCode::kAborted) {
HandleError("Decoding error", status);
Shutdown(logger_->MakeException("Decoding error.", status));
return;
}
......@@ -553,7 +530,7 @@ void DecoderTemplate<Traits>::OnFlushDone(media::Status status) {
DCHECK_EQ(pending_request_->type, Request::Type::kFlush);
if (!status.is_ok()) {
HandleError("Flushing error", status);
Shutdown(logger_->MakeException("Flushing error.", status));
return;
}
......@@ -594,6 +571,11 @@ void DecoderTemplate<Traits>::OnOutput(uint32_t reset_generation,
nullptr, Traits::MakeOutput(std::move(output), context));
}
template <typename Traits>
void DecoderTemplate<Traits>::ContextDestroyed() {
logger_->Neuter();
}
template <typename Traits>
void DecoderTemplate<Traits>::Trace(Visitor* visitor) const {
visitor->Trace(script_state_);
......@@ -606,11 +588,6 @@ void DecoderTemplate<Traits>::Trace(Visitor* visitor) const {
ExecutionContextLifecycleObserver::Trace(visitor);
}
template <typename Traits>
void DecoderTemplate<Traits>::ContextDestroyed() {
parent_media_log_ = nullptr;
}
template <typename Traits>
bool DecoderTemplate<Traits>::HasPendingActivity() const {
return pending_request_ || !requests_.IsEmpty();
......
......@@ -16,10 +16,11 @@
#include "third_party/blink/renderer/bindings/core/v8/script_promise_resolver.h"
#include "third_party/blink/renderer/bindings/modules/v8/v8_codec_state.h"
#include "third_party/blink/renderer/bindings/modules/v8/v8_web_codecs_error_callback.h"
#include "third_party/blink/renderer/core/execution_context/execution_context_lifecycle_observer.h"
#include "third_party/blink/renderer/modules/modules_export.h"
#include "third_party/blink/renderer/modules/webcodecs/codec_config_eval.h"
#include "third_party/blink/renderer/modules/webcodecs/codec_logger.h"
#include "third_party/blink/renderer/platform/bindings/script_wrappable.h"
#include "third_party/blink/renderer/platform/context_lifecycle_observer.h"
#include "third_party/blink/renderer/platform/heap/garbage_collected.h"
#include "third_party/blink/renderer/platform/heap/heap.h"
#include "third_party/blink/renderer/platform/heap/heap_allocator.h"
......@@ -115,7 +116,6 @@ class MODULES_EXPORT DecoderTemplate
bool ProcessDecodeRequest(Request* request);
bool ProcessFlushRequest(Request* request);
bool ProcessResetRequest(Request* request);
void HandleError(std::string context, media::Status);
void ResetAlgorithm();
void Shutdown(DOMException* ex = nullptr);
......@@ -147,15 +147,7 @@ class MODULES_EXPORT DecoderTemplate
// Could be a configure, flush, or reset. Decodes go in |pending_decodes_|.
Member<Request> pending_request_;
// |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 to |decoder_|.
std::unique_ptr<media::MediaLog> media_log_;
std::unique_ptr<CodecLogger> logger_;
media::GpuVideoAcceleratorFactories* gpu_factories_ = nullptr;
......
......@@ -12,7 +12,6 @@
#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"
......@@ -37,7 +36,6 @@
#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"
......@@ -170,24 +168,14 @@ VideoEncoder::VideoEncoder(ScriptState* script_state,
UseCounter::Count(ExecutionContext::From(script_state),
WebFeature::kWebCodecs);
ExecutionContext* context = GetExecutionContext();
logger_ = std::make_unique<CodecLogger>(
GetExecutionContext(), Thread::MainThread()->GetTaskRunner());
DCHECK(context);
media::MediaLog* log = logger_->log();
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>(
log->SetProperty<media::MediaLogProperty::kFrameTitle>(
std::string("VideoEncoder(WebCodecs)"));
media_log_->SetProperty<media::MediaLogProperty::kFrameUrl>(
log->SetProperty<media::MediaLogProperty::kFrameUrl>(
GetExecutionContext()->Url().GetString().Ascii());
output_callback_ = init->output();
......@@ -300,9 +288,10 @@ bool VideoEncoder::VerifyCodecSupport(ParsedConfig* config,
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>(
media::MediaLog* log = logger_->log();
log->SetProperty<media::MediaLogProperty::kVideoDecoderName>(encoder_name);
log->SetProperty<media::MediaLogProperty::kIsPlatformVideoDecoder>(
is_hw_accelerated);
}
......@@ -314,11 +303,11 @@ void VideoEncoder::CreateAndInitializeEncoderOnEncoderSupportKnown(
media_encoder_ = CreateMediaVideoEncoder(*active_config_);
if (!media_encoder_) {
HandleError(
HandleError(logger_->MakeException(
"Encoder creation error.",
media::Status(media::StatusCode::kEncoderInitializationError,
"Unable to create encoder (most likely unsupported "
"codec/acceleration requirement combination)"));
"codec/acceleration requirement combination)")));
return;
}
......@@ -336,7 +325,8 @@ void VideoEncoder::CreateAndInitializeEncoderOnEncoderSupportKnown(
DCHECK(self->active_config_);
if (!status.is_ok()) {
self->HandleError("Encoder initialization error.", status);
self->HandleError(self->logger_->MakeException(
"Encoder initialization error.", status));
}
self->stall_request_processing_ = false;
......@@ -548,6 +538,9 @@ void VideoEncoder::ResetInternal() {
}
void VideoEncoder::HandleError(DOMException* ex) {
if (state_.AsEnum() == V8CodecState::Enum::kClosed)
return;
// Save a temp before we clear the callback.
V8WebCodecsErrorCallback* error_callback = error_callback_.Get();
......@@ -560,6 +553,9 @@ void VideoEncoder::HandleError(DOMException* ex) {
media_encoder_.reset();
output_callback_.Clear();
// Prevent further logging.
logger_->Neuter();
if (!script_state_->ContextIsValid() || !error_callback)
return;
......@@ -567,16 +563,6 @@ void VideoEncoder::HandleError(DOMException* ex) {
error_callback->InvokeAndReportException(nullptr, ex);
}
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);
}
void VideoEncoder::EnqueueRequest(Request* request) {
requests_.push_back(request);
ProcessRequests();
......@@ -618,7 +604,8 @@ void VideoEncoder::ProcessEncode(Request* request) {
return;
DCHECK_CALLED_ON_VALID_SEQUENCE(self->sequence_checker_);
if (!status.is_ok()) {
self->HandleError("Encoding error.", status);
self->HandleError(
self->logger_->MakeException("Encoding error.", status));
}
self->ProcessRequests();
};
......@@ -627,9 +614,8 @@ void VideoEncoder::ProcessEncode(Request* request) {
if (frame->HasGpuMemoryBuffer() && !support_nv12_) {
frame = ConvertToI420Frame(frame);
if (!frame) {
HandleError("Unexpected frame format.",
media::Status(media::StatusCode::kEncoderFailedEncode,
"Unexpected frame format"));
HandleError(logger_->MakeException(
"Unexpected frame format.", media::StatusCode::kEncoderFailedEncode));
return;
}
}
......@@ -706,7 +692,8 @@ void VideoEncoder::ProcessReconfigure(Request* request) {
return;
DCHECK_CALLED_ON_VALID_SEQUENCE(self->sequence_checker_);
if (!status.is_ok()) {
self->HandleError("Encoder reconfiguration error.", status);
self->HandleError(self->logger_->MakeException(
"Encoder initialization error.", status));
self->stall_request_processing_ = false;
return;
}
......@@ -750,7 +737,8 @@ void VideoEncoder::ProcessFlush(Request* request) {
if (status.is_ok()) {
req->resolver.Release()->Resolve();
} else {
self->HandleError("Flushing error.", status);
self->HandleError(
self->logger_->MakeException("Flushing error.", status));
req->resolver.Release()->Reject();
}
self->stall_request_processing_ = false;
......@@ -800,7 +788,7 @@ void VideoEncoder::CallOutputCallback(
}
void VideoEncoder::ContextDestroyed() {
parent_media_log_ = nullptr;
logger_->Neuter();
}
bool VideoEncoder::HasPendingActivity() const {
......
......@@ -19,6 +19,7 @@
#include "third_party/blink/renderer/bindings/modules/v8/v8_video_encoder_output_callback.h"
#include "third_party/blink/renderer/bindings/modules/v8/v8_web_codecs_error_callback.h"
#include "third_party/blink/renderer/modules/modules_export.h"
#include "third_party/blink/renderer/modules/webcodecs/codec_logger.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"
......@@ -120,7 +121,6 @@ class MODULES_EXPORT VideoEncoder final
media::VideoEncoderOutput output,
base::Optional<media::VideoEncoder::CodecDescription> codec_desc);
void HandleError(DOMException* ex);
void HandleError(std::string context, media::Status);
void EnqueueRequest(Request* request);
void ProcessRequests();
void ProcessEncode(Request* request);
......@@ -140,20 +140,12 @@ class MODULES_EXPORT VideoEncoder final
const ParsedConfig& config);
bool CanReconfigure(ParsedConfig& original_config, ParsedConfig& new_config);
std::unique_ptr<CodecLogger> logger_;
std::unique_ptr<media::VideoEncoder> media_encoder_;
// This flag maybe removed when all encoders can handle NV12 frame.
bool support_nv12_ = false;
// |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<ParsedConfig> active_config_;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment