From 8774bcbe78a412402a086156bb79478395130dba Mon Sep 17 00:00:00 2001 From: Dragos Vingarzan Date: Sun, 4 Oct 2026 13:44:22 +0200 Subject: [PATCH 1/4] fix: avoid input DTX buffering for permanent audio mixers --- daemon/codec.c | 22 +++++++-- include/codec.h | 1 + t/Makefile | 9 +++- t/auto-daemon-tests-mixer-dtx.pl | 85 ++++++++++++++++++++++++++++++++ 4 files changed, 111 insertions(+), 6 deletions(-) create mode 100644 t/auto-daemon-tests-mixer-dtx.pl diff --git a/daemon/codec.c b/daemon/codec.c index 68fd50e71..5c3496c4c 100644 --- a/daemon/codec.c +++ b/daemon/codec.c @@ -894,8 +894,12 @@ static void __make_transcoder(struct codec_handler *handler, rtp_payload_type *d cn_payload_type, packet_decoded_encode, __ssrc_handler_transcode_new); } static bool __make_audio_player_decoder(struct codec_handler *handler, rtp_payload_type *dest, - bool pcm_dtmf_detect) + bool pcm_dtmf_detect, bool mixer_clock) { + // Changing clock ownership must discard an existing input DTX queue as well as its decoder. + if (handler->mixer_clock != mixer_clock && handler->transcoder) + __handler_shutdown(handler); + handler->mixer_clock = mixer_clock; return __make_transcoder_full(handler, dest, NULL, -1, pcm_dtmf_detect, -1, packet_decoded_audio_player, __ssrc_handler_decode_new); } @@ -1426,7 +1430,11 @@ static GTree *codec_make_prefs_tree(rtp_payload_type *pt, struct codec_store *cs } static bool codec_update_audio_player_flags(struct call_media *sink, const sdp_ng_flags *flags) { - bool explicit_player = MEDIA_ISSET(sink, AUDIO_PLAYER) && !MEDIA_ISSET(sink, AUDIO_PLAYER_IMPLICIT); + // Permanent selection overrides an earlier implicit player and implicit global defaults. + bool explicit_player = rtpe_config.use_audio_player == UAP_ALWAYS + || (flags && flags->audio_player == AP_FORCE) + || ((!flags || flags->audio_player == AP_DEFAULT) + && MEDIA_ISSET(sink, AUDIO_PLAYER) && !MEDIA_ISSET(sink, AUDIO_PLAYER_IMPLICIT)); MEDIA_CLEAR(sink, AUDIO_PLAYER_IMPLICIT); if (explicit_player) return true; @@ -1459,6 +1467,7 @@ void __codec_handlers_update(struct call_media *source, struct call_media *sink, return; bool use_audio_player = codec_update_audio_player_flags(sink, a.flags); + bool permanent_audio_player = use_audio_player && !MEDIA_ISSET(sink, AUDIO_PLAYER_IMPLICIT); /* required for updating the transcoding attrs of subscriber */ struct media_subscription *ms = call_get_media_subscription(source->media_subscribers_ht, sink); @@ -1855,7 +1864,7 @@ sink_pt_fixed:; sink_dtmf_pt ? sink_dtmf_pt->payload_type : -1, pcm_dtmf_detect, sink_cn_pt ? sink_cn_pt->payload_type : -1); else - __make_audio_player_decoder(handler, sink_pt, pcm_dtmf_detect); + __make_audio_player_decoder(handler, sink_pt, pcm_dtmf_detect, permanent_audio_player); // for DTMF delay: we pretend that there is no output DTMF payload type (sink_dtmf_pt == NULL) // so that DTMF is converted to audio (so it can be replaced with silence). we still want // to output DTMF event packets when we can though, so we need to remember the DTMF payload @@ -1897,7 +1906,7 @@ sink_pt_fixed:; // change all passthrough handlers also to transcoders while (passthrough_handlers) { struct codec_handler *handler = passthrough_handlers->data; - if (!__make_audio_player_decoder(handler, pref_dest_codec, false)) + if (!__make_audio_player_decoder(handler, pref_dest_codec, false, permanent_audio_player)) __convert_passthrough_ssrc(handler); passthrough_handlers = g_slist_delete_link(passthrough_handlers, passthrough_handlers); @@ -3402,6 +3411,11 @@ static void __buffer_delay_seq(struct delay_buffer *dbuf, struct media_packet *m static bool __dtx_should_do(struct codec_ssrc_handler *ch) { if (!ch) return false; + // Permanent mixers already generate continuous output and fill silent intervals. Scheduling + // their inputs through a second DTX clock can accumulate source-specific delay during AMR SID gaps. + // Keep the existing DTX path for ordinary transcoding and implicitly enabled audio players. + if (ch->handler->mixer_clock && ch->handler->packet_decoded == packet_decoded_audio_player) + return false; if (!ch->decoder) return false; if (!decoder_has_dtx(ch->decoder)) diff --git a/include/codec.h b/include/codec.h index 074a3c2c2..fa23e43b5 100644 --- a/include/codec.h +++ b/include/codec.h @@ -60,6 +60,7 @@ struct codec_handler { bool transcoder:1; bool pcm_dtmf_detect:1; bool blackhole:1; + bool mixer_clock:1; // permanent mixer supplies output timing and silence independently size_t payload_len; // for short-packet blocking diff --git a/t/Makefile b/t/Makefile index db85b9593..e6baf5297 100644 --- a/t/Makefile +++ b/t/Makefile @@ -90,7 +90,7 @@ include ../lib/common.Makefile daemon-tests-bundle daemon-tests-dtls daemon-tests-rollback \ daemon-tests-rollback-redis daemon-tests-redis-srtp-index \ daemon-tests-recording daemon-tests-create daemon-tests-created-subscriptions daemon-tests-alias \ - daemon-tests-kernel-api \ + daemon-tests-kernel-api daemon-tests-mixer-dtx \ daemon-tests-video daemon-tests-inject daemon-tests-dtx-dtmf TESTS := test-bitstr aes-crypt aead-aes-crypt test-const_str_hash.strhash @@ -143,7 +143,7 @@ daemon-tests: daemon-tests-main daemon-tests-jb daemon-tests-pubsub daemon-tests daemon-tests-bundle daemon-tests-dtls daemon-tests-rollback daemon-tests-rollback-redis \ daemon-tests-redis-srtp-index \ daemon-tests-recording daemon-tests-create daemon-tests-created-subscriptions \ - daemon-tests-dtx daemon-tests-dtx-cn daemon-tests-dtx-no-shift daemon-tests-dtx-dtmf \ + daemon-tests-dtx daemon-tests-dtx-cn daemon-tests-dtx-no-shift daemon-tests-dtx-dtmf daemon-tests-mixer-dtx \ daemon-tests-alias \ daemon-tests-kernel-api \ daemon-tests-video daemon-tests-inject @@ -184,6 +184,11 @@ daemon-tests-dtx: daemon-test-deps daemon-tests-dtx-dtmf: daemon-test-deps ./auto-test-helper "$@" perl -I../perl auto-daemon-tests-dtx-dtmf.pl +daemon-tests-mixer-dtx: daemon-test-deps + ./auto-test-helper "$@" perl -I../perl auto-daemon-tests-mixer-dtx.pl + ./auto-test-helper "$@-transcoding" perl -I../perl auto-daemon-tests-mixer-dtx.pl --audio-player=transcoding + ./auto-test-helper "$@-play-media" perl -I../perl auto-daemon-tests-mixer-dtx.pl --audio-player=play-media + daemon-tests-dtx-no-shift: daemon-test-deps ./auto-test-helper "$@" perl -I../perl auto-daemon-tests-dtx-no-shift.pl diff --git a/t/auto-daemon-tests-mixer-dtx.pl b/t/auto-daemon-tests-mixer-dtx.pl new file mode 100644 index 000000000..51e233fc8 --- /dev/null +++ b/t/auto-daemon-tests-mixer-dtx.pl @@ -0,0 +1,85 @@ +#!/usr/bin/perl + +# Conference mixer timing regression. + +use strict; +use warnings; +use NGCP::Rtpengine::Test; +use NGCP::Rtpclient::SRTP; +use NGCP::Rtpengine::AutoTest; +use Test::More; +use Time::HiRes qw(time); +use IO::Select; + +# A deliberately large DTX delay makes clock ownership observable without AMR fixtures or long calls. +autotest_start(qw(--config-file=none -t -1 -i 203.0.113.1 -n 2223 -f -L 7 -E + --dtx-delay=200 --max-dtx=1), @ARGV) or die; + +# Reuse the call and SSRC to cover decoder/DTX cleanup when clock ownership changes. +my ($source, $listener) = new_call([qw(198.51.100.10 5000)], [qw(198.51.100.10 5002)]); +my $phase = 0; +for my $mode ('always', 'transcoding', 'always', 'off', 'always') { + my $name = "DTX clock ownership: $phase $mode"; + my $offer = rtpe_req('offer', $name, { + 'from-tag' => ft(), + 'audio player' => $mode, + codec => { transcode => ['L16/16000/1'] }, + sdp => "v=0\r\no=- 1 1 IN IP4 198.51.100.10\r\ns=test\r\nt=0 0\r\n" . + "c=IN IP4 198.51.100.10\r\nm=audio 5000 RTP/AVP 0\r\na=sendrecv\r\n", + }); + my $answer = rtpe_req('answer', $name, { + 'from-tag' => ft(), + 'to-tag' => tt(), + 'audio player' => $mode, + sdp => "v=0\r\no=- 1 1 IN IP4 198.51.100.10\r\ns=test\r\nt=0 0\r\n" . + "c=IN IP4 198.51.100.10\r\nm=audio 5002 RTP/AVP 96\r\n" . + "a=rtpmap:96 L16/16000/1\r\na=sendrecv\r\n", + }); + my ($input_port) = $answer->{sdp} =~ /m=audio (\d+)/; + ok($input_port, 'source media address was negotiated'); + my $select = IO::Select->new($listener); + while ($select->can_read(0)) { + my $discard; + $listener->recv($discard, 65535); + } + my $start = time(); + my $seq = 100 + $phase * 100; + my $ts = 100000 + $phase * 16000; + # Two paced frames also populate the recent payload tracker used by the DTX timer. + snd($source, $input_port, rtp(0, $seq, $ts, 0x5678, "\x40" x 160)); + Time::HiRes::sleep(0.02); + snd($source, $input_port, rtp(0, $seq + 1, $ts + 160, 0x5678, "\x40" x 160)); + my ($delay, $last_ts); + while (time() - $start < 0.6) { + next unless $select->can_read(0.02); + my $packet; + $listener->recv($packet, 65535); + next unless length($packet) >= 12; + my ($pt, $ts) = unpack('x C x2 N', $packet); + next unless ($pt & 0x7f) == 96; + if (defined $last_ts && $mode ne 'off') { + is(($ts - $last_ts) & 0xffffffff, 320, 'mixer output clock remains continuous'); + } + $last_ts = $ts; + my @samples = unpack('s>*', substr($packet, 12)); + if (grep { abs($_) > 500 } @samples) { + $delay = time() - $start; + last; + } + } + ok(defined $delay, 'speech reaches the listener'); + if (defined $delay) { + if ($mode eq 'always') { + cmp_ok($delay, '<', 0.14, 'permanent mixer does not queue input behind the DTX timer'); + } + else { + cmp_ok($delay, '>=', 0.18, 'ordinary and implicit transcoders retain configured DTX handling'); + } + } + # Let both tone frames leave the mixer before measuring the next signalling update. + Time::HiRes::sleep(0.3); + $phase++; +} + +rtpe_req('delete', 'DTX clock ownership cleanup', {'from-tag' => ft()}); +done_testing(); From 4f655e587fe6b6dd7a6a4080d1e72068199ddd18 Mon Sep 17 00:00:00 2001 From: Dragos Vingarzan Date: Fri, 9 Oct 2026 15:58:15 +0200 Subject: [PATCH 2/4] test: verify mixer clock continuity per SSRC and reject stale phase audio --- t/auto-daemon-tests-mixer-dtx.pl | 42 ++++++++++++++++++++++++-------- 1 file changed, 32 insertions(+), 10 deletions(-) diff --git a/t/auto-daemon-tests-mixer-dtx.pl b/t/auto-daemon-tests-mixer-dtx.pl index 51e233fc8..25489e8a1 100644 --- a/t/auto-daemon-tests-mixer-dtx.pl +++ b/t/auto-daemon-tests-mixer-dtx.pl @@ -45,29 +45,51 @@ my $start = time(); my $seq = 100 + $phase * 100; my $ts = 100000 + $phase * 16000; + # Distinct PCMU levels decode to -3900, -3388, -2876, -2364 and -1884 respectively. + # Recognise this phase's audio instead of accepting late speech from the previous configuration. + my $payload = chr(0x30 + $phase * 4) x 160; + my $sample = (-3900, -3388, -2876, -2364, -1884)[$phase]; # Two paced frames also populate the recent payload tracker used by the DTX timer. - snd($source, $input_port, rtp(0, $seq, $ts, 0x5678, "\x40" x 160)); + snd($source, $input_port, rtp(0, $seq, $ts, 0x5678, $payload)); Time::HiRes::sleep(0.02); - snd($source, $input_port, rtp(0, $seq + 1, $ts + 160, 0x5678, "\x40" x 160)); - my ($delay, $last_ts); + snd($source, $input_port, rtp(0, $seq + 1, $ts + 160, 0x5678, $payload)); + my ($delay, $speech_ssrc, $last_seq, $last_ts); + my $clock_intervals = 0; while (time() - $start < 0.6) { next unless $select->can_read(0.02); my $packet; $listener->recv($packet, 65535); next unless length($packet) >= 12; - my ($pt, $ts) = unpack('x C x2 N', $packet); + my ($pt, $out_seq, $out_ts, $ssrc) = unpack('x C n N N', $packet); next unless ($pt & 0x7f) == 96; - if (defined $last_ts && $mode ne 'off') { - is(($ts - $last_ts) & 0xffffffff, 320, 'mixer output clock remains continuous'); - } - $last_ts = $ts; my @samples = unpack('s>*', substr($packet, 12)); - if (grep { abs($_) > 500 } @samples) { + if (!defined $delay && (grep { abs($_ - $sample) < 20 } @samples) >= 16) { $delay = time() - $start; - last; + $speech_ssrc = $ssrc; + } + next unless defined $delay && $ssrc == $speech_ssrc; + last if $mode eq 'off'; + # A replacement mixer and the old transcoder have independent SSRCs and timestamp origins. + # Check the stream carrying current speech, including its silence after the two input frames. + if (defined $last_seq) { + my $seq_delta = ($out_seq - $last_seq) & 0xffff; + my $ts_delta = ($out_ts - $last_ts) & 0xffffffff; + my $context = sprintf('%s SSRC=%08x seq=%u->%u timestamp=%u->%u', + $name, $ssrc, $last_seq, $out_seq, $last_ts, $out_ts); + is($seq_delta, 1, 'mixer output sequence remains continuous') or diag($context); + if ($seq_delta == 1) { + is($ts_delta, 320, 'mixer output clock remains continuous') or diag($context); + $clock_intervals++; + } } + $last_seq = $out_seq; + $last_ts = $out_ts; + last if $clock_intervals >= 3; } ok(defined $delay, 'speech reaches the listener'); + if ($mode ne 'off') { + cmp_ok($clock_intervals, '>=', 3, 'checked multiple mixer clock intervals after speech'); + } if (defined $delay) { if ($mode eq 'always') { cmp_ok($delay, '<', 0.14, 'permanent mixer does not queue input behind the DTX timer'); From 84729e891af8553d8a036bc0f2713f92e58998c4 Mon Sep 17 00:00:00 2001 From: Dragos Vingarzan Date: Fri, 9 Oct 2026 16:11:27 +0200 Subject: [PATCH 3/4] test: widen DTX timing separation for shared CI runners --- t/auto-daemon-tests-mixer-dtx.pl | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/t/auto-daemon-tests-mixer-dtx.pl b/t/auto-daemon-tests-mixer-dtx.pl index 25489e8a1..21d914066 100644 --- a/t/auto-daemon-tests-mixer-dtx.pl +++ b/t/auto-daemon-tests-mixer-dtx.pl @@ -11,9 +11,11 @@ use Time::HiRes qw(time); use IO::Select; -# A deliberately large DTX delay makes clock ownership observable without AMR fixtures or long calls. +# Leave a wide gap between prompt mixer output and the configured DTX wait. +# The observation includes scheduling of both the daemon and this test on shared CI runners. +my $dtx_delay = 0.8; # seconds autotest_start(qw(--config-file=none -t -1 -i 203.0.113.1 -n 2223 -f -L 7 -E - --dtx-delay=200 --max-dtx=1), @ARGV) or die; + --max-dtx=1), '--dtx-delay=' . ($dtx_delay * 1000), @ARGV) or die; # Reuse the call and SSRC to cover decoder/DTX cleanup when clock ownership changes. my ($source, $listener) = new_call([qw(198.51.100.10 5000)], [qw(198.51.100.10 5002)]); @@ -55,7 +57,7 @@ snd($source, $input_port, rtp(0, $seq + 1, $ts + 160, 0x5678, $payload)); my ($delay, $speech_ssrc, $last_seq, $last_ts); my $clock_intervals = 0; - while (time() - $start < 0.6) { + while (time() - $start < $dtx_delay + 0.8) { next unless $select->can_read(0.02); my $packet; $listener->recv($packet, 65535); @@ -92,10 +94,14 @@ } if (defined $delay) { if ($mode eq 'always') { - cmp_ok($delay, '<', 0.14, 'permanent mixer does not queue input behind the DTX timer'); + cmp_ok($delay, '<', $dtx_delay / 2, 'permanent mixer does not queue input behind the DTX timer') + or diag(sprintf('%s: observed %.3f s with configured DTX delay %.3f s', + $name, $delay, $dtx_delay)); } else { - cmp_ok($delay, '>=', 0.18, 'ordinary and implicit transcoders retain configured DTX handling'); + cmp_ok($delay, '>=', $dtx_delay - 0.05, 'ordinary and implicit transcoders retain configured DTX handling') + or diag(sprintf('%s: observed %.3f s with configured DTX delay %.3f s', + $name, $delay, $dtx_delay)); } } # Let both tone frames leave the mixer before measuring the next signalling update. From 013eaf1823f51752da45c3440cdad86087420757 Mon Sep 17 00:00:00 2001 From: Dragos Vingarzan Date: Fri, 9 Oct 2026 16:46:59 +0200 Subject: [PATCH 4/4] fix: prevent mixer catch-up bursts after timer stalls --- daemon/audio_player.c | 9 +++++++++ t/auto-daemon-tests-mixer-dtx.pl | 28 +++++++++++++++++++++++----- 2 files changed, 32 insertions(+), 5 deletions(-) diff --git a/daemon/audio_player.c b/daemon/audio_player.c index 326ab743a..bee886e4c 100644 --- a/daemon/audio_player.c +++ b/daemon/audio_player.c @@ -31,6 +31,15 @@ static bool audio_player_run(struct media_player *mp) { if (!ap || !ap->ptime_us) return false; + // Skip missed timer ticks after a stall instead of encoding a catch-up burst. + // A burst can exceed the encoder's send-timer window and interleave queued and new packets. + int64_t now = now_us(); + int64_t late = now - mp->next_run; + if (late >= ap->ptime_us) { + mp->next_run += late / ap->ptime_us * ap->ptime_us; + rtpe_now = mp->next_run; + } + ap->last_run = rtpe_now; // equals mp->next_run unsigned int size; diff --git a/t/auto-daemon-tests-mixer-dtx.pl b/t/auto-daemon-tests-mixer-dtx.pl index 21d914066..e9b51deaa 100644 --- a/t/auto-daemon-tests-mixer-dtx.pl +++ b/t/auto-daemon-tests-mixer-dtx.pl @@ -19,6 +19,26 @@ # Reuse the call and SSRC to cover decoder/DTX cleanup when clock ownership changes. my ($source, $listener) = new_call([qw(198.51.100.10 5000)], [qw(198.51.100.10 5002)]); +my $idle_select = IO::Select->new($source, $listener); +my $source_select = IO::Select->new($source); + +# Consume idle media so the preload harness's Unix datagram queues do not fill and block signalling. +sub drain_media { + my ($select, $duration) = @_; + my $until = time() + $duration; + while (1) { + my $wait = $until - time(); + $wait = 0 if $wait < 0; + $wait = 0.02 if $wait > 0.02; + my @ready = $select->can_read($wait); + for my $socket (@ready) { + my $discard; + $socket->recv($discard, 65535); + } + last if time() >= $until && !@ready; + } +} + my $phase = 0; for my $mode ('always', 'transcoding', 'always', 'off', 'always') { my $name = "DTX clock ownership: $phase $mode"; @@ -40,10 +60,7 @@ my ($input_port) = $answer->{sdp} =~ /m=audio (\d+)/; ok($input_port, 'source media address was negotiated'); my $select = IO::Select->new($listener); - while ($select->can_read(0)) { - my $discard; - $listener->recv($discard, 65535); - } + drain_media($idle_select, 0); my $start = time(); my $seq = 100 + $phase * 100; my $ts = 100000 + $phase * 16000; @@ -58,6 +75,7 @@ my ($delay, $speech_ssrc, $last_seq, $last_ts); my $clock_intervals = 0; while (time() - $start < $dtx_delay + 0.8) { + drain_media($source_select, 0); next unless $select->can_read(0.02); my $packet; $listener->recv($packet, 65535); @@ -105,7 +123,7 @@ } } # Let both tone frames leave the mixer before measuring the next signalling update. - Time::HiRes::sleep(0.3); + drain_media($idle_select, 0.3); $phase++; }