Fix addCodec to return error if payload type exists in codecs list

mediaengine.go

@@ -7,6 +7,7 @@
 package webrtc
 
 import (
+	"errors"
 	"fmt"
 	"strconv"
 	"strings"
@@ -55,6 +56,9 @@
 	MimeTypeFlexFEC = "video/flexfec"
 )
 
+// ErrCodecAlreadyRegistered indicates that a codec has already been registered for the same payload type.
+var ErrCodecAlreadyRegistered = fmt.Errorf("codec already registered for same payload type")
+
 type mediaEngineHeaderExtension struct {
 	uri              string
 	isAudio, isVideo bool
@@ -244,17 +248,20 @@
 }
 
 // addCodec will append codec if it not exists.
-func (m *MediaEngine) addCodec(codecs []RTPCodecParameters, codec RTPCodecParameters) []RTPCodecParameters {
+func (m *MediaEngine) addCodec(codecs []RTPCodecParameters, codec RTPCodecParameters) ([]RTPCodecParameters, error) {
 	for _, c := range codecs {
-		if c.MimeType == codec.MimeType &&
-			fmtp.ClockRateEqual(c.MimeType, c.ClockRate, codec.ClockRate) &&
-			fmtp.ChannelsEqual(c.MimeType, c.Channels, codec.Channels) &&
-			c.PayloadType == codec.PayloadType {
-			return codecs
+		if c.PayloadType == codec.PayloadType {
+			if c.MimeType == codec.MimeType &&
+				fmtp.ClockRateEqual(c.MimeType, c.ClockRate, codec.ClockRate) &&
+				fmtp.ChannelsEqual(c.MimeType, c.Channels, codec.Channels) {
+				return codecs, nil
+			}
+
+			return codecs, ErrCodecAlreadyRegistered
 		}
 	}
 
-	return append(codecs, codec)
+	return append(codecs, codec), nil
 }
 
 // RegisterCodec adds codec to the MediaEngine
@@ -263,17 +270,18 @@
 	m.mu.Lock()
 	defer m.mu.Unlock()
 
+	var err error
 	codec.statsID = fmt.Sprintf("RTPCodec-%d", time.Now().UnixNano())
 	switch typ {
 	case RTPCodecTypeAudio:
-		m.audioCodecs = m.addCodec(m.audioCodecs, codec)
+		m.audioCodecs, err = m.addCodec(m.audioCodecs, codec)
 	case RTPCodecTypeVideo:
-		m.videoCodecs = m.addCodec(m.videoCodecs, codec)
+		m.videoCodecs, err = m.addCodec(m.videoCodecs, codec)
 	default:
 		return ErrUnknownType
 	}
 
-	return nil
+	return err
 }
 
 // RegisterHeaderExtension adds a header extension to the MediaEngine
@@ -577,14 +585,21 @@
 	return nil
 }
 
-func (m *MediaEngine) pushCodecs(codecs []RTPCodecParameters, typ RTPCodecType) {
+func (m *MediaEngine) pushCodecs(codecs []RTPCodecParameters, typ RTPCodecType) error {
+	var joinedErr error
 	for _, codec := range codecs {
+		var err error
 		if typ == RTPCodecTypeAudio {
-			m.negotiatedAudioCodecs = m.addCodec(m.negotiatedAudioCodecs, codec)
+			m.negotiatedAudioCodecs, err = m.addCodec(m.negotiatedAudioCodecs, codec)
 		} else if typ == RTPCodecTypeVideo {
-			m.negotiatedVideoCodecs = m.addCodec(m.negotiatedVideoCodecs, codec)
+			m.negotiatedVideoCodecs, err = m.addCodec(m.negotiatedVideoCodecs, codec)
+		}
+		if err != nil {
+			joinedErr = errors.Join(joinedErr, err)
 		}
 	}
+
+	return joinedErr
 }
 
 // Update the MediaEngine from a remote description.
@@ -645,13 +660,16 @@
 		// use exact matches when they exist, otherwise fall back to partial
 		switch {
 		case len(exactMatches) > 0:
-			m.pushCodecs(exactMatches, typ)
+			err = m.pushCodecs(exactMatches, typ)
 		case len(partialMatches) > 0:
-			m.pushCodecs(partialMatches, typ)
+			err = m.pushCodecs(partialMatches, typ)
 		default:
 			// no match, not negotiated
 			continue
 		}
+		if err != nil {
+			return err
+		}
 
 		if err := m.updateHeaderExtensionFromMediaSection(media); err != nil {
 			return err

mediaengine_test.go

@@ -592,6 +592,26 @@ func TestMediaEngineDoubleRegister(t *testing.T) {
 	assert.Equal(t, len(mediaEngine.audioCodecs), 1)
 }
 
+// If a user attempts to register a codec with same payload but with different
+// codec we should just discard duplicate calls.
+func TestMediaEngineDoubleRegisterDifferentCodec(t *testing.T) {
+	mediaEngine := MediaEngine{}
+
+	assert.NoError(t, mediaEngine.RegisterCodec(
+		RTPCodecParameters{
+			RTPCodecCapability: RTPCodecCapability{MimeTypeG722, 8000, 0, "", nil},
+			PayloadType:        111,
+		}, RTPCodecTypeAudio))
+
+	assert.Error(t, ErrCodecAlreadyRegistered, mediaEngine.RegisterCodec(
+		RTPCodecParameters{
+			RTPCodecCapability: RTPCodecCapability{MimeTypeOpus, 48000, 0, "", nil},
+			PayloadType:        111,
+		}, RTPCodecTypeAudio))
+
+	assert.Equal(t, len(mediaEngine.audioCodecs), 1)
+}
+
 // The cloned MediaEngine instance should be able to update negotiated header extensions.
 func TestUpdateHeaderExtenstionToClonedMediaEngine(t *testing.T) {
 	src := MediaEngine{}

rtptransceiver_test.go

@@ -18,8 +18,8 @@ func Test_RTPTransceiver_SetCodecPreferences(t *testing.T) {
 	api := NewAPI(WithMediaEngine(mediaEngine))
 	assert.NoError(t, mediaEngine.RegisterDefaultCodecs())
 
-	mediaEngine.pushCodecs(mediaEngine.videoCodecs, RTPCodecTypeVideo)
-	mediaEngine.pushCodecs(mediaEngine.audioCodecs, RTPCodecTypeAudio)
+	assert.NoError(t, mediaEngine.pushCodecs(mediaEngine.videoCodecs, RTPCodecTypeVideo))
+	assert.NoError(t, mediaEngine.pushCodecs(mediaEngine.audioCodecs, RTPCodecTypeAudio))
 
 	tr := RTPTransceiver{kind: RTPCodecTypeVideo, api: api, codecs: mediaEngine.videoCodecs}
 	assert.EqualValues(t, mediaEngine.videoCodecs, tr.getCodecs())

sdp_test.go

@@ -683,8 +683,8 @@ func TestPopulateSDP(t *testing.T) { //nolint:cyclop,maintidx
 		me := &MediaEngine{}
 		assert.NoError(t, me.RegisterDefaultCodecs())
 		api := NewAPI(WithMediaEngine(me))
-		me.pushCodecs(me.videoCodecs, RTPCodecTypeVideo)
-		me.pushCodecs(me.audioCodecs, RTPCodecTypeAudio)
+		assert.NoError(t, me.pushCodecs(me.videoCodecs, RTPCodecTypeVideo))
+		assert.NoError(t, me.pushCodecs(me.audioCodecs, RTPCodecTypeAudio))
 
 		tr := &RTPTransceiver{kind: RTPCodecTypeVideo, api: api, codecs: me.videoCodecs}
 		tr.setDirection(RTPTransceiverDirectionRecvonly)