Skip to content

Commit

Permalink
fix: return err if codec with same payload type exists in addCodec
Browse files Browse the repository at this point in the history
  • Loading branch information
itzmanish committed Jan 22, 2025
1 parent feeeebf commit 36f7b00
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 9 deletions.
21 changes: 12 additions & 9 deletions mediaengine.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ const (
MimeTypeFlexFEC = "video/flexfec"
)

var ErrCodecAlreadyRegistered = fmt.Errorf("codec already registered for same payload type")

Check failure on line 58 in mediaengine.go

View workflow job for this annotation

GitHub Actions / lint / Go

exported: exported var ErrCodecAlreadyRegistered should have comment or be unexported (revive)

type mediaEngineHeaderExtension struct {
uri string
isAudio, isVideo bool
Expand Down Expand Up @@ -244,14 +246,14 @@ 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 && c.PayloadType == codec.PayloadType {
return codecs
if c.PayloadType == codec.PayloadType {
return codecs, ErrCodecAlreadyRegistered
}
}

return append(codecs, codec)
return append(codecs, codec), nil
}

// RegisterCodec adds codec to the MediaEngine
Expand All @@ -261,17 +263,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
Expand Down Expand Up @@ -573,9 +576,9 @@ func (m *MediaEngine) updateHeaderExtension(id int, extension string, typ RTPCod
func (m *MediaEngine) pushCodecs(codecs []RTPCodecParameters, typ RTPCodecType) {
for _, codec := range codecs {
if typ == RTPCodecTypeAudio {
m.negotiatedAudioCodecs = m.addCodec(m.negotiatedAudioCodecs, codec)
m.negotiatedAudioCodecs, _ = m.addCodec(m.negotiatedAudioCodecs, codec)
} else if typ == RTPCodecTypeVideo {
m.negotiatedVideoCodecs = m.addCodec(m.negotiatedVideoCodecs, codec)
m.negotiatedVideoCodecs, _ = m.addCodec(m.negotiatedVideoCodecs, codec)
}
}
}
Expand Down
19 changes: 19 additions & 0 deletions mediaengine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -583,7 +583,26 @@ func TestMediaEngineDoubleRegister(t *testing.T) {
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)
}

// 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,
Expand Down

0 comments on commit 36f7b00

Please sign in to comment.