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/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..e9b51deaa --- /dev/null +++ b/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();