From 7990d127b8fefce12a62682d30fab656d8fdc69a Mon Sep 17 00:00:00 2001 From: Richard Fuchs Date: Wed, 12 Jul 2023 09:40:20 -0400 Subject: [PATCH] MT#56128 convert sdp_manipulations to array Use an indexed array to generically support all possible media types instead of hard-coding a few possible types. This makes lookup faster and is also future-proof. Add helper functions to allocate and lookup the appropriate object. Change-Id: I28406b2c5a6829330f41601fa52e738d9a8ee315 --- daemon/call_interfaces.c | 102 +++++++++--------------------- daemon/sdp.c | 127 +++++++++++--------------------------- include/call_interfaces.h | 19 +++++- include/sdp.h | 16 ++--- 4 files changed, 86 insertions(+), 178 deletions(-) diff --git a/daemon/call_interfaces.c b/daemon/call_interfaces.c index d2f325336..38a9eba63 100644 --- a/daemon/call_interfaces.c +++ b/daemon/call_interfaces.c @@ -588,21 +588,16 @@ INLINE void ng_osrtp_option(struct sdp_ng_flags *out, str *s, void *dummy) { } } -INLINE void ng_sdp_attr_manipulations(struct sdp_manipulations_common ** sm_ptr, bencode_item_t *value) { +INLINE void ng_sdp_attr_manipulations(struct sdp_ng_flags *flags, bencode_item_t *value) { if (!value || value->type != BENCODE_DICTIONARY) { ilog(LOG_WARN, "SDP manipulations: Wrong type for this type of command."); return; } - /* check if it's not allocated already for this monologue */ - if (!*sm_ptr) - *sm_ptr = g_slice_alloc0(sizeof(**sm_ptr)); - for (bencode_item_t *it = value->child; it; it = it->sibling) { bencode_item_t *command_action = it->sibling ? it->sibling : NULL; - enum media_type media; str media_type; GQueue * q_ptr = NULL; GHashTable ** ht = NULL; @@ -614,8 +609,12 @@ INLINE void ng_sdp_attr_manipulations(struct sdp_manipulations_common ** sm_ptr, if (!bencode_get_str(it, &media_type)) continue; - media = (!str_cmp(&media_type, "none") || !str_cmp(&media_type, "global")) ? - MT_UNKNOWN : codec_get_type(&media_type); + struct sdp_manipulations *sm = sdp_manipulations_get_by_name(flags, &media_type); + if (!sm) { + ilog(LOG_WARN, "SDP manipulations: unsupported SDP section '" STR_FORMAT "' targeted.", + STR_FMT(&media_type)); + continue; + } for (bencode_item_t *it_c = command_action->child; it_c; it_c = it_c->sibling) { @@ -632,22 +631,8 @@ INLINE void ng_sdp_attr_manipulations(struct sdp_manipulations_common ** sm_ptr, switch (__csh_lookup(&command_type)) { /* CMD_ADD / CMD_SUBST commands */ - case CSH_LOOKUP("substitute"):; - - switch (media) { - case MT_UNKNOWN: - ht = &(*sm_ptr)->subst_commands_glob; - break; - case MT_AUDIO: - ht = &(*sm_ptr)->subst_commands_audio; - break; - case MT_VIDEO: - ht = &(*sm_ptr)->subst_commands_video; - break; - default: - ilog(LOG_WARN, "SDP manipulations: unsupported SDP section targeted."); - continue; - } + case CSH_LOOKUP("substitute"): + ht = &sm->subst_commands; /* a table can already be allocated by similar commands in previous iterations */ if (!*ht) @@ -676,21 +661,8 @@ INLINE void ng_sdp_attr_manipulations(struct sdp_manipulations_common ** sm_ptr, } break; - case CSH_LOOKUP("add"):; - switch (media) { - case MT_UNKNOWN: - q_ptr = &(*sm_ptr)->add_commands_glob; - break; - case MT_AUDIO: - q_ptr = &(*sm_ptr)->add_commands_audio; - break; - case MT_VIDEO: - q_ptr = &(*sm_ptr)->add_commands_video; - break; - default: - ilog(LOG_WARN, "SDP manipulations: unsupported SDP section targeted."); - continue; - } + case CSH_LOOKUP("add"): + q_ptr = &sm->add_commands; for (bencode_item_t *it_v = command_value->child; it_v; it_v = it_v->sibling) { @@ -705,21 +677,8 @@ INLINE void ng_sdp_attr_manipulations(struct sdp_manipulations_common ** sm_ptr, break; /* CMD_REM commands */ - case CSH_LOOKUP("remove"):; - switch (media) { - case MT_UNKNOWN: - ht = &(*sm_ptr)->rem_commands_glob; - break; - case MT_AUDIO: - ht = &(*sm_ptr)->rem_commands_audio; - break; - case MT_VIDEO: - ht = &(*sm_ptr)->rem_commands_video; - break; - default: - ilog(LOG_WARN, "SDP manipulations: unsupported SDP section targeted."); - continue; - } + case CSH_LOOKUP("remove"): + ht = &sm->rem_commands; /* a table can already be allocated by similar commands in previous iterations */ if (!*ht) @@ -1435,7 +1394,7 @@ static void call_ng_main_flags(struct sdp_ng_flags *out, str *key, bencode_item_ case CSH_LOOKUP("SDP-attr"): if (value->type != BENCODE_DICTIONARY) break; - ng_sdp_attr_manipulations(&out->sdp_manipulations, value); + ng_sdp_attr_manipulations(out, value); break; case CSH_LOOKUP("received from"): case CSH_LOOKUP("received-from"): @@ -1833,26 +1792,22 @@ static void call_ng_process_flags(struct sdp_ng_flags *out, bencode_item_t *inpu call_ng_dict_iter(out, input, call_ng_main_flags); } -static void ng_sdp_attr_manipulations_free(struct sdp_manipulations_common * sdp_manipulations) { - if (sdp_manipulations->rem_commands_glob) - g_hash_table_destroy(sdp_manipulations->rem_commands_glob); - if (sdp_manipulations->rem_commands_audio) - g_hash_table_destroy(sdp_manipulations->rem_commands_audio); - if (sdp_manipulations->rem_commands_video) - g_hash_table_destroy(sdp_manipulations->rem_commands_video); +static void ng_sdp_attr_manipulations_free(struct sdp_manipulations * array[__MT_MAX]) { + for (int i = 0; i < __MT_MAX; i++) { + struct sdp_manipulations *sdp_manipulations = array[i]; + if (!sdp_manipulations) + continue; - if (sdp_manipulations->subst_commands_glob) - g_hash_table_destroy(sdp_manipulations->subst_commands_glob); - if (sdp_manipulations->subst_commands_audio) - g_hash_table_destroy(sdp_manipulations->subst_commands_audio); - if (sdp_manipulations->subst_commands_video) - g_hash_table_destroy(sdp_manipulations->subst_commands_video); + if (sdp_manipulations->rem_commands) + g_hash_table_destroy(sdp_manipulations->rem_commands); + if (sdp_manipulations->subst_commands) + g_hash_table_destroy(sdp_manipulations->subst_commands); + g_queue_clear_full(&sdp_manipulations->add_commands, free); - g_queue_clear_full(&sdp_manipulations->add_commands_glob, free); - g_queue_clear_full(&sdp_manipulations->add_commands_audio, free); - g_queue_clear_full(&sdp_manipulations->add_commands_video, free); + g_slice_free1(sizeof(*sdp_manipulations), sdp_manipulations); - g_slice_free1(sizeof(*sdp_manipulations), sdp_manipulations); + array[i] = NULL; + } } void call_ng_free_flags(struct sdp_ng_flags *flags) { @@ -1877,8 +1832,7 @@ void call_ng_free_flags(struct sdp_ng_flags *flags) { g_queue_clear_full(&flags->sdes_order, free); g_queue_clear_full(&flags->sdes_offerer_pref, free); - if (flags->sdp_manipulations) - ng_sdp_attr_manipulations_free(flags->sdp_manipulations); + ng_sdp_attr_manipulations_free(flags->sdp_manipulations); } static enum load_limit_reasons call_offer_session_limit(void) { diff --git a/daemon/sdp.c b/daemon/sdp.c index 888e1a4b9..0398eb3f4 100644 --- a/daemon/sdp.c +++ b/daemon/sdp.c @@ -295,8 +295,8 @@ INLINE void chopper_append_c(struct sdp_chopper *c, const char *s); * Checks whether a given type of SDP manipulation exists for a given session level. */ static int sdp_manipulate_check(enum command_type command_type, - struct sdp_manipulations_common * sdp_manipulations, - enum media_type media_type, str * attr_name) { + struct sdp_manipulations * sdp_manipulations, + str * attr_name) { /* no need for checks, if not given in flags */ if (!sdp_manipulations) @@ -310,19 +310,7 @@ static int sdp_manipulate_check(enum command_type command_type, if (!attr_name || !attr_name->len) break; - switch (media_type) { - case MT_AUDIO: - ht = sdp_manipulations->subst_commands_audio; - break; - case MT_VIDEO: - ht = sdp_manipulations->subst_commands_video; - break; - case MT_UNKNOWN: - ht = sdp_manipulations->subst_commands_glob; - break; - default: - break; - } + ht = sdp_manipulations->subst_commands; str * l = ht ? g_hash_table_lookup(ht, attr_name) : NULL; if (l) @@ -330,20 +318,8 @@ static int sdp_manipulate_check(enum command_type command_type, break; case CMD_ADD: - switch (media_type) { - case MT_AUDIO: - q_ptr = &sdp_manipulations->add_commands_audio; - break; - case MT_VIDEO: - q_ptr = &sdp_manipulations->add_commands_video; - break; - case MT_UNKNOWN: - q_ptr = &sdp_manipulations->add_commands_glob; - break; - default: - break; - } - if (q_ptr && q_ptr->head) + q_ptr = &sdp_manipulations->add_commands; + if (q_ptr->head) return 1; break; @@ -351,19 +327,7 @@ static int sdp_manipulate_check(enum command_type command_type, if (!attr_name || !attr_name->len) break; - switch (media_type) { - case MT_AUDIO: - ht = sdp_manipulations->rem_commands_audio; - break; - case MT_VIDEO: - ht = sdp_manipulations->rem_commands_video; - break; - case MT_UNKNOWN: - ht = sdp_manipulations->rem_commands_glob; - break; - default: - break; - } + ht = sdp_manipulations->rem_commands; if (ht && g_hash_table_lookup(ht, attr_name)) return 1; break; @@ -380,20 +344,9 @@ static int sdp_manipulate_check(enum command_type command_type, * Adds values into a requested session level (global, audio, video) */ static void sdp_manipulations_add(struct sdp_chopper *chop, - struct sdp_manipulations_common * sdp_manipulations, - enum media_type media_type) { + struct sdp_manipulations * sdp_manipulations) { - GQueue * q_ptr = NULL; - switch (media_type) { - case MT_AUDIO: - q_ptr = &sdp_manipulations->add_commands_audio; - break; - case MT_VIDEO: - q_ptr = &sdp_manipulations->add_commands_video; - break; - default: /* MT_UNKNOWN */ - q_ptr = &sdp_manipulations->add_commands_glob; - } + GQueue * q_ptr = &sdp_manipulations->add_commands; for (GList *l = q_ptr->head; l; l = l->next) { @@ -409,21 +362,10 @@ static void sdp_manipulations_add(struct sdp_chopper *chop, * Substitute values for a requested session level (global, audio, video) */ static void sdp_manipulations_subst(struct sdp_chopper *chop, - struct sdp_manipulations_common * sdp_manipulations, - enum media_type media_type, str * attr_name) { + struct sdp_manipulations * sdp_manipulations, + str * attr_name) { - GHashTable * ht = NULL; - - switch (media_type) { - case MT_AUDIO: - ht = sdp_manipulations->subst_commands_audio; - break; - case MT_VIDEO: - ht = sdp_manipulations->subst_commands_video; - break; - default: /* MT_UNKNOWN */ - ht = sdp_manipulations->subst_commands_glob; - } + GHashTable * ht = sdp_manipulations->subst_commands; str * cmd_subst_value = ht ? g_hash_table_lookup(ht, attr_name) : NULL; if (!cmd_subst_value) @@ -2314,7 +2256,7 @@ void sdp_chopper_destroy_ret(struct sdp_chopper *chop, str *ret) { /* processing existing session attributes (those present in offer/answer) */ static int process_session_attributes(struct sdp_chopper *chop, struct sdp_attributes *attrs, - struct sdp_ng_flags *flags, struct sdp_manipulations_common * sdp_manipulations) + struct sdp_ng_flags *flags) { GList *l; struct sdp_attribute *attr; @@ -2322,12 +2264,14 @@ static int process_session_attributes(struct sdp_chopper *chop, struct sdp_attri for (l = attrs->list.head; l; l = l->next) { attr = l->data; + struct sdp_manipulations *sdp_manipulations = sdp_manipulations_get_by_id(flags, MT_UNKNOWN); + /* if attr is supposed to be removed don't add to the chop->output */ - if (sdp_manipulate_check(CMD_REM, sdp_manipulations, MT_UNKNOWN, &attr->line_value)) + if (sdp_manipulate_check(CMD_REM, sdp_manipulations, &attr->line_value)) goto strip; /* if attr is supposed to be substituted don't add to the chop->output, but add another value */ - if (sdp_manipulate_check(CMD_SUBST, sdp_manipulations, MT_UNKNOWN, &attr->line_value)) + if (sdp_manipulate_check(CMD_SUBST, sdp_manipulations, &attr->line_value)) goto strip_with_subst; switch (attr->attr) { @@ -2396,7 +2340,7 @@ strip_with_subst: return -1; if (skip_over(chop, &attr->full_line)) return -1; - sdp_manipulations_subst(chop, sdp_manipulations, MT_UNKNOWN, &attr->line_value); + sdp_manipulations_subst(chop, sdp_manipulations, &attr->line_value); } return 0; @@ -2404,7 +2348,7 @@ strip_with_subst: /* processing existing media attributes (those present in offer/answer) */ static int process_media_attributes(struct sdp_chopper *chop, struct sdp_media *sdp, - struct sdp_ng_flags *flags, struct call_media *media, struct sdp_manipulations_common * sdp_manipulations) + struct sdp_ng_flags *flags, struct call_media *media) { GList *l; struct sdp_attributes *attrs = &sdp->attributes; @@ -2417,12 +2361,15 @@ static int process_media_attributes(struct sdp_chopper *chop, struct sdp_media * if (MEDIA_ISSET(media, GENERATOR)) goto strip; + struct sdp_manipulations *sdp_manipulations = sdp_manipulations_get_by_id(flags, + sdp->media_type_id); + /* if attr is supposed to be removed don't add to the chop->output */ - if (sdp_manipulate_check(CMD_REM, sdp_manipulations, sdp->media_type_id, &attr->line_value)) + if (sdp_manipulate_check(CMD_REM, sdp_manipulations, &attr->line_value)) goto strip; /* if attr is supposed to be substituted don't add to the chop->output, but add another value */ - if (sdp_manipulate_check(CMD_SUBST, sdp_manipulations, sdp->media_type_id, &attr->line_value)) + if (sdp_manipulate_check(CMD_SUBST, sdp_manipulations, &attr->line_value)) goto strip_with_subst; switch (attr->attr) { @@ -2520,7 +2467,7 @@ strip_with_subst: return -1; if (skip_over(chop, &attr->full_line)) return -1; - sdp_manipulations_subst(chop, sdp_manipulations, sdp->media_type_id, &attr->line_value); + sdp_manipulations_subst(chop, sdp_manipulations, &attr->line_value); } return 0; @@ -2900,9 +2847,9 @@ static void append_attr_to_gstring(GString *s, char * name, const str * value, { str attr; str_init(&attr, name); - struct sdp_manipulations_common *sdp_manipulations = flags->sdp_manipulations; + struct sdp_manipulations *sdp_manipulations = sdp_manipulations_get_by_id(flags, media_type); /* take into account SDP arbitrary manipulations */ - if (sdp_manipulate_check(CMD_REM, sdp_manipulations, media_type, &attr)) { + if (sdp_manipulate_check(CMD_REM, sdp_manipulations, &attr)) { ilog(LOG_DEBUG, "Cannot insert: '%s' because prevented by SDP manipulations", name); return; } @@ -2924,9 +2871,9 @@ static void append_attr_int_to_gstring(GString *s, char * name, const int * valu { str attr; str_init(&attr, name); - struct sdp_manipulations_common *sdp_manipulations = flags->sdp_manipulations; + struct sdp_manipulations *sdp_manipulations = sdp_manipulations_get_by_id(flags, media_type); /* take into account SDP arbitrary manipulations */ - if (sdp_manipulate_check(CMD_REM, sdp_manipulations, media_type, &attr)) { + if (sdp_manipulate_check(CMD_REM, sdp_manipulations, &attr)) { ilog(LOG_DEBUG, "Cannot insert: '%s' because prevented by SDP manipulations", name); return; } @@ -3058,8 +3005,6 @@ static const char *replace_sdp_media_section(struct sdp_chopper *chop, struct ca bool is_active = true; - struct sdp_manipulations_common *sdp_manipulations = flags->sdp_manipulations; - if (flags->ice_option != ICE_FORCE_RELAY && call_media->type_id != MT_MESSAGE) { err = "failed to replace media type"; if (replace_media_type(chop, sdp_media, call_media)) @@ -3095,7 +3040,7 @@ static const char *replace_sdp_media_section(struct sdp_chopper *chop, struct ca } err = "failed to process media attributes"; - if (process_media_attributes(chop, sdp_media, flags, call_media, sdp_manipulations)) + if (process_media_attributes(chop, sdp_media, flags, call_media)) goto error; copy_up_to_end_of(chop, &sdp_media->s); @@ -3127,8 +3072,6 @@ int sdp_replace(struct sdp_chopper *chop, GQueue *sessions, struct call_monologu unsigned int media_index = 0; - struct sdp_manipulations_common *sdp_manipulations = flags->sdp_manipulations; - for (l = sessions->head; l; l = l->next) { session = l->data; @@ -3216,7 +3159,7 @@ int sdp_replace(struct sdp_chopper *chop, GQueue *sessions, struct call_monologu if (!MEDIA_ISSET(call_media, PASSTHRU)) { err = "failed to process session attributes"; - if (process_session_attributes(chop, &session->attributes, flags, sdp_manipulations)) + if (process_session_attributes(chop, &session->attributes, flags)) goto error; } @@ -3226,8 +3169,9 @@ int sdp_replace(struct sdp_chopper *chop, GQueue *sessions, struct call_monologu print_sdp_session_section(chop->output, flags, call_media); /* ADD arbitrary SDP manipulations for a session sessions */ - if (sdp_manipulate_check(CMD_ADD, sdp_manipulations, MT_UNKNOWN, NULL)) - sdp_manipulations_add(chop, sdp_manipulations, MT_UNKNOWN); + struct sdp_manipulations *sdp_manipulations = sdp_manipulations_get_by_id(flags, MT_UNKNOWN); + if (sdp_manipulate_check(CMD_ADD, sdp_manipulations, NULL)) + sdp_manipulations_add(chop, sdp_manipulations); for (k = session->media_streams.head; k; k = k->next) { sdp_media = k->data; @@ -3301,8 +3245,9 @@ int sdp_replace(struct sdp_chopper *chop, GQueue *sessions, struct call_monologu } /* ADD arbitrary SDP manipulations for audio/video media sessions */ - if (sdp_manipulate_check(CMD_ADD, sdp_manipulations, sdp_media->media_type_id, NULL)) - sdp_manipulations_add(chop, sdp_manipulations, sdp_media->media_type_id); + sdp_manipulations = sdp_manipulations_get_by_id(flags, sdp_media->media_type_id); + if (sdp_manipulate_check(CMD_ADD, sdp_manipulations, NULL)) + sdp_manipulations_add(chop, sdp_manipulations); media_index++; } diff --git a/include/call_interfaces.h b/include/call_interfaces.h index 1f2b27992..41db9fd9f 100644 --- a/include/call_interfaces.h +++ b/include/call_interfaces.h @@ -61,7 +61,7 @@ struct sdp_ng_flags { str dtls_fingerprint; /* commands to manipulate attr lines in SDP */ - struct sdp_manipulations_common * sdp_manipulations; + struct sdp_manipulations * sdp_manipulations[__MT_MAX]; enum { ICE_DEFAULT = 0, @@ -254,5 +254,22 @@ int call_interfaces_init(void); void call_interfaces_free(void); void call_interfaces_timer(void); +INLINE struct sdp_manipulations *sdp_manipulations_get_by_id(struct sdp_ng_flags *f, enum media_type id) { + if (id < 0 || id >= G_N_ELEMENTS(f->sdp_manipulations)) + return NULL; + if (!f->sdp_manipulations[id]) + f->sdp_manipulations[id] = g_slice_alloc0(sizeof(*f->sdp_manipulations[id])); + return f->sdp_manipulations[id]; +} + +INLINE struct sdp_manipulations *sdp_manipulations_get_by_name(struct sdp_ng_flags *f, const str *s) { + if (!str_cmp(s, "none") || !str_cmp(s, "global")) + return sdp_manipulations_get_by_id(f, MT_UNKNOWN); + enum media_type id = codec_get_type(s); + if (id == MT_OTHER) + return NULL; + return sdp_manipulations_get_by_id(f, id); +} + #endif diff --git a/include/sdp.h b/include/sdp.h index 7dbe97f69..49073200b 100644 --- a/include/sdp.h +++ b/include/sdp.h @@ -16,18 +16,10 @@ enum command_type { /* A structure for SDP arbitrary manipulations on all levels of SDP: * session (global), media (audio/video). Works only on `a=` lines. */ -struct sdp_manipulations_common { - GQueue add_commands_glob; - GQueue add_commands_audio; - GQueue add_commands_video; - - GHashTable * rem_commands_glob; - GHashTable * rem_commands_audio; - GHashTable * rem_commands_video; - - GHashTable * subst_commands_glob; - GHashTable * subst_commands_audio; - GHashTable * subst_commands_video; +struct sdp_manipulations { + GQueue add_commands; + GHashTable * rem_commands; + GHashTable * subst_commands; }; struct ice_candidate;