Skip to content

Conversation

ticpu
Copy link

@ticpu ticpu commented Jul 2, 2025

This is in relation to #2226 and its discussions.

It includes a comprehensive list of patches that were suggested in isolation but that work better as a whole.

Thanks to @celliso1, @shaunjstokes, @magiclin99 and all others who have contributed to fixing this issue.

Obsoletes: #2582, #2623.
Fixes: #2226

@celliso1
Copy link

celliso1 commented Jul 2, 2025

The commit shows redundant use of actual_samples_per_second:

				uint32_t codec_rate = imp->actual_samples_per_second;

				if (!strcasecmp(map->rm_encoding, "opus")) {
					codec_rate = imp->actual_samples_per_second;
				}`

Did you mean this instead ? So only opus is affected:

				uint32_t codec_rate = imp->samples_per_second;

				if (!strcasecmp(map->rm_encoding, "opus")) {
					codec_rate = imp->actual_samples_per_second;
				}`

See #2582 (comment) regarding #2623.

@ticpu
Copy link
Author

ticpu commented Jul 3, 2025

@celliso1 oh.. my bad. True, it does affect recordings in an unpredictable way like this. I will correct this, thank you.

@ticpu ticpu force-pushed the mod_opus_transcoding_fixes branch from ad06e74 to 8cd8876 Compare July 3, 2025 16:01
@ticpu
Copy link
Author

ticpu commented Jul 17, 2025

Can confirm this did cause issues with G722 not negotiating 8kHz vs 16kHz, we'll be testing that fix soon.

Right now, I also have other issues with our mod_oreka having weird saturated audio and are still diagnosing if it is related to this PR or FreeSWITCH update.

@ticpu ticpu marked this pull request as draft August 1, 2025 22:25
@ticpu
Copy link
Author

ticpu commented Aug 1, 2025

Moving to draft as hard-coding samples = 480 causes issues in many places other than to the call itself. Media bugs, recordings, captures are all affected.
I'm currently working on a different patch that will use switch_rtp_set_interval to fix it and looks promising.

@ticpu ticpu force-pushed the mod_opus_transcoding_fixes branch from 8cd8876 to ac8b8af Compare August 20, 2025 16:04
@ticpu ticpu force-pushed the mod_opus_transcoding_fixes branch from ac8b8af to d3b0e76 Compare September 26, 2025 14:48
shaunjstokes and others added 2 commits September 26, 2025 10:51
In vars.xml , when you use codec settings like this:
`<X-PRE-PROCESS cmd="set" data="internal_codec_prefs=OPUS@16000h@1c,PCMU"/>`

The switch_core_media.c line 5520 only takes the OPUS default rate (48k)
instead of 16k in the config.

Because of this, FS won't be able to choose opus if your client asks for 16k
opus. You will see same log symptom as signalwire#2226

We've found that the patch signalwire#2582 has the side effect of causing calls that
fail to negotiate the same codec as Leg A for Leg B to fail with SIP
488 INCOMPATIBLE_DESTINATION. That's not an option for us, we need backwards
compatibility with other codecs.

Rather than apply this for all codecs it should only apply for Opus. Now
calls using Opus and other codecs establish correctly on both legs, and there
are no issues with codec negotiation when the codecs on Leg A and Leg B don't
match.

Tested-by: Jérôme Poulin <jeromepoulin@gmail.com>
Co-authored-by: magiclin99 <magiclin99@gmail.com>
When the configured candidates are OPUS 48kHz, PCMA,..., the negotiation
unexpectedly matches OPUS 16k with OPUS 48kHz. This occurs because the
implementation updates the remote sample rate after the initial media
negotiation (cooperation loop), leading to incorrect matching.

Before:
```
2025-07-04 15:56:19.826475 96.27% [DEBUG] sofia.c:7586 Remote SDP:
v=0
o=- 3960604579 3960604579 IN IP4 52.76.213.59
s=pjmedia
b=AS:50
t=0 0
a=X-nat:0
m=audio 24598 RTP/AVP 96 120
c=IN IP4 52.76.213.59
b=TIAS:32000
a=rtpmap:96 opus/48000/2
a=fmtp:96 useinbandfec=1;maxplaybackrate=16000;sprop-maxcapturerate=16000
a=rtpmap:120 telephone-event/48000
a=fmtp:120 0-16
a=ssrc:184830022 cname:742327632f371b3d
a=rtcp:24599

switch_core_media.c:5526 Audio Codec Compare [opus:96:48000:20:0:1]/[opus:116:48000:20:0:1]
switch_core_media.c:5569 Audio Codec Compare [opus:116:48000:20:0:1] is saved as a near-match
switch_core_media.c:5526 Audio Codec Compare [opus:96:16000:20:0:1]/[PCMU:0:8000:20:64000:1]
switch_core_media.c:5526 Audio Codec Compare [opus:96:16000:20:0:1]/[G729:18:8000:20:8000:1]
switch_core_media.c:5526 Audio Codec Compare [opus:96:16000:20:0:1]/[GSM:3:8000:20:13200:1]
```

As shown in the log above, the sample rate compared with the first supported
codec is 48kHz. It should be 16kHz as configured by `sprop-maxcapturerate`.

After this patch, the sample rate is updated to 16kHz before comparison:

```
2025-07-04 17:24:02.999514 97.50% [DEBUG] sofia.c:7586 Remote SDP:
v=0
o=- 3960609842 3960609842 IN IP4 52.76.213.59
s=pjmedia
b=AS:50
t=0 0
a=X-nat:0
m=audio 19208 RTP/AVP 96 120
c=IN IP4 52.76.213.59
b=TIAS:32000
a=rtpmap:96 opus/48000/2
a=fmtp:96 useinbandfec=1;maxplaybackrate=16000;sprop-maxcapturerate=16000
a=rtpmap:120 telephone-event/48000
a=fmtp:120 0-16
a=ssrc:17356830 cname:4cb67c6474c259d7
a=rtcp:19209

switch_core_media.c:5529 Audio Codec Compare [opus:96:16000:20:0:1]/[opus:116:48000:20:0:1]
switch_core_media.c:5529 Audio Codec Compare [opus:96:16000:20:0:1]/[PCMU:0:8000:20:64000:1]
switch_core_media.c:5529 Audio Codec Compare [opus:96:16000:20:0:1]/[G729:18:8000:20:8000:1]
switch_core_media.c:5529 Audio Codec Compare [opus:96:16000:20:0:1]/[GSM:3:8000:20:13200:1]
```

Tested-By: Jérôme Poulin <jeromepoulin@gmail.com>
@ticpu ticpu force-pushed the mod_opus_transcoding_fixes branch from d3b0e76 to cc5cca3 Compare September 26, 2025 14:51
@ticpu
Copy link
Author

ticpu commented Sep 26, 2025

Update on this, even my other fix to adjust RTP scaling isn't fully reliable. Some situations still cause either recordings or audio issues. The only way I found to get around all audio issues is to go through the normal transcoding path whenever there's a frequency mismatch.

I'll be testing the patch from @nltd101 in combination with @shaunjstokes one then mark this ready if all parts of the solution end up working. I improved the commit message from PR #2833 to include the details of the fix as well.

I think both commits will be required to have a fully working system using Opus both on the SBC and internally.

Edit: I'll go ahead and test these commits on a couple softphones, then move to the PBX and SBC. We really need Opus to never do near-matches that can result in broken audio. I'll come back in about 2 weeks with the results of our testing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect OPUS negotiation
4 participants