From dfae8d6a22761a3f5be3a94c13a51b4199cf2a30 Mon Sep 17 00:00:00 2001 From: Richard Fuchs Date: Tue, 24 Mar 2020 13:49:01 -0400 Subject: [PATCH] TT#78201 add symmetric-codecs flag closes #953 Change-Id: I848f501709f48927a7156033ccd42eacd742e2d8 --- README.md | 25 +++ daemon/call_interfaces.c | 3 + daemon/codec.c | 102 +++++++++ include/call_interfaces.h | 1 + t/auto-daemon-tests.pl | 440 ++++++++++++++++++++++++++++++++++++++ 5 files changed, 571 insertions(+) diff --git a/README.md b/README.md index c32600465..4330e7a33 100644 --- a/README.md +++ b/README.md @@ -699,6 +699,31 @@ Optionally included keys are: This flag should be given as part of the `answer` message. + - `symmetric codecs` + + This flag instructs *rtpengine* to honour the list of codecs accepted by answer, including + their order, and match them up with the list of codecs that *rtpengine* itself produces + when transcoding. It must be using in an `answer` message and is ignored in an `offer`. + + By default, any supported codec that was originally offered will be accepted by + *rtpengine* when transcoding, and the first codec listed will be used as output codec, + even if neither this codec nor its transcoded counterpart was accepted by the answer. + With this flag given, *rtpengine* will prefer the codecs listed in the answer over + the codecs listed in the offer and re-order the answer accordingly. This can lead to + a high-priority codec given in the offer to be listed as low-priority codec in the + answer, and vice versa. On the other hand, it can lead to the transcoding engine to be + disabled when it isn't needed. + + For example: The original offer lists codecs `PCMA` and `opus`. *Rtpengine* is instructed + to add `G722` as a transcoded codec in the offer, and so the offer produced by + *rtpengine* lists `PCMA`, `opus`, and `G722`. If *rtpengine* were to receive any + G.722 media, it would transcode it to PCMA as this is the codec preferred by the + offer. The answer now accepts `opus` and rejects the other two codecs. Without this + flag, the answer produced by *rtpengine* would contain both `PCMA` and `opus`, because + receiving G.722 would still be a possibility and so would have to be transcoded to + PCMA. With this flag however, *rtpengine* honours the single accepted codec from the + answer and so is able to eliminate PCMA from its own answer as it's not needed. + - `all` Only relevant to the `unblock media` message. Instructs *rtpengine* to remove not only a diff --git a/daemon/call_interfaces.c b/daemon/call_interfaces.c index e226d3f54..869486c93 100644 --- a/daemon/call_interfaces.c +++ b/daemon/call_interfaces.c @@ -716,6 +716,9 @@ static void call_ng_flags_flags(struct sdp_ng_flags *out, str *s, void *dummy) { case CSH_LOOKUP("asymmetric-codecs"): out->asymmetric_codecs = 1; break; + case CSH_LOOKUP("symmetric-codecs"): + out->symmetric_codecs = 1; + break; case CSH_LOOKUP("inject-DTMF"): out->inject_dtmf = 1; break; diff --git a/daemon/codec.c b/daemon/codec.c index 31436c8ab..70bb650d2 100644 --- a/daemon/codec.c +++ b/daemon/codec.c @@ -498,6 +498,105 @@ static void __eliminate_rejected_codecs(struct call_media *receiver, struct call } } +// transfers ownership of payload type objects from a queue to a hash table. +// duplicates are removed. +static GHashTable *__payload_type_queue_hash(GQueue *prefs, GQueue *order) { + GHashTable *ret = g_hash_table_new_full(g_direct_hash, g_direct_equal, NULL, + (GDestroyNotify) payload_type_free); + g_queue_init(order); + for (GList *l = prefs->head; l; l = l->next) { + struct rtp_payload_type *pt = l->data; + if (g_hash_table_lookup(ret, GINT_TO_POINTER(pt->payload_type))) { + ilog(LOG_DEBUG, "Removing duplicate RTP payload type %i", pt->payload_type); + payload_type_free(pt); + continue; + } + g_hash_table_insert(ret, GINT_TO_POINTER(pt->payload_type), pt); + g_queue_push_tail(order, GINT_TO_POINTER(pt->payload_type)); + } + + // ownership has been transferred + g_queue_clear(prefs); + + return ret; +} + +static void __symmetric_codecs(struct call_media *receiver, struct call_media *sink, + int *sink_transcoding) +{ + if (!MEDIA_ISSET(sink, TRANSCODE)) + return; + if (!*sink_transcoding) + return; + + // sink still looks like it's transcoding. reconstruct our answer to the receiver + // (receiver->prefs_recv) based on the codecs accepted by the sink (sink->prefs_send). + + GQueue prefs_recv_order, prefs_send_order; + GHashTable *prefs_recv = __payload_type_queue_hash(&receiver->codecs_prefs_recv, &prefs_recv_order); + GHashTable *prefs_send = __payload_type_queue_hash(&receiver->codecs_prefs_send, &prefs_send_order); + + // ownership of the objects has been transferred. clear out old structures. + g_hash_table_remove_all(receiver->codecs_recv); + g_hash_table_remove_all(receiver->codec_names_recv); + g_hash_table_remove_all(receiver->codecs_send); + g_hash_table_remove_all(receiver->codec_names_send); + + // reconstruct list based on other side's preference. + int transcoding = 0; + + for (GList *l = sink->codecs_prefs_send.head; l; l = l->next) { + struct rtp_payload_type *pt = l->data; + // do we have a matching output? + struct rtp_payload_type *out_pt = g_hash_table_lookup(prefs_recv, + GINT_TO_POINTER(pt->payload_type)); + if (out_pt && g_hash_table_lookup(prefs_send, GINT_TO_POINTER(pt->payload_type))) { + // add it to the list + ilog(LOG_DEBUG, "Adding symmetric RTP payload type %i", pt->payload_type); + g_hash_table_steal(prefs_recv, GINT_TO_POINTER(pt->payload_type)); + __rtp_payload_type_add_recv(receiver, out_pt); + // and our send leg + out_pt = g_hash_table_lookup(prefs_send, GINT_TO_POINTER(pt->payload_type)); + if (out_pt) { + g_hash_table_steal(prefs_send, GINT_TO_POINTER(pt->payload_type)); + __rtp_payload_type_add_send(receiver, out_pt); + } + continue; + } + // we must transcode after all. + ilog(LOG_DEBUG, "RTP payload type %i is not symmetric and must be transcoded", + pt->payload_type); + transcoding = 1; + } + + if (!transcoding) + *sink_transcoding = 0; + else { + // append any leftover codecs + while (prefs_recv_order.length) { + void *ptype = g_queue_pop_head(&prefs_recv_order); + struct rtp_payload_type *out_pt = g_hash_table_lookup(prefs_recv, ptype); + if (!out_pt) + continue; + g_hash_table_steal(prefs_recv, ptype); + __rtp_payload_type_add_recv(receiver, out_pt); + } + while (prefs_send_order.length) { + void *ptype = g_queue_pop_head(&prefs_send_order); + struct rtp_payload_type *out_pt = g_hash_table_lookup(prefs_send, ptype); + if (!out_pt) + continue; + g_hash_table_steal(prefs_send, ptype); + __rtp_payload_type_add_send(receiver, out_pt); + } + } + + g_hash_table_destroy(prefs_recv); + g_queue_clear(&prefs_recv_order); + g_hash_table_destroy(prefs_send); + g_queue_clear(&prefs_send_order); +} + static void __check_dtmf_injector(const struct sdp_ng_flags *flags, struct call_media *receiver, struct rtp_payload_type *pref_dest_codec, GHashTable *output_transcoders, int dtmf_payload_type) @@ -736,6 +835,9 @@ void codec_handlers_update(struct call_media *receiver, struct call_media *sink, // similarly, if the sink can receive a codec that the receiver can't send, it's also transcoding __check_send_codecs(receiver, sink, flags, dtmf_sinks, &sink_transcoding); + if (flags && flags->opmode == OP_ANSWER && flags->symmetric_codecs) + __symmetric_codecs(receiver, sink, &sink_transcoding); + ilog(LOG_DEBUG, "%i DTMF sink entries", g_hash_table_size(dtmf_sinks)); int dtmf_payload_type = __dtmf_payload_type(dtmf_sinks, pref_dest_codec); diff --git a/include/call_interfaces.h b/include/call_interfaces.h index 89d2d2413..d744fd72e 100644 --- a/include/call_interfaces.h +++ b/include/call_interfaces.h @@ -75,6 +75,7 @@ struct sdp_ng_flags { original_sendrecv:1, always_transcode:1, asymmetric_codecs:1, + symmetric_codecs:1, inject_dtmf:1, t38_decode:1, t38_force:1, diff --git a/t/auto-daemon-tests.pl b/t/auto-daemon-tests.pl index abdce172e..42f4c98bb 100755 --- a/t/auto-daemon-tests.pl +++ b/t/auto-daemon-tests.pl @@ -18,6 +18,446 @@ my ($sock_a, $sock_b, $port_a, $port_b, $ssrc, $resp, $srtp_ctx_a, $srtp_ctx_b, +# symmetric-codec flag (GH 953) + +new_call(); + +offer('gh 953 w/o flag', { ICE => 'remove', codec => { transcode => ['G722'] } }, < 'remove', }, < 'remove', codec => { transcode => ['G722'] } }, < 'remove', flags => ['symmetric codecs'] }, < 'remove', codec => { transcode => ['G722'] } }, < 'remove', }, < 'remove', codec => { transcode => ['G722'] } }, < 'remove', flags => ['symmetric codecs'] }, < 'remove', codec => { transcode => ['G722'] } }, < 'remove', }, < 'remove', codec => { transcode => ['G722'] } }, < 'remove', flags => ['symmetric codecs'] }, <