From 332878fa2cb57350e2e31ac91276ee1bee026ff0 Mon Sep 17 00:00:00 2001 From: sirzooro Date: Thu, 22 Jan 2026 09:48:12 +0100 Subject: [PATCH] AlwaysNegotiateDataChannels configuration flag This change implements a new Configuration.AlwaysNegotiateDataChannels flag that forces an "application" (data/SCTP) media section to be present in SDP offers even before any DataChannel is created. This is implementation of WebRTC Extensions section 15, "Always negotiating data channels". --- .gitignore | 1 + configuration.go | 4 ++ configuration_js.go | 4 ++ js_utils.go | 25 +++++++ peerconnection.go | 16 +++-- peerconnection_go_test.go | 138 ++++++++++++++++++++++++++++++++++++++ peerconnection_js.go | 26 +++---- peerconnection_test.go | 75 +++++++++++++++++---- 8 files changed, 261 insertions(+), 28 deletions(-) diff --git a/.gitignore b/.gitignore index 23945578..dc042459 100644 --- a/.gitignore +++ b/.gitignore @@ -26,3 +26,4 @@ cover.out examples/sfu-ws/cert.pem examples/sfu-ws/key.pem wasm_exec.js +webrtc.test diff --git a/configuration.go b/configuration.go index d9d82291..369493bd 100644 --- a/configuration.go +++ b/configuration.go @@ -52,4 +52,8 @@ type Configuration struct { // SDPSemantics controls the type of SDP offers accepted by and // SDP answers generated by the PeerConnection. SDPSemantics SDPSemantics `json:"sdpSemantics,omitempty"` + + // AlwaysNegotiateDataChannels specifies whether the application prefers + // to always negotiate data channels in the initial SDP offer. + AlwaysNegotiateDataChannels bool `json:"alwaysNegotiateDataChannels,omitempty"` } diff --git a/configuration_js.go b/configuration_js.go index 6111d9ae..5a88f0d8 100644 --- a/configuration_js.go +++ b/configuration_js.go @@ -37,5 +37,9 @@ type Configuration struct { // ICECandidatePoolSize describes the size of the prefetched ICE pool. ICECandidatePoolSize uint8 + // AlwaysNegotiateDataChannels specifies whether the application prefers + // to always negotiate data channels in the initial SDP offer. + AlwaysNegotiateDataChannels bool + Certificates []Certificate `json:"certificates,omitempty"` } diff --git a/js_utils.go b/js_utils.go index 07d582e1..9ee5c1ed 100644 --- a/js_utils.go +++ b/js_utils.go @@ -118,6 +118,23 @@ func valueToStrings(val js.Value) []string { return result } +func valueToBoolOrFalse(val js.Value) bool { + if val.IsNull() || val.IsUndefined() { + return false + } + + return val.Bool() +} + +func valueToBoolPointer(val js.Value) *bool { + if val.IsNull() || val.IsUndefined() { + return nil + } + b := val.Bool() + + return &b +} + func stringPointerToValue(val *string) js.Value { if val == nil { return js.Undefined() @@ -132,6 +149,14 @@ func uint16PointerToValue(val *uint16) js.Value { return js.ValueOf(*val) } +func boolToValueOrUndefined(val bool) js.Value { + if !val { + return js.Undefined() + } + + return js.ValueOf(val) +} + func boolPointerToValue(val *bool) js.Value { if val == nil { return js.Undefined() diff --git a/peerconnection.go b/peerconnection.go index 506e9c30..0fec5f76 100644 --- a/peerconnection.go +++ b/peerconnection.go @@ -250,6 +250,7 @@ func (pc *PeerConnection) initConfiguration(configuration Configuration) error { pc.configuration.ICETransportPolicy = configuration.ICETransportPolicy pc.configuration.SDPSemantics = configuration.SDPSemantics + pc.configuration.AlwaysNegotiateDataChannels = configuration.AlwaysNegotiateDataChannels sanitizedICEServers := configuration.getICEServers() if len(sanitizedICEServers) > 0 { @@ -583,6 +584,12 @@ func (pc *PeerConnection) SetConfiguration(configuration Configuration) error { // https://www.w3.org/TR/webrtc/#set-the-configuration (step #7) pc.configuration.ICETransportPolicy = configuration.ICETransportPolicy + // AlwaysNegotiateDataChannels is treated like other zero-value configuration + // fields: only a non-zero value (true) updates the existing setting. + if configuration.AlwaysNegotiateDataChannels { + pc.configuration.AlwaysNegotiateDataChannels = configuration.AlwaysNegotiateDataChannels + } + // Step #8: ICE candidate pool size is not implemented in pion/webrtc. // The value is stored in configuration but candidate pooling is not supported. @@ -2790,7 +2797,7 @@ func (pc *PeerConnection) startRTP( } pc.startRTPReceivers(remoteDesc, currentTransceivers) - if d := haveDataChannel(remoteDesc); d != nil { + if d := haveDataChannel(remoteDesc); d != nil && d.MediaName.Port.Value != 0 { pc.startSCTP(getMaxMessageSize(d)) } } @@ -2848,7 +2855,7 @@ func (pc *PeerConnection) generateUnmatchedSDP( mediaSections = append(mediaSections, mediaSection{id: "audio", transceivers: audio}) } - if pc.sctpTransport.dataChannelsRequested != 0 { + if pc.configuration.AlwaysNegotiateDataChannels || pc.sctpTransport.dataChannelsRequested != 0 { mediaSections = append(mediaSections, mediaSection{id: "data", data: true}) } } else { @@ -2859,7 +2866,7 @@ func (pc *PeerConnection) generateUnmatchedSDP( mediaSections = append(mediaSections, mediaSection{id: t.Mid(), transceivers: []*RTPTransceiver{t}}) } - if pc.sctpTransport.dataChannelsRequested != 0 { + if pc.configuration.AlwaysNegotiateDataChannels || pc.sctpTransport.dataChannelsRequested != 0 { mediaSections = append(mediaSections, mediaSection{id: strconv.Itoa(len(mediaSections)), data: true}) } } @@ -3019,7 +3026,8 @@ func (pc *PeerConnection) generateMatchedSDP( } } - if pc.sctpTransport.dataChannelsRequested != 0 && !alreadyHaveApplicationMediaSection { + if (pc.configuration.AlwaysNegotiateDataChannels || pc.sctpTransport.dataChannelsRequested != 0) && + !alreadyHaveApplicationMediaSection { if detectedPlanB { mediaSections = append(mediaSections, mediaSection{id: "data", data: true}) } else { diff --git a/peerconnection_go_test.go b/peerconnection_go_test.go index 20ee6bd5..4dfed479 100644 --- a/peerconnection_go_test.go +++ b/peerconnection_go_test.go @@ -34,6 +34,7 @@ import ( "github.com/pion/webrtc/v4/internal/util" "github.com/pion/webrtc/v4/pkg/rtcerr" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) // newPair creates two new peer connections (an offerer and an answerer) using @@ -296,6 +297,16 @@ func TestPeerConnection_SetConfiguration_Go(t *testing.T) { } } +func TestPeerConnection_GetConfiguration_Go(t *testing.T) { + pc, err := NewPeerConnection(Configuration{}) + assert.NoError(t, err) + + cfg := pc.GetConfiguration() + assert.Equal(t, false, cfg.AlwaysNegotiateDataChannels) + + assert.NoError(t, pc.Close()) +} + func TestPeerConnection_EventHandlers_Go(t *testing.T) { lim := test.TimeOut(time.Second * 5) defer lim.Stop() @@ -2530,3 +2541,130 @@ func TestCreateAnswerPassiveOfferActiveAnswer(t *testing.T) { assert.Equal(t, answerRole, DTLSRoleClient) assert.NoError(t, pc.Close()) } + +func TestAlwaysNegotiateDataChannel_InitialOffer_Go(t *testing.T) { + pcDefault, err := NewPeerConnection(Configuration{}) + assert.NoError(t, err) + + offerDefault, err := pcDefault.CreateOffer(nil) + assert.NoError(t, err) + assert.Nil(t, haveDataChannel(&offerDefault)) + assert.NoError(t, pcDefault.Close()) + + pc, err := NewPeerConnection(Configuration{AlwaysNegotiateDataChannels: true}) + assert.NoError(t, err) + + offer, err := pc.CreateOffer(nil) + assert.NoError(t, err) + assert.NotNil(t, haveDataChannel(&offer)) + assert.NoError(t, pc.Close()) +} + +func TestAlwaysNegotiateDataChannels_CreateDataChannel(t *testing.T) { //nolint:cyclop + lim := test.TimeOut(time.Second * 30) + defer lim.Stop() + + report := test.CheckRoutines(t) + defer report() + + cfg := Configuration{AlwaysNegotiateDataChannels: true} + + pcOffer, err := NewPeerConnection(cfg) + require.NoError(t, err) + pcAnswer, err := NewPeerConnection(cfg) + require.NoError(t, err) + + defer closePairNow(t, pcOffer, pcAnswer) + + negotiationNeeded := make(chan struct{}, 1) + pcOffer.OnNegotiationNeeded(func() { + select { + case negotiationNeeded <- struct{}{}: + default: + } + }) + + remoteDataChannel := make(chan *DataChannel, 1) + pcAnswer.OnDataChannel(func(dc *DataChannel) { + select { + case remoteDataChannel <- dc: + default: + } + }) + + connectedWG := untilConnectionState(PeerConnectionStateConnected, pcOffer, pcAnswer) + require.NoError(t, signalPairWithOptions(pcOffer, pcAnswer, withDisableInitialDataChannel(true))) + + connected := make(chan struct{}) + go func() { + connectedWG.Wait() + close(connected) + }() + + select { + case <-connected: + case <-time.After(10 * time.Second): + assert.FailNow(t, "connection establishment timed out") + } + + // Verify no data channels initially exist + pcOffer.sctpTransport.lock.Lock() + offerDCCount := len(pcOffer.sctpTransport.dataChannels) + pcOffer.sctpTransport.lock.Unlock() + pcAnswer.sctpTransport.lock.Lock() + answerDCCount := len(pcAnswer.sctpTransport.dataChannels) + pcAnswer.sctpTransport.lock.Unlock() + require.Equal(t, 0, offerDCCount) + require.Equal(t, 0, answerDCCount) + + select { + case <-remoteDataChannel: + assert.FailNow(t, "unexpected OnDataChannel before CreateDataChannel") + case <-time.After(100 * time.Millisecond): + } + + // Now create a data channel and verify it works as expected + localDC, err := pcOffer.CreateDataChannel("post-connect", nil) + require.NoError(t, err) + + localOpened := make(chan struct{}, 1) + localDC.OnOpen(func() { + select { + case localOpened <- struct{}{}: + default: + } + }) + + select { + case <-negotiationNeeded: + assert.FailNow(t, "unexpected OnNegotiationNeeded for CreateDataChannel") + case <-time.After(250 * time.Millisecond): + } + + var remoteDC *DataChannel + select { + case remoteDC = <-remoteDataChannel: + case <-time.After(5 * time.Second): + assert.FailNow(t, "timed out waiting for remote OnDataChannel") + } + + remoteOpened := make(chan struct{}, 1) + remoteDC.OnOpen(func() { + select { + case remoteOpened <- struct{}{}: + default: + } + }) + + select { + case <-localOpened: + case <-time.After(5 * time.Second): + assert.FailNow(t, "timed out waiting for local data channel open") + } + + select { + case <-remoteOpened: + case <-time.After(5 * time.Second): + assert.FailNow(t, "timed out waiting for remote data channel open") + } +} diff --git a/peerconnection_js.go b/peerconnection_js.go index 99c90d10..87460bdd 100644 --- a/peerconnection_js.go +++ b/peerconnection_js.go @@ -561,12 +561,13 @@ func (pc *PeerConnection) SCTP() *SCTPTransport { // js.Undefined(), which will result in the default value being used. func configurationToValue(configuration Configuration) js.Value { return js.ValueOf(map[string]any{ - "iceServers": iceServersToValue(configuration.ICEServers), - "iceTransportPolicy": stringEnumToValueOrUndefined(configuration.ICETransportPolicy.String()), - "bundlePolicy": stringEnumToValueOrUndefined(configuration.BundlePolicy.String()), - "rtcpMuxPolicy": stringEnumToValueOrUndefined(configuration.RTCPMuxPolicy.String()), - "peerIdentity": stringToValueOrUndefined(configuration.PeerIdentity), - "iceCandidatePoolSize": uint8ToValueOrUndefined(configuration.ICECandidatePoolSize), + "iceServers": iceServersToValue(configuration.ICEServers), + "iceTransportPolicy": stringEnumToValueOrUndefined(configuration.ICETransportPolicy.String()), + "bundlePolicy": stringEnumToValueOrUndefined(configuration.BundlePolicy.String()), + "rtcpMuxPolicy": stringEnumToValueOrUndefined(configuration.RTCPMuxPolicy.String()), + "peerIdentity": stringToValueOrUndefined(configuration.PeerIdentity), + "iceCandidatePoolSize": uint8ToValueOrUndefined(configuration.ICECandidatePoolSize), + "alwaysNegotiateDataChannels": boolToValueOrUndefined(configuration.AlwaysNegotiateDataChannels), // Note: Certificates are not currently supported. // "certificates": configuration.Certificates, @@ -616,12 +617,13 @@ func valueToConfiguration(configValue js.Value) Configuration { return Configuration{} } return Configuration{ - ICEServers: valueToICEServers(configValue.Get("iceServers")), - ICETransportPolicy: NewICETransportPolicy(valueToStringOrZero(configValue.Get("iceTransportPolicy"))), - BundlePolicy: newBundlePolicy(valueToStringOrZero(configValue.Get("bundlePolicy"))), - RTCPMuxPolicy: newRTCPMuxPolicy(valueToStringOrZero(configValue.Get("rtcpMuxPolicy"))), - PeerIdentity: valueToStringOrZero(configValue.Get("peerIdentity")), - ICECandidatePoolSize: valueToUint8OrZero(configValue.Get("iceCandidatePoolSize")), + ICEServers: valueToICEServers(configValue.Get("iceServers")), + ICETransportPolicy: NewICETransportPolicy(valueToStringOrZero(configValue.Get("iceTransportPolicy"))), + BundlePolicy: newBundlePolicy(valueToStringOrZero(configValue.Get("bundlePolicy"))), + RTCPMuxPolicy: newRTCPMuxPolicy(valueToStringOrZero(configValue.Get("rtcpMuxPolicy"))), + PeerIdentity: valueToStringOrZero(configValue.Get("peerIdentity")), + ICECandidatePoolSize: valueToUint8OrZero(configValue.Get("iceCandidatePoolSize")), + AlwaysNegotiateDataChannels: valueToBoolOrFalse(configValue.Get("alwaysNegotiateDataChannels")), // Note: Certificates are not supported. // Certificates []Certificate diff --git a/peerconnection_test.go b/peerconnection_test.go index de5eaee7..88e4a366 100644 --- a/peerconnection_test.go +++ b/peerconnection_test.go @@ -31,17 +31,46 @@ func newPair() (pcOffer *PeerConnection, pcAnswer *PeerConnection, err error) { return pca, pcb, nil } -func signalPairWithModification( +type signalPairOptions struct { + disableInitialDataChannel bool + modificationFunc func(string) string +} + +func withModificationFunc(f func(string) string) func(*signalPairOptions) { + return func(o *signalPairOptions) { + o.modificationFunc = f + } +} + +func withDisableInitialDataChannel(disable bool) func(*signalPairOptions) { + return func(o *signalPairOptions) { + o.disableInitialDataChannel = disable + } +} + +func signalPairWithOptions( pcOffer *PeerConnection, pcAnswer *PeerConnection, - modificationFunc func(string) string, + opts ...func(*signalPairOptions), ) error { - // Note(albrow): We need to create a data channel in order to trigger ICE - // candidate gathering in the background for the JavaScript/Wasm bindings. If - // we don't do this, the complete offer including ICE candidates will never be - // generated. - if _, err := pcOffer.CreateDataChannel("initial_data_channel", nil); err != nil { - return err + var options signalPairOptions + for _, o := range opts { + o(&options) + } + + modificationFunc := options.modificationFunc + if modificationFunc == nil { + modificationFunc = func(s string) string { return s } + } + + if !options.disableInitialDataChannel { + // Note(albrow): We need to create a data channel in order to trigger ICE + // candidate gathering in the background for the JavaScript/Wasm bindings. If + // we don't do this, the complete offer including ICE candidates will never be + // generated. + if _, err := pcOffer.CreateDataChannel("initial_data_channel", nil); err != nil { + return err + } } offer, err := pcOffer.CreateOffer(nil) @@ -72,6 +101,18 @@ func signalPairWithModification( return pcOffer.SetRemoteDescription(*pcAnswer.LocalDescription()) } +func signalPairWithModification( + pcOffer *PeerConnection, + pcAnswer *PeerConnection, + modificationFunc func(string) string, +) error { + return signalPairWithOptions( + pcOffer, + pcAnswer, + withModificationFunc(modificationFunc), + ) +} + func signalPair(pcOffer *PeerConnection, pcAnswer *PeerConnection) error { return signalPairWithModification( pcOffer, @@ -168,10 +209,11 @@ func TestPeerConnection_SetConfiguration(t *testing.T) { Username: "unittest", }, }, - ICETransportPolicy: ICETransportPolicyAll, - BundlePolicy: BundlePolicyBalanced, - RTCPMuxPolicy: RTCPMuxPolicyRequire, - ICECandidatePoolSize: 5, + ICETransportPolicy: ICETransportPolicyAll, + BundlePolicy: BundlePolicyBalanced, + RTCPMuxPolicy: RTCPMuxPolicyRequire, + ICECandidatePoolSize: 5, + AlwaysNegotiateDataChannels: true, }) if err != nil { return pc, err @@ -251,6 +293,14 @@ func TestPeerConnection_SetConfiguration(t *testing.T) { }, wantErr: &rtcerr.InvalidModificationError{Err: ErrModifyingICECandidatePoolSize}, }, + { + name: "enable AlwaysNegotiateDataChannels", + init: func() (*PeerConnection, error) { + return NewPeerConnection(Configuration{}) + }, + config: Configuration{AlwaysNegotiateDataChannels: true}, + wantErr: nil, + }, } { pc, err := test.init() assert.NoError(t, err, "SetConfiguration %q: init failed", test.name) @@ -285,6 +335,7 @@ func TestPeerConnection_GetConfiguration(t *testing.T) { // See: https://github.com/pion/webrtc/issues/513. // assert.Equal(t, len(expected.Certificates), len(actual.Certificates)) assert.Equal(t, expected.ICECandidatePoolSize, actual.ICECandidatePoolSize) + assert.False(t, actual.AlwaysNegotiateDataChannels) assert.NoError(t, pc.Close()) }