Fixes bug where codec case name was not being parsed if lower case. #1452
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a bug where audio and video codec names were not being properly parsed when they appeared in lowercase in SDP (Session Description Protocol) data. The fix ensures case-insensitive comparison for codec names and normalizes them to uppercase.
- Updates G722 codec name comparison to be case-insensitive using
StringComparison.OrdinalIgnoreCase - Normalizes codec names to uppercase in both
AudioFormatandVideoFormatconstructors - Refactors OPUS codec format to use a predefined constant instead of manual construction
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/net/SDP/SDPAudioVideoMediaFormat.cs | Implements case-insensitive codec name parsing and uppercase normalization |
| src/net/RTP/RTPSession.cs | Adds commented logging statement for debugging |
| src/app/Media/Codecs/AudioEncoder.cs | Replaces manual OPUS format creation with predefined constant |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
| } | ||
|
|
||
| return new AudioFormat(ID, name, clockRate, rtpClockRate, channels, Fmtp); | ||
| return new AudioFormat(ID, name?.ToUpper(), clockRate, rtpClockRate, channels, Fmtp); |
There was a problem hiding this comment.
Consider using StringComparison.OrdinalIgnoreCase instead of converting to uppercase. This approach is more efficient and follows the same pattern established in the G722 comparison above.
| return new AudioFormat(ID, name?.ToUpper(), clockRate, rtpClockRate, channels, Fmtp); | |
| return new AudioFormat(ID, name, clockRate, rtpClockRate, channels, Fmtp); |
| if (TryParseRtpmap(Rtpmap, out var name, out int clockRate, out _)) | ||
| { | ||
| return new VideoFormat(ID, name, clockRate, Fmtp); | ||
| return new VideoFormat(ID, name?.ToUpper(), clockRate, Fmtp); |
There was a problem hiding this comment.
Consider using StringComparison.OrdinalIgnoreCase instead of converting to uppercase. This approach is more efficient and follows the same pattern established in the G722 comparison above.
| return new VideoFormat(ID, name?.ToUpper(), clockRate, Fmtp); | |
| return new VideoFormat(ID, name, clockRate, Fmtp); |
| { | ||
| //logger.LogTrace("SendAudio: durationRtpUnits={DurationRtpUnits}, sample size={Sample}", durationRtpUnits, sample?.Length); | ||
|
|
||
| AudioStream?.SendAudio(durationRtpUnits, sample); |
There was a problem hiding this comment.
Remove commented debug code. If this logging is needed for debugging, consider adding it as a proper feature with appropriate log levels or removing it entirely.
| AudioStream?.SendAudio(durationRtpUnits, sample); | |
| { | |
| AudioStream?.SendAudio(durationRtpUnits, sample); |
No description provided.