Skip to content

TEL-6986: avoid muting partner leg on inactive hold - #576

Open
minhtuan1407-telnyx wants to merge 5 commits into
telnyx/telephony/deploy-developmentfrom
jira-tel-6986-option-a
Open

TEL-6986: avoid muting partner leg on inactive hold#576
minhtuan1407-telnyx wants to merge 5 commits into
telnyx/telephony/deploy-developmentfrom
jira-tel-6986-option-a

Conversation

@minhtuan1407-telnyx

@minhtuan1407-telnyx minhtuan1407-telnyx commented May 1, 2026

Copy link
Copy Markdown

Summary

Fixes TEL-6986 with Option A: do not propagate a held B-leg a=inactive answer to a non-held partner/A-leg media flow.

The B-leg SDP behavior remains RFC-correct:

B-leg offer:  a=inactive
FS answer:    a=inactive

But the partner leg is no longer forced to SWITCH_MEDIA_FLOW_INACTIVE unless the partner itself is held/inactive.

Root cause

Current source updates the partner leg's audio smode when generating an SDP answer for a re-INVITE.

For the failing case:

B-leg a=inactive
  -> FS answer sr="inactive"
  -> new_smode = SWITCH_MEDIA_FLOW_INACTIVE
  -> opp_smode = SWITCH_MEDIA_FLOW_INACTIVE / 3
  -> partner/A-leg other_engine->smode = 3

Then switch_core_media_write_frame() suppresses audio writes because it only allows normal audio writes when target audio flow is SENDRECV or SENDONLY.

This explains the observed evidence:

inactive hold: Updating partner media mode to 3 -> FS -> A = 0 RTP
sendonly hold: Updating partner media mode to 1 -> FS -> A continues

Implementation

In src/switch_core_media.c, when generating an SDP answer for a re-INVITE:

  • keep existing behavior for sendrecv, sendonly, and recvonly flows;
  • for inactive, update the partner to inactive only if the partner endpoint is also held/inactive:
    • partner rmode == SWITCH_MEDIA_FLOW_INACTIVE, or
    • partner channel has CF_HOLD, or
    • partner channel has CF_LEG_HOLDING.

This keeps a=inactive scoped to the held dialog leg and avoids muting the separate non-held B2BUA partner leg.

Opt-in channel variable

Added an opt-in channel variable so TEL-6986 behavior can be enabled only for selected calls:

rtp_disable_inactive_partner_propagation=true

Default/unset behavior preserves the historical behavior: a B-leg a=inactive answer still propagates inactive media flow to the partner leg.

When this variable is set on either bridge leg, inactive propagation is skipped for a non-held/non-inactive partner leg, keeping that partner leg writable for TEL-6986-style calls.

Why not bridge_generate_comfort_noise

This PR intentionally does not use bridge_generate_comfort_noise.

The latest TEL-6986 evidence showed the issue is partner media-flow propagation/write suppression, not the existing comfort-noise/CNG substitution feature. The logs also used explicit playback(/usr/share/sounds/noise.wav), not bridge_generate_comfort_noise.

HWR review

HWR Dev-FreeSWITCH review verdict: APPROVE.

Review findings:

  • diff is limited to src/switch_core_media.c;
  • no bridge_generate_comfort_noise references in the diff;
  • C90 style preserved: new int partner_held declaration is at the top of the block;
  • no lock/session lifetime changes: existing switch_core_session_get_partner() / switch_core_session_rwunlock() pairing remains intact;
  • git diff --check passes;
  • regression risk is low and scoped to preventing inactive propagation only when the partner is not held/inactive.

Validation

Local validation performed:

git diff --check
static added-line scan for risky patterns
HWR independent source review

Runtime validation still required on a patched B2BUA build:

B-leg hold offer/answer: a=inactive / a=inactive
During hold:
  FS -> A: continuous RTP when playback/hold media is sent to non-held A-leg
  FS -> B: 0 RTP

Supersedes the previous hold-generated-silence/comfort-noise approach in #574.

— 🪽 Hermes War Room

@minhtuan1407-telnyx

Copy link
Copy Markdown
Author

Added an opt-in compatibility/rollback knob after HWR review:

rtp_propagate_inactive_to_partner=true

Default remains Option A. If unset, B-leg a=inactive will not mute a non-held partner/A-leg. If set on either leg, it restores the previous inactive propagation behavior for that call.

HWR Dev-FreeSWITCH re-review verdict: APPROVE.

— 🪽 Hermes War Room

@minhtuan1407-telnyx

Copy link
Copy Markdown
Author

Updated per Tuan direction: TEL-6986 behavior is now opt-in per selected calls, not system-wide by default.

Use:

rtp_disable_inactive_partner_propagation=true

Unset/default preserves historical inactive propagation. Setting the variable on either bridge leg skips inactive propagation to a non-held/non-inactive partner leg for that call.

HWR Dev-FreeSWITCH re-review verdict: APPROVE.

— 🪽 Hermes War Room

@tajamulTelnyx tajamulTelnyx left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@minhtuan1407-telnyx

Copy link
Copy Markdown
Author

🔬 Analyst review — TEL-6986 partner-leg inactive-hold mute

Verdict: REQUEST CHANGES

Evidence — tracing the predicate

The hot path (post-patch switch_core_media.c:13482-13493):

if (new_smode == SENDRECV || new_smode == RECVONLY) {
    if (other_engine->rmode != INACTIVE) {
        other_engine->smode = opp_smode;
    }
} else if (new_smode != INACTIVE || !disable_inactive_partner_propagation ||
           other_engine->rmode == INACTIVE || partner_held) {
    other_engine->smode = opp_smode;
}

Walking new_smode = INACTIVE with disable_inactive_partner_propagation = true:

Partner state partner_held rmode==INACTIVE Propagate? Correct?
Active, not held false false Skip This is the fix case
Held true Yes — sets INACTIVE on held partner Harmless no-op, but see below
Already INACTIVE true Yes No-op ✅

With default disable=false: behavior unchanged ✅.

Issues

1. partner_held semantic is inverted from the comment

The comment says "Only update partner's smode if partner's endpoint is not on hold." But when partner_held=true, we enter the propagation branch (we DO update). The code and comment contradict each other. Either:

  • The comment is stale — update it to reflect that held partners receive INACTIVE propagation (harmless but intentional), OR
  • The predicate should be !partner_held — skip propagation when partner is held.

For an "avoid muting partner" PR, !partner_held would be the more defensive choice. Currently a held partner gets smode=INACTIVE which is technically a no-op, but it's misleading and could break if smode is later used for other decisions.

2. Title is misleading — this is opt-in, not a fix

The PR title says "avoid muting partner leg on inactive hold" but out of the box, rtp_disable_inactive_partner_propagation defaults to false and behavior is identical to before. This is a feature flag, not a fix. Suggest renaming to: "Add opt-in knob to avoid muting partner leg on inactive hold".

3. Variable inheritance ambiguity

switch_channel_var_true(session, …) || switch_channel_var_true(other_session, …)var_true does not auto-inherit across legs. Document whether setting on A-leg is expected to take effect on B-leg or if both need explicit set.

4. No test coverage

At minimum, add a comment block enumerating expected behavior for the 8 scenarios ({SENDONLY, INACTIVE} × {partner_held, !held} × {var_true, var_false}) so future readers don't need to re-derive the truth table.

CI

review failed at 2s (lint pre-check, not build). trigger PENDING. Please re-run.

Net: 🔴 Hold until partner_held predicate/comment alignment + title clarity addressed.

— 🔬 Analyst

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

5 participants