From 4cbeb15c07fbbb2594ff0d7bb5f6e945bcdc58d1 Mon Sep 17 00:00:00 2001 From: Richard Fuchs Date: Fri, 22 Oct 2021 14:23:49 -0400 Subject: [PATCH] TT#147451 obsolete necessity for `from-tag` in unsub/sub ans Change-Id: I409dbfb032265d0da39bff4cb63bf6dd5388cde4 --- README.md | 12 +++++---- daemon/call.c | 46 +++++++++++++++++++++++------------ daemon/call_interfaces.c | 34 ++++++++++++-------------- daemon/janus.c | 3 ++- include/call.h | 4 +-- t/auto-daemon-tests-pubsub.pl | 35 +++++++++++++------------- 6 files changed, 76 insertions(+), 58 deletions(-) diff --git a/README.md b/README.md index 5ff28cc49..ab723bd87 100644 --- a/README.md +++ b/README.md @@ -2114,13 +2114,16 @@ the option to manipulate the codecs. The reply message will also contain the `to-tag` (corresponding to the subscription, either generated or taken from the received message). +If a `subscribe request` is made for an existing `to-tag` then all existing +subscriptions for that `to-tag` are deleted before the new subscriptions are +created. + `subscribe answer` Message -------------------------- This message is expected to be received after responding to a `subscribe -request` message. The message should contain the same `from-tag` and `to-tag` -is the reply to the `subscribe request` (although `label` etc can also be used -instead of the `from-tag`), as well as the answer SDP in `sdp`. +request` message. The message should contain the same `to-tag` as the reply to +the `subscribe request` as well as the answer SDP in `sdp`. By default, the answer SDP must accept all codecs that were presented in the offer SDP (given in the reply to `subscribe request`). If not all codecs were @@ -2139,8 +2142,7 @@ forwarding will start to the endpoint given in the answer SDP. --------------------- This message is a counterpart to `subsscribe answer` to stop an established -subscription. The subscription to be stopped is identified by `from-tag` and -`to`tag`. +subscription. The subscription to be stopped is identified by the `to-tag`. The *tcp-ng* Control Protocol ========================= diff --git a/daemon/call.c b/daemon/call.c index ffa7d858f..dfbd737e6 100644 --- a/daemon/call.c +++ b/daemon/call.c @@ -2780,6 +2780,13 @@ static void __unsubscribe_all_offer_answer_subscribers(struct call_monologue *ml l = next; } } +static void __unsubscribe_from_all(struct call_monologue *ml) { + for (GList *l = ml->subscriptions.head; l; ) { + GList *next = l->next; + __unsubscribe_one_link(ml, l); + l = next; + } +} void __add_subscription(struct call_monologue *which, struct call_monologue *to, bool offer_answer) { if (g_hash_table_lookup(which->subscriptions_ht, to)) { ilog(LOG_DEBUG, "Tag '" STR_FORMAT_M "' is already subscribed to '" STR_FORMAT_M "'", @@ -2886,6 +2893,8 @@ int monologue_publish(struct call_monologue *ml, GQueue *streams, struct sdp_ng_ int monologue_subscribe_request(struct call_monologue *src_ml, struct call_monologue *dst_ml, struct sdp_ng_flags *flags) { + __unsubscribe_from_all(dst_ml); + __call_monologue_init_from_flags(dst_ml, flags); GList *dst_media_it = NULL; @@ -2934,15 +2943,20 @@ int monologue_subscribe_request(struct call_monologue *src_ml, struct call_monol return -1; } + __add_subscription(dst_ml, src_ml, false); + __update_init_subscribers(src_ml, NULL, NULL); __update_init_subscribers(dst_ml, NULL, NULL); return 0; } /* called with call->master_lock held in W */ -int monologue_subscribe_answer(struct call_monologue *src_ml, struct call_monologue *dst_ml, - struct sdp_ng_flags *flags, GQueue *streams) -{ +int monologue_subscribe_answer(struct call_monologue *dst_ml, struct sdp_ng_flags *flags, GQueue *streams) { + if (!dst_ml->subscriptions.length) + return -1; + struct call_subscription *cs = dst_ml->subscriptions.head->data; + struct call_monologue *src_ml = cs->monologue; + GList *dst_media_it = NULL; GList *src_media_it = NULL; @@ -2981,9 +2995,6 @@ int monologue_subscribe_answer(struct call_monologue *src_ml, struct call_monolo MEDIA_SET(dst_media, INITIALIZED); } - __unsubscribe_one(dst_ml, src_ml); - __add_subscription(dst_ml, src_ml, false); - __update_init_subscribers(dst_ml, streams, flags); __update_init_subscribers(src_ml, NULL, NULL); @@ -2994,17 +3005,22 @@ int monologue_subscribe_answer(struct call_monologue *src_ml, struct call_monolo } /* called with call->master_lock held in W */ -int monologue_unsubscribe(struct call_monologue *src_ml, struct call_monologue *dst_ml, - struct sdp_ng_flags *flags) -{ - if (!__unsubscribe_one(dst_ml, src_ml)) - return -1; +int monologue_unsubscribe(struct call_monologue *dst_ml, struct sdp_ng_flags *flags) { + for (GList *l = dst_ml->subscriptions.head; l; ) { + GList *next = l->next; + struct call_subscription *cs = l->data; + struct call_monologue *src_ml = cs->monologue; - __update_init_subscribers(dst_ml, NULL, NULL); - __update_init_subscribers(src_ml, NULL, NULL); + __unsubscribe_one_link(dst_ml, l); - __dialogue_unkernelize(src_ml); - __dialogue_unkernelize(dst_ml); + __update_init_subscribers(dst_ml, NULL, NULL); + __update_init_subscribers(src_ml, NULL, NULL); + + __dialogue_unkernelize(src_ml); + __dialogue_unkernelize(dst_ml); + + l = next; + } return 0; } diff --git a/daemon/call_interfaces.c b/daemon/call_interfaces.c index 1e7d14088..811000fe3 100644 --- a/daemon/call_interfaces.c +++ b/daemon/call_interfaces.c @@ -2634,20 +2634,19 @@ 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 *err = NULL; 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; AUTO_CLEANUP_NULL(struct call *call, call_unlock_release); - struct call_monologue *source_ml; - // get source monologue - err = media_block_match(&call, &source_ml, &flags, input, OP_REQ_ANSWER); - if (err) - return err; + call_ng_process_flags(&flags, input, OP_REQ_ANSWER); + + if (!flags.call_id.s) + return "No call-id in message"; + call = call_get_opmode(&flags.call_id, OP_REQ_ANSWER); + if (!call) + return "Unknown call-ID"; - if (!source_ml) - return "No call participant specified"; if (!flags.to_tag.s) return "No to-tag in message"; if (!flags.sdp.len) @@ -2663,7 +2662,7 @@ const char *call_subscribe_answer_ng(bencode_item_t *input, bencode_item_t *outp if (sdp_streams(&parsed, &streams, &flags)) return "Incomplete SDP specification"; - int ret = monologue_subscribe_answer(source_ml, dest_ml, &flags, &streams); + int ret = monologue_subscribe_answer(dest_ml, &flags, &streams); if (ret) return "Failed to process subscription answer"; @@ -2672,18 +2671,17 @@ const char *call_subscribe_answer_ng(bencode_item_t *input, bencode_item_t *outp const char *call_unsubscribe_ng(bencode_item_t *input, bencode_item_t *output) { - const char *err = NULL; AUTO_CLEANUP(struct sdp_ng_flags flags, call_ng_free_flags); AUTO_CLEANUP_NULL(struct call *call, call_unlock_release); - struct call_monologue *source_ml; - // get source monologue - err = media_block_match(&call, &source_ml, &flags, input, OP_OTHER); - if (err) - return err; + call_ng_process_flags(&flags, input, OP_REQ_ANSWER); + + if (!flags.call_id.s) + return "No call-id in message"; + call = call_get_opmode(&flags.call_id, OP_REQ_ANSWER); + if (!call) + return "Unknown call-ID"; - if (!source_ml) - return "No call participant specified"; if (!flags.to_tag.s) return "No to-tag in message"; @@ -2692,7 +2690,7 @@ const char *call_unsubscribe_ng(bencode_item_t *input, bencode_item_t *output) { if (!dest_ml) return "To-tag not found"; - int ret = monologue_unsubscribe(source_ml, dest_ml, &flags); + int ret = monologue_unsubscribe(dest_ml, &flags); if (ret) return "Failed to unsubscribe"; diff --git a/daemon/janus.c b/daemon/janus.c index aed29f308..6a209a8d6 100644 --- a/daemon/janus.c +++ b/daemon/janus.c @@ -723,6 +723,7 @@ static const char *janus_videoroom_start(struct websocket_message *wm, struct ja struct call_monologue *source_ml = call_get_monologue(call, &source_handle_str); if (!source_ml) return "Feed not found"; + // XXX verify that dest_ml is subscribed to source_ml AUTO_CLEANUP_GBUF(dest_handle_buf); dest_handle_buf = g_strdup_printf("%" PRIu64, handle->id); @@ -732,7 +733,7 @@ static const char *janus_videoroom_start(struct websocket_message *wm, struct ja if (!dest_ml) return "Subscriber not found"; - int ret = monologue_subscribe_answer(source_ml, dest_ml, &flags, &streams); + int ret = monologue_subscribe_answer(dest_ml, &flags, &streams); if (ret) return "Failed to process subscription answer"; } diff --git a/include/call.h b/include/call.h index ad4441372..f79e7d09b 100644 --- a/include/call.h +++ b/include/call.h @@ -621,9 +621,9 @@ void codecs_offer_answer(struct call_media *media, struct call_media *other_medi struct stream_params *sp, struct sdp_ng_flags *flags); int monologue_publish(struct call_monologue *ml, GQueue *streams, struct sdp_ng_flags *flags); int monologue_subscribe_request(struct call_monologue *src, struct call_monologue *dst, struct sdp_ng_flags *); -int monologue_subscribe_answer(struct call_monologue *src, struct call_monologue *dst, struct sdp_ng_flags *, +int monologue_subscribe_answer(struct call_monologue *dst, struct sdp_ng_flags *, GQueue *); -int monologue_unsubscribe(struct call_monologue *src, struct call_monologue *dst, struct sdp_ng_flags *); +int monologue_unsubscribe(struct call_monologue *dst, struct sdp_ng_flags *); int monologue_destroy(struct call_monologue *ml); int call_delete_branch(const str *callid, const str *branch, const str *fromtag, const str *totag, bencode_item_t *output, int delete_delay); diff --git a/t/auto-daemon-tests-pubsub.pl b/t/auto-daemon-tests-pubsub.pl index c680a52ab..28da91a0f 100755 --- a/t/auto-daemon-tests-pubsub.pl +++ b/t/auto-daemon-tests-pubsub.pl @@ -96,7 +96,7 @@ SDP is $ftr, ft(), 'from-tag matches'; subscribe_answer('sub, multi codec, sub w diff codec', - { 'from-tag' => ft(), 'to-tag' => $ttr, flags => ['allow transcoding'] }, < $ttr, flags => ['allow transcoding'] }, < ft(), 'to-tag' => $ttr }, < $ttr }, < ft(), 'to-tag' => $ttr }, < $ttr }, < 'foo', 'to-tag' => $ttr }, < $ttr }, < 'foo' }, < 'bar' }, < 'bar', 'to-tag' => $ttr }, < $ttr }, < ft(), 'to-tag' => $ttr }, < $ttr }, < ft(), 'to-tag' => $ttr }, < $ttr }, < ft(), 'to-tag' => $ttr }, < $ttr }, < ft(), 'to-tag' => $ttr }, < $ttr }, < ft(), 'to-tag' => $ttr }, < $ttr }, < ft(), 'to-tag' => $ttr }, < $ttr }, < ft(), 'to-tag' => $ttr }, < $ttr }, < ft(), 'to-tag' => $ttr }, < $ttr }, < ft(), 'to-tag' => $ttr }, < $ttr }, < ft(), 'to-tag' => $ttr, flags => ['allow transcoding'] }, < $ttr, flags => ['allow transcoding'] }, <