Skip to content

REGRESSION(269844@main): [GStreamer] WebCodecs encoders now closing from non-main thread #19688

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
webkit-commit-queue merged 1 commit into from
Oct 29, 2023
Merged

REGRESSION(269844@main): [GStreamer] WebCodecs encoders now closing from non-main thread #19688

webkit-commit-queue merged 1 commit into from
Oct 29, 2023

Conversation

@philn
Copy link
Member

@philn philn commented Oct 29, 2023 •
edited by webkit-early-warning-system
Loading

264d86d

REGRESSION(269844@main): [GStreamer] WebCodecs encoders now closing from non-main thread
https://bugs.webkit.org/show_bug.cgi?id=263846

Reviewed by Youenn Fablet.

Schedule the close/reset of encoders and decoders in the script execution context. Otherwise the
resetEncoder() might attempt to trigger rejection of pending flush promises from a non main thread,
leading to ASSERTs hit in Debug builds.

* LayoutTests/platform/glib/TestExpectations:
* LayoutTests/platform/glib/imported/w3c/web-platform-tests/webcodecs/video-encoder-config.https.any.worker-expected.txt:
* Source/WebCore/platform/audio/gstreamer/AudioEncoderGStreamer.cpp:
(WebCore::GStreamerAudioEncoder::create):
* Source/WebCore/platform/graphics/gstreamer/VideoEncoderGStreamer.cpp:
(WebCore::GStreamerVideoEncoder::create):

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

dbcee73

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

@philn philn self-assigned this Oct 29, 2023
@philn philn added the Media Bugs related to the HTML 5 Media elements. label Oct 29, 2023
@webkit-early-warning-system
Copy link
Collaborator

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

EWS run on previous version of this PR (hash 13a2a15)

Details

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

@philn philn requested a review from youennf October 29, 2023 11:15
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Oct 29, 2023
@philn philn removed the merging-blocked Applied to prevent a change from being merged label Oct 29, 2023
@philn philn changed the title REGRESSION(269844@main): WebCodecs encoders/decoders now closing from non-main thread REGRESSION(269844@main): [GStreamer] WebCodecs encoders now closing from non-main thread Oct 29, 2023
@webkit-early-warning-system
Copy link
Collaborator

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

EWS run on previous version of this PR (hash 54a4982)

Details

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

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Oct 29, 2023
@philn philn removed the merging-blocked Applied to prevent a change from being merged label Oct 29, 2023
@webkit-early-warning-system
Copy link
Collaborator

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

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

Details

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

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Oct 29, 2023
@philn philn removed the merging-blocked Applied to prevent a change from being merged label Oct 29, 2023
@webkit-early-warning-system
Copy link
Collaborator

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

EWS run on previous version of this PR (hash 32dcf13)

Details

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

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Oct 29, 2023
@philn philn removed the merging-blocked Applied to prevent a change from being merged label Oct 29, 2023
@webkit-early-warning-system
Copy link
Collaborator

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

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

Details

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

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Oct 29, 2023
@webkit-early-warning-system
Copy link
Collaborator

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

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

Details

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

@philn philn added merge-queue Applied to send a pull request to merge-queue and removed merging-blocked Applied to prevent a change from being merged labels Oct 29, 2023
@philn
REGRESSION(269844@main): [GStreamer] WebCodecs encoders now closing f…
264d86d 
…rom non-main thread

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

Reviewed by Youenn Fablet.

Schedule the close/reset of encoders and decoders in the script execution context. Otherwise the
resetEncoder() might attempt to trigger rejection of pending flush promises from a non main thread,
leading to ASSERTs hit in Debug builds.

* LayoutTests/platform/glib/TestExpectations:
* LayoutTests/platform/glib/imported/w3c/web-platform-tests/webcodecs/video-encoder-config.https.any.worker-expected.txt:
* Source/WebCore/platform/audio/gstreamer/AudioEncoderGStreamer.cpp:
(WebCore::GStreamerAudioEncoder::create):
* Source/WebCore/platform/graphics/gstreamer/VideoEncoderGStreamer.cpp:
(WebCore::GStreamerVideoEncoder::create):

Canonical link: https://commits.webkit.org/269913@main
@webkit-commit-queue
Copy link
Collaborator

webkit-commit-queue commented Oct 29, 2023

Committed 269913@main (264d86d): https://commits.webkit.org/269913@main

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

@webkit-commit-queue webkit-commit-queue merged commit 264d86d into WebKit:main Oct 29, 2023
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Oct 29, 2023
@philn philn deleted the eng/263846 branch October 30, 2023 08:07
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

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.

5 participants

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