From 22d8db9e72c00d3bd7a523f1fa276928b9e51044 Mon Sep 17 00:00:00 2001 From: Richard Fuchs Date: Thu, 17 Feb 2022 13:14:04 -0500 Subject: [PATCH] TT#14008 don't accept stray answer codecs that were not offered Special handling for codec lists that were received as part of an answer: If the list includes a codec that was not offered, ignore that codec. This prevents transcoders from being set up that were not requested. This brought to light some tests that were actually broken. Change-Id: Iac71056ec5e10b5de5567917974f2c4e0261eb0c --- daemon/call.c | 25 ++++++++++++++----------- daemon/codec.c | 40 ++++++++++++++++++++++++++++++---------- include/codec.h | 4 ++-- t/auto-daemon-tests.pl | 6 +++--- t/test-transcode.c | 4 ++-- 5 files changed, 51 insertions(+), 28 deletions(-) diff --git a/daemon/call.c b/daemon/call.c index 0006f21cc..4e160acb2 100644 --- a/daemon/call.c +++ b/daemon/call.c @@ -2347,9 +2347,11 @@ void codecs_offer_answer(struct call_media *media, struct call_media *other_medi other_media->index); if (flags) { if (flags->reuse_codec) - codec_store_populate_reuse(&other_media->codecs, &sp->codecs, flags->codec_set); + codec_store_populate_reuse(&other_media->codecs, &sp->codecs, flags->codec_set, + false); else - codec_store_populate(&other_media->codecs, &sp->codecs, flags->codec_set); + codec_store_populate(&other_media->codecs, &sp->codecs, flags->codec_set, + false); 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) @@ -2359,7 +2361,8 @@ void codecs_offer_answer(struct call_media *media, struct call_media *other_medi codec_store_accept(&other_media->codecs, &flags->codec_consume, &sp->codecs); codec_store_track(&other_media->codecs, &flags->codec_mask); } else - codec_store_populate(&other_media->codecs, &sp->codecs, NULL); + codec_store_populate(&other_media->codecs, &sp->codecs, NULL, + false); // we don't update the answerer side if the offer is not RTP but is going // to RTP (i.e. T.38 transcoding) - instead we leave the existing codec list @@ -2374,9 +2377,9 @@ void codecs_offer_answer(struct call_media *media, struct call_media *other_medi STR_FMT(&media->monologue->tag), media->index); if (flags && flags->reuse_codec) - codec_store_populate_reuse(&media->codecs, &sp->codecs, NULL); + codec_store_populate_reuse(&media->codecs, &sp->codecs, NULL, false); else - codec_store_populate(&media->codecs, &sp->codecs, NULL); + codec_store_populate(&media->codecs, &sp->codecs, NULL, false); } if (flags) { codec_store_strip(&media->codecs, &flags->codec_strip, flags->codec_except); @@ -2406,9 +2409,9 @@ void codecs_offer_answer(struct call_media *media, struct call_media *other_medi STR_FMT(&other_media->monologue->tag), other_media->index); if (flags->reuse_codec) - codec_store_populate_reuse(&other_media->codecs, &sp->codecs, flags->codec_set); + codec_store_populate_reuse(&other_media->codecs, &sp->codecs, flags->codec_set, true); else - codec_store_populate(&other_media->codecs, &sp->codecs, flags->codec_set); + codec_store_populate(&other_media->codecs, &sp->codecs, flags->codec_set, true); 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); @@ -2951,7 +2954,7 @@ 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, NULL); + codec_store_populate(&media->codecs, &sp->codecs, NULL, false); if (codec_store_accept_one(&media->codecs, &flags->codec_accept, flags->accept_any ? true : false)) return -1; @@ -3021,7 +3024,7 @@ static int monologue_subscribe_request1(struct call_monologue *src_ml, struct ca if (__media_init_from_flags(src_media, dst_media, sp, flags) == 1) continue; - codec_store_populate(&dst_media->codecs, &src_media->codecs, NULL); + codec_store_populate(&dst_media->codecs, &src_media->codecs, NULL, false); 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); @@ -3121,12 +3124,12 @@ int monologue_subscribe_answer(struct call_monologue *dst_ml, struct sdp_ng_flag continue; if (flags && flags->allow_transcoding) { - codec_store_populate(&dst_media->codecs, &sp->codecs, flags->codec_set); + codec_store_populate(&dst_media->codecs, &sp->codecs, flags->codec_set, true); 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, NULL); + codec_store_populate(&dst_media->codecs, &sp->codecs, NULL, true); if (!codec_store_is_full_answer(&src_media->codecs, &dst_media->codecs)) return -1; } diff --git a/daemon/codec.c b/daemon/codec.c index 814e6fa5d..a31ddf33f 100644 --- a/daemon/codec.c +++ b/daemon/codec.c @@ -4189,7 +4189,9 @@ static void codec_store_add_end(struct codec_store *cs, struct rtp_payload_type codec_store_add_link(cs, pt, NULL); } -void codec_store_populate_reuse(struct codec_store *dst, struct codec_store *src, GHashTable *codec_set) { +void codec_store_populate_reuse(struct codec_store *dst, struct codec_store *src, GHashTable *codec_set, + bool answer_only) +{ // start fresh struct call_media *media = dst->media; struct call *call = media ? media->call : NULL; @@ -4198,13 +4200,23 @@ void codec_store_populate_reuse(struct codec_store *dst, struct codec_store *src struct rtp_payload_type *pt = l->data; struct rtp_payload_type *orig_pt = g_hash_table_lookup(dst->codecs, GINT_TO_POINTER(pt->payload_type)); - ilogs(codec, LOG_DEBUG, "Adding codec " STR_FORMAT " (%i)", - STR_FMT(&pt->encoding_with_params), - pt->payload_type); - - if (!orig_pt) { - __codec_options_set(call, pt, codec_set); - codec_store_add_end(dst, pt); + + if (orig_pt) + ilogs(codec, LOG_DEBUG, "Retaining codec " STR_FORMAT " (%i)", + STR_FMT(&pt->encoding_with_params), + pt->payload_type); + else { + if (!answer_only) { + ilogs(codec, LOG_DEBUG, "Adding codec " STR_FORMAT " (%i) to end of list", + STR_FMT(&pt->encoding_with_params), + pt->payload_type); + __codec_options_set(call, pt, codec_set); + codec_store_add_end(dst, pt); + } + else + ilogs(codec, LOG_DEBUG, "Not adding stray answer codec " STR_FORMAT " (%i)", + STR_FMT(&pt->encoding_with_params), + pt->payload_type); } } if(dst->codec_prefs.head){ @@ -4228,10 +4240,12 @@ void codec_store_check_empty(struct codec_store *dst, struct codec_store *src) { ilog(LOG_WARN, "Usage error: List of codecs empty. Restoring original list of codecs. " "Results may be unexpected."); - codec_store_populate(dst, src, NULL); + codec_store_populate(dst, src, NULL, false); } -void codec_store_populate(struct codec_store *dst, struct codec_store *src, GHashTable *codec_set) { +void codec_store_populate(struct codec_store *dst, struct codec_store *src, GHashTable *codec_set, + bool answer_only) +{ // start fresh struct codec_store orig_dst; codec_store_move(&orig_dst, dst); @@ -4243,6 +4257,12 @@ void codec_store_populate(struct codec_store *dst, struct codec_store *src, GHas struct rtp_payload_type *pt = l->data; struct rtp_payload_type *orig_pt = g_hash_table_lookup(orig_dst.codecs, GINT_TO_POINTER(pt->payload_type)); + if (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; + } ilogs(codec, LOG_DEBUG, "Adding codec " STR_FORMAT " (%i)", STR_FMT(&pt->encoding_with_params), pt->payload_type); diff --git a/include/codec.h b/include/codec.h index f8bbb6121..5097b565b 100644 --- a/include/codec.h +++ b/include/codec.h @@ -94,8 +94,8 @@ void codec_update_all_handlers(struct call_monologue *ml); void codec_store_cleanup(struct codec_store *cs); void codec_store_init(struct codec_store *cs, struct call_media *); -void codec_store_populate(struct codec_store *, struct codec_store *, GHashTable *); -void codec_store_populate_reuse(struct codec_store *, struct codec_store *, GHashTable *); +void codec_store_populate(struct codec_store *, struct codec_store *, GHashTable *, bool answer_only); +void codec_store_populate_reuse(struct codec_store *, struct codec_store *, GHashTable *, bool answer_only); void codec_store_add_raw(struct codec_store *cs, struct rtp_payload_type *pt); void codec_store_strip(struct codec_store *, GQueue *strip, GHashTable *except); void codec_store_offer(struct codec_store *, GQueue *, struct codec_store *); diff --git a/t/auto-daemon-tests.pl b/t/auto-daemon-tests.pl index 2f89bebaf..f1f8114ca 100755 --- a/t/auto-daemon-tests.pl +++ b/t/auto-daemon-tests.pl @@ -5693,9 +5693,9 @@ o=Z 58440449 0 IN IP4 192.168.1.1 s=Z c=IN IP4 192.168.1.1 t=0 0 -m=audio 8000 RTP/AVP 96 97 -a=rtpmap:96 opus/48000 -a=rtpmap:97 telephone-event/48000 +m=audio 8000 RTP/AVP 97 102 +a=rtpmap:97 opus/48000 +a=rtpmap:102 telephone-event/48000 a=sendrecv -------------------------------------- v=0 diff --git a/t/test-transcode.c b/t/test-transcode.c index 5af2bbb19..b6f8822ba 100644 --- a/t/test-transcode.c +++ b/t/test-transcode.c @@ -1473,7 +1473,7 @@ int main(void) { sdp_pt(0, PCMU, 8000); sdp_pt(8, PCMA, 8000); answer(); - expect(B, "7/PCMA/8000 0/PCMU/8000 8/PCMA/8000"); + expect(B, "0/PCMU/8000 8/PCMA/8000"); sdp_pt(0, PCMU, 8000); sdp_pt(8, PCMA, 8000); sdp_pt(9, PCMA, 8000); @@ -1483,7 +1483,7 @@ int main(void) { sdp_pt(0, PCMU, 8000); sdp_pt(8, PCMA, 8000); answer(); - expect(B, "7/PCMA/8000 0/PCMU/8000 8/PCMA/8000"); + expect(B, "0/PCMU/8000 8/PCMA/8000"); end(); start();