From 16e7b9465f8a0b36a8085cede371657415e66745 Mon Sep 17 00:00:00 2001 From: Richard Fuchs Date: Tue, 8 Aug 2023 15:09:33 -0400 Subject: [PATCH] MT#55283 add `allow-asymmetric-codecs` option Change-Id: I6e9a6e04b9e6192b906c872e478784efb3fe5c6a --- daemon/call.c | 47 ++++++--- daemon/call_interfaces.c | 4 + daemon/codec.c | 101 ++++++++++++------ include/call_interfaces.h | 1 + include/codec.h | 2 + lib/rtplib.c | 3 +- lib/rtplib.h | 4 + t/auto-daemon-tests.pl | 208 ++++++++++++++++++++++++++++++++++++++ 8 files changed, 321 insertions(+), 49 deletions(-) diff --git a/daemon/call.c b/daemon/call.c index ef47d680c..fec234f33 100644 --- a/daemon/call.c +++ b/daemon/call.c @@ -2385,7 +2385,8 @@ static void codecs_offer(struct call_media *media, struct call_media *other_medi .codec_set = flags->codec_set); else codec_store_populate(&other_media->codecs, &sp->codecs, - .codec_set = flags->codec_set); + .codec_set = flags->codec_set, + .allow_asymmetric = !!flags->allow_asymmetric_codecs); codec_store_strip(&other_media->codecs, &flags->codec_strip, flags->codec_except); codec_store_offer(&other_media->codecs, &flags->codec_offer, &sp->codecs); if (!other_media->codecs.strip_full) @@ -2412,7 +2413,8 @@ static void codecs_offer(struct call_media *media, struct call_media *other_medi if (flags && flags->reuse_codec) codec_store_populate_reuse(&media->codecs, &sp->codecs); else - codec_store_populate(&media->codecs, &sp->codecs); + codec_store_populate(&media->codecs, &sp->codecs, + .allow_asymmetric = !!(flags && flags->allow_asymmetric_codecs)); } if (flags) { codec_store_strip(&media->codecs, &flags->codec_strip, flags->codec_except); @@ -2428,13 +2430,15 @@ static void codecs_offer(struct call_media *media, struct call_media *other_medi codec_tracker_update(&media->codecs); // set up handlers - codec_handlers_update(media, other_media, .flags = flags, .sp = sp); + codec_handlers_update(media, other_media, .flags = flags, .sp = sp, + .allow_asymmetric = !!(flags && flags->allow_asymmetric_codecs)); // updating the handlers may have removed some codecs, so run update the supp codecs again codec_tracker_update(&media->codecs); // finally set up handlers again based on final results - codec_handlers_update(media, other_media, .flags = flags, .sp = sp, .sub = dialogue[1]); + codec_handlers_update(media, other_media, .flags = flags, .sp = sp, .sub = dialogue[1], + .allow_asymmetric = !!(flags && flags->allow_asymmetric_codecs)); } __attribute__((nonnull(1, 2, 3, 4, 5))) @@ -2459,7 +2463,8 @@ static void codecs_answer(struct call_media *media, struct call_media *other_med else codec_store_populate(&other_media->codecs, &sp->codecs, .codec_set = flags->codec_set, - .answer_only = codec_answer_only); + .answer_only = codec_answer_only, + .allow_asymmetric = !!flags->allow_asymmetric_codecs); codec_store_strip(&other_media->codecs, &flags->codec_strip, flags->codec_except); codec_store_offer(&other_media->codecs, &flags->codec_offer, &sp->codecs); codec_store_check_empty(&other_media->codecs, &sp->codecs); @@ -2467,7 +2472,7 @@ static void codecs_answer(struct call_media *media, struct call_media *other_med // update callee side codec handlers again (second pass after the offer) as we // might need to update some handlers, e.g. when supplemental codecs have been // rejected - codec_handlers_update(other_media, media); + codec_handlers_update(other_media, media, .allow_asymmetric = !!flags->allow_asymmetric_codecs); // finally set up our caller side codecs ilogs(codec, LOG_DEBUG, "Codec answer for " STR_FORMAT " #%u", @@ -2476,15 +2481,18 @@ static void codecs_answer(struct call_media *media, struct call_media *other_med codec_store_answer(&media->codecs, &other_media->codecs, flags); // set up handlers - codec_handlers_update(media, other_media, .flags = flags, .sp = sp); + codec_handlers_update(media, other_media, .flags = flags, .sp = sp, + .allow_asymmetric = !!flags->allow_asymmetric_codecs); // updating the handlers may have removed some codecs, so run update the supp codecs again codec_tracker_update(&media->codecs); codec_tracker_update(&other_media->codecs); // finally set up handlers again based on final results - codec_handlers_update(media, other_media, .flags = flags, .sp = sp, .sub = dialogue[1]); - codec_handlers_update(other_media, media, .sub = dialogue[0]); + codec_handlers_update(media, other_media, .flags = flags, .sp = sp, .sub = dialogue[1], + .allow_asymmetric = !!flags->allow_asymmetric_codecs); + codec_handlers_update(other_media, media, .sub = dialogue[0], + .allow_asymmetric = !!flags->allow_asymmetric_codecs); // activate audio player if needed (not done by codec_handlers_update without `flags`) audio_player_activate(media); @@ -3065,7 +3073,8 @@ int monologue_publish(struct call_monologue *ml, GQueue *streams, struct sdp_ng_ __media_init_from_flags(media, NULL, sp, flags); - codec_store_populate(&media->codecs, &sp->codecs); + codec_store_populate(&media->codecs, &sp->codecs, + .allow_asymmetric = !!flags->allow_asymmetric_codecs); if (codec_store_accept_one(&media->codecs, &flags->codec_accept, !!flags->accept_any)) return -1; @@ -3137,7 +3146,8 @@ static int monologue_subscribe_request1(struct call_monologue *src_ml, struct ca __media_init_from_flags(src_media, dst_media, sp, flags); - codec_store_populate(&dst_media->codecs, &src_media->codecs); + codec_store_populate(&dst_media->codecs, &src_media->codecs, + .allow_asymmetric = !!flags->allow_asymmetric_codecs); codec_store_strip(&dst_media->codecs, &flags->codec_strip, flags->codec_except); codec_store_strip(&dst_media->codecs, &flags->codec_consume, flags->codec_except); codec_store_strip(&dst_media->codecs, &flags->codec_mask, flags->codec_except); @@ -3145,7 +3155,8 @@ static int monologue_subscribe_request1(struct call_monologue *src_ml, struct ca codec_store_transcode(&dst_media->codecs, &flags->codec_transcode, &sp->codecs); codec_store_synthesise(&dst_media->codecs, &src_media->codecs); - codec_handlers_update(dst_media, src_media, .flags = flags, .sp = sp); + codec_handlers_update(dst_media, src_media, .flags = flags, .sp = sp, + .allow_asymmetric = !!flags->allow_asymmetric_codecs); if (!flags->inactive) bf_copy(&dst_media->media_flags, MEDIA_FLAG_SEND, &src_media->media_flags, SP_FLAG_RECV); @@ -3251,18 +3262,22 @@ int monologue_subscribe_answer(struct call_monologue *dst_ml, struct sdp_ng_flag if (flags->allow_transcoding) { codec_store_populate(&dst_media->codecs, &sp->codecs, .codec_set = flags->codec_set, - .answer_only = true); + .answer_only = true, + .allow_asymmetric = !!flags->allow_asymmetric_codecs); codec_store_strip(&dst_media->codecs, &flags->codec_strip, flags->codec_except); codec_store_offer(&dst_media->codecs, &flags->codec_offer, &sp->codecs); } else { - codec_store_populate(&dst_media->codecs, &sp->codecs, .answer_only = true); + codec_store_populate(&dst_media->codecs, &sp->codecs, .answer_only = true, + .allow_asymmetric = !!flags->allow_asymmetric_codecs); if (!codec_store_is_full_answer(&src_media->codecs, &dst_media->codecs)) return -1; } - codec_handlers_update(src_media, dst_media, .flags = flags); - codec_handlers_update(dst_media, src_media, .flags = flags, .sp = sp, .sub = rev_cs); + codec_handlers_update(src_media, dst_media, .flags = flags, + .allow_asymmetric = !!flags->allow_asymmetric_codecs); + codec_handlers_update(dst_media, src_media, .flags = flags, .sp = sp, .sub = rev_cs, + .allow_asymmetric = !!flags->allow_asymmetric_codecs); __dtls_logic(flags, dst_media, sp); diff --git a/daemon/call_interfaces.c b/daemon/call_interfaces.c index 02f6db1af..c09e91893 100644 --- a/daemon/call_interfaces.c +++ b/daemon/call_interfaces.c @@ -1105,6 +1105,10 @@ static void call_ng_flags_flags(struct sdp_ng_flags *out, str *s, void *dummy) { case CSH_LOOKUP("allow-transcoding"): out->allow_transcoding = 1; break; + case CSH_LOOKUP("allow-asymmetric-codecs"): + case CSH_LOOKUP("allow-asymmetric-codec"): + out->allow_asymmetric_codecs = 1; + break; case CSH_LOOKUP("inject-DTMF"): case CSH_LOOKUP("inject-dtmf"): out->inject_dtmf = 1; diff --git a/daemon/codec.c b/daemon/codec.c index aac6fa2bd..d65cb31db 100644 --- a/daemon/codec.c +++ b/daemon/codec.c @@ -54,6 +54,8 @@ static struct timerthread codec_timers_thread; static void rtp_payload_type_copy(struct rtp_payload_type *dst, const struct rtp_payload_type *src); static void codec_store_add_raw_order(struct codec_store *cs, struct rtp_payload_type *pt); +static struct rtp_payload_type *codec_store_find_compatible(struct codec_store *cs, + const struct rtp_payload_type *pt); static void __rtp_payload_type_add_name(GHashTable *, struct rtp_payload_type *pt); static void codec_calc_lost(struct ssrc_ctx *ssrc, uint16_t seq); @@ -351,8 +353,10 @@ static struct codec_handler *__handler_new(const struct rtp_payload_type *pt, st static void __make_passthrough(struct codec_handler *handler, int dtmf_pt, int cn_pt) { __handler_shutdown(handler); - ilogs(codec, LOG_DEBUG, "Using passthrough handler for " STR_FORMAT " with DTMF %i, CN %i", - STR_FMT(&handler->source_pt.encoding_with_params), dtmf_pt, cn_pt); + ilogs(codec, LOG_DEBUG, "Using passthrough handler for " STR_FORMAT " (%i) with DTMF %i, CN %i", + STR_FMT(&handler->source_pt.encoding_with_params), + handler->source_pt.payload_type, + dtmf_pt, cn_pt); if (handler->source_pt.codec_def && handler->source_pt.codec_def->dtmf) handler->handler_func = handler_func_dtmf; else { @@ -640,8 +644,9 @@ static void __check_codec_list(GHashTable **supplemental_sinks, struct rtp_paylo pdc = first_tc_codec; if (pdc && pref_dest_codec) { *pref_dest_codec = pdc; - ilogs(codec, LOG_DEBUG, "Default sink codec is " STR_FORMAT, - STR_FMT(&(*pref_dest_codec)->encoding_with_params)); + ilogs(codec, LOG_DEBUG, "Default sink codec is " STR_FORMAT " (%i)", + STR_FMT(&(*pref_dest_codec)->encoding_with_params), + (*pref_dest_codec)->payload_type); } } @@ -1135,23 +1140,7 @@ void __codec_handlers_update(struct call_media *receiver, struct call_media *sin if (!sink_pt) { // no matching/identical output codec. maybe we have the same output codec, // but with a different payload type or a different format? - GQueue *dest_codecs = g_hash_table_lookup(sink->codecs.codec_names, &pt->encoding); - if (dest_codecs) { - // the sink supports this codec - check offered formats - for (GList *k = dest_codecs->head; k; k = k->next) { - unsigned int dest_ptype = GPOINTER_TO_UINT(k->data); - sink_pt = g_hash_table_lookup(sink->codecs.codecs, - GINT_TO_POINTER(dest_ptype)); - if (!sink_pt) - continue; - if (sink_pt->clock_rate != pt->clock_rate || - sink_pt->channels != pt->channels) { - sink_pt = NULL; - continue; - } - break; - } - } + sink_pt = codec_store_find_compatible(&sink->codecs, pt); } if (sink_pt && !pt->codec_def->supplemental) { @@ -1253,8 +1242,9 @@ sink_pt_fixed:; // we can now decide whether we can do passthrough, or transcode // different codecs? this will only be true for non-supplemental codecs - // XXX needs more intelligent fmtp matching - if (!rtp_payload_type_eq_nf(pt, sink_pt)) + if (!a.allow_asymmetric && pt->payload_type != sink_pt->payload_type) + goto transcode; + if (!rtp_payload_type_fmt_eq_nf(pt, sink_pt)) goto transcode; // supplemental codecs are always matched up. we want them as passthrough if @@ -1311,8 +1301,10 @@ sink_pt_fixed:; goto transcode; // everything matches - we can do passthrough - ilogs(codec, LOG_DEBUG, "Sink supports codec " STR_FORMAT " for passthrough", - STR_FMT(&pt->encoding_with_params)); + ilogs(codec, LOG_DEBUG, "Sink supports codec " STR_FORMAT " (%i) for passthrough (to %i)", + STR_FMT(&pt->encoding_with_params), + pt->payload_type, + sink_pt->payload_type); __make_passthrough_gsl(handler, &passthrough_handlers, sink_dtmf_pt, sink_cn_pt, use_ssrc_passthrough); goto next; @@ -4806,6 +4798,40 @@ static void codec_store_add_end(struct codec_store *cs, struct rtp_payload_type codec_store_add_link(cs, pt, NULL); } +static struct rtp_payload_type *codec_store_find_compatible_q(struct codec_store *cs, GQueue *q, + const struct rtp_payload_type *pt) +{ + if (!q) + return NULL; + for (GList *l = q->head; l; l = l->next) { + struct rtp_payload_type *ret = g_hash_table_lookup(cs->codecs, l->data); + if (rtp_payload_type_fmt_eq_compat(ret, pt)) + return ret; + } + return NULL; +} +static struct rtp_payload_type *codec_store_find_compatible(struct codec_store *cs, + const struct rtp_payload_type *pt) +{ + struct rtp_payload_type *ret; + ret = codec_store_find_compatible_q(cs, + g_hash_table_lookup(cs->codec_names, &pt->encoding_with_full_params), + pt); + if (ret) + return ret; + ret = codec_store_find_compatible_q(cs, + g_hash_table_lookup(cs->codec_names, &pt->encoding_with_params), + pt); + if (ret) + return ret; + ret = codec_store_find_compatible_q(cs, + g_hash_table_lookup(cs->codec_names, &pt->encoding), + pt); + if (ret) + return ret; + return NULL; +} + void __codec_store_populate_reuse(struct codec_store *dst, struct codec_store *src, struct codec_store_args a) { // start fresh struct call_media *media = dst->media; @@ -4816,6 +4842,8 @@ void __codec_store_populate_reuse(struct codec_store *dst, struct codec_store *s struct rtp_payload_type *orig_pt = g_hash_table_lookup(dst->codecs, GINT_TO_POINTER(pt->payload_type)); + pt->reverse_payload_type = pt->payload_type; + if (orig_pt) ilogs(codec, LOG_DEBUG, "Retaining codec " STR_FORMAT " (%i)", STR_FMT(&pt->encoding_with_params), @@ -4871,16 +4899,24 @@ void __codec_store_populate(struct codec_store *dst, struct codec_store *src, st struct rtp_payload_type *orig_pt = g_hash_table_lookup(orig_dst.codecs, GINT_TO_POINTER(pt->payload_type)); if (a.answer_only && !orig_pt) { - ilogs(codec, LOG_DEBUG, "Not adding stray answer codec " STR_FORMAT " (%i)", - STR_FMT(&pt->encoding_with_params), - pt->payload_type); - continue; + if (a.allow_asymmetric) + orig_pt = codec_store_find_compatible(&orig_dst, pt); + if (!orig_pt) { + ilogs(codec, LOG_DEBUG, "Not adding stray answer codec " STR_FORMAT " (%i)", + STR_FMT(&pt->encoding_with_params), + pt->payload_type); + continue; + } } ilogs(codec, LOG_DEBUG, "Adding codec " STR_FORMAT " (%i)", STR_FMT(&pt->encoding_with_params), pt->payload_type); + + pt->reverse_payload_type = pt->payload_type; + if (orig_pt) { // carry over existing options + pt->payload_type = orig_pt->payload_type; pt->ptime = orig_pt->ptime; pt->for_transcoding = orig_pt->for_transcoding; pt->accepted = orig_pt->accepted; @@ -5303,8 +5339,11 @@ void codec_store_answer(struct codec_store *dst, struct codec_store *src, struct STR_FMT(&h->dest_pt.encoding_with_params), h->dest_pt.payload_type); if (!g_hash_table_lookup(dst->codecs, GINT_TO_POINTER(h->dest_pt.payload_type))) { - if (h->passthrough) - codec_store_add_end(dst, pt); + if (h->passthrough) { + struct rtp_payload_type copy = *pt; + copy.payload_type = pt->reverse_payload_type; + codec_store_add_end(dst, ©); + } else codec_store_add_end(dst, &h->dest_pt); if (!is_supp) diff --git a/include/call_interfaces.h b/include/call_interfaces.h index 12128e74c..9ffe2922a 100644 --- a/include/call_interfaces.h +++ b/include/call_interfaces.h @@ -167,6 +167,7 @@ struct sdp_ng_flags { single_codec:1, reuse_codec:1, allow_transcoding:1, + allow_asymmetric_codecs:1, early_media:1, accept_any:1, inject_dtmf:1, diff --git a/include/codec.h b/include/codec.h index 7eff38985..5de3b268a 100644 --- a/include/codec.h +++ b/include/codec.h @@ -114,6 +114,7 @@ void codec_update_all_source_handlers(struct call_monologue *ml, const struct sd struct codec_store_args { GHashTable *codec_set; bool answer_only; + bool allow_asymmetric; }; __attribute__((nonnull(1))) @@ -172,6 +173,7 @@ struct chu_args { const struct sdp_ng_flags *flags; const struct stream_params *sp; struct call_subscription *sub; + bool allow_asymmetric; }; #define codec_handlers_update(r, s, ...) \ __codec_handlers_update(r, s, (struct chu_args) {__VA_ARGS__}) diff --git a/lib/rtplib.c b/lib/rtplib.c index f1b3b8921..766404c9e 100644 --- a/lib/rtplib.c +++ b/lib/rtplib.c @@ -145,8 +145,7 @@ const struct rtp_payload_type *rtp_get_rfc_codec(const str *codec) { } // helper function: matches only basic params, without matching payload type number -__attribute__((nonnull(1, 2))) -static bool rtp_payload_type_fmt_eq_nf(const struct rtp_payload_type *a, const struct rtp_payload_type *b) { +bool rtp_payload_type_fmt_eq_nf(const struct rtp_payload_type *a, const struct rtp_payload_type *b) { if (a->clock_rate != b->clock_rate) return false; if (a->channels != b->channels) diff --git a/lib/rtplib.h b/lib/rtplib.h index fa02a084e..d3055282a 100644 --- a/lib/rtplib.h +++ b/lib/rtplib.h @@ -92,6 +92,7 @@ struct rtp_codec_format { struct rtp_payload_type { int payload_type; + int reverse_payload_type; str encoding_with_params; // "opus/48000/2" str encoding_with_full_params; // "opus/48000/1" str encoding; // "opus" @@ -129,6 +130,9 @@ __attribute__((nonnull(1, 2))) bool rtp_payload_type_eq_exact(const struct rtp_payload_type *a, const struct rtp_payload_type *b); __attribute__((nonnull(1, 2))) bool rtp_payload_type_eq_compat(const struct rtp_payload_type *a, const struct rtp_payload_type *b); +// matches only basic params but not payload type number +__attribute__((nonnull(1, 2))) +bool rtp_payload_type_fmt_eq_nf(const struct rtp_payload_type *a, const struct rtp_payload_type *b); // matches only basic params and payload type number __attribute__((nonnull(1, 2))) bool rtp_payload_type_eq_nf(const struct rtp_payload_type *, const struct rtp_payload_type *); diff --git a/t/auto-daemon-tests.pl b/t/auto-daemon-tests.pl index 902671675..caf199e49 100755 --- a/t/auto-daemon-tests.pl +++ b/t/auto-daemon-tests.pl @@ -82,6 +82,214 @@ sub stun_succ { +new_call; + +offer('AMR asymmetric, control', {}, < ['allow asymmetric codecs']}, < [qw/SIPREC all/]}, <