[core/media] Opus: multi-rate transcoding negotiation + RFC 7587 RTP timestamp (#2226) - #3064
[core/media] Opus: multi-rate transcoding negotiation + RFC 7587 RTP timestamp (#2226)#3064celliso1 wants to merge 4 commits into
Conversation
|
@andywolk @morbit85 this consolidates the stalled Opus@16k negotiation PRs (#2582 / #2623 / #2833) and adds the RFC 7587 timestamp fix for #2226, recording-safe (mod_opus.c untouched, avoids the samples=480 approach @ticpu flagged). A per-issue "Failure modes covered" matrix and the validation table are in the description. cc @ticpu (diagnosed the samples=480 recording/playback breakage and tested the negotiation commits), @nltd101, @shaunjstokes, @magiclin99. Authorship is preserved on the cherry-picks; a sanity-check that I represented your fixes |
0a4c720 to
c7f4f48
Compare
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> (cherry picked from commit 0646de8) Signed-off-by: Calvin Ellison <cellison@youmail.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> (cherry picked from commit cc5cca3) Signed-off-by: Calvin Ellison <cellison@youmail.com>
) RFC 7587 requires Opus RTP timestamps to advance at a fixed 48 kHz clock regardless of the payload sample rate. For 16k/8k Opus this was wrong in two coupled spots, so a same-rate transcode (AMR-WB/16000 -> Opus, as offered by some carriers) advanced the Opus timestamp at 16 kHz (320/20ms) instead of 48 kHz (960/20ms), breaking audio on endpoints that honour the RFC 7587 clock. 1. switch_core_media_set_codec(): after negotiation, switch_rtp_change_interval() reset samples_per_interval to read_impl.samples_per_packet and omitted the Opus 48 kHz clock that switch_rtp_new() already applies at init. Derive the interval from samples_per_second for Opus, matching the init path. 2. switch_core_session_write_frame() encode path: it only regenerated the RTP timestamp (instead of passing the source timestamp through) when samples_per_packet changed. A same-rate transcode keeps the same samples_per_packet (AMR-WB 16k and Opus 16k both pack 320 samples/20ms), so the 16 kHz source timestamp was passed onto the 48 kHz Opus stream. Also regenerate when samples_per_second (the RTP clock rate) differs. This reuses FreeSWITCH's existing timestamp-regeneration path rather than a per-packet timestamp rescaler, and leaves samples_per_packet at the true PCM frame count (160/80), so media bugs, recording and playback that read it are unaffected. Validated with a same-rate AMR-WB->Opus transcode: the B-leg Opus RTP timestamp delta goes from 320 to 960, with recording intact. Signed-off-by: Calvin Ellison <cellison@youmail.com>
…ire#2226) Regression test for the recording-safe property of the RFC 7587 timestamp fix. The Opus RTP clock is 48 kHz (samples_per_second == 48000) regardless of the audio rate, but samples_per_packet must stay the TRUE decoded PCM frame count (rate * ptime), because media bugs, record_session and playback size PCM frames from it. Assert the 8k/16k/48k opus implementations register 160/320/960 samples per 20 ms frame (not the 48 kHz-domain value), catching any reintroduction of a mod_opus.c samples=480 hack. Signed-off-by: Calvin Ellison <cellison@youmail.com>
c7f4f48 to
2c829fd
Compare
|
Our test setup is not available until mid to end of August, I can't try these yet but really want and need to test these as we have some situations that we still degrade to G722 to prevent issues. |
Description
Problem
Two related defects break Opus at a non-48 kHz payload rate (issue #2226):
Negotiation. When a leg offers
opus/48000constrained to 16 kHz(
maxplaybackrate=16000/sprop-maxcapturerate=16000), the codec-match loopcompares the constrained rate (16000) against Opus's registered 48 kHz rate;
because they differ it never records Opus@16k as a match, so Opus is dropped
from the candidate set. What the caller gets then depends entirely on the rest
of the offer:
alongside): negotiation falls through to it, so the call connects but on
that codec (narrowband) even though both ends support Opus@16k;
488 Not Acceptable Here.So PCMU is only the incidental fallback that happened to be in the offer, not
the defect — the defect is that Opus@16k is never matched. Regressed in 1.10.10.
RFC 7587 RTP timestamp. RFC 7587 §4.1 requires the Opus RTP timestamp to
advance at a fixed 48 kHz clock regardless of the payload rate. For a
same-rate transcode (e.g.
AMR-WB/16000↔Opus, as some carriers offer),FreeSWITCH advanced the Opus timestamp at the 16 kHz payload rate (320/20 ms)
instead of 48 kHz (960/20 ms), producing wrong-clock RTP that breaks audio on
RFC-7587-conformant endpoints. Resampled paths (e.g. PCMU 8k→Opus) were already
correct, so the bug is specific to same-rate Opus transcodes.
Fix
Commits 1–2 fix negotiation (Opus-scoped rate comparison + updating the remote
rate before the comparison loop). Commit 3 fixes the timestamp in two coupled spots:
switch_core_media_set_codec(): after negotiationswitch_rtp_change_interval()reset the RTP interval to
samples_per_packetand omitted the Opus 48 kHz clockthat
switch_rtp_new()already applies at init — mirror it.switch_core_session_write_frame(): it only regenerated the RTP timestamp (vspassing the source timestamp through) when
samples_per_packetchanged. Asame-rate transcode keeps the same
samples_per_packet(AMR-WB 16k and Opus 16kboth pack 320 samples/20 ms), so also regenerate when
samples_per_second(theRTP clock rate) differs.
Recording-safe by construction. This reuses FreeSWITCH's existing
timestamp-regeneration path and leaves
samples_per_packetat the true PCM framecount (160/80), so nothing that consumes the codec's per-frame sample count
changes. An earlier approach hard-coded
samples = 480inmod_opus.c; that alsofixes the on-wire timestamp but mislabels the per-frame sample count that media
bugs, captures and playback rely on — @ticpu reported it causing broken
playback/captures and one-way audio on #2832, which is why that approach was
abandoned. This PR does not touch
mod_opus.c. (Note:record_sessionspecifically recomputes its frame length from the byte count, so it is robust to
the
samples=480mislabel — verified — but the clean approach here avoids thedual-purpose field for all consumers.)
Failure modes covered
Every symptom across the referenced issues/PRs maps to a fix here — or is
explicitly avoided/out of scope:
"near-match" against Opus@48k). → commit 1 (Opus-scoped rate comparison, the
Update switch_core_media.c - Patch for #2226 #2623 form). Validated: Opus@16k now selected.
sprop-maxcapturerate=16000) was applied onlymid-loop, so Opus@16k didn't compare cleanly. → commit 2 (hoist
remote_codec_ratebefore the match loop). Validated: selected at the true16 kHz rate.
488on mismatched non-Opus legs. Notreintroduced: the rate override is guarded by
rm_encoding=="opus"andfmtp_remote_codec_rateis only set for Opus offers, so the non-Opusnegotiation path is byte-identical to stock. Verified in the diff.
playback glitches and one-way audio. → commit 3 (48 kHz RTP clock, RFC 7587).
Validated: B-leg ts delta 320→960.
samples=480approach broke recording, playback and captures.Avoided by construction:
mod_opus.candsamples_per_packetstay stock; thefix lives entirely in
switch_core_media.c. Validated:record_sessionWAVintact.
Out of scope (untouched, not regressed): non-Opus codec negotiation and G.722's
8 kHz-clock / 16 kHz-audio handling — the negotiation change is Opus-scoped.
Validation
Built and driven through a SIP transcoding harness (real AMR-WB media replayed
into FreeSWITCH, bridged to an Opus@16k UAS, RTP timestamps measured on the wire):
488if Opus offered alone)record_sessionType of Change
Related Issues
Credit / relationship to prior PRs
This consolidates and supersedes the stalled negotiation PRs, with credit to the
original authors (their commits are cherry-picked here, authorship preserved):
caused SIP 488 on mismatched non-Opus legs, so scoped to Opus below)
remote_codec_ratebefore comparisonsamples=480timestamp approach breaks recording/playback/captures, and tested the
negotiation commits
The RFC 7587 timestamp fix (commit 3) is the piece none of the above landed:
#2832 sidestepped the timestamp by forcing a transcode but left the clock wrong;
this PR fixes the clock itself, recording-safe.
Testing
Profiled on production PSTN to Opus VoIP app.
Checklist