diff --git a/mediaengine.go b/mediaengine.go index 6a791deca43..299726fa920 100644 --- a/mediaengine.go +++ b/mediaengine.go @@ -7,6 +7,7 @@ package webrtc import ( + "errors" "fmt" "strconv" "strings" @@ -55,6 +56,9 @@ const ( 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 @@ func (m *MediaEngine) RegisterDefaultCodecs() error { } // 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 @@ func (m *MediaEngine) RegisterCodec(codec RTPCodecParameters, typ RTPCodecType) 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 @@ func (m *MediaEngine) updateHeaderExtension(id int, extension string, typ RTPCod 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 @@ func (m *MediaEngine) updateFromRemoteDescription(desc sdp.SessionDescription) e // 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 diff --git a/mediaengine_test.go b/mediaengine_test.go index 7f0ae392781..15c273387bc 100644 --- a/mediaengine_test.go +++ b/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{} diff --git a/rtptransceiver_test.go b/rtptransceiver_test.go index b7e1a72284c..298556a6a2e 100644 --- a/rtptransceiver_test.go +++ b/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()) diff --git a/sdp_test.go b/sdp_test.go index 700f16fc52f..89caa6fd27c 100644 --- a/sdp_test.go +++ b/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)