Repository: pion/webrtc (go)

Issue:
Guard against MediaEngine being shared between PeerConnections
A `MediaEngine` should only be used by one PeerConnection. We need to find a way to assert that it has never been used before.

Pull request description:
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

#### Description

#### Reference issue
Fixes #...


Patch:
diff --git a/mediaengine.go b/mediaengine.go
index 28449eb4..9bc8490f 100644
--- a/mediaengine.go
+++ b/mediaengine.go
@@ -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 {
diff --git a/peerconnection.go b/peerconnection.go
index ca845fc5..38d98c08 100644
--- a/peerconnection.go
+++ b/peerconnection.go
@@ -105,6 +105,10 @@ 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.
diff --git a/settingengine.go b/settingengine.go
index dbbd3f3e..8091e6a2 100644
--- a/settingengine.go
+++ b/settingengine.go
@@ -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
+}


Test patch:
diff --git a/settingengine_test.go b/settingengine_test.go
index 892b07a0..e0e99bae 100644
--- a/settingengine_test.go
+++ b/settingengine_test.go
@@ -139,3 +139,46 @@ 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())
+
+		offerer, answerer, err := NewAPI(WithMediaEngine(m)).newPair(Configuration{})
+		assert.NoError(t, err)
+
+		_, err = offerer.AddTransceiverFromKind(RTPCodecTypeVideo)
+		assert.NoError(t, err)
+
+		assert.NoError(t, signalPair(offerer, answerer))
+
+		assert.False(t, m.negotiatedVideo)
+		assert.Empty(t, m.negotiatedVideoCodecs)
+
+		assert.NoError(t, offerer.Close())
+		assert.NoError(t, answerer.Close())
+	})
+
+	t.Run("No Copy", func(t *testing.T) {
+		m := &MediaEngine{}
+		assert.NoError(t, m.RegisterDefaultCodecs())
+
+		s := SettingEngine{}
+		s.DisableMediaEngineCopy(true)
+
+		offerer, answerer, err := NewAPI(WithMediaEngine(m), WithSettingEngine(s)).newPair(Configuration{})
+		assert.NoError(t, err)
+
+		_, err = offerer.AddTransceiverFromKind(RTPCodecTypeVideo)
+		assert.NoError(t, err)
+
+		assert.NoError(t, signalPair(offerer, answerer))
+
+		assert.True(t, m.negotiatedVideo)
+		assert.NotEmpty(t, m.negotiatedVideoCodecs)
+
+		assert.NoError(t, offerer.Close())
+		assert.NoError(t, answerer.Close())
+	})
+}