Skip to content
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

GstWebRTC backports from upstream #1387

Draft
wants to merge 15 commits into
base: wpe-2.38
Choose a base branch
from
Draft

GstWebRTC backports from upstream #1387

wants to merge 15 commits into from

Conversation

cadubentzen
Copy link

Include backports from upstream related to GstWebRTC.

Depends on GStreamer 1.24 backports from WebPlatformForEmbedded/buildroot#542

@cadubentzen cadubentzen marked this pull request as ready for review August 28, 2024 09:36
@cadubentzen cadubentzen requested a review from philn as a code owner August 28, 2024 09:36
youennf and others added 15 commits August 28, 2024 17:15
@youennf @cadubentzen
Move out of OnTrack/OnRemoveTrack libwebrtc callbacks
2584afa 
https://bugs.webkit.org/show_bug.cgi?id=244709
rdar://problem/99481570

Reviewed by Eric Carlson.

OnTrack/OnRemoveTrack are not standard compliant.
We miss some cases where these should be called (say rollback).
The timing of these callbacks is also not well aligned with the specification.

Instead, everytime a description is applied successfully, we store the current transceiver states from the backend.
We then compute the corresponding events from the transceiver states.

Covered by existing and rebased tests.

* LayoutTests/imported/w3c/web-platform-tests/webrtc/RTCPeerConnection-setRemoteDescription-rollback-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/webrtc/RTCRtpSender-setStreams.https-expected.txt:
* Source/WebCore/Modules/mediastream/MediaStream.h:
* Source/WebCore/Modules/mediastream/MediaStreamTrack.cpp:
(WebCore::MediaStreamTrack::trackMutedChanged):
* Source/WebCore/Modules/mediastream/MediaStreamTrack.h:
(WebCore::MediaStreamTrack::setShouldFireMuteEventImmediately):
* Source/WebCore/Modules/mediastream/PeerConnectionBackend.cpp:
(WebCore::PeerConnectionBackend::setLocalDescription):
(WebCore::setAssociatedRemoteStreams):
(WebCore::isDirectionReceiving):
(WebCore::processRemoteTracks):
(WebCore::PeerConnectionBackend::setLocalDescriptionSucceeded):
(WebCore::PeerConnectionBackend::setRemoteDescriptionSucceeded):
* Source/WebCore/Modules/mediastream/PeerConnectionBackend.h:
(WebCore::PeerConnectionBackend::DescriptionStates::isolatedCopy):
* Source/WebCore/Modules/mediastream/RTCRtpReceiver.h:
* Source/WebCore/Modules/mediastream/RTCRtpTransceiver.h:
* Source/WebCore/Modules/mediastream/gstreamer/GStreamerMediaEndpoint.cpp:
(WebCore::GStreamerMediaEndpoint::doSetRemoteDescription):
* Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp:
(WebCore::LibWebRTCMediaEndpoint::mediaStreamFromRTCStreamId):
(WebCore::LibWebRTCMediaEndpoint::addPendingTrackEvent):
(WebCore::LibWebRTCMediaEndpoint::collectTransceivers):
(WebCore::LibWebRTCMediaEndpoint::addIceCandidate):
(WebCore::LibWebRTCMediaEndpoint::OnIceCandidate):
(WebCore::LibWebRTCMediaEndpointTransceiverState::isolatedCopy):
(WebCore::toLibWebRTCMediaEndpointTransceiverState):
(WebCore::transceiverStatesFromPeerConnection):
(WebCore::LibWebRTCMediaEndpoint::setLocalSessionDescriptionSucceeded):
(WebCore::LibWebRTCMediaEndpoint::setLocalSessionDescriptionFailed):
(WebCore::LibWebRTCMediaEndpoint::setRemoteSessionDescriptionSucceeded):
(WebCore::LibWebRTCMediaEndpoint::mediaStreamFromRTCStream): Deleted.
(WebCore::LibWebRTCMediaEndpoint::newTransceiver): Deleted.
(WebCore::LibWebRTCMediaEndpoint::removeRemoteTrack): Deleted.
(WebCore::LibWebRTCMediaEndpoint::OnTrack): Deleted.
(WebCore::LibWebRTCMediaEndpoint::OnRemoveTrack): Deleted.
* Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.h:

Canonical link: https://commits.webkit.org/254128@main
@philn @cadubentzen
Incoming video track renderer should rely on the track intrinsic size h…
fd293f7 
…ttps://bugs.webkit.org/show_bug.cgi?id=274093

Reviewed by Xabier Rodriguez-Calvar.

Pixel and display aspect ratios shouldn't be applied for WebRTC video tracks. The intrinsic size is
re-used as it is. The avf MediaStream player behaves similarly.

* Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:
(WebCore::MediaPlayerPrivateGStreamer::updateVideoSizeAndOrientationFromCaps):
* Source/WebCore/platform/mediastream/gstreamer/RealtimeIncomingVideoSourceGStreamer.cpp:
(WebCore::RealtimeIncomingVideoSourceGStreamer::ensureSizeAndFramerate):
(WebCore::RealtimeIncomingVideoSourceGStreamer::dispatchSample):

Canonical link: https://commits.webkit.org/278745@main
@cadubentzen
Missing a=msid in offer after setting codec preferences https://bugs.…
2ac597c 
…webkit.org/show_bug.cgi?id=275157

Reviewed by Philippe Normand.

When setting codec preferences on GstWebRTCRTPTransceiver, the caps object
did not contain an "a-msid" field, which causes the offer created by the
webrtcbin containing that transceiver to not include an `a=msid` line for it.

The issue is fixed by reusing the `a-msid` field from the pre-existent
codec preference, if one exists.

* LayoutTests/webrtc/msid-setCodecPreferences-expected.txt: Added.
* LayoutTests/webrtc/msid-setCodecPreferences.html: Added.
* Source/WebCore/Modules/mediastream/gstreamer/GStreamerRtpTransceiverBackend.cpp:
(WebCore::getMsidFromCurrentCodecPreferences):
(WebCore::GStreamerRtpTransceiverBackend::setCodecPreferences):

Canonical link: https://commits.webkit.org/279746@main
@cadubentzen
Missing media in SDP if setConfiguration() is called after createData…
631510a 
…Channel() or addTransceiver() https://bugs.webkit.org/show_bug.cgi?id=273318

Reviewed by Xabier Rodriguez-Calvar.

GStreamerMediaEndpoint::setConfiguration() was tearing down the pipeline
if one already existed and creating a new one, which is an issue if any data channels
or transceivers are created before RTCPeerConnection.setConfiguration().

The issue is fixed by creating the pipeline earlier in GStreamerMediaEndpoint's contructor,
so that data channels or transceivers aren't discarded if created/added before setConfiguration().

Credit to Philippe Normand <[email protected]> for finding the issue and fixing it.
I wrote the layout test, which fails without his fix.

* LayoutTests/webrtc/setConfiguration-after-createDataChannel-or-addTransceiver-expected.txt: Added.
* LayoutTests/webrtc/setConfiguration-after-createDataChannel-or-addTransceiver.html: Added.
* Source/WebCore/Modules/mediastream/gstreamer/GStreamerMediaEndpoint.cpp:
(WebCore::GStreamerMediaEndpoint::GStreamerMediaEndpoint):
(WebCore::GStreamerMediaEndpoint::setConfiguration):

Canonical link: https://commits.webkit.org/278099@main
@philn @cadubentzen
webrtc/connection-state.html started failing after update to 1.24.0 h…
77afee3 
…ttps://bugs.webkit.org/show_bug.cgi?id=271243 <rdar://problem/125014192>

Reviewed by Xabier Rodriguez-Calvar.

The `doneGatheringCandidates()` method is called by the PeerConnectionBackend when it's notified
from gst-webrtc that the ICE gathering is finished, so we don't need to call it ourselves. The
end-of-candidates SDP attribute shouldn't appear in the offer/answer the end-point reports either.
This is covered by the webrtc/libwebrtc/descriptionGetters.html test.

* LayoutTests/platform/glib/TestExpectations:
* Source/WebCore/Modules/mediastream/gstreamer/GStreamerMediaEndpoint.cpp:
(WebCore::fetchDescription):
(WebCore::GStreamerMediaEndpoint::doSetLocalDescription):
(WebCore::GStreamerMediaEndpoint::doSetRemoteDescription):
(WebCore::GStreamerMediaEndpoint::onIceCandidate):

Canonical link: https://commits.webkit.org/276412@main
@cadubentzen
m_maxMessageSize should be infinity() instead of max() in `RTCS…
416b31a 
…ctpTransport.h`

https://bugs.webkit.org/show_bug.cgi?id=274025
rdar://problem/128306030

Reviewed by Youenn Fablet.

This patch aligns WebKit with web specification [1]:

[1] https://w3c.github.io/webrtc-pc/#dfn-update-the-data-max-message-size

"If both remoteMaxMessageSize and canSendSize are 0, set
[[MaxMessageSize]] to the positive Infinity value."

* Source/WebCore/Modules/mediastream/RTCSctpTransport.h:
(double m_maxMessageSize):
* LayoutTests/imported/w3c/web-platform-tests/webrtc/RTCSctpTransport-maxMessageSize-expected.txt: Rebaselined

Canonical link: https://commits.webkit.org/279039@main
@youennf @cadubentzen
Most of WPT webrtc/RTCSctpTransport-maxMessageSize.html tests are fai…
872134e 
…ling https://bugs.webkit.org/show_bug.cgi?id=274442 rdar://128444396

Reviewed by Philippe Normand.

We implement https://w3c.github.io/webrtc-pc/#sctp-transport-update-mms.
This is called when successfully applying a SDP description as per specification.
In this implementation, the assumption is that canSendSize is 0.
The specific value of 65536 is handled by libwebrtc.

* LayoutTests/imported/w3c/web-platform-tests/webrtc/RTCSctpTransport-maxMessageSize-expected.txt:
* Source/WebCore/Modules/mediastream/PeerConnectionBackend.cpp:
(WebCore::PeerConnectionBackend::setLocalDescriptionSucceeded):
(WebCore::PeerConnectionBackend::setRemoteDescriptionSucceeded):
* Source/WebCore/Modules/mediastream/PeerConnectionBackend.h:
* Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp:
(WebCore::RTCPeerConnection::updateSctpBackend):
* Source/WebCore/Modules/mediastream/RTCPeerConnection.h:
* Source/WebCore/Modules/mediastream/RTCSctpTransport.cpp:
(WebCore::RTCSctpTransport::onStateChanged):
(WebCore::RTCSctpTransport::updateMaxMessageSize):
* Source/WebCore/Modules/mediastream/RTCSctpTransport.h:
* Source/WebCore/Modules/mediastream/gstreamer/GStreamerMediaEndpoint.cpp:
(WebCore::GStreamerMediaEndpoint::doSetLocalDescription):
(WebCore::GStreamerMediaEndpoint::doSetRemoteDescription):
* Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp:
(WebCore::SctpTransportState::maxMessageSize const):
(WebCore::LibWebRTCMediaEndpoint::setLocalSessionDescriptionSucceeded):
(WebCore::LibWebRTCMediaEndpoint::setRemoteSessionDescriptionSucceeded):

Canonical link: https://commits.webkit.org/279111@main
@cadubentzen
Add debug logging of TransceiverStates https://bugs.webkit.org/show_b…
d2b1a82 
…ug.cgi?id=275146

Reviewed by Philippe Normand.

Add debug logging of TransceiverStates, to aid debugging while moving
the GStreamer backend out of m_pendingTrackEvents in PeerConnectionBackend.

* Source/WebCore/Modules/mediastream/PeerConnectionBackend.cpp:
(WebCore::PeerConnectionBackend::setLocalDescriptionSucceeded):
(WebCore::PeerConnectionBackend::setRemoteDescriptionSucceeded):
(WebCore::toJSONObject):
(WebCore::toJSONArray):
(WebCore::toJSONString):
(WTF::LogArgument<WebCore::PeerConnectionBackend::TransceiverState>::toString):
(WTF::LogArgument<WebCore::PeerConnectionBackend::TransceiverStates>::toString):
* Source/WebCore/Modules/mediastream/PeerConnectionBackend.h:
(WebCore::PeerConnectionBackend::DescriptionStates::isolatedCopy):
* Source/WebCore/platform/mediastream/RTCRtpTransceiverDirection.h:

Canonical link: https://commits.webkit.org/279742@main
@calvaris @cadubentzen
[MediaStream][GStreamer] Hardware decoder/sinks do not have src pads
18da406 
https://bugs.webkit.org/show_bug.cgi?id=276170

Reviewed by Philippe Normand.

In the MediaStream use case, we are trying to get decoder stats from a pad probe at the src pad of the video decoder.
The problem comes with the decoder/sink elements that don't have one because they are the end of the pipeline.

* Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:
(WebCore::MediaPlayerPrivateGStreamer::configureVideoDecoder):

Canonical link: https://commits.webkit.org/280622@main
@philn @cadubentzen
[GStreamer][WebRTC] Minor improvements in incoming track processor
bd22872 
https://bugs.webkit.org/show_bug.cgi?id=276988

Reviewed by Xabier Rodriguez-Calvar.

The msid information can be present in the pad caps, so when that's the case inspecting the SDP is
not required. Also we now send the force-key-unit event only for video tracks.

* Source/WebCore/platform/mediastream/gstreamer/GStreamerIncomingTrackProcessor.cpp:
(WebCore::GStreamerIncomingTrackProcessor::configure):
(WebCore::GStreamerIncomingTrackProcessor::incomingTrackProcessor):

Canonical link: https://commits.webkit.org/281286@main
@philn @cadubentzen
[GStreamer][WebRTC] Simplified pipeline for incoming tracks
88bdf0a 
https://bugs.webkit.org/show_bug.cgi?id=276989

Reviewed by Xabier Rodriguez-Calvar.

The incoming track processor now feeds a single sink, no tee or dynamic pipeline manipulations
involved anymore. This brings back a timeout in webrtc/h265.html, but it will be fixed once we have
track events dispatching fixed (bug #275685).

* LayoutTests/platform/glib/TestExpectations:
* Source/WebCore/Modules/mediastream/gstreamer/GStreamerMediaEndpoint.cpp:
(WebCore::GStreamerMediaEndpoint::setConfiguration):
(WebCore::GStreamerMediaEndpoint::connectIncomingTrack):
(WebCore::GStreamerMediaEndpoint::connectPad):
* Source/WebCore/platform/mediastream/gstreamer/GStreamerIncomingTrackProcessor.cpp:
(WebCore::GStreamerIncomingTrackProcessor::configure):
(WebCore::GStreamerIncomingTrackProcessor::incomingTrackProcessor):
(WebCore::GStreamerIncomingTrackProcessor::createParser):
(WebCore::GStreamerIncomingTrackProcessor::stats):
* Source/WebCore/platform/mediastream/gstreamer/GStreamerIncomingTrackProcessor.h:
* Source/WebCore/platform/mediastream/gstreamer/GStreamerMediaStreamSource.cpp:
* Source/WebCore/platform/mediastream/gstreamer/GStreamerWebRTCCommon.h:
* Source/WebCore/platform/mediastream/gstreamer/RealtimeIncomingAudioSourceGStreamer.cpp:
* Source/WebCore/platform/mediastream/gstreamer/RealtimeIncomingSourceGStreamer.cpp:
(WebCore::RealtimeIncomingSourceGStreamer::RealtimeIncomingSourceGStreamer):
(WebCore::RealtimeIncomingSourceGStreamer::setBin):
(WebCore::RealtimeIncomingSourceGStreamer::registerClient):
(WebCore::RealtimeIncomingSourceGStreamer::unregisterClient):
(WebCore::RealtimeIncomingSourceGStreamer::unregisterClientLocked):
(WebCore::RealtimeIncomingSourceGStreamer::forEachClient):
(WebCore::RealtimeIncomingSourceGStreamer::handleUpstreamEvent):
(WebCore::RealtimeIncomingSourceGStreamer::handleUpstreamQuery):
(WebCore::RealtimeIncomingSourceGStreamer::handleDownstreamEvent):
(WebCore::RealtimeIncomingSourceGStreamer::setUpstreamBin): Deleted.
(WebCore::RealtimeIncomingSourceGStreamer::startProducingData): Deleted.
(WebCore::RealtimeIncomingSourceGStreamer::stopProducingData): Deleted.
(WebCore::RealtimeIncomingSourceGStreamer::configureAppSink): Deleted.
(WebCore::RealtimeIncomingSourceGStreamer::configureFakeVideoSink): Deleted.
* Source/WebCore/platform/mediastream/gstreamer/RealtimeIncomingSourceGStreamer.h:
(WebCore::RealtimeIncomingSourceGStreamer::bin const):
(WebCore::RealtimeIncomingSourceGStreamer::bin): Deleted.
(WebCore::RealtimeIncomingSourceGStreamer::setIsUpstreamDecoding): Deleted.
(WebCore::RealtimeIncomingSourceGStreamer::dispatchSample): Deleted.
* Source/WebCore/platform/mediastream/gstreamer/RealtimeIncomingVideoSourceGStreamer.cpp:
(WebCore::RealtimeIncomingVideoSourceGStreamer::setBin):
(WebCore::RealtimeIncomingVideoSourceGStreamer::dispatchSample):
(WebCore::RealtimeIncomingVideoSourceGStreamer::setUpstreamBin): Deleted.
* Source/WebCore/platform/mediastream/gstreamer/RealtimeIncomingVideoSourceGStreamer.h:

Canonical link: https://commits.webkit.org/281394@main
@philn @cadubentzen
[GStreamer][WebRTC] Apply platform-specific quirks for incoming tracks
4bfbd25 
https://bugs.webkit.org/show_bug.cgi?id=276769

Reviewed by Xabier Rodriguez-Calvar.

On platforms where quirks are required, keep hardware-accelerated parsers out of the WebRTC
pipeline. They are instead used from the playback pipeline. The LibWebRTC backend had support for
this already, this patch brings the same feature to the GstWebRTC backend.

* Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.cpp:
(WebCore::gstGetAutoplugSelectResult):
* Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.h:
* Source/WebCore/platform/mediastream/gstreamer/GStreamerIncomingTrackProcessor.cpp:
(WebCore::GStreamerIncomingTrackProcessor::createParser):
* Source/WebCore/platform/mediastream/libwebrtc/gstreamer/GStreamerVideoDecoderFactory.cpp:
(WebCore::GStreamerWebRTCVideoDecoder::getGstAutoplugSelectResult): Deleted.

Canonical link: https://commits.webkit.org/281292@main
@cadubentzen
[GStreamer][WebRTC] Emit 'track' event before resolving setRemoteDesc…
1fc1e0b 
…ription's promise

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

Reviewed by Philippe Normand.

The track event should be emitted after sucessfully applying a remote
description and before the promise from setRemoteDescription is resolved.
However, up until now the GstWebRTC backend emitted the event based on
the 'pad-added' signal. The timing of the signal doesn't match what the
spec expects.

Fix the issue by querying the transceiver states after applying a remote
description sucessfully. By providing the transceiver states to
PeerConnectionBackend::setRemoteDescriptionSucceeded(), it's able to
emit the track events correctly acccording to the spec.

A test is added to check that the track event is emitted before
setRemoteDescription resolves, which would fail before this patch in
the GstWebRTC backend.

* LayoutTests/platform/glib/TestExpectations:
* LayoutTests/platform/glib/fast/mediastream/RTCPeerConnection-inspect-answer-expected.txt:
* LayoutTests/platform/glib/fast/mediastream/RTCPeerConnection-media-setup-single-dialog-expected.txt:
* LayoutTests/platform/glib/fast/mediastream/RTCPeerConnection-setRemoteDescription-offer-expected.txt:
* LayoutTests/platform/glib/imported/w3c/web-platform-tests/webrtc/RTCPeerConnection-setLocalDescription-parameterless.https-expected.txt: Removed.
* LayoutTests/platform/glib/imported/w3c/web-platform-tests/webrtc/protocol/split.https-expected.txt: Added.
* LayoutTests/webrtc/setRemoteDescription-track-expected.txt: Added.
* LayoutTests/webrtc/setRemoteDescription-track.html: Added.
* Source/WebCore/Modules/mediastream/PeerConnectionBackend.cpp:
(WebCore::PeerConnectionBackend::setRemoteDescriptionSucceeded):
(WebCore::PeerConnectionBackend::setRemoteDescriptionFailed):
(WebCore::PeerConnectionBackend::stop):
(WebCore::PeerConnectionBackend::dispatchTrackEvent): Deleted.
(WebCore::PeerConnectionBackend::addPendingTrackEvent): Deleted.
* Source/WebCore/Modules/mediastream/PeerConnectionBackend.h:
* Source/WebCore/Modules/mediastream/gstreamer/GStreamerMediaEndpoint.cpp:
(WebCore::GStreamerMediaEndpoint::initializePipeline):
(WebCore::GStreamerMediaEndpointTransceiverState::isolatedCopy):
(WebCore::getMediaStreamIdsFromSDPMedia):
(WebCore::isRecvDirection):
(WebCore::toGStreamerMediaEndpointTransceiverState):
(WebCore::transceiverStatesFromWebRTCBin):
(WebCore::GStreamerMediaEndpoint::doSetLocalDescription):
(WebCore::GStreamerMediaEndpoint::setTransceiverCodecPreferences):
(WebCore::GStreamerMediaEndpoint::doSetRemoteDescription):
(WebCore::GStreamerMediaEndpoint::setDescription):
(WebCore::GStreamerMediaEndpoint::connectIncomingTrack):
* Source/WebCore/Modules/mediastream/gstreamer/GStreamerMediaEndpoint.h:
* Source/WebCore/Modules/mediastream/gstreamer/GStreamerPeerConnectionBackend.cpp:
(WebCore::GStreamerPeerConnectionBackend::addPendingTrackEvent): Deleted.
(WebCore::GStreamerPeerConnectionBackend::dispatchPendingTrackEvents): Deleted.
* Source/WebCore/Modules/mediastream/gstreamer/GStreamerPeerConnectionBackend.h:
* Source/WebCore/platform/mediastream/MediaStreamTrackPrivate.cpp:
(WebCore::MediaStreamTrackPrivate::dataFlowStarted):
* Source/WebCore/platform/mediastream/MediaStreamTrackPrivate.h:
(WebCore::MediaStreamTrackPrivateObserver::dataFlowStarted):
* Source/WebCore/platform/mediastream/gstreamer/GStreamerMediaStreamSource.cpp:
(webkitMediaStreamSrcCharacteristicsChanged):

Canonical link: https://commits.webkit.org/281892@main
@cadubentzen
[WebRTC] Fix build with GCC 9
06c498c
@cadubentzen
wip: do not set immediate-output=TRUE on westerossink
6fe795e
@cadubentzen cadubentzen force-pushed the cadubentzen/GstWebRTC branch from 04ad7eb to 6fe795e Compare August 30, 2024 10:56
@cadubentzen cadubentzen marked this pull request as draft September 2, 2024 08:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@emutavchi emutavchi emutavchi left review comments

@philn philn philn approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@cadubentzen @philn @emutavchi @youennf @calvaris