From 521532599cbfb16bb817af05e0ef160f1245efd6 Mon Sep 17 00:00:00 2001 From: Donat Zenichev Date: Sun, 19 Feb 2023 21:22:23 +0100 Subject: [PATCH] MT#56128 sdp manipulations: improve structures design In order to improve the efficieny of the code (make lookups faster) use GQueues for CMD_ADD manipulations and GHashTable for CMD_REM manipulations. This gives the following benefits: - faster lookup (check), if any SDP command is to be applied (no need to iterate through all values of commands, as it used to be) - keep a sequence for CMD_ADD values given by option flag(s) In order to keep the code more lightweight and clear, add a separate struct ptr to queues and hash tables into the call_interfaces header. Change-Id: I7e45aca4062750c7b8959473edb410ed76cc04e7 --- daemon/call_interfaces.c | 132 ++++++++++++++++++++++------------ daemon/sdp.c | 147 ++++++++++++++++++++++---------------- include/call_interfaces.h | 7 +- include/sdp.h | 23 +++--- 4 files changed, 189 insertions(+), 120 deletions(-) diff --git a/daemon/call_interfaces.c b/daemon/call_interfaces.c index 6d0382f25..c743aeef8 100644 --- a/daemon/call_interfaces.c +++ b/daemon/call_interfaces.c @@ -62,7 +62,6 @@ static void call_ng_flags_str_ht(struct sdp_ng_flags *out, str *s, void *htp); static void call_ng_flags_str_q_multi(struct sdp_ng_flags *out, str *s, void *qp); static void ng_stats_ssrc(bencode_item_t *dict, struct ssrc_hash *ht); static str *str_dup_escape(const str *s); -static void sdp_command_free(void * p); static int call_stream_address_gstring(GString *o, struct packet_stream *ps, enum stream_address_format format) { int len, ret; @@ -606,13 +605,17 @@ INLINE void ng_osrtp_option(struct sdp_ng_flags *out, str *s, void *dummy) { } } -INLINE void ng_sdp_attr_manipulations(GQueue *sdp_attr_manipulations, bencode_item_t *value) { +INLINE void ng_sdp_attr_manipulations(struct sdp_manipulations_common ** sm_ptr, 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; @@ -633,7 +636,7 @@ INLINE void ng_sdp_attr_manipulations(GQueue *sdp_attr_manipulations, bencode_it { bencode_item_t *command_value = it_c->sibling ? it_c->sibling : NULL; - if (!command_value) /* if no value, makes no sense to continue */ + if (!command_value) /* if no value, makes no sense to check further */ continue; /* detect command type */ @@ -641,46 +644,77 @@ INLINE void ng_sdp_attr_manipulations(GQueue *sdp_attr_manipulations, bencode_it if (!bencode_get_str(it_c, &command_type)) continue; - struct sdp_command * command_obj; - command_obj = g_slice_alloc0(sizeof(*command_obj)); - command_obj->media_type_id = media; - switch (__csh_lookup(&command_type)) { - case CSH_LOOKUP("add"): - command_obj->command_type_id = CMD_ADD; - break; - case CSH_LOOKUP("remove"): - command_obj->command_type_id = CMD_REM; + + /* CMD_ADD commands */ + case CSH_LOOKUP("add"):; + GQueue * q_ptr = NULL; + + 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: unspported SDP section targeted."); + continue; + } + + for (bencode_item_t *it_v = command_value->child; it_v; it_v = it_v->sibling) + { + /* detect command value */ + str command_value; + if (!bencode_get_str(it_v, &command_value)) + continue; + + str * s_copy = str_dup_escape(&command_value); + g_queue_push_tail(q_ptr, s_copy); + } break; - default: - ilog(LOG_WARN, "SDP manipulations: Unknown SDP manipulation command type."); - g_slice_free1(sizeof(*command_obj), command_obj); - continue; - } - GHashTable ** ht = &command_obj->command_values; - *ht = g_hash_table_new_full(str_case_hash, str_case_equal, free, NULL); + /* CMD_REM commands */ + case CSH_LOOKUP("remove"):; + GHashTable ** ht = NULL; + + 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: unspported SDP section targeted."); + continue; + } - for (bencode_item_t *it_v = command_value->child; it_v; it_v = it_v->sibling) - { - /* detect command value */ - str command_value; - if (!bencode_get_str(it_v, &command_value)) - continue; + /* a table can already be allocated by similar commands in previous iterations */ + if (!*ht) + *ht = g_hash_table_new_full(str_case_hash, str_case_equal, free, NULL); - str *s_copy = str_dup_escape(&command_value); - g_hash_table_replace(*ht, s_copy, s_copy); - } + for (bencode_item_t *it_v = command_value->child; it_v; it_v = it_v->sibling) + { + /* detect command value */ + str command_value; + if (!bencode_get_str(it_v, &command_value)) + continue; - /* no values - is a bad command */ - if (g_hash_table_size(command_obj->command_values) == 0) - { - ilog(LOG_WARN, "SDP manipulations: An issue with given value(s), can't handle it."); - sdp_command_free(command_obj); - /* this command had no values, but still continue checking other commands */ - continue; - } else { - g_queue_push_tail(sdp_attr_manipulations, command_obj); + str *s_copy = str_dup_escape(&command_value); + g_hash_table_replace(*ht, s_copy, s_copy); + } + break; + + default: + ilog(LOG_WARN, "SDP manipulations: Unknown SDP manipulation command type."); + continue; } } } @@ -1343,7 +1377,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_attr_manipulations, value); + ng_sdp_attr_manipulations(&out->sdp_manipulations, value); break; case CSH_LOOKUP("received from"): case CSH_LOOKUP("received-from"): @@ -1699,11 +1733,19 @@ 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 sdp_command_free(void * p) -{ - struct sdp_command * attr = p; - g_hash_table_destroy(attr->command_values); - g_slice_free1(sizeof(*attr), attr); +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); + + 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); } void call_ng_free_flags(struct sdp_ng_flags *flags) { @@ -1718,7 +1760,6 @@ void call_ng_free_flags(struct sdp_ng_flags *flags) { if (flags->frequencies) g_array_free(flags->frequencies, true); - g_queue_clear_full(&flags->sdp_attr_manipulations, sdp_command_free); g_queue_clear_full(&flags->from_tags, free); g_queue_clear_full(&flags->codec_offer, free); g_queue_clear_full(&flags->codec_transcode, free); @@ -1728,6 +1769,9 @@ void call_ng_free_flags(struct sdp_ng_flags *flags) { g_queue_clear_full(&flags->codec_mask, free); 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); } static enum load_limit_reasons call_offer_session_limit(void) { diff --git a/daemon/sdp.c b/daemon/sdp.c index 46b946ca6..1ce8521ad 100644 --- a/daemon/sdp.c +++ b/daemon/sdp.c @@ -278,37 +278,24 @@ struct sdp_attribute { /* example: a=rtpmap:8 PCMA/8000 */ } u; }; - - +/** + * Globaly visible variables for this file. + */ static char __id_buf[6*2 + 1]; // 6 hex encoded characters const str rtpe_instance_id = STR_CONST_INIT(__id_buf); +/** + * Declarations for inner functions/helpers. + */ static void attr_free(void *p); static void attr_insert(struct sdp_attributes *attrs, struct sdp_attribute *attr); - - INLINE void chopper_append_c(struct sdp_chopper *c, const char *s); - -static int sdp_manipulate_command_cmp(gconstpointer a, gconstpointer b) { - const struct sdp_command * sc = a; - const struct sdp_command_fictitious * sc_lookup = b; - if (sc->command_type_id != sc_lookup->command_type_id) - return 1; - if (sc->media_type_id != sc_lookup->media_type_id) - return 1; - /* here expected only lookup by one value */ - if (NULL != sc_lookup->command_value) { - if (!g_hash_table_lookup(sc->command_values, sc_lookup->command_value)) - return 1; - } - return 0; -} - /** * Checks whether a given type of SDP manipulation exists for a given session level. */ -static int sdp_manipulate_check(enum command_type command_type, GQueue *sdp_manipulate, +static int sdp_manipulate_check(enum command_type command_type, + struct sdp_manipulations_common * sdp_manipulations, enum media_type media_type, str *attr_name) { /* for now we only support session lvl, audio and media streams */ if (media_type == MT_OTHER) @@ -318,10 +305,47 @@ static int sdp_manipulate_check(enum command_type command_type, GQueue *sdp_mani return 0; } - struct sdp_command_fictitious fictitious = {command_type, media_type, attr_name}; + /* no need for checks, if not given in flags */ + if (!sdp_manipulations) + return 0; - if (g_queue_find_custom(sdp_manipulate, &fictitious, sdp_manipulate_command_cmp)) - return 1; + switch (command_type) { + case CMD_ADD:; + 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; + } + if (q_ptr && q_ptr->head && !g_queue_is_empty(q_ptr)) + return 1; + break; + + case CMD_REM:; + GHashTable *ht = NULL; + switch (media_type) { + case MT_AUDIO: + ht = sdp_manipulations->rem_commands_audio; + break; + case MT_VIDEO: + ht = sdp_manipulations->rem_commands_video; + break; + default: /* MT_UNKNOWN */ + ht = sdp_manipulations->rem_commands_glob; + } + if (ht && g_hash_table_lookup(ht, attr_name)) + return 1; + break; + + default: + ilog(LOG_WARNING, "SDP manipulations: Unsupported command type '%d', can't manipulate these attributes.", + command_type); + } return 0; } @@ -329,30 +353,29 @@ static int sdp_manipulate_check(enum command_type command_type, GQueue *sdp_mani /** * Adds values into a requested session level (global, audio, video) */ -static void sdp_manipulations_add(struct sdp_chopper *chop, GQueue *sdp_manipulate, +static void sdp_manipulations_add(struct sdp_chopper *chop, + struct sdp_manipulations_common * sdp_manipulations, enum media_type media_type) { - for (GList *l = sdp_manipulate->head; l; l = l->next) - { - struct sdp_command * command = l->data; - GList *ll = NULL; - - if (command->command_type_id != CMD_ADD || - command->media_type_id != media_type) - continue; - /* TODO: add lookup if not already in the body */ + 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; + } - ll = g_hash_table_get_values(command->command_values); - for (GList *l = ll; l; l = l->next) - { - str * value = l->data; - chopper_append_c(chop, "a="); - chopper_append_c(chop, value->s); - chopper_append_c(chop, "\r\n"); - } + for (GList *l = q_ptr->head; l; l = l->next) + { + str * attr_value = l->data; - if (ll) - g_list_free(ll); + chopper_append_c(chop, "a="); + chopper_append_c(chop, attr_value->s); + chopper_append_c(chop, "\r\n"); } } @@ -2238,7 +2261,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, GQueue *sdp_manipulate) + struct sdp_ng_flags *flags, struct sdp_manipulations_common * sdp_manipulations) { GList *l; struct sdp_attribute *attr; @@ -2247,7 +2270,7 @@ static int process_session_attributes(struct sdp_chopper *chop, struct sdp_attri attr = l->data; /* if attr is supposed to be removed don't add to the chop->output */ - if (sdp_manipulate_check(CMD_REM, sdp_manipulate, MT_UNKNOWN, &attr->line_value)) + if (sdp_manipulate_check(CMD_REM, sdp_manipulations, MT_UNKNOWN, &attr->line_value)) goto strip; switch (attr->attr) { @@ -2315,7 +2338,7 @@ strip: /* 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, GQueue *sdp_manipulate) + struct sdp_ng_flags *flags, struct call_media *media, struct sdp_manipulations_common * sdp_manipulations) { GList *l; struct sdp_attributes *attrs = &sdp->attributes; @@ -2329,7 +2352,7 @@ static int process_media_attributes(struct sdp_chopper *chop, struct sdp_media * goto strip; /* if attr is supposed to be removed don't add to the chop->output */ - if (sdp_manipulate_check(CMD_REM, sdp_manipulate, sdp->media_type_id, &attr->line_value)) + if (sdp_manipulate_check(CMD_REM, sdp_manipulations, sdp->media_type_id, &attr->line_value)) goto strip; switch (attr->attr) { @@ -2797,10 +2820,10 @@ static void append_attr_to_gstring(GString *s, char * name, const str * value, { str attr; str_init(&attr, name); - GQueue *sdp_manipulate = &flags->sdp_attr_manipulations; + struct sdp_manipulations_common *sdp_manipulations = flags->sdp_manipulations; /* take into account SDP arbitary manipulations */ - if (sdp_manipulate_check(CMD_REM, sdp_manipulate, media_type, &attr)) { + if (sdp_manipulate_check(CMD_REM, sdp_manipulations, media_type, &attr)) { ilog(LOG_DEBUG, "Cannot insert: '%s' because prevented by SDP manipulations", name); return; } @@ -2823,10 +2846,10 @@ static void append_attr_char_to_gstring(GString *s, char * name, const char * va { str attr; str_init(&attr, name); - GQueue *sdp_manipulate = &flags->sdp_attr_manipulations; + struct sdp_manipulations_common *sdp_manipulations = flags->sdp_manipulations; /* take into account SDP arbitary manipulations */ - if (sdp_manipulate_check(CMD_REM, sdp_manipulate, media_type, &attr)) { + if (sdp_manipulate_check(CMD_REM, sdp_manipulations, media_type, &attr)) { ilog(LOG_DEBUG, "Cannot insert: '%s' because prevented by SDP manipulations", name); return; } @@ -2849,10 +2872,10 @@ static void append_attr_int_to_gstring(GString *s, char * name, const int * valu { str attr; str_init(&attr, name); - GQueue *sdp_manipulate = &flags->sdp_attr_manipulations; + struct sdp_manipulations_common *sdp_manipulations = flags->sdp_manipulations; /* take into account SDP arbitary manipulations */ - if (sdp_manipulate_check(CMD_REM, sdp_manipulate, media_type, &attr)) { + if (sdp_manipulate_check(CMD_REM, sdp_manipulations, media_type, &attr)) { ilog(LOG_DEBUG, "Cannot insert: '%s' because prevented by SDP manipulations", name); return; } @@ -2985,7 +3008,7 @@ static const char *replace_sdp_media_section(struct sdp_chopper *chop, struct ca bool is_active = true; - GQueue *sdp_manipulate = &flags->sdp_attr_manipulations; + 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"; @@ -3022,7 +3045,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_manipulate)) + if (process_media_attributes(chop, sdp_media, flags, call_media, sdp_manipulations)) goto error; copy_up_to_end_of(chop, &sdp_media->s); @@ -3055,7 +3078,7 @@ int sdp_replace(struct sdp_chopper *chop, GQueue *sessions, struct call_monologu m = monologue->medias.head; media_index = 1; - GQueue *sdp_manipulate = &flags->sdp_attr_manipulations; + struct sdp_manipulations_common *sdp_manipulations = flags->sdp_manipulations; for (l = sessions->head; l; l = l->next) { session = l->data; @@ -3145,7 +3168,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_manipulate)) + if (process_session_attributes(chop, &session->attributes, flags, sdp_manipulations)) goto error; } @@ -3155,8 +3178,8 @@ int sdp_replace(struct sdp_chopper *chop, GQueue *sessions, struct call_monologu print_sdp_session_section(chop->output, flags, call_media); /* ADD arbitary SDP manipulations for a session sessions */ - if (sdp_manipulate_check(CMD_ADD, sdp_manipulate, MT_UNKNOWN, NULL)) - sdp_manipulations_add(chop, sdp_manipulate, MT_UNKNOWN); + if (sdp_manipulate_check(CMD_ADD, sdp_manipulations, MT_UNKNOWN, NULL)) + sdp_manipulations_add(chop, sdp_manipulations, MT_UNKNOWN); for (k = session->media_streams.head; k; k = k->next) { sdp_media = k->data; @@ -3233,8 +3256,8 @@ int sdp_replace(struct sdp_chopper *chop, GQueue *sessions, struct call_monologu } /* ADD arbitary SDP manipulations for audio/video media sessions */ - if (sdp_manipulate_check(CMD_ADD, sdp_manipulate, sdp_media->media_type_id, NULL)) - sdp_manipulations_add(chop, sdp_manipulate, sdp_media->media_type_id); + 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); media_index++; m = m->next; diff --git a/include/call_interfaces.h b/include/call_interfaces.h index 7fb47eb97..c90325896 100644 --- a/include/call_interfaces.h +++ b/include/call_interfaces.h @@ -8,7 +8,7 @@ #include "bencode.h" #include "socket.h" #include "call.h" - +#include "sdp.h" struct call; struct call_stats; @@ -57,7 +57,10 @@ struct sdp_ng_flags { GQueue sdes_order; /* the order, in which crypto suites are being added to the SDP */ GQueue sdes_offerer_pref; /* preferred crypto suites to be selected for the offerer */ str dtls_fingerprint; - GQueue sdp_attr_manipulations; /* commands to manipulate attr lines in SDP */ + + /* commands to manipulate attr lines in SDP */ + struct sdp_manipulations_common * sdp_manipulations; + enum { ICE_DEFAULT = 0, ICE_REMOVE, diff --git a/include/sdp.h b/include/sdp.h index adf81f551..598361e71 100644 --- a/include/sdp.h +++ b/include/sdp.h @@ -13,18 +13,17 @@ enum command_type { CMD_SUBST, }; -/* command to manipulate attr in SDP body */ -struct sdp_command { - enum command_type command_type_id; /* command to apply */ - enum media_type media_type_id; /* scope (session, media audio, media video) */ - GHashTable * command_values; /* a list of command values, values in str format */ -}; - -/* This one is used only for lookups */ -struct sdp_command_fictitious { - enum command_type command_type_id; - enum media_type media_type_id; - str * command_value; +/* A structure for SDP arbitary 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; }; struct ice_candidate;