[Core] Do not hang up the call on a write-path decoder error

[yosif111]

Description

One RTP frame that the decoder rejects currently kills an entire bridged call — both legs.

When switch_core_session_write_frame() has to transcode (an Opus leg bridged to a G.711 leg, say), the decode happens on the write path. Any decode failure lands in the default: case of the status switch, which logs Codec %s decoder error! and then goto error with the fatal status still set. switch_ivr_bridge() treats every non-SWITCH_STATUS_SUCCESS write as terminal, leaves the bridge loop, and both legs are hung up — with NORMAL_CLEARING, so nothing in the CDR or hangup-cause reporting points at the decoder.

The read path already does the right thing. switch_core_io.c logs the same error, drops the frame and keeps the call up, using a session->decoder_errors counter and a limit of 10. The write path is the only decode site that dies on the first bad frame.

It also already contains the precedent for the correct behaviour a few lines above: case SWITCH_STATUS_BREAK: sets SWITCH_STATUS_SUCCESS and jumps to the same error label — i.e. drop the frame, don't fail the write. This change makes an isolated decode error behave the same way.

The fix adds session->write_decoder_errors, returns SWITCH_STATUS_SUCCESS for the first nine consecutive failures so the undecodable frame is simply dropped, and lets the tenth propagate as before so a genuinely broken stream still terminates. The counter resets after any successful decode. It is a separate field from decoder_errors because the read and write paths use different codecs and run on different threads.

Prior history

This is a long-standing issue and the project has been round it once already:

• Reported on freeswitch-users in November 2014, no fix ever posted: https://lists.freeswitch.org/pipermail/freeswitch-users/2014-November/109328.html
• [mod_opus] Do not hangup call on decode error - fix 86a5ee3509 #982 (2020) worked around it in mod_opus by returning SWITCH_STATUS_NOOP from switch_opus_decode(), explicitly because "this makes switch_core_media hangup the call".
• [mod_opus] Fix status returned by switch_opus_decode(). Should be SWITCH_STATUS_FALSE instead of SWITCH_STATUS_NOOP. Add a unit-test. #2381 (2024) correctly reverted that, since NOOP does not mean "error" — which re-exposed the core behaviour [mod_opus] Do not hangup call on decode error - fix 86a5ee3509 #982 had been papering over.

The review discussion on both of those PRs agrees that hanging up on a decode error is wrong; the disagreement was only over which status the codec should return. This PR fixes it at the site that actually decides to tear the call down, so no codec has to lie about its status to keep a call alive.

Type of Change

• Bug fix
• New feature
• Breaking change
• Documentation update
• Code cleanup / refactor

Related Issues

Fixes #3102
Relates to #982, #2381

Testing

• Added/updated unit tests
• Tested manually

Repro recipe (enough to reproduce with sipp or any RTP injector):

1. Bridge an Opus leg to a PCMA/PCMU leg so switch_core_session_write_frame() must transcode. An armed jitter buffer (rtp_jitter_buffer_during_bridge=true) makes this much easier to hit in the wild but is not required.
2. Inject one RTP packet on the Opus leg whose payload is an Opus framing violation. Two bytes suffice: 0xFF 0x00 — a code-3 TOC with a frame count of 0, which opus_decode() rejects with OPUS_INVALID_PACKET.
3. Unpatched, the call ends within ~20 ms:

[ERR]    mod_opus.c:931          Decoder Error: corrupted stream fs:960 plc:false!
[ERR]    switch_core_media.c:16061  Codec OPUS (STANDARD) decoder error!
[DEBUG]  switch_ivr_bridge.c:819    <b-leg> ending bridge by request from write function
[NOTICE] Hangup <a-leg> [NORMAL_CLEARING]
[NOTICE] Hangup <b-leg> [NORMAL_CLEARING]

A/B tested on 1.11.1 with two builds differing only by this hunk:

Injected on the Opus leg	Unpatched	Patched
1 invalid frame	both legs hung up, NORMAL_CLEARING	same decoder error! log, frame dropped, call continues with a single 20 ms gap
12 consecutive invalid frames	hung up on the 1st	hung up on the 10th, as intended
Isolated invalid frames spread over a call	hung up on the 1st	never terminates — counter resets on each successful decode

Build verification: the branch is built against master (c1bb5c6) — ./bootstrap.sh -j && ./configure && make libfreeswitch.la on Debian bookworm/amd64.

Field data: this has been running in a production deployment carrying Opus↔G.711 transcoding bridges. Before the fix it accounted for roughly 0.5% of answered bridged calls per day being dropped mid-conversation with no diagnosable hangup cause; after the fix that class of drop is gone and the decoder error! log line still appears at the same rate, so the bad frames are being dropped rather than hidden.

Checklist

• I have read the CONTRIBUTING guidelines
• My code follows the project's style guidelines
• I have added tests for my changes (if applicable)
• I have updated documentation (if applicable)
• All existing tests pass — not run locally, leaving this to CI

Additional Notes

One deliberate difference from switch_core_io.c. On the read path the session->decoder_errors = 0; reset sits immediately after the switch, on the same fall-through the tolerated-error branch takes — so that counter is zeroed again on every tolerated error and never actually reaches the limit. Here the error branch ends in goto error, which skips the reset, so write_decoder_errors really does count consecutive failures and the give-up path is reachable. Happy to change it to match switch_core_io.c byte-for-byte instead if you'd rather have exact symmetry, or to send a follow-up fixing the read-path reset placement — just say which you prefer.

On the threshold. 10 is taken from the read path rather than chosen; it is a fixed constant there too. If you'd prefer it configurable (a global or a channel variable) I'm glad to add that.

On a unit test. I didn't add one because there is no existing harness that exercises switch_core_session_write_frame() with a bridged pair; tests/unit/switch_core_codec.c (added in #2381) covers switch_opus_decode() returning SWITCH_STATUS_FALSE, which is the input to this path but not the behaviour being fixed. Happy to add one if you can point me at the right place to hang it.

🤖 Generated with Claude Code