Skip to content

[WebCodecs] imported/w3c/web-platform-tests/webcodecs/video-encoder-config.https.any.html has two sub-tests failing - #78231

Merged
webkit-commit-queue merged 1 commit into
WebKit:mainfrom
philn:eng/328842
Oct 27, 2026
Merged

[WebCodecs] imported/w3c/web-platform-tests/webcodecs/video-encoder-config.https.any.html has two sub-tests failing#78231
webkit-commit-queue merged 1 commit into
WebKit:mainfrom
philn:eng/328842

Conversation

@philn

@philn philn commented Oct 25, 2026 •
edited by webkit-early-warning-system
Loading

Copy link
Copy Markdown
Member

cf3e6fe

[WebCodecs] imported/w3c/web-platform-tests/webcodecs/video-encoder-config.https.any.html has two sub-tests failing
https://bugs.webkit.org/show_bug.cgi?id=328842

Reviewed by Youenn Fablet.

Reject video encoder configurations containing a zero bitrate.

* LayoutTests/imported/w3c/web-platform-tests/webcodecs/video-encoder-config.https.any-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/webcodecs/video-encoder-config.https.any.worker-expected.txt:
* Source/WebCore/Modules/webcodecs/WebCodecsVideoEncoder.cpp:
(WebCore::isValidEncoderConfig):

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

881a1be

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios   🛠 mac   🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-cmake-debug   🧪 wpe-wk2   🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2   🧪 api-mac   🧪 api-wpe
✅ 🧪 ios-wk2-wpt   🧪 api-mac-cmake-debug   🧪 jsc-wpe
✅ 🧪 api-ios   🧪 mac-wk2 ✅ 🛠 gtk3-gcc
✅ 🛠 ios-safer-cpp ✅ 🧪 mac-cmake-debug-wk2 ✅ 🛠 gtk
✅ 🛠 vision   🧪 mac-wk2-stress   🧪 gtk-wk2
✅ 🛠 🧪 merge ✅ 🛠 vision-sim   🧪 mac-intel-wk2   🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🛠 mac-safer-cpp ✅ 🛠 playstation
✅ 🛠 tv   🧪 mac-site-isolation
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@philn philn self-assigned this Oct 25, 2026
@philn philn added the Media Bugs related to the HTML 5 Media elements. label Oct 25, 2026
@webkit-early-warning-system

webkit-early-warning-system commented Oct 25, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

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

Details

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-cmake-debug ✅ 🧪 wpe-wk2 ✅ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 api-mac-cmake-debug
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk3-gcc
✅ 🛠 ios-safer-cpp ✅ 🧪 mac-cmake-debug-wk2 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-intel-wk2 ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🛠 mac-safer-cpp ✅ 🛠 playstation
✅ 🛠 tv ✅ 🧪 mac-site-isolation
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@philn
philn requested a review from youennf October 25, 2026 15:54
@philn philn mentioned this pull request Oct 25, 2026
Merged
@philn
philn requested a review from calvaris October 26, 2026 11:35
youennf
youennf reviewed Oct 26, 2026
View reviewed changes

@youennf youennf left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r=me for bitrate change.
I am less sure about the other video frame big size case.

Comment thread Source/WebCore/Modules/webcodecs/WebCodecsVideoEncoder.cpp
return false;

if (config.bitrate && !*config.bitrate)
return false;

@youennf youennf Oct 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should have the spec mention this case.

@philn philn Oct 26, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

w3c/webcodecs#984

Comment thread Source/WebCore/Modules/webcodecs/WebCodecsVideoEncoder.cpp Outdated
if (config.codec.startsWith("avc1."_s) && (!!(config.width % 2) || !!(config.height % 2)))
return Exception { ExceptionCode::TypeError, "H264 only supports even sized frames"_s };

if (config.hardwareAcceleration == HardwareAcceleration::PreferHardware && (config.width >= 30000 || config.height >= 30000))

@youennf youennf Oct 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is not covered, should we move it to another PR.
Also why the choice of 30000, is it the same as Chromium and/or Firefox

@philn philn Oct 26, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked only Chromium so far, I think the limit is per codec, https://chromium.googlesource.com/chromium/src/+/main/third_party/blink/renderer/modules/webcodecs/video_encoder.cc#242 so using a fixed value here doesn't seem correct actually.

@philn philn Oct 27, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm removing this change...

@webkit-early-warning-system

webkit-early-warning-system commented Oct 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

EWS run on current version of this PR (hash 881a1be)

Details

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios   🛠 mac   🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-cmake-debug   🧪 wpe-wk2   🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2   🧪 api-mac   🧪 api-wpe
✅ 🧪 ios-wk2-wpt   🧪 api-mac-cmake-debug   🧪 jsc-wpe
✅ 🧪 api-ios   🧪 mac-wk2 ✅ 🛠 gtk3-gcc
✅ 🛠 ios-safer-cpp ✅ 🧪 mac-cmake-debug-wk2 ✅ 🛠 gtk
✅ 🛠 vision   🧪 mac-wk2-stress   🧪 gtk-wk2
✅ 🛠 🧪 merge ✅ 🛠 vision-sim   🧪 mac-intel-wk2   🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🛠 mac-safer-cpp ✅ 🛠 playstation
✅ 🛠 tv   🧪 mac-site-isolation
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@philn philn added the merge-queue Applied to send a pull request to merge-queue label Oct 27, 2026
@philn
[WebCodecs] imported/w3c/web-platform-tests/webcodecs/video-encoder-c…
cf3e6fe 
…onfig.https.any.html has two sub-tests failing

https://bugs.webkit.org/show_bug.cgi?id=328842

Reviewed by Youenn Fablet.

Reject video encoder configurations containing a zero bitrate.

* LayoutTests/imported/w3c/web-platform-tests/webcodecs/video-encoder-config.https.any-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/webcodecs/video-encoder-config.https.any.worker-expected.txt:
* Source/WebCore/Modules/webcodecs/WebCodecsVideoEncoder.cpp:
(WebCore::isValidEncoderConfig):

Canonical link: https://commits.webkit.org/324824@main
@webkit-commit-queue

webkit-commit-queue commented Oct 27, 2026

Copy link
Copy Markdown
Collaborator

Committed 324824@main (cf3e6fe): https://commits.webkit.org/324824@main

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

@webkit-commit-queue
webkit-commit-queue merged commit cf3e6fe into WebKit:main Oct 27, 2026
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Oct 27, 2026
@philn
philn deleted the eng/328842 branch October 27, 2026 10:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@youennf youennf youennf approved these changes
@calvaris calvaris Awaiting requested review from calvaris

Assignees

@philn philn

Labels

Media Bugs related to the HTML 5 Media elements.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@philn @webkit-early-warning-system @webkit-commit-queue @youennf