fix(sip): fix one-way audio after long hold

[edospadoni]

Summary

This PR fixes a one-way audio bug in the SIP plugin that occurs after a call is on hold for a long time (roughly 11+ minutes).

When the call resumes, the remote party can still hear the Janus side, but the Janus side no longer receives audio from the peer. The RTP packets arrive at Janus and the SIP relay thread forwards them, but srtp_protect rejects them with srtp_err_status_replay_old before they reach the WebRTC PeerConnection, so the browser's packetsReceived counter stays frozen while packetsSent keeps growing.

Root cause

Two separate issues in plugins/janus_sip.c:

1. Relay thread not notified when only SDP direction changes

janus_sip_sdp_process() only sets session->media.updated = TRUE (and writes to the wake-up pipe) when the changed flag is set, which currently only happens on IP / port differences. When the peer answers a hold/unhold re-INVITE with the same IP and port but a changed a=sendrecv / a=recvonly direction, the relay thread is never woken up, so session->media.audio_recv stays out of date from the thread's point of view.

2. Outbound audio seq number can regress after a long hold

During hold, packets are dropped by the relay thread (audio_recv == FALSE) and never reach janus_rtp_header_update. The peer's RTP sequence number keeps advancing (~50 pps for G.711). After ~11 minutes of hold the peer's 16-bit seq_number wraps past 65535 and comes back to a lower value.

When the first packet after unhold is processed, janus_rtp_header_update maps the peer's (now wrapped) seq through acontext and produces an output seq that is lower than the last seq that was already successfully protected by pc->dtls->srtp_out. libsrtp's replay database treats the new index as "old" and srtp_protect returns srtp_err_status_replay_old; the encrypted packet is never sent to the browser.

The bug is intermittent because it only manifests when the combined active time + hold duration causes the peer's seq to wrap uint16 into a range 128–32768 below the last protected seq (SRTP's "dead zone" where ROC is not automatically adjusted).

Fix

Two minimal changes in plugins/janus_sip.c:

1. In janus_sip_sdp_process(), remember the previous audio_recv / audio_send / video_recv / video_send values and force *changed = TRUE whenever any of them differs after processing the SDP. This guarantees the relay thread is woken up on every hold/unhold transition, even when IP and port are unchanged.

2. In janus_sip_relay_thread(), introduce a local guint16 audio_out_seq that is incremented once per relayed audio packet, and overwrite the RTP header seq number with it right after janus_rtp_header_update. The timestamp continues to come from acontext (so silence/gap handling is preserved). This makes the outbound audio seq strictly monotonic regardless of what the peer does during hold, so srtp_protect can never see a regression and libsrtp's natural 65535→0 wrap continues to be handled transparently via ROC.

Total change: 1 file, 38 insertions, 9 deletions. No public API change.

Verification

Deployed in production on a NethVoice/Asterisk + Janus installation on 2026-04-17. Monitored for 7 days:

Metric	Before fix	After fix (7 days)
Errors srtp_err_status_replay_old on SIP audio handles	~100/s burst per occurrence	0
Unhold events after ≥11 min hold	each triggered the bug	50+ handled cleanly
Unhold events after ≥22 min hold (single uint16 wrap)	systematic failure	25+ handled cleanly
Unhold events after ≥44 min hold (double wrap)	—	4+ handled cleanly (longest observed: 69 min)

Across ~250+ unhold events we observed ~26 concrete cases where audio_out_seq produced a strictly monotonic value while the pre-fix code path (acontext.last_seq) would have produced a regressed value in SRTP's dead zone. All of these would have been silent audio on the browser side before the fix.

[januscla]

Thanks for your contribution, @edospadoni! Please make sure you sign our CLA, as it's a required step before we can merge this.

[lminiero]

Thanks for your contribution!

the relay thread is never woken up

I'm not sure what you mean by that. The relay thread in the SIP plugin never goes to "sleep", since it's always actively monitoring incoming sockets. The fact some packets will be ignored doesn't mean they're not received by the thread first. As such, "Relay thread not notified when only SDP direction changes" doesn't make much sense to me: no one notifies the thread, the thread is always alive, and simply monitors session->media.updated at every cycle to check if anything changed. Anyway, I'll review the code to see what you mean more precisely and comment there in case.

The SRTP issue is indeed a known one, instead, as it appeared in a few other plugins too (for different use cases that may lead to paused media that is then resumed). We fixed those issues in those other plugins, but not in the SIP plugin. Which is why I think that:

2. In janus_sip_relay_thread(), introduce a local guint16 audio_out_seq that is incremented once per relayed audio packet, and overwrite the RTP header seq number with it right after janus_rtp_header_update

is actually not the way to do that. We already have ways to tell the RTP header function to reset things because the stream was paused for a while. It's a property called seq_reset in the janus_rtp_switching_context: when set to true, itensures that the sequence number is automatically rewritten to be the one after the last we received, and after that everything will go on smoothly. This is what the VideoRoom plugin does when a subscription that was paused is resumed, for instance:

https://github.com/meetecho/janus-gateway/blob/master/src/plugins/janus_videoroom.c#L11592-L11593

As such, you shouldn't track sequence numbers at all or change things manually, or you'll mess with the way the RTP context works, which could potentially cause issues. Just set that property to true when media is supposed to be delivered again and the core will do the job for you.

[lminiero]

I see now what you mean by "not waking up the thread". You meant that a change is not detected, since direction changed but the address didn't, and so the if(session->media.updated) block in janus_sip_relay_thread isn't invoked. I still don't see why that should be an issue, since the media direction would be updated in the session media object anyway, and so the thread would simply start referring to those instead. If address and port didn't change, that block in janus_sip_relay_thread doesn't need invoking, since its only purpose is to reconnect the sockets. Can you clarify why you think it's needed instead?

[edospadoni]

@lminiero thanks for the review.

On fix #1 (*changed = TRUE for direction-only changes): you're right, it's not needed. I added it early on assuming the relay thread had to be woken up, but going back through the logs the thread was already seeing the updated audio_recv and relaying packets fine. The drops were happening later inside srtp_protect, so that change was a red herring. I'll drop it.

On fix #2: thanks for the pointer to seq_reset and the VideoRoom example, I wasn't aware of it. That's clearly the right way to do this and gives the same result as my custom counter without inventing anything new. I'll rework the patch to set session->media.acontext.seq_reset = TRUE (and the same for vcontext) inside janus_sip_sdp_process() whenever audio_recv / video_recv goes from FALSE to TRUE.

I'll push the updated patch shortly.

[edospadoni]

@lminiero done. Patch is now down to a single change in janus_sip_sdp_process(): it remembers audio_recv / video_recv, and on a FALSE → TRUE transition sets acontext.seq_reset / vcontext.seq_reset (same pattern as VideoRoom). Dropped the *changed = TRUE change as discussed.

[lminiero]

Thanks for the quick turnaround! Have you checked if this does indeed still work as expected for you?

[edospadoni]

Did a quick local test and it behaves the same as the previous patch (which makes sense since seq_reset produces the same last_seq + 1 rebase that my custom counter was doing). We're going to deploy this version on our production box and keep an eye on the logs over the next few days, will follow up if anything looks off.

[lminiero]

Thanks! As soon as you say it's ok I'll merge, backport to 0.x, and port the same fix to the NoSIP plugin too (whidh shares the same approach for media). I'll need to change the fix a bit for #3514, as there may be more than one audio and video stream there.

[edospadoni]

@lminiero we've kept it running on our production box for the past 5 days and monitored the logs across many hold/unhold cycles, including long ones. The fix looks solid. From our side it's good to merge whenever you want.

[lminiero]

@edospadoni thanks for taking the time to validate this, and thanks again for your contribution! Merging right away, and I'll then backport to 0.x and update the NoSIP plugin too 🙏

[lminiero]

Actually the NoSIP plugin doesn't have an interface for audio_recv and video_recv, but only for sending. As such, an update of that would first require us to do that, and then integrate the changes. This can be done later on, so I'll just backport to 0.x for now.

[edospadoni]

Do you have a rough idea of when this will land in a tagged release, and which version it'll be in (1.4.2)? Asking so we can plan when to move our production box off our fork and back onto an official build.

[lminiero]

It will be in 1.4.2 (and 0.16.2), but I don't know yet when a new version will be tagged, sorry.

[edospadoni]

No worries, thanks anyway. I'll keep an eye on the releases for 1.4.2 / 0.16.2. Thanks again for the quick review and merge!