Commit c0e73fce authored by Eugene Zemtsov's avatar Eugene Zemtsov Committed by Commit Bot

[webcodecs] Make sure callbacks aren't called after encoder.reset()

Bug: 1146170
Change-Id: Idd807b7f6e2618c8b4e1f161a2c66d74b4b40374
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2531901Reviewed-by: default avatarDan Sanders <sandersd@chromium.org>
Commit-Queue: Eugene Zemtsov <eugene@chromium.org>
Cr-Commit-Position: refs/heads/master@{#826478}
parent 42e68028
...@@ -345,6 +345,7 @@ void VideoEncoder::configure(const VideoEncoderConfig* config, ...@@ -345,6 +345,7 @@ void VideoEncoder::configure(const VideoEncoderConfig* config,
state_ = V8CodecState(V8CodecState::Enum::kConfigured); state_ = V8CodecState(V8CodecState::Enum::kConfigured);
Request* request = MakeGarbageCollected<Request>(); Request* request = MakeGarbageCollected<Request>();
request->reset_count = reset_count_;
request->type = Request::Type::kConfigure; request->type = Request::Type::kConfigure;
active_config_ = std::move(parsed_config); active_config_ = std::move(parsed_config);
EnqueueRequest(request); EnqueueRequest(request);
...@@ -390,11 +391,12 @@ void VideoEncoder::encode(VideoFrame* frame, ...@@ -390,11 +391,12 @@ void VideoEncoder::encode(VideoFrame* frame,
frame->destroy(); frame->destroy();
Request* request = MakeGarbageCollected<Request>(); Request* request = MakeGarbageCollected<Request>();
request->reset_count = reset_count_;
request->type = Request::Type::kEncode; request->type = Request::Type::kEncode;
request->frame = internal_frame; request->frame = internal_frame;
request->encodeOpts = opts; request->encodeOpts = opts;
++requested_encodes_; ++requested_encodes_;
return EnqueueRequest(request); EnqueueRequest(request);
} }
void VideoEncoder::close(ExceptionState& exception_state) { void VideoEncoder::close(ExceptionState& exception_state) {
...@@ -405,7 +407,7 @@ void VideoEncoder::close(ExceptionState& exception_state) { ...@@ -405,7 +407,7 @@ void VideoEncoder::close(ExceptionState& exception_state) {
state_ = V8CodecState(V8CodecState::Enum::kClosed); state_ = V8CodecState(V8CodecState::Enum::kClosed);
ClearRequests(); ResetInternal();
media_encoder_.reset(); media_encoder_.reset();
output_callback_.Clear(); output_callback_.Clear();
error_callback_.Clear(); error_callback_.Clear();
...@@ -422,6 +424,7 @@ ScriptPromise VideoEncoder::flush(ExceptionState& exception_state) { ...@@ -422,6 +424,7 @@ ScriptPromise VideoEncoder::flush(ExceptionState& exception_state) {
Request* request = MakeGarbageCollected<Request>(); Request* request = MakeGarbageCollected<Request>();
request->resolver = request->resolver =
MakeGarbageCollected<ScriptPromiseResolver>(script_state_); MakeGarbageCollected<ScriptPromiseResolver>(script_state_);
request->reset_count = reset_count_;
request->type = Request::Type::kFlush; request->type = Request::Type::kFlush;
EnqueueRequest(request); EnqueueRequest(request);
return request->resolver->Promise(); return request->resolver->Promise();
...@@ -429,17 +432,17 @@ ScriptPromise VideoEncoder::flush(ExceptionState& exception_state) { ...@@ -429,17 +432,17 @@ ScriptPromise VideoEncoder::flush(ExceptionState& exception_state) {
void VideoEncoder::reset(ExceptionState& exception_state) { void VideoEncoder::reset(ExceptionState& exception_state) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
// TODO: Not fully implemented yet
if (ThrowIfCodecStateClosed(state_, "reset", exception_state)) if (ThrowIfCodecStateClosed(state_, "reset", exception_state))
return; return;
ClearRequests(); ResetInternal();
state_ = V8CodecState(V8CodecState::Enum::kUnconfigured); state_ = V8CodecState(V8CodecState::Enum::kUnconfigured);
} }
void VideoEncoder::ClearRequests() { void VideoEncoder::ResetInternal() {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
reset_count_++;
while (!requests_.empty()) { while (!requests_.empty()) {
Request* pending_req = requests_.TakeFirst(); Request* pending_req = requests_.TakeFirst();
DCHECK(pending_req); DCHECK(pending_req);
...@@ -449,6 +452,7 @@ void VideoEncoder::ClearRequests() { ...@@ -449,6 +452,7 @@ void VideoEncoder::ClearRequests() {
pending_req->resolver.Release()->Reject(ex); pending_req->resolver.Release()->Reject(ex);
} }
} }
stall_request_processing_ = false;
} }
void VideoEncoder::HandleError(DOMException* ex) { void VideoEncoder::HandleError(DOMException* ex) {
...@@ -457,7 +461,7 @@ void VideoEncoder::HandleError(DOMException* ex) { ...@@ -457,7 +461,7 @@ void VideoEncoder::HandleError(DOMException* ex) {
state_ = V8CodecState(V8CodecState::Enum::kClosed); state_ = V8CodecState(V8CodecState::Enum::kClosed);
ClearRequests(); ResetInternal();
// Errors are permanent. Shut everything down. // Errors are permanent. Shut everything down.
error_callback_.Clear(); error_callback_.Clear();
...@@ -510,7 +514,7 @@ void VideoEncoder::ProcessEncode(Request* request) { ...@@ -510,7 +514,7 @@ void VideoEncoder::ProcessEncode(Request* request) {
auto done_callback = [](VideoEncoder* self, Request* req, auto done_callback = [](VideoEncoder* self, Request* req,
media::Status status) { media::Status status) {
if (!self) if (!self || self->reset_count_ != req->reset_count)
return; return;
DCHECK_CALLED_ON_VALID_SEQUENCE(self->sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(self->sequence_checker_);
if (!status.is_ok()) { if (!status.is_ok()) {
...@@ -556,11 +560,11 @@ void VideoEncoder::ProcessConfigure(Request* request) { ...@@ -556,11 +560,11 @@ void VideoEncoder::ProcessConfigure(Request* request) {
} }
auto output_cb = WTF::BindRepeating(&VideoEncoder::CallOutputCallback, auto output_cb = WTF::BindRepeating(&VideoEncoder::CallOutputCallback,
WrapWeakPersistent(this)); WrapWeakPersistent(this), reset_count_);
auto done_callback = [](VideoEncoder* self, Request* req, auto done_callback = [](VideoEncoder* self, Request* req,
media::Status status) { media::Status status) {
if (!self) if (!self || self->reset_count_ != req->reset_count)
return; return;
DCHECK_CALLED_ON_VALID_SEQUENCE(self->sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(self->sequence_checker_);
DCHECK(self->active_config_); DCHECK(self->active_config_);
...@@ -589,9 +593,9 @@ void VideoEncoder::ProcessFlush(Request* request) { ...@@ -589,9 +593,9 @@ void VideoEncoder::ProcessFlush(Request* request) {
auto done_callback = [](VideoEncoder* self, Request* req, auto done_callback = [](VideoEncoder* self, Request* req,
media::Status status) { media::Status status) {
DCHECK(req->resolver); if (!self || self->reset_count_ != req->reset_count)
if (!self)
return; return;
DCHECK(req->resolver);
DCHECK_CALLED_ON_VALID_SEQUENCE(self->sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(self->sequence_checker_);
if (status.is_ok()) { if (status.is_ok()) {
req->resolver.Release()->Resolve(); req->resolver.Release()->Resolve();
...@@ -612,10 +616,12 @@ void VideoEncoder::ProcessFlush(Request* request) { ...@@ -612,10 +616,12 @@ void VideoEncoder::ProcessFlush(Request* request) {
} }
void VideoEncoder::CallOutputCallback( void VideoEncoder::CallOutputCallback(
uint32_t reset_count,
media::VideoEncoderOutput output, media::VideoEncoderOutput output,
base::Optional<media::VideoEncoder::CodecDescription> codec_desc) { base::Optional<media::VideoEncoder::CodecDescription> codec_desc) {
if (!script_state_->ContextIsValid() || !output_callback_ || if (!script_state_->ContextIsValid() || !output_callback_ ||
state_.AsEnum() != V8CodecState::Enum::kConfigured) state_.AsEnum() != V8CodecState::Enum::kConfigured ||
reset_count != reset_count_)
return; return;
EncodedVideoMetadata metadata; EncodedVideoMetadata metadata;
......
...@@ -90,12 +90,15 @@ class MODULES_EXPORT VideoEncoder final : public ScriptWrappable { ...@@ -90,12 +90,15 @@ class MODULES_EXPORT VideoEncoder final : public ScriptWrappable {
void Trace(Visitor*) const; void Trace(Visitor*) const;
Type type; Type type;
// Current value of VideoEncoder.reset_count_ when request was created.
uint32_t reset_count = 0;
Member<VideoFrame> frame; // used by kEncode Member<VideoFrame> frame; // used by kEncode
Member<const VideoEncoderEncodeOptions> encodeOpts; // used by kEncode Member<const VideoEncoderEncodeOptions> encodeOpts; // used by kEncode
Member<ScriptPromiseResolver> resolver; // used by kFlush Member<ScriptPromiseResolver> resolver; // used by kFlush
}; };
void CallOutputCallback( void CallOutputCallback(
uint32_t reset_count,
media::VideoEncoderOutput output, media::VideoEncoderOutput output,
base::Optional<media::VideoEncoder::CodecDescription> codec_desc); base::Optional<media::VideoEncoder::CodecDescription> codec_desc);
void HandleError(DOMException* ex); void HandleError(DOMException* ex);
...@@ -106,7 +109,7 @@ class MODULES_EXPORT VideoEncoder final : public ScriptWrappable { ...@@ -106,7 +109,7 @@ class MODULES_EXPORT VideoEncoder final : public ScriptWrappable {
void ProcessConfigure(Request* request); void ProcessConfigure(Request* request);
void ProcessFlush(Request* request); void ProcessFlush(Request* request);
void ClearRequests(); void ResetInternal();
std::unique_ptr<ParsedConfig> ParseConfig(const VideoEncoderConfig*, std::unique_ptr<ParsedConfig> ParseConfig(const VideoEncoderConfig*,
ExceptionState&); ExceptionState&);
...@@ -124,6 +127,10 @@ class MODULES_EXPORT VideoEncoder final : public ScriptWrappable { ...@@ -124,6 +127,10 @@ class MODULES_EXPORT VideoEncoder final : public ScriptWrappable {
Member<V8WebCodecsErrorCallback> error_callback_; Member<V8WebCodecsErrorCallback> error_callback_;
HeapDeque<Member<Request>> requests_; HeapDeque<Member<Request>> requests_;
int32_t requested_encodes_ = 0; int32_t requested_encodes_ = 0;
// How many times reset() was called on the encoder. It's used to decide
// when a callback needs to be dismissed because reset() was called between
// an operation and its callback.
uint32_t reset_count_ = 0;
// Some kConfigure and kFlush requests can't be executed in parallel with // Some kConfigure and kFlush requests can't be executed in parallel with
// kEncode. This flag stops processing of new requests in the requests_ queue // kEncode. This flag stops processing of new requests in the requests_ queue
......
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