From 71addf5118cf67214930633458a9963837d1ab82 Mon Sep 17 00:00:00 2001 From: Richard Fuchs Date: Thu, 24 Apr 2025 14:22:31 -0400 Subject: [PATCH] MT#62687 support codec preferences Change-Id: Ib8eaca5b2223cf7eb40e9dc68e8ab05782bd4f99 --- daemon/codec.c | 87 ++-- daemon/main.c | 10 +- docs/rtpengine.md | 53 +++ include/codec.h | 6 +- t/Makefile | 5 +- t/auto-daemon-tests-codec-prefs.pl | 671 +++++++++++++++++++++++++++++ t/test6.conf | 29 ++ 7 files changed, 830 insertions(+), 31 deletions(-) create mode 100644 t/auto-daemon-tests-codec-prefs.pl create mode 100644 t/test6.conf diff --git a/daemon/codec.c b/daemon/codec.c index 15567d4b4..7adf8875c 100644 --- a/daemon/codec.c +++ b/daemon/codec.c @@ -376,6 +376,29 @@ static struct codec_handler codec_handler_stub_blackhole = { +static unsigned int __codec_pipeline_hash(const struct codec_pipeline_index *i) { + return str_case_hash(&i->src.encoding) + ^ str_case_hash(&i->dst.encoding) + ^ i->src.clock_rate + ^ i->dst.clock_rate + ^ i->src.channels + ^ i->dst.channels; +} + +static gboolean __codec_pipeline_eq(const struct codec_pipeline_index *a, const struct codec_pipeline_index *b) { + return str_case_equal(&a->src.encoding, &b->src.encoding) + && str_case_equal(&a->dst.encoding, &b->dst.encoding) + && a->src.clock_rate == b->src.clock_rate + && a->dst.clock_rate == b->dst.clock_rate + && a->src.channels == b->src.channels + && a->dst.channels == b->dst.channels; +} + +TYPED_GHASHTABLE_IMPL(transcode_config_ht, __codec_pipeline_hash, __codec_pipeline_eq, NULL, NULL) + +static transcode_config_ht rtpe_transcode_config; +static bool have_codec_preferences; + static void __transform_handler_shutdown(struct transform_handler *tfh) { if (!tfh) return; @@ -1358,6 +1381,31 @@ static int __codec_handler_eq(const struct codec_handler_index *h, const struct TYPED_GHASHTABLE_IMPL(codec_handlers_ht, __codec_handler_hash, __codec_handler_eq, NULL, NULL) +static int direct_rev_cmp(const void *a, const void *b) { + return GPOINTER_TO_INT(a) < GPOINTER_TO_INT(b) ? 1 : GPOINTER_TO_INT(a) > GPOINTER_TO_INT(b) ? -1 : 0; +} + +static GTree *codec_make_prefs_tree(rtp_payload_type *pt, struct codec_store *cs) { + if (!have_codec_preferences) + return NULL; + + GTree *ret = g_tree_new(direct_rev_cmp); + + for (__auto_type l = cs->codec_prefs.head; l; l = l->next) { + __auto_type dst = l->data; + struct codec_pipeline_index pi = { .src = *pt, .dst = *dst }; + __auto_type lookup = t_hash_table_lookup(rtpe_transcode_config, &pi); + int pref = 0; + if (lookup) + pref = lookup->preference; + // first one of each unique preference wins + if (!g_tree_lookup(ret, GINT_TO_POINTER(pref))) + g_tree_insert(ret, GINT_TO_POINTER(pref), dst); + } + + return ret; +} + /** * receiver - media / sink - other_media * call must be locked in W @@ -1486,6 +1534,8 @@ void __codec_handlers_update(struct call_media *receiver, struct call_media *sin rtp_payload_type *pt = l->data; rtp_payload_type *sink_pt = NULL; + g_autoptr(GTree) codec_preferences = codec_make_prefs_tree(pt, &sink->codecs); + ilogs(codec, LOG_DEBUG, "Checking receiver codec " STR_FORMAT "/" STR_FORMAT " (%i)", STR_FMT(&pt->encoding_with_full_params), STR_FMT0(&pt->format_parameters), @@ -1530,13 +1580,19 @@ void __codec_handlers_update(struct call_media *receiver, struct call_media *sin sink_pt = pt; } + rtp_payload_type *weighted_pref_dest_codec = NULL; + if (codec_preferences) + weighted_pref_dest_codec = rtpe_g_tree_first(codec_preferences); + if (!weighted_pref_dest_codec) + weighted_pref_dest_codec = pref_dest_codec; + if (sink_pt && !pt->codec_def->supplemental) { // we have a matching output codec. do we actually want to use it, or // do we want to transcode to something else? // ignore the preference here - for now, all `for_transcoding` codecs // take preference - if (pref_dest_codec && pref_dest_codec->for_transcoding) - sink_pt = pref_dest_codec; + if (weighted_pref_dest_codec && weighted_pref_dest_codec->for_transcoding) + sink_pt = weighted_pref_dest_codec; } // ignore DTMF sink if we're blocking DTMF in PCM replacement mode @@ -1545,7 +1601,7 @@ void __codec_handlers_update(struct call_media *receiver, struct call_media *sin // still no output? pick the preferred sink codec if (!sink_pt) - sink_pt = pref_dest_codec; + sink_pt = weighted_pref_dest_codec; if (!sink_pt) { ilogs(codec, LOG_DEBUG, "No suitable output codec for " STR_FORMAT "/" STR_FORMAT, @@ -6326,28 +6382,6 @@ static void codec_worker(void *d) { thread_waker_del(&waker); } -static unsigned int __codec_pipeline_hash(const struct codec_pipeline_index *i) { - return str_case_hash(&i->src.encoding) - ^ str_case_hash(&i->dst.encoding) - ^ i->src.clock_rate - ^ i->dst.clock_rate - ^ i->src.channels - ^ i->dst.channels; -} - -static gboolean __codec_pipeline_eq(const struct codec_pipeline_index *a, const struct codec_pipeline_index *b) { - return str_case_equal(&a->src.encoding, &b->src.encoding) - && str_case_equal(&a->dst.encoding, &b->dst.encoding) - && a->src.clock_rate == b->src.clock_rate - && a->dst.clock_rate == b->dst.clock_rate - && a->src.channels == b->src.channels - && a->dst.channels == b->dst.channels; -} - -TYPED_GHASHTABLE_IMPL(transcode_config_ht, __codec_pipeline_hash, __codec_pipeline_eq, NULL, NULL) - -transcode_config_ht rtpe_transcode_config; - #endif void codecs_init(void) { @@ -6387,6 +6421,9 @@ void codecs_init(void) { .channels = tcc->i.dst.channels, .clockrate = tcc->i.dst.clock_rate, }); + + if (tcc->preference) + have_codec_preferences = true; } #endif } diff --git a/daemon/main.c b/daemon/main.c index 69ea9b02e..931acd52b 100644 --- a/daemon/main.c +++ b/daemon/main.c @@ -432,9 +432,17 @@ static void do_transcode_config(const char *name, charp_ht ht, struct transcode_ die("Failed to parse source codec '%s' in transcode config '%s'", src, name); char *tfm = t_hash_table_lookup(ht, "transform"); + char *pref_s = t_hash_table_lookup(ht, "preference"); #ifdef HAVE_CODEC_CHAIN char *cc = t_hash_table_lookup(ht, "codec-chain"); #endif + + int pref = 0; + if (pref_s) + pref = atoi(pref_s); + + tc->preference = pref; + if (tfm) { if (!endpoint_parse_any_getaddrinfo_full(&tc->transform, tfm)) die("Failed to parse transform endpoint '%s' in transcode config '%s'", tfm, name); @@ -452,7 +460,7 @@ static void do_transcode_config(const char *name, charp_ht ht, struct transcode_ tc->codec_chain = true; } #endif - else + else if (!pref) die("Transcode config '%s' has no verdict", name); } diff --git a/docs/rtpengine.md b/docs/rtpengine.md index 68fd82f55..e5e935679 100644 --- a/docs/rtpengine.md +++ b/docs/rtpengine.md @@ -1858,6 +1858,59 @@ For example, if __source=PCMU__ and __destination=PCMA__ are set, then this particular transcoding config section would be considered if the received codec is PCMU and the requested output codec is PCMA. +### __preference__ Verdict + +This verdict can be used on its own, or can be used in combination with some +other verdict. It configures a preference for a particular codec combination, +relative to other codec combinations. The default preference (none configured) +is zero. Codec combinations with higher (positive) preference values will be +considered as better options than the default, while lower (negative) +preference values will be considered as worse options. + +For example consider the following configuration: + + transcode-config = tc + + [tc-PCMA-PCMU-higher] + source = PCMA + destination = PCMU + preference = 5 + + [tc-GSM-G723-lower] + source = G723 + destination = GSM + preference = -5 + +Consider a received offer SDP listing codecs GSM, PCMU, and G722 (in that +order), with transcoding to PCMA and G723 requested. The outgoing offer SDP +would therefore list GSM, PCMU, G722, PCMA, and G723 (in that order). + +If the answer were to accept any of GSM, PCMU, or G722, then pass-through +without transcoding would occur. + +If the answer were to accept any of the other codecs that were requested for +transcoding (PCMA or G723), then normally the first offered codec (GSM) would +be used as transcoding partner. But: + +If the answer were to accept PCMA, then GSM as transcoding partner would be +considered with a preference of zero, as no config section matches this pair. +However the config section `tc-PCMA-PCMU-higher` matches PCMU as transcoding +partner for PCMA and would give that pair a higher preference (5). PCMU would +therefore win with the highest preference, and transcoding would occur between +PCMA and PCMU. + +If the answer were to accept G723, then the config section `tc-GSM-G723-lower` +would be considered and would give GSM as transcoding partner a negative +preference of -5. The next possible codec (PCMU) doesn't have a matching config +section and so would be considered with a preference of zero. PCMU would +therefore win and transcoding would occur between G723 and PCMU. + +_NOTE: These config sections operate directionally, meaning that in the above +example, a config section listing `source = GSM` and `destination = G723` would +not be considered. If a codec pair ought to receive the same preference value +regardless of the direction, then it must be listed twice, with source and +destination swapped._ + ### __transform__ Verdict To use the __transform__ verdict, the config key __transform=...__ must be set. diff --git a/include/codec.h b/include/codec.h index 4b0340848..482ad049f 100644 --- a/include/codec.h +++ b/include/codec.h @@ -116,6 +116,8 @@ struct transcode_config { struct codec_pipeline_index i; // parsed + int preference; // default: 0 + // transform verdict endpoint_t transform; str local_interface; @@ -272,10 +274,6 @@ INLINE struct codec_handler *codec_handler_lookup(codec_handlers_ht ht, int pt, } -extern transcode_config_ht rtpe_transcode_config; - - - #else INLINE void __codec_handlers_update(struct call_media *receiver, struct call_media *sink, struct chu_args a) diff --git a/t/Makefile b/t/Makefile index 204e22414..63f836257 100644 --- a/t/Makefile +++ b/t/Makefile @@ -150,7 +150,7 @@ daemon-tests: daemon-tests-main daemon-tests-jb daemon-tests-pubsub daemon-tests daemon-tests-templ-def daemon-tests-templ-def-offer \ daemon-tests-sdp-manipulations daemon-tests-sdes-manipulations \ daemon-tests-sdp-orig-replacements daemon-tests-moh daemon-tests-evs-dtx daemon-tests-transform \ - daemon-tests-transcode-config + daemon-tests-transcode-config daemon-tests-codec-prefs daemon-test-deps: tests-preload.so $(MAKE) -C ../daemon @@ -252,6 +252,9 @@ daemon-tests-t38: daemon-test-deps spandsp_recv_fax_pcm spandsp_recv_fax_t38 \ daemon-tests-evs-dtx: daemon-test-deps ./auto-test-helper "$@" perl -I../perl auto-daemon-tests-evs-dtx.pl +daemon-tests-codec-prefs: daemon-test-deps + ./auto-test-helper "$@" perl -I../perl auto-daemon-tests-codec-prefs.pl + test-bitstr: test-bitstr.o test-mix-buffer: test-mix-buffer.o $(COMMONOBJS) mix_buffer.o ssrc.o rtp.o crypto.o helpers.o \ diff --git a/t/auto-daemon-tests-codec-prefs.pl b/t/auto-daemon-tests-codec-prefs.pl new file mode 100644 index 000000000..eff4ef9d6 --- /dev/null +++ b/t/auto-daemon-tests-codec-prefs.pl @@ -0,0 +1,671 @@ +#!/usr/bin/perl + +use strict; +use warnings; +use NGCP::Rtpengine::Test; +use NGCP::Rtpclient::SRTP; +use NGCP::Rtpengine::AutoTest; +use Test::More; +use NGCP::Rtpclient::ICE; +use POSIX; + +autotest_start(qw(--config-file=test6.conf)) + or die; + + +new_call; + +offer('control - neutral passthrough', { codec => { transcode => ['PCMA', 'PCMU'] } }, < { transcode => ['PCMA', 'PCMU'] } }, < { transcode => ['PCMA', 'PCMU'] } }, < { transcode => ['PCMA', 'PCMU'] } }, < { transcode => ['PCMA', 'PCMU'] } }, < { transcode => ['PCMA', 'PCMU', 'speex'] } }, < { transcode => ['PCMA', 'PCMU', 'opus'] } }, < { transcode => ['PCMA', 'PCMU', 'opus'] } }, < { transcode => ['PCMA', 'PCMU', 'opus'] } }, < { transcode => ['PCMA', 'G723'] } }, < { transcode => ['PCMA', 'G723'] } }, < { transcode => ['PCMA', 'G723'] } }, < { transcode => ['PCMA', 'G723'] } }, < { transcode => ['PCMA', 'G723'] } }, <