fix: avoid input DTX buffering for permanent audio mixers

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;

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))

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
 

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
 

t/auto-daemon-tests-mixer-dtx.pl

@@ -0,0 +1,131 @@
+#!/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;
+
+# 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
+	--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)]);
+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";
+	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);
+	drain_media($idle_select, 0);
+	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, $payload));
+	Time::HiRes::sleep(0.02);
+	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 < $dtx_delay + 0.8) {
+		drain_media($source_select, 0);
+		next unless $select->can_read(0.02);
+		my $packet;
+		$listener->recv($packet, 65535);
+		next unless length($packet) >= 12;
+		my ($pt, $out_seq, $out_ts, $ssrc) = unpack('x C n N N', $packet);
+		next unless ($pt & 0x7f) == 96;
+		my @samples = unpack('s>*', substr($packet, 12));
+		if (!defined $delay && (grep { abs($_ - $sample) < 20 } @samples) >= 16) {
+			$delay = time() - $start;
+			$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, '<', $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, '>=', $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.
+	drain_media($idle_select, 0.3);
+	$phase++;
+}
+
+rtpe_req('delete', 'DTX clock ownership cleanup', {'from-tag' => ft()});
+done_testing();