Skip to content
Permalink
Browse files
[WebCodecs] Flush before reconfiguring the encoder
https://bugs.webkit.org/show_bug.cgi?id=246994
rdar://problem/101532393

Reviewed by Eric Carlson.

In case we reconfigure an encoder, we will trigger a flush to get all encoded chunks before we change the configuration.
This allows to get the right configuration attached to encoded chunks.
This also ensures we get all encoded chunks since otherwise, there could be a race since we recreate a new encoder when calling configure.
The race is that we need to get all encoded chunks before the old encoder gets destroyed.
Drive-by fix of WebCodecsVideoEncoder::flush where we throwing an exception while we should reject the promise.

* LayoutTests/TestExpectations:
* LayoutTests/imported/w3c/web-platform-tests/webcodecs/reconfiguring-encoder.https.any_h264_annexb-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/webcodecs/reconfiguring-encoder.https.any_h264_avc-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/webcodecs/reconfiguring-encoder.https.any_vp8-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/webcodecs/reconfiguring-encoder.https.any_vp9_p0-expected.txt:
* Source/WebCore/Modules/webcodecs/WebCodecsVideoEncoder.cpp:
(WebCore::WebCodecsVideoEncoder::configure):
(WebCore::WebCodecsVideoEncoder::flush):
* Source/WebCore/Modules/webcodecs/WebCodecsVideoEncoder.h:

Canonical link: https://commits.webkit.org/255957@main
  • Loading branch information
youennf committed Oct 25, 2022
1 parent bb74e46 commit cac80a1b3b935765f6371aa888b9ad58eec562bd
Show file tree
Hide file tree
Showing 7 changed files with 22 additions and 21 deletions.
@@ -236,8 +236,6 @@ webrtc/peerConnection-rvfc.html [ Skip ]

imported/w3c/web-platform-tests/webcodecs/full-cycle-test.https.any.html?h264_annexb [ Pass Failure ]
imported/w3c/web-platform-tests/webcodecs/full-cycle-test.https.any.html?h264_avc [ Pass Failure ]
imported/w3c/web-platform-tests/webcodecs/reconfiguring-encoder.https.any.html?h264_annexb [ Pass Failure ]
imported/w3c/web-platform-tests/webcodecs/reconfiguring-encoder.https.any.html?h264_avc [ Pass Failure ]
imported/w3c/web-platform-tests/webcodecs/temporal-svc-encoding.https.any.html?h264 [ Pass Failure ]
imported/w3c/web-platform-tests/webcodecs/videoDecoder-codec-specific.https.any.html?h264_annexb [ Pass Failure ]
imported/w3c/web-platform-tests/webcodecs/videoDecoder-codec-specific.https.any.worker.html?h264_annexb [ Pass Failure ]
@@ -1,7 +1,3 @@
CONSOLE MESSAGE: Error: assert_unreached: assert_equals: expected 800 but got 640 Reached unreachable code
CONSOLE MESSAGE: Error: assert_unreached: assert_equals: expected 800 but got 640 Reached unreachable code

Harness Error (FAIL), message = Error: assert_unreached: assert_equals: expected 800 but got 640 Reached unreachable code

PASS Reconfiguring encoder

@@ -1,7 +1,3 @@
CONSOLE MESSAGE: Error: assert_unreached: assert_equals: expected 800 but got 640 Reached unreachable code
CONSOLE MESSAGE: Error: assert_unreached: assert_equals: expected 800 but got 640 Reached unreachable code

Harness Error (FAIL), message = Error: assert_unreached: assert_equals: expected 800 but got 640 Reached unreachable code

PASS Reconfiguring encoder

@@ -1,6 +1,3 @@
CONSOLE MESSAGE: Error: assert_unreached: assert_equals: expected 800 but got 640 Reached unreachable code

Harness Error (FAIL), message = Error: assert_unreached: assert_equals: expected 800 but got 640 Reached unreachable code

PASS Reconfiguring encoder

@@ -1,6 +1,3 @@
CONSOLE MESSAGE: Error: assert_unreached: assert_equals: expected 800 but got 640 Reached unreachable code

Harness Error (FAIL), message = Error: assert_unreached: assert_equals: expected 800 but got 640 Reached unreachable code

PASS Reconfiguring encoder

@@ -93,6 +93,22 @@ ExceptionOr<void> WebCodecsVideoEncoder::configure(WebCodecsVideoEncoderConfig&&
m_state = WebCodecsCodecState::Configured;
m_isKeyChunkRequired = true;

if (m_internalEncoder) {
queueControlMessageAndProcess([this, config]() mutable {
m_isMessageQueueBlocked = true;
m_internalEncoder->flush([this, weakedThis = WeakPtr { *this }, config = WTFMove(config)]() mutable {
if (!weakedThis)
return;

if (m_state == WebCodecsCodecState::Closed || !scriptExecutionContext())
return;

m_isMessageQueueBlocked = false;
processControlMessageQueue();
});
});
}

queueControlMessageAndProcess([this, config = WTFMove(config), identifier = scriptExecutionContext()->identifier()]() mutable {
m_isMessageQueueBlocked = true;
m_baseConfiguration = config;
@@ -209,10 +225,12 @@ ExceptionOr<void> WebCodecsVideoEncoder::encode(Ref<WebCodecsVideoFrame>&& frame
return { };
}

ExceptionOr<void> WebCodecsVideoEncoder::flush(Ref<DeferredPromise>&& promise)
void WebCodecsVideoEncoder::flush(Ref<DeferredPromise>&& promise)
{
if (m_state != WebCodecsCodecState::Configured)
return Exception { InvalidStateError, "VideoEncoder is not configured"_s };
if (m_state != WebCodecsCodecState::Configured) {
promise->reject(Exception { InvalidStateError, "VideoEncoder is not configured"_s });
return;
}

m_pendingFlushPromises.append(promise.copyRef());
queueControlMessageAndProcess([this, clearFlushPromiseCount = m_clearFlushPromiseCount]() mutable {
@@ -223,7 +241,6 @@ ExceptionOr<void> WebCodecsVideoEncoder::flush(Ref<DeferredPromise>&& promise)
m_pendingFlushPromises.takeFirst()->resolve();
});
});
return { };
}

ExceptionOr<void> WebCodecsVideoEncoder::reset()
@@ -64,7 +64,7 @@ class WebCodecsVideoEncoder

ExceptionOr<void> configure(WebCodecsVideoEncoderConfig&&);
ExceptionOr<void> encode(Ref<WebCodecsVideoFrame>&&, WebCodecsVideoEncoderEncodeOptions&&);
ExceptionOr<void> flush(Ref<DeferredPromise>&&);
void flush(Ref<DeferredPromise>&&);
ExceptionOr<void> reset();
ExceptionOr<void> close();

0 comments on commit cac80a1

Please sign in to comment.