[Cocoa] Remote video codecs should surface any error happening during encoding to web pages #20876

youennf · 2023-11-24T17:13:10Z

`279e75f`

[Cocoa] Remote video codecs should surface any error happening during encoding to web pages
https://bugs.webkit.org/show_bug.cgi?id=265307
rdar://118770776

Reviewed by Eric Carlson.

We were not exposing the encoder errors to JavaScript, while the spec asks us to do so.
We add to H264 and HEVC encoders a way to be notified of any error, and update the code so that,
either the success or error callback is called for every encoding task.

We update LibWebRTCCodecs to make encoding and flush IPC messages with a response handler.
This allows to call the WebCodecs encoding task completion handler with the result of the encoding task.
We use NativePromise for both as a convenience and we make sure that the order of messages is guaranteed.

Covered by a new test which triggers an encoding error by using a low H264 encoding level with large video frames.
This should in theory be handled at configure step but we are not yet checking precisely enough this.
When we do this, we will need to change the test and it will probably be pass for all platforms as the checking code might be in WebKit and not below.
This test is only passing in Sonoma since below, the underlying encoder is not triggering an encoding error.

* LayoutTests/TestExpectations:
* LayoutTests/http/wpt/webcodecs/encoder-task-failing-expected.txt: Added.
* LayoutTests/http/wpt/webcodecs/encoder-task-failing.html: Added.
* LayoutTests/platform/mac-wk2/TestExpectations:
* Source/ThirdParty/libwebrtc/Configurations/libwebrtc.exp:
* Source/ThirdParty/libwebrtc/Source/webrtc/sdk/WebKit/WebKitEncoder.h:
* Source/ThirdParty/libwebrtc/Source/webrtc/sdk/WebKit/WebKitEncoder.mm:
(-[WK_RTCLocalVideoH264H265Encoder setErrorCallback:]):
(webrtc::createLocalEncoder):
* Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/base/RTCVideoEncoder.h:
* Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/components/video_codec/RTCVideoEncoderH264.h:
* Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/components/video_codec/RTCVideoEncoderH264.mm:
(-[RTCVideoEncoderH264 setErrorCallback:]):
(-[RTCVideoEncoderH264 encode:codecSpecificInfo:frameTypes:]):
(-[RTCVideoEncoderH264 frameWasEncoded:flags:sampleBuffer:codecSpecificInfo:width:height:renderTimeMs:timestamp:duration:rotation:isKeyFrameRequired:]):
* Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/components/video_codec/RTCVideoEncoderH265.h:
* Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/components/video_codec/RTCVideoEncoderH265.mm:
(-[RTCVideoEncoderH265 setErrorCallback:]):
(-[RTCVideoEncoderH265 encode:codecSpecificInfo:frameTypes:]):
(-[RTCVideoEncoderH265 frameWasEncoded:flags:sampleBuffer:width:height:renderTimeMs:timestamp:rotation:]):
* Source/WebKit/GPUProcess/webrtc/LibWebRTCCodecsProxy.h:
* Source/WebKit/GPUProcess/webrtc/LibWebRTCCodecsProxy.messages.in:
* Source/WebKit/GPUProcess/webrtc/LibWebRTCCodecsProxy.mm:
(WebKit::LibWebRTCCodecsProxy::createEncoder):
(WebKit::LibWebRTCCodecsProxy::encodeFrame):
(WebKit::LibWebRTCCodecsProxy::notifyEncoderResult):
(WebKit::LibWebRTCCodecsProxy::flushEncoder):
* Source/WebKit/WebProcess/GPU/media/RemoteVideoCodecFactory.cpp:
(WebKit::RemoteVideoEncoderCallbacks::close):
(WebKit::RemoteVideoEncoder::encode):
(WebKit::RemoteVideoEncoderCallbacks::notifyEncodedChunk):
(WebKit::RemoteVideoEncoderCallbacks::addDuration): Deleted.
* Source/WebKit/WebProcess/GPU/webrtc/LibWebRTCCodecs.cpp:
(WebKit::LibWebRTCCodecs::encodeFrameInternal):
(WebKit::LibWebRTCCodecs::encodeFrame):
(WebKit::LibWebRTCCodecs::flushEncoder):
(WebKit::LibWebRTCCodecs::flushEncoderCompleted): Deleted.
* Source/WebKit/WebProcess/GPU/webrtc/LibWebRTCCodecs.h:
* Source/WebKit/WebProcess/GPU/webrtc/LibWebRTCCodecs.messages.in:

Canonical link: https://commits.webkit.org/271377@main

eb64486

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	~~🧪 wpe-wk2~~
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	~~🧪 ios-wk2-wpt~~	✅ 🧪 mac-wk1	✅ 🛠 gtk
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 tv	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 api-gtk
	✅ 🛠 tv-sim	✅ 🧪 mac-wk2-stress
✅ 🛠 🧪 merge	✅ 🛠 watch
	✅ 🛠 watch-sim

webkit-early-warning-system · 2023-11-24T17:13:16Z

EWS run on previous version of this PR (hash 88a81df)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	❌ 🧪 wpe-wk2
✅ 🧪 webkitperl	❌ 🧪 ios-wk2	✅ 🧪 api-mac	❌ 🧪 api-wpe
	❌ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 gtk
	✅ 🧪 api-ios	❌ 🧪 mac-wk2	❌ 🧪 gtk-wk2
	✅ 🛠 tv	✅ 🧪 mac-AS-debug-wk2	❌ 🧪 api-gtk
	✅ 🛠 tv-sim	❌ 🧪 mac-wk2-stress
	✅ 🛠 watch
	✅ 🛠 watch-sim

eric-carlson

r=me once the bots are hapoy

...ce/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/components/video_codec/RTCVideoEncoderH264.mm

webkit-early-warning-system · 2023-11-27T08:47:11Z

EWS run on previous version of this PR (hash 9759b7c)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
❌ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 gtk
	✅ 🧪 api-ios	❌ 🧪 mac-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 tv	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 api-gtk
	✅ 🛠 tv-sim	❌ 🧪 mac-wk2-stress
	✅ 🛠 watch
	✅ 🛠 watch-sim

webkit-early-warning-system · 2023-11-28T07:26:30Z

EWS run on previous version of this PR (hash bb427b8)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
❌ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 gtk
	✅ 🧪 api-ios	❌ 🧪 mac-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 tv	✅ 🧪 mac-AS-debug-wk2	❌ 🧪 api-gtk
	✅ 🛠 tv-sim	❌ 🧪 mac-wk2-stress
	✅ 🛠 watch
	✅ 🛠 watch-sim

webkit-early-warning-system · 2023-11-29T11:38:27Z

EWS run on previous version of this PR (hash c21b3f1)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
❌ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 gtk
	✅ 🧪 api-ios	❌ 🧪 mac-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 tv	❌ 🧪 mac-AS-debug-wk2	✅ 🧪 api-gtk
	✅ 🛠 tv-sim	❌ 🧪 mac-wk2-stress
	✅ 🛠 watch
	✅ 🛠 watch-sim

webkit-early-warning-system · 2023-12-01T07:09:13Z

EWS run on current version of this PR (hash eb64486)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	~~🧪 wpe-wk2~~
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	~~🧪 ios-wk2-wpt~~	✅ 🧪 mac-wk1	✅ 🛠 gtk
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 tv	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 api-gtk
	✅ 🛠 tv-sim	✅ 🧪 mac-wk2-stress
✅ 🛠 🧪 merge	✅ 🛠 watch
	✅ 🛠 watch-sim

… encoding to web pages https://bugs.webkit.org/show_bug.cgi?id=265307 rdar://118770776 Reviewed by Eric Carlson. We were not exposing the encoder errors to JavaScript, while the spec asks us to do so. We add to H264 and HEVC encoders a way to be notified of any error, and update the code so that, either the success or error callback is called for every encoding task. We update LibWebRTCCodecs to make encoding and flush IPC messages with a response handler. This allows to call the WebCodecs encoding task completion handler with the result of the encoding task. We use NativePromise for both as a convenience and we make sure that the order of messages is guaranteed. Covered by a new test which triggers an encoding error by using a low H264 encoding level with large video frames. This should in theory be handled at configure step but we are not yet checking precisely enough this. When we do this, we will need to change the test and it will probably be pass for all platforms as the checking code might be in WebKit and not below. This test is only passing in Sonoma since below, the underlying encoder is not triggering an encoding error. * LayoutTests/TestExpectations: * LayoutTests/http/wpt/webcodecs/encoder-task-failing-expected.txt: Added. * LayoutTests/http/wpt/webcodecs/encoder-task-failing.html: Added. * LayoutTests/platform/mac-wk2/TestExpectations: * Source/ThirdParty/libwebrtc/Configurations/libwebrtc.exp: * Source/ThirdParty/libwebrtc/Source/webrtc/sdk/WebKit/WebKitEncoder.h: * Source/ThirdParty/libwebrtc/Source/webrtc/sdk/WebKit/WebKitEncoder.mm: (-[WK_RTCLocalVideoH264H265Encoder setErrorCallback:]): (webrtc::createLocalEncoder): * Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/base/RTCVideoEncoder.h: * Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/components/video_codec/RTCVideoEncoderH264.h: * Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/components/video_codec/RTCVideoEncoderH264.mm: (-[RTCVideoEncoderH264 setErrorCallback:]): (-[RTCVideoEncoderH264 encode:codecSpecificInfo:frameTypes:]): (-[RTCVideoEncoderH264 frameWasEncoded:flags:sampleBuffer:codecSpecificInfo:width:height:renderTimeMs:timestamp:duration:rotation:isKeyFrameRequired:]): * Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/components/video_codec/RTCVideoEncoderH265.h: * Source/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/components/video_codec/RTCVideoEncoderH265.mm: (-[RTCVideoEncoderH265 setErrorCallback:]): (-[RTCVideoEncoderH265 encode:codecSpecificInfo:frameTypes:]): (-[RTCVideoEncoderH265 frameWasEncoded:flags:sampleBuffer:width:height:renderTimeMs:timestamp:rotation:]): * Source/WebKit/GPUProcess/webrtc/LibWebRTCCodecsProxy.h: * Source/WebKit/GPUProcess/webrtc/LibWebRTCCodecsProxy.messages.in: * Source/WebKit/GPUProcess/webrtc/LibWebRTCCodecsProxy.mm: (WebKit::LibWebRTCCodecsProxy::createEncoder): (WebKit::LibWebRTCCodecsProxy::encodeFrame): (WebKit::LibWebRTCCodecsProxy::notifyEncoderResult): (WebKit::LibWebRTCCodecsProxy::flushEncoder): * Source/WebKit/WebProcess/GPU/media/RemoteVideoCodecFactory.cpp: (WebKit::RemoteVideoEncoderCallbacks::close): (WebKit::RemoteVideoEncoder::encode): (WebKit::RemoteVideoEncoderCallbacks::notifyEncodedChunk): (WebKit::RemoteVideoEncoderCallbacks::addDuration): Deleted. * Source/WebKit/WebProcess/GPU/webrtc/LibWebRTCCodecs.cpp: (WebKit::LibWebRTCCodecs::encodeFrameInternal): (WebKit::LibWebRTCCodecs::encodeFrame): (WebKit::LibWebRTCCodecs::flushEncoder): (WebKit::LibWebRTCCodecs::flushEncoderCompleted): Deleted. * Source/WebKit/WebProcess/GPU/webrtc/LibWebRTCCodecs.h: * Source/WebKit/WebProcess/GPU/webrtc/LibWebRTCCodecs.messages.in: Canonical link: https://commits.webkit.org/271377@main

webkit-commit-queue · 2023-12-01T12:05:41Z

Committed 271377@main (279e75f): https://commits.webkit.org/271377@main

Reviewed commits have been landed. Closing PR #20876 and removing active labels.

…odecs are a constant failure https://bugs.webkit.org/show_bug.cgi?id=265483 rdar://118901410 Reviewed by Jean-Yves Avenard. x86_64 VTB encoder does not like some low resolutions, increase to 200x200 to handle this correctly.. In the future, WebKit#20876 will surface such errors instead of silently failing. * LayoutTests/http/wpt/webcodecs/h264-encoder-default-config.https.any.js: (async encoderTest): Canonical link: https://commits.webkit.org/271464@main

youennf requested a review from cdumez as a code owner November 24, 2023 17:13

youennf self-assigned this Nov 24, 2023

youennf added the Media Bugs related to the HTML 5 Media elements. label Nov 24, 2023

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Nov 24, 2023

eric-carlson approved these changes Nov 24, 2023

View reviewed changes

...ce/ThirdParty/libwebrtc/Source/webrtc/sdk/objc/components/video_codec/RTCVideoEncoderH264.mm Outdated Show resolved Hide resolved

youennf force-pushed the eng/Cocoa-Remote-video-codecs-should-surface-any-error-happening-during-encoding-to-web-pages branch from 88a81df to 9759b7c Compare November 27, 2023 08:47

youennf force-pushed the eng/Cocoa-Remote-video-codecs-should-surface-any-error-happening-during-encoding-to-web-pages branch from 9759b7c to bb427b8 Compare November 28, 2023 07:26

youennf force-pushed the eng/Cocoa-Remote-video-codecs-should-surface-any-error-happening-during-encoding-to-web-pages branch from bb427b8 to c21b3f1 Compare November 29, 2023 11:38

youennf force-pushed the eng/Cocoa-Remote-video-codecs-should-surface-any-error-happening-during-encoding-to-web-pages branch from c21b3f1 to eb64486 Compare December 1, 2023 07:09

youennf removed the merging-blocked Applied to prevent a change from being merged label Dec 1, 2023

youennf mentioned this pull request Dec 1, 2023

NEW TEST (271087@main): [ macOS x86_64 wk2 ] 2 tests in http/wpt/webcodecs are a constant failure #21150

Merged

youennf added the merge-queue Applied to send a pull request to merge-queue label Dec 1, 2023

webkit-commit-queue force-pushed the eng/Cocoa-Remote-video-codecs-should-surface-any-error-happening-during-encoding-to-web-pages branch from eb64486 to 279e75f Compare December 1, 2023 12:05

webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Dec 1, 2023

webkit-commit-queue merged commit 279e75f into WebKit:main Dec 1, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Cocoa] Remote video codecs should surface any error happening during encoding to web pages #20876

[Cocoa] Remote video codecs should surface any error happening during encoding to web pages #20876

youennf commented Nov 24, 2023 •

edited by webkit-early-warning-system

Loading

webkit-early-warning-system commented Nov 24, 2023 •

edited

Loading

eric-carlson left a comment

webkit-early-warning-system commented Nov 27, 2023 •

edited

Loading

webkit-early-warning-system commented Nov 28, 2023 •

edited

Loading

webkit-early-warning-system commented Nov 29, 2023 •

edited

Loading

webkit-early-warning-system commented Dec 1, 2023 •

edited

Loading

webkit-commit-queue commented Dec 1, 2023

[Cocoa] Remote video codecs should surface any error happening during encoding to web pages #20876

[Cocoa] Remote video codecs should surface any error happening during encoding to web pages #20876

Conversation

youennf commented Nov 24, 2023 • edited by webkit-early-warning-system Loading

279e75f

webkit-early-warning-system commented Nov 24, 2023 • edited Loading

eric-carlson left a comment

Choose a reason for hiding this comment

webkit-early-warning-system commented Nov 27, 2023 • edited Loading

webkit-early-warning-system commented Nov 28, 2023 • edited Loading

webkit-early-warning-system commented Nov 29, 2023 • edited Loading

webkit-early-warning-system commented Dec 1, 2023 • edited Loading

webkit-commit-queue commented Dec 1, 2023

youennf commented Nov 24, 2023 •

edited by webkit-early-warning-system

Loading

`279e75f`

webkit-early-warning-system commented Nov 24, 2023 •

edited

Loading

webkit-early-warning-system commented Nov 27, 2023 •

edited

Loading

webkit-early-warning-system commented Nov 28, 2023 •

edited

Loading

webkit-early-warning-system commented Nov 29, 2023 •

edited

Loading

webkit-early-warning-system commented Dec 1, 2023 •

edited

Loading