Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: pion/webrtc
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: v3.0.6
Choose a base ref
...
head repository: pion/webrtc
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: v3.0.7
Choose a head ref
  • 3 commits
  • 6 files changed
  • 2 contributors

Commits on Feb 10, 2021

  1. Disable MediaEngine Copy by Default 

    If an API is shared between PeerConnections they would use the same
MediaEngine. A MediaEngine contains negotiated PayloadTypes so if the
PeerConnections were answering you would end up in invalid states.

Add DisableMediaEngineCopy to SettingEngine in case user needs old
behavior.

Resolves #1662
    @Sean-Der
    Sean-Der committed Feb 10, 2021
    Copy the full SHA
    c8b7aa3 View commit details

  2. Update module pion/dtls/v2 to v2.0.5 

    Generated by renovateBot
    @renovate-bot @Sean-Der
    renovate-bot authored and Sean-Der committed Feb 10, 2021
    Copy the full SHA
    5a6f532 View commit details

  3. Fix MediaEngine Copy 

    Copy the entire API. Since the MediaEngine is a pointer that would
destroy the MediaEngine that is used by other PeerConnections

Relates to #1662
    @Sean-Der
    Sean-Der committed Feb 10, 2021
    Copy the full SHA
    8902641 View commit details
Showing with 111 additions and 2 deletions.
  1. +1 −1 go.mod
  2. +6 −0 go.sum
  3. +10 −0 mediaengine.go
  4. +9 −1 peerconnection.go
  5. +8 −0 settingengine.go
  6. +77 −0 settingengine_test.go
2 changes: 1 addition & 1 deletion go.mod
View file
Original file line number Diff line number Diff line change
@@ -6,7 +6,7 @@ require (
github.com/onsi/ginkgo v1.14.2 // indirect
github.com/onsi/gomega v1.10.3 // indirect
github.com/pion/datachannel v1.4.21
github.com/pion/dtls/v2 v2.0.4
github.com/pion/dtls/v2 v2.0.5
github.com/pion/ice/v2 v2.0.15
github.com/pion/interceptor v0.0.9
github.com/pion/logging v0.2.2
6 changes: 6 additions & 0 deletions go.sum
View file
Original file line number Diff line number Diff line change
@@ -38,6 +38,8 @@ github.com/pion/datachannel v1.4.21 h1:3ZvhNyfmxsAqltQrApLPQMhSFNA+aT87RqyCq4OXm
github.com/pion/datachannel v1.4.21/go.mod h1:oiNyP4gHx2DIwRzX/MFyH0Rz/Gz05OgBlayAI2hAWjg=
github.com/pion/dtls/v2 v2.0.4 h1:WuUcqi6oYMu/noNTz92QrF1DaFj4eXbhQ6dzaaAwOiI=
github.com/pion/dtls/v2 v2.0.4/go.mod h1:qAkFscX0ZHoI1E07RfYPoRw3manThveu+mlTDdOxoGI=
github.com/pion/dtls/v2 v2.0.5 h1:jgQJRK2IJ9eWQAcUEZN4M0tnCi5X/cERnxH9J8qOjR0=
github.com/pion/dtls/v2 v2.0.5/go.mod h1:QuDII+8FVvk9Dp5t5vYIMTo7hh7uBkra+8QIm7QGm10=
github.com/pion/ice/v2 v2.0.15 h1:KZrwa2ciL9od8+TUVJiYTNsCW9J5lktBjGwW1MacEnQ=
github.com/pion/ice/v2 v2.0.15/go.mod h1:ZIiVGevpgAxF/cXiIVmuIUtCb3Xs4gCzCbXB6+nFkSI=
github.com/pion/interceptor v0.0.9 h1:fk5hTdyLO3KURQsf/+RjMpEm4NE3yeTY9Kh97b5BvwA=
@@ -88,6 +90,8 @@ golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACk
golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
golang.org/x/crypto v0.0.0-20201016220609-9e8e0b390897 h1:pLI5jrR7OSLijeIDcmRxNmw2api+jEfxLoykJVice/E=
golang.org/x/crypto v0.0.0-20201016220609-9e8e0b390897/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
golang.org/x/crypto v0.0.0-20201221181555-eec23a3978ad h1:DN0cp81fZ3njFcrLCytUHRSUkqBjfTo4Tx9RJTWs0EY=
golang.org/x/crypto v0.0.0-20201221181555-eec23a3978ad/go.mod h1:jdWPYTVW3xRLrWPugEBEK3UY2ZEsg3UU495nc5E+M+I=
golang.org/x/net v0.0.0-20180906233101-161cd47e91fd/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=
golang.org/x/net v0.0.0-20191126235420-ef20fe5d7933/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
@@ -106,13 +110,15 @@ golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5h
golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20190904154756-749cb33beabd/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20191005200804-aed5e4c7ecf9/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20191026070338-33540a1f6037/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20191120155948-bd437916bb0e/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20200323222414-85ca7c5b95cd/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20200519105757-fe76b779f299/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f h1:+Nyd8tzPX9R7BWHguqsrbFdRx3WQ/1ib8I44HXV5yTA=
golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20201119102817-f84b799fce68 h1:nxC68pudNYkKU6jWhgrqdreuFiOQWj1Fs7T3VrH4Pjw=
golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/term v0.0.0-20201117132131-f5c789dd3221/go.mod h1:Nr5EML6q2oocZ2LXRh80K7BxOlk5/8JxuGnuhpl+muw=
golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk=
10 changes: 10 additions & 0 deletions mediaengine.go
View file
Original file line number Diff line number Diff line change
@@ -271,6 +271,16 @@ func (m *MediaEngine) getHeaderExtensionID(extension RTPHeaderExtensionCapabilit
return
}

// copy copies any user modifiable state of the MediaEngine
// all internal state is reset
func (m *MediaEngine) copy() *MediaEngine {
return &MediaEngine{
videoCodecs: append([]RTPCodecParameters{}, m.videoCodecs...),
audioCodecs: append([]RTPCodecParameters{}, m.audioCodecs...),
headerExtensions: append([]mediaEngineHeaderExtension{}, m.headerExtensions...),
}
}

func (m *MediaEngine) getCodecByPayload(payloadType PayloadType) (RTPCodecParameters, RTPCodecType, error) {
for _, codec := range m.negotiatedVideoCodecs {
if codec.PayloadType == payloadType {
10 changes: 9 additions & 1 deletion peerconnection.go
View file
Original file line number Diff line number Diff line change
@@ -133,7 +133,13 @@ func (api *API) NewPeerConnection(configuration Configuration) (*PeerConnection,
log: api.settingEngine.LoggerFactory.NewLogger("pc"),
}

pc.interceptorRTCPWriter = api.interceptor.BindRTCPWriter(interceptor.RTCPWriterFunc(pc.writeRTCP))
if !api.settingEngine.disableMediaEngineCopy {
pc.api = &API{
settingEngine: api.settingEngine,
mediaEngine: api.mediaEngine.copy(),
interceptor: api.interceptor,
}
}

var err error
if err = pc.initConfiguration(configuration); err != nil {
@@ -169,6 +175,8 @@ func (api *API) NewPeerConnection(configuration Configuration) (*PeerConnection,
}
})

pc.interceptorRTCPWriter = api.interceptor.BindRTCPWriter(interceptor.RTCPWriterFunc(pc.writeRTCP))

return pc, nil
}

8 changes: 8 additions & 0 deletions settingengine.go
View file
Original file line number Diff line number Diff line change
@@ -59,6 +59,7 @@ type SettingEngine struct {
LoggerFactory logging.LoggerFactory
iceTCPMux ice.TCPMux
iceProxyDialer proxy.Dialer
disableMediaEngineCopy bool
}

// DetachDataChannels enables detaching data channels. When enabled
@@ -255,3 +256,10 @@ func (e *SettingEngine) SetICETCPMux(tcpMux ice.TCPMux) {
func (e *SettingEngine) SetICEProxyDialer(d proxy.Dialer) {
e.iceProxyDialer = d
}

// DisableMediaEngineCopy stops the MediaEngine from being copied. This allows a user to modify
// the MediaEngine after the PeerConnection has been constructed. This is useful if you wish to
// modify codecs after signaling. Make sure not to share MediaEngines between PeerConnections.
func (e *SettingEngine) DisableMediaEngineCopy(isDisabled bool) {
e.disableMediaEngineCopy = isDisabled
}
77 changes: 77 additions & 0 deletions settingengine_test.go
View file
Original file line number Diff line number Diff line change
@@ -139,3 +139,80 @@ func TestSettingEngine_SetICETCP(t *testing.T) {

assert.Equal(t, tcpMux, settingEngine.iceTCPMux)
}

func TestSettingEngine_SetDisableMediaEngineCopy(t *testing.T) {
t.Run("Copy", func(t *testing.T) {
m := &MediaEngine{}
assert.NoError(t, m.RegisterDefaultCodecs())

api := NewAPI(WithMediaEngine(m))

offerer, answerer, err := api.newPair(Configuration{})
assert.NoError(t, err)

_, err = offerer.AddTransceiverFromKind(RTPCodecTypeVideo)
assert.NoError(t, err)

assert.NoError(t, signalPair(offerer, answerer))

// Assert that the MediaEngine the user created isn't modified
assert.False(t, m.negotiatedVideo)
assert.Empty(t, m.negotiatedVideoCodecs)

// Assert that the internal MediaEngine is modified
assert.True(t, offerer.api.mediaEngine.negotiatedVideo)
assert.NotEmpty(t, offerer.api.mediaEngine.negotiatedVideoCodecs)

assert.NoError(t, offerer.Close())
assert.NoError(t, answerer.Close())

newOfferer, newAnswerer, err := api.newPair(Configuration{})
assert.NoError(t, err)

// Assert that the first internal MediaEngine hasn't been cleared
assert.True(t, offerer.api.mediaEngine.negotiatedVideo)
assert.NotEmpty(t, offerer.api.mediaEngine.negotiatedVideoCodecs)

// Assert that the new internal MediaEngine isn't modified
assert.False(t, newOfferer.api.mediaEngine.negotiatedVideo)
assert.Empty(t, newAnswerer.api.mediaEngine.negotiatedVideoCodecs)

assert.NoError(t, newOfferer.Close())
assert.NoError(t, newAnswerer.Close())
})

t.Run("No Copy", func(t *testing.T) {
m := &MediaEngine{}
assert.NoError(t, m.RegisterDefaultCodecs())

s := SettingEngine{}
s.DisableMediaEngineCopy(true)

api := NewAPI(WithMediaEngine(m), WithSettingEngine(s))

offerer, answerer, err := api.newPair(Configuration{})
assert.NoError(t, err)

_, err = offerer.AddTransceiverFromKind(RTPCodecTypeVideo)
assert.NoError(t, err)

assert.NoError(t, signalPair(offerer, answerer))

// Assert that the user MediaEngine was modified, so no copy happened
assert.True(t, m.negotiatedVideo)
assert.NotEmpty(t, m.negotiatedVideoCodecs)

assert.NoError(t, offerer.Close())
assert.NoError(t, answerer.Close())

offerer, answerer, err = api.newPair(Configuration{})
assert.NoError(t, err)

// Assert that the new internal MediaEngine was modified, so no copy happened
assert.True(t, offerer.api.mediaEngine.negotiatedVideo)
assert.NotEmpty(t, offerer.api.mediaEngine.negotiatedVideoCodecs)

assert.NoError(t, offerer.Close())
assert.NoError(t, answerer.Close())
})
}