Skip to content

[Gstreamer][WebRTC] webrtc/video-disabled-black.html fails #15978

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
merged 1 commit into from
Aug 4, 2023
Merged

[Gstreamer][WebRTC] webrtc/video-disabled-black.html fails #15978

merged 1 commit into from
Aug 4, 2023

Conversation

philn
Copy link
Member

@philn philn commented Jul 21, 2023 •
edited by webkit-early-warning-system
Loading

d9e6730

[Gstreamer][WebRTC] webrtc/video-disabled-black.html fails
https://bugs.webkit.org/show_bug.cgi?id=254212
<rdar://problem/107046108>

Reviewed by Xabier Rodriguez-Calvar.

When disabling an incoming WebRTC track the mediastreamsrc now updates and propagates the
corresponding GstStream. This allows decodebin3 to properly reconfigure itself, by getting rid of
the decode chain (the disabled stream is not encoded).

This patch also disables the `parse-streams` property on urisourcebin for MSE and MediaStreams, it's
needed mostly for buffering adaptivedemux-based streams anyway. Parsing will then be done
downstream, by decodebin3, which also simplifies input stream reconfigurations that are now managed
internally by decodebin3.

* LayoutTests/platform/glib/TestExpectations:
* Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:
(WebCore::MediaPlayerPrivateGStreamer::configureElement):
* Source/WebCore/platform/mediastream/gstreamer/GStreamerMediaStreamSource.cpp:
(webkitMediaStreamNew):
(webkitMediaStreamSrcPostStreamCollection):

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

21b59d4

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 Jul 21, 2023
@philn philn added Platform Portability improvements and other general platform improvements not driven directly by site bugs. skip-ews Applied to prevent a change from being run on EWS labels Jul 21, 2023
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jul 21, 2023 •
edited
Loading

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

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 calvaris July 21, 2023 09:16
@philn
Copy link
Member Author

philn commented Jul 21, 2023

Draft due to dependency on #15977

@philn philn removed the skip-ews Applied to prevent a change from being run on EWS label Jul 21, 2023
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jul 21, 2023 •
edited
Loading

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

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

@philn philn marked this pull request as ready for review July 21, 2023 10:56
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jul 21, 2023
@philn philn marked this pull request as draft July 21, 2023 14:35
@philn philn removed the merging-blocked Applied to prevent a change from being merged label Jul 21, 2023
@philn philn marked this pull request as ready for review July 21, 2023 15:43
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jul 21, 2023 •
edited
Loading

EWS run on previous version of this PR (hash 843911d)

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 Jul 21, 2023
@philn
Copy link
Member Author

philn commented Jul 22, 2023

The test failures exposed another GStreamer bug, MR -> https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/5089

calvaris
calvaris approved these changes Jul 24, 2023
View reviewed changes
@philn philn removed the merging-blocked Applied to prevent a change from being merged label Jul 27, 2023
@philn philn added the skip-ews Applied to prevent a change from being run on EWS label Jul 27, 2023
@webkit-early-warning-system
Copy link
Collaborator

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

EWS run on previous version of this PR (hash 10421b9)

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 removed the skip-ews Applied to prevent a change from being run on EWS label Aug 4, 2023
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Aug 4, 2023 •
edited
Loading

EWS run on current version of this PR (hash 21b59d4)

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 the merge-queue Applied to send a pull request to merge-queue label Aug 4, 2023
@philn
[Gstreamer][WebRTC] webrtc/video-disabled-black.html fails
d9e6730 
https://bugs.webkit.org/show_bug.cgi?id=254212
<rdar://problem/107046108>

Reviewed by Xabier Rodriguez-Calvar.

When disabling an incoming WebRTC track the mediastreamsrc now updates and propagates the
corresponding GstStream. This allows decodebin3 to properly reconfigure itself, by getting rid of
the decode chain (the disabled stream is not encoded).

This patch also disables the `parse-streams` property on urisourcebin for MSE and MediaStreams, it's
needed mostly for buffering adaptivedemux-based streams anyway. Parsing will then be done
downstream, by decodebin3, which also simplifies input stream reconfigurations that are now managed
internally by decodebin3.

* LayoutTests/platform/glib/TestExpectations:
* Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:
(WebCore::MediaPlayerPrivateGStreamer::configureElement):
* Source/WebCore/platform/mediastream/gstreamer/GStreamerMediaStreamSource.cpp:
(webkitMediaStreamNew):
(webkitMediaStreamSrcPostStreamCollection):

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

Committed 266589@main (d9e6730): https://commits.webkit.org/266589@main

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

@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Aug 4, 2023
@webkit-commit-queue webkit-commit-queue merged commit d9e6730 into WebKit:main Aug 4, 2023
@philn philn deleted the eng/254212 branch August 4, 2023 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@calvaris calvaris calvaris approved these changes

Assignees

@philn philn

Labels
Platform Portability improvements and other general platform improvements not driven directly by site bugs.
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 @calvaris @webkit-ews-buildbot