From e561a1cc0c342cca8ad7e5865b9e369c631d32fe Mon Sep 17 00:00:00 2001 From: Richard Fuchs Date: Tue, 28 Mar 2023 10:45:42 -0400 Subject: [PATCH] MT#56447 refactor trickle ICE handling Move trickle ICE handling out of __media_init_from_flags and into a higher-level function. This obsoletes the special magic return value to indicate a trickle ICE failure. All methods accepting trickle ICE updates must now explicitly call the trickle ICE update function. The requirement to have a full dialogue for trickle ICE updates is removed as trickle ICE only affects one side. Change-Id: I0850e1858876ca7bcdd39b7144b53b5a4afed53e --- daemon/call.c | 24 +++-------------- daemon/call_interfaces.c | 31 +++++++++++----------- daemon/control_ng.c | 4 +-- daemon/ice.c | 55 +++++++++++++++++++++++++++++++++++---- include/call.h | 1 - include/call_interfaces.h | 5 ++-- include/ice.h | 7 +++-- 7 files changed, 79 insertions(+), 48 deletions(-) diff --git a/daemon/call.c b/daemon/call.c index 464fc2f33..e59e8c846 100644 --- a/daemon/call.c +++ b/daemon/call.c @@ -2861,22 +2861,11 @@ static void __update_media_label(struct call_media *media, struct call_media *ot } // `media` can be NULL -static int __media_init_from_flags(struct call_media *other_media, struct call_media *media, +static void __media_init_from_flags(struct call_media *other_media, struct call_media *media, struct stream_params *sp, struct sdp_ng_flags *flags) { struct call *call = other_media->call; - if (flags && flags->fragment) { - // trickle ICE SDP fragment. don't do anything other than update - // the ICE stuff. - if (!MEDIA_ISSET(other_media, TRICKLE_ICE)) - return ERROR_NO_ICE_AGENT; - if (!other_media->ice_agent) - return ERROR_NO_ICE_AGENT; - ice_update(other_media->ice_agent, sp, false); - return 1; // done, continue - } - if (flags && flags->opmode == OP_OFFER && flags->reset) { if (media) MEDIA_CLEAR(media, INITIALIZED); @@ -2986,8 +2975,6 @@ static int __media_init_from_flags(struct call_media *other_media, struct call_m if (sp->desired_family) media->desired_family = sp->desired_family; } - - return 0; } unsigned int proto_num_ports(unsigned int sp_ports, struct call_media *media, struct sdp_ng_flags *flags, @@ -3052,8 +3039,7 @@ int monologue_offer_answer(struct call_monologue *dialogue[2], GQueue *streams, * THIS side (recipient) before, then the structs will be populated with * details already. */ - if (__media_init_from_flags(other_media, media, sp, flags) == 1) - continue; + __media_init_from_flags(other_media, media, sp, flags); codecs_offer_answer(media, other_media, sp, flags); @@ -3370,8 +3356,7 @@ static int monologue_subscribe_request1(struct call_monologue *src_ml, struct ca if (rev_idx_diff == 0 && src_media->index > dst_media->index) rev_idx_diff = src_media->index - dst_media->index; - if (__media_init_from_flags(src_media, dst_media, sp, flags) == 1) - continue; + __media_init_from_flags(src_media, dst_media, sp, flags); codec_store_populate(&dst_media->codecs, &src_media->codecs, NULL, false); codec_store_strip(&dst_media->codecs, &flags->codec_strip, flags->codec_except); @@ -3471,8 +3456,7 @@ int monologue_subscribe_answer(struct call_monologue *dst_ml, struct sdp_ng_flag struct call_media *dst_media = __get_media(dst_ml, sp, flags, 0); struct call_media *src_media = __get_media(src_ml, sp, flags, index++); - if (__media_init_from_flags(dst_media, NULL, sp, flags) == 1) - continue; + __media_init_from_flags(dst_media, NULL, sp, flags); if (flags && flags->allow_transcoding) { codec_store_populate(&dst_media->codecs, &sp->codecs, flags->codec_set, true); diff --git a/daemon/call_interfaces.c b/daemon/call_interfaces.c index 026ce1efa..35e9fb6f8 100644 --- a/daemon/call_interfaces.c +++ b/daemon/call_interfaces.c @@ -1989,9 +1989,9 @@ static const char *call_offer_answer_ng(struct ng_buffer *ngbuf, bencode_item_t call = call_get(&flags.call_id); // SDP fragments for trickle ICE must always operate on an existing call - if (!call && opmode == OP_OFFER && flags.fragment) { - queue_sdp_fragment(ngbuf, &streams, &flags); + if (opmode == OP_OFFER && trickle_ice_update(ngbuf, call, &flags, &streams)) { errstr = NULL; + // SDP fragments for trickle ICE are consumed with no replacement returned goto out; } @@ -2064,17 +2064,8 @@ static const char *call_offer_answer_ng(struct ng_buffer *ngbuf, bencode_item_t bool do_dequeue = true; ret = monologue_offer_answer(dialogue, &streams, &flags); - if (!ret) { - // SDP fragments for trickle ICE are consumed with no replacement returned - if (!flags.fragment) - ret = sdp_replace(chopper, &parsed, dialogue[1], &flags); - } - else if (ret == ERROR_NO_ICE_AGENT && flags.fragment) { - queue_sdp_fragment(ngbuf, &streams, &flags); - ret = 0; - do_dequeue = false; - } - + if (!ret) + ret = sdp_replace(chopper, &parsed, dialogue[1], &flags); if (!ret) save_last_sdp(dialogue[0], &sdp, &parsed, &streams); @@ -2088,7 +2079,7 @@ static const char *call_offer_answer_ng(struct ng_buffer *ngbuf, bencode_item_t } if (do_dequeue) - dequeue_sdp_fragments(dialogue); + dequeue_sdp_fragments(dialogue[0]); rwlock_unlock_w(&call->master_lock); @@ -3283,7 +3274,8 @@ found_sink: } -const char *call_publish_ng(bencode_item_t *input, bencode_item_t *output, const char *addr, +const char *call_publish_ng(struct ng_buffer *ngbuf, bencode_item_t *input, bencode_item_t *output, + const char *addr, const endpoint_t *sin) { AUTO_CLEANUP(struct sdp_ng_flags flags, call_ng_free_flags); @@ -3310,6 +3302,10 @@ const char *call_publish_ng(bencode_item_t *input, bencode_item_t *output, const return "Incomplete SDP specification"; call = call_get_or_create(&flags.call_id, false, false); + + if (trickle_ice_update(ngbuf, call, &flags, &streams)) + return NULL; + updated_created_from(call, addr, sin); struct call_monologue *ml = call_get_or_create_monologue(call, &flags.from_tag); @@ -3464,7 +3460,7 @@ const char *call_subscribe_request_ng(bencode_item_t *input, bencode_item_t *out } -const char *call_subscribe_answer_ng(bencode_item_t *input, bencode_item_t *output) { +const char *call_subscribe_answer_ng(struct ng_buffer *ngbuf, bencode_item_t *input, bencode_item_t *output) { AUTO_CLEANUP(struct sdp_ng_flags flags, call_ng_free_flags); AUTO_CLEANUP(GQueue parsed, sdp_free) = G_QUEUE_INIT; AUTO_CLEANUP(GQueue streams, sdp_streams_free) = G_QUEUE_INIT; @@ -3478,6 +3474,9 @@ const char *call_subscribe_answer_ng(bencode_item_t *input, bencode_item_t *outp if (!call) return "Unknown call-ID"; + if (trickle_ice_update(ngbuf, call, &flags, &streams)) + return NULL; + if (!flags.to_tag.s) return "No to-tag in message"; if (!flags.sdp.len) diff --git a/daemon/control_ng.c b/daemon/control_ng.c index c3b63326a..7ae2d9016 100644 --- a/daemon/control_ng.c +++ b/daemon/control_ng.c @@ -311,7 +311,7 @@ int control_ng_process(str *buf, const endpoint_t *sin, char *addr, command = NGC_STATISTICS; break; case CSH_LOOKUP("publish"): - errstr = call_publish_ng(dict, resp, addr, sin); + errstr = call_publish_ng(ngbuf, dict, resp, addr, sin); command = NGC_PUBLISH; break; case CSH_LOOKUP("subscribe request"): @@ -319,7 +319,7 @@ int control_ng_process(str *buf, const endpoint_t *sin, char *addr, command = NGC_SUBSCRIBE_REQ; break; case CSH_LOOKUP("subscribe answer"): - errstr = call_subscribe_answer_ng(dict, resp); + errstr = call_subscribe_answer_ng(ngbuf, dict, resp); command = NGC_SUBSCRIBE_ANS; break; case CSH_LOOKUP("unsubscribe"): diff --git a/daemon/ice.c b/daemon/ice.c index 1a1f6a163..95b263737 100644 --- a/daemon/ice.c +++ b/daemon/ice.c @@ -98,6 +98,31 @@ static GHashTable *sdp_fragments; +void ice_update_media_streams(struct call_monologue *ml, GQueue *streams) { + unsigned int media_idx = 0; + + for (GList *l = streams->head; l; l = l->next) { + struct stream_params *sp = l->data; + if (media_idx >= ml->medias->len) + break; + struct call_media *media = ml->medias->pdata[media_idx++]; + if (!media) + continue; + + if (!media->ice_agent) { + ilogs(ice, LOG_WARN, "Media for trickle ICE update is not ICE-enabled"); + continue; + } + if (!MEDIA_ISSET(media, TRICKLE_ICE)) { + ilogs(ice, LOG_WARN, "Media for trickle ICE update is not trickle-ICE-enabled"); + continue; + } + + ice_update(media->ice_agent, sp, false); + } +} + + static unsigned int frag_key_hash(const void *A) { const struct fragment_key *a = A; return str_hash(&a->call_id) ^ str_hash(&a->from_tag); @@ -121,7 +146,7 @@ static void fragment_key_free(void *p) { g_free(k->from_tag.s); g_slice_free1(sizeof(*k), k); } -void queue_sdp_fragment(struct ng_buffer *ngbuf, GQueue *streams, struct sdp_ng_flags *flags) { +static void queue_sdp_fragment(struct ng_buffer *ngbuf, GQueue *streams, struct sdp_ng_flags *flags) { ilog(LOG_DEBUG, "Queuing up SDP fragment for " STR_FORMAT_M "/" STR_FORMAT_M, STR_FMT_M(&flags->call_id), STR_FMT_M(&flags->from_tag)); @@ -142,12 +167,32 @@ void queue_sdp_fragment(struct ng_buffer *ngbuf, GQueue *streams, struct sdp_ng_ g_queue_push_tail(frags, frag); mutex_unlock(&sdp_fragments_lock); } +bool trickle_ice_update(struct ng_buffer *ngbuf, struct call *call, struct sdp_ng_flags *flags, + GQueue *streams) +{ + if (!flags->fragment) + return false; + + if (!call) { + queue_sdp_fragment(ngbuf, streams, flags); + return true; + } + struct call_monologue *ml = call_get_monologue(call, &flags->from_tag); + if (!ml) { + queue_sdp_fragment(ngbuf, streams, flags); + return true; + } + + ice_update_media_streams(ml, streams); + + return true; +} #define MAX_FRAG_AGE 3000000 -void dequeue_sdp_fragments(struct call_monologue *dialogue[2]) { +void dequeue_sdp_fragments(struct call_monologue *monologue) { struct fragment_key k; ZERO(k); - k.call_id = dialogue[0]->call->callid; - k.from_tag = dialogue[0]->tag; + k.call_id = monologue->call->callid; + k.from_tag = monologue->tag; GQueue *frags = NULL; @@ -168,7 +213,7 @@ void dequeue_sdp_fragments(struct call_monologue *dialogue[2]) { ilog(LOG_DEBUG, "Dequeuing SDP fragment for " STR_FORMAT_M "/" STR_FORMAT_M, STR_FMT_M(&k.call_id), STR_FMT_M(&k.from_tag)); - monologue_offer_answer(dialogue, &frag->streams, &frag->flags); + ice_update_media_streams(monologue, &frag->streams); next: fragment_free(frag); diff --git a/include/call.h b/include/call.h index d61e8c15c..d036dc5d3 100644 --- a/include/call.h +++ b/include/call.h @@ -88,7 +88,6 @@ enum { #define ERROR_NO_FREE_PORTS -100 #define ERROR_NO_FREE_LOGS -101 -#define ERROR_NO_ICE_AGENT -102 #ifndef RTP_LOOP_PROTECT #define RTP_LOOP_PROTECT 28 /* number of bytes */ diff --git a/include/call_interfaces.h b/include/call_interfaces.h index 99da87c05..aa4558f8c 100644 --- a/include/call_interfaces.h +++ b/include/call_interfaces.h @@ -236,9 +236,10 @@ const char *call_stop_media_ng(bencode_item_t *, bencode_item_t *); const char *call_play_dtmf_ng(bencode_item_t *, bencode_item_t *); void ng_call_stats(struct call *call, const str *fromtag, const str *totag, bencode_item_t *output, struct call_stats *totals); -const char *call_publish_ng(bencode_item_t *, bencode_item_t *, const char *, const endpoint_t *); +const char *call_publish_ng(struct ng_buffer *, bencode_item_t *, bencode_item_t *, const char *, + const endpoint_t *); const char *call_subscribe_request_ng(bencode_item_t *, bencode_item_t *); -const char *call_subscribe_answer_ng(bencode_item_t *, bencode_item_t *); +const char *call_subscribe_answer_ng(struct ng_buffer *, bencode_item_t *, bencode_item_t *); const char *call_unsubscribe_ng(bencode_item_t *, bencode_item_t *); void save_last_sdp(struct call_monologue *ml, str *sdp, GQueue *parsed, GQueue *streams); diff --git a/include/ice.h b/include/ice.h index af10e51b8..47847b3d4 100644 --- a/include/ice.h +++ b/include/ice.h @@ -158,6 +158,7 @@ void ice_foundation(str *); void ice_agent_init(struct ice_agent **agp, struct call_media *media); void ice_update(struct ice_agent *, struct stream_params *, bool allow_restart); +void ice_update_media_streams(struct call_monologue *ml, GQueue *streams); void ice_shutdown(struct ice_agent **); void ice_restart(struct ice_agent *); @@ -170,8 +171,10 @@ int ice_request(struct stream_fd *, const endpoint_t *, struct stun_attrs *); int ice_response(struct stream_fd *, const endpoint_t *src, struct stun_attrs *attrs, void *transaction); -void queue_sdp_fragment(struct ng_buffer *ngbuf, GQueue *streams, struct sdp_ng_flags *flags); -void dequeue_sdp_fragments(struct call_monologue *dialogue[2]); +void dequeue_sdp_fragments(struct call_monologue *); +bool trickle_ice_update(struct ng_buffer *ngbuf, struct call *call, struct sdp_ng_flags *flags, + GQueue *streams); + void ice_slow_timer(void);