zishuo/webrtc
1
0
Fork 0
mirror of https://github.com/pion/webrtc.git synced 2025-10-23 15:13:28 +08:00
Code Issues Packages Projects Releases Wiki Activity

Fix MediaEngine Copy

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

Relates to #1662
This commit is contained in:
Sean DuBois
2021-02-09 21:57:34 -08:00
parent 5a6f532023
commit 8902641f91
2 changed files with 45 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
peerconnection.go
View File

@@ -105,10 +105,6 @@ func NewPeerConnection(configuration Configuration) (*PeerConnection, error) {
// NewPeerConnection creates a new PeerConnection with the provided configuration against the received API object
func (api *API) NewPeerConnection(configuration Configuration) (*PeerConnection, error) {
if !api.settingEngine.disableMediaEngineCopy {
api.mediaEngine = api.mediaEngine.copy()
}
// https://w3c.github.io/webrtc-pc/#constructor (Step #2)
// Some variables defined explicitly despite their implicit zero values to
// allow better readability to understand what is happening.
@@ -137,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 {
@@ -173,6 +175,8 @@ func (api *API) NewPeerConnection(configuration Configuration) (*PeerConnection,
}
})
pc.interceptorRTCPWriter = api.interceptor.BindRTCPWriter(interceptor.RTCPWriterFunc(pc.writeRTCP))
return pc, nil
}

38
settingengine_test.go
View File

@@ -145,7 +145,9 @@ func TestSettingEngine_SetDisableMediaEngineCopy(t *testing.T) {
m := &MediaEngine{}
assert.NoError(t, m.RegisterDefaultCodecs())
offerer, answerer, err := NewAPI(WithMediaEngine(m)).newPair(Configuration{})
api := NewAPI(WithMediaEngine(m))
offerer, answerer, err := api.newPair(Configuration{})
assert.NoError(t, err)
_, err = offerer.AddTransceiverFromKind(RTPCodecTypeVideo)
@@ -153,11 +155,30 @@ func TestSettingEngine_SetDisableMediaEngineCopy(t *testing.T) {
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) {
@@ -167,7 +188,9 @@ func TestSettingEngine_SetDisableMediaEngineCopy(t *testing.T) {
s := SettingEngine{}
s.DisableMediaEngineCopy(true)
offerer, answerer, err := NewAPI(WithMediaEngine(m), WithSettingEngine(s)).newPair(Configuration{})
api := NewAPI(WithMediaEngine(m), WithSettingEngine(s))
offerer, answerer, err := api.newPair(Configuration{})
assert.NoError(t, err)
_, err = offerer.AddTransceiverFromKind(RTPCodecTypeVideo)
@@ -175,10 +198,21 @@ func TestSettingEngine_SetDisableMediaEngineCopy(t *testing.T) {
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())
})
}