Skip to content

Conversation

@philn
Copy link
Member

@philn philn commented Apr 27, 2023

72fe397

[GStreamer][WebRTC] Tighten EndPoint pipeline with playback pipeline
https://bugs.webkit.org/show_bug.cgi?id=256041

Reviewed by Xabier Rodriguez-Calvar.

The RTP depayloaders and parsers are now wrapped by the RealtimeIncomingSourceGStreamer sub-classes,
using parsebin. The relationship between the incoming source appsink and the mediastreamsrc appsrc
elements is also stronger, latency is now properly propagated and queries are relayed. This all
helps improving lip-sync of incoming WebRTC tracks.

* Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:
(WebCore::MediaPlayerPrivateGStreamer::configureElement):
(WebCore::MediaPlayerPrivateGStreamer::configureDepayloader): Deleted.
* Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h:
* Source/WebCore/platform/mediastream/gstreamer/GStreamerMediaStreamSource.cpp:
(webkitMediaStreamSrcAddTrack):
* Source/WebCore/platform/mediastream/gstreamer/RealtimeIncomingSourceGStreamer.cpp:
(WebCore::RealtimeIncomingSourceGStreamer::RealtimeIncomingSourceGStreamer):
(WebCore::RealtimeIncomingSourceGStreamer::registerClient):
(WebCore::RealtimeIncomingSourceGStreamer::unregisterClient):
(WebCore::RealtimeIncomingSourceGStreamer::handleUpstreamEvent):
(WebCore::RealtimeIncomingSourceGStreamer::handleUpstreamQuery):
* Source/WebCore/platform/mediastream/gstreamer/RealtimeIncomingSourceGStreamer.h:
(WebCore::RealtimeIncomingSourceGStreamer::bin):

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

993b131

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 Apr 27, 2023
@philn philn added the Platform Portability improvements and other general platform improvements not driven directly by site bugs. label Apr 27, 2023
@philn philn requested a review from calvaris April 27, 2023 15:13
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Apr 27, 2023
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't you assert if that has a value and then then it?

Copy link
Member Author

Choose a reason for hiding this comment

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

can't parse the question :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you mean

ASSERT(internalSource->m_webrtcSourceClientId)
auto clientId = internalSource->m_webrtcSourceClientId.value();

¿

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I meant that I think you should assert on the value to have an actual value and then retrieving it. Now you're doing the other way around and it looks weird, but I might be missing something.

@philn philn removed the merging-blocked Applied to prevent a change from being merged label Apr 28, 2023
@philn philn added the merge-queue Applied to send a pull request to merge-queue label Apr 28, 2023
https://bugs.webkit.org/show_bug.cgi?id=256041

Reviewed by Xabier Rodriguez-Calvar.

The RTP depayloaders and parsers are now wrapped by the RealtimeIncomingSourceGStreamer sub-classes,
using parsebin. The relationship between the incoming source appsink and the mediastreamsrc appsrc
elements is also stronger, latency is now properly propagated and queries are relayed. This all
helps improving lip-sync of incoming WebRTC tracks.

* Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:
(WebCore::MediaPlayerPrivateGStreamer::configureElement):
(WebCore::MediaPlayerPrivateGStreamer::configureDepayloader): Deleted.
* Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.h:
* Source/WebCore/platform/mediastream/gstreamer/GStreamerMediaStreamSource.cpp:
(webkitMediaStreamSrcAddTrack):
* Source/WebCore/platform/mediastream/gstreamer/RealtimeIncomingSourceGStreamer.cpp:
(WebCore::RealtimeIncomingSourceGStreamer::RealtimeIncomingSourceGStreamer):
(WebCore::RealtimeIncomingSourceGStreamer::registerClient):
(WebCore::RealtimeIncomingSourceGStreamer::unregisterClient):
(WebCore::RealtimeIncomingSourceGStreamer::handleUpstreamEvent):
(WebCore::RealtimeIncomingSourceGStreamer::handleUpstreamQuery):
* Source/WebCore/platform/mediastream/gstreamer/RealtimeIncomingSourceGStreamer.h:
(WebCore::RealtimeIncomingSourceGStreamer::bin):

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

Committed 263499@main (72fe397): https://commits.webkit.org/263499@main

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

@webkit-commit-queue webkit-commit-queue merged commit 72fe397 into WebKit:main Apr 28, 2023
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Apr 28, 2023
@philn philn deleted the eng/256041 branch April 28, 2023 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Platform Portability improvements and other general platform improvements not driven directly by site bugs.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants