From 3eb68c819f98f81e3da8649d0f9ec70964314e61 Mon Sep 17 00:00:00 2001 From: Donat Zenichev Date: Wed, 26 Jun 2024 14:59:21 +0200 Subject: [PATCH] MT#60386 sdp_create: don't carry stream_params Don't carry parameters required for processing in the `sdp_create()` via the `stream_params`, but rahter handle them like: - parsing in `sdp_parse()` - `sdp_media` -> `stream_params` in sdp_streams() - `stream_params` -> `call_media` in ` __media_init_from_flags()` Additionally: update the test "subscribe_request AMR asymmetric". This is because we seem to never actually take into account presence of bandwidth data in offer/answer model preceding the subscribe request. Change-Id: I5b4b19ae244c6bbf961d5ea7c18b6747519144db --- daemon/call.c | 20 +++++++++++++++++++ daemon/call_interfaces.c | 4 ++-- daemon/janus.c | 4 ++-- daemon/sdp.c | 43 ++++++++++++++++++---------------------- include/call.h | 4 ++++ include/sdp.h | 2 +- t/auto-daemon-tests.pl | 5 +++++ 7 files changed, 53 insertions(+), 29 deletions(-) diff --git a/daemon/call.c b/daemon/call.c index fff35ba3c..d7d768a29 100644 --- a/daemon/call.c +++ b/daemon/call.c @@ -2879,6 +2879,26 @@ static void __media_init_from_flags(struct call_media *other_media, struct call_ if (sp->desired_family) media->desired_family = sp->desired_family; } + + /* desired endpoint's address */ + other_media->desired_address = NULL; + if (media) + media->desired_address = NULL; + if (sp->rtp_endpoint.address.family) { + other_media->desired_address = &sp->rtp_endpoint.address; + if (media) + media->desired_address = &sp->rtp_endpoint.address; + } + + /* desired bandwidth */ + other_media->desired_bandwidth_as = sp->media_session_as; + other_media->desired_bandwidth_rr = sp->media_session_rr; + other_media->desired_bandwidth_rs = sp->media_session_rs; + if (media) { + media->desired_bandwidth_as = sp->media_session_as; + media->desired_bandwidth_rr = sp->media_session_rr; + media->desired_bandwidth_rs = sp->media_session_rs; + } } unsigned int proto_num_ports(unsigned int sp_ports, struct call_media *media, sdp_ng_flags *flags, diff --git a/daemon/call_interfaces.c b/daemon/call_interfaces.c index ed8217054..e6d16be98 100644 --- a/daemon/call_interfaces.c +++ b/daemon/call_interfaces.c @@ -3690,7 +3690,7 @@ const char *call_publish_ng(ng_buffer *ngbuf, bencode_item_t *input, bencode_ite if (ret) ilog(LOG_ERR, "Publish error"); // XXX close call? handle errors? - ret = sdp_create(&sdp_out, ml, &streams, &flags); + ret = sdp_create(&sdp_out, ml, &flags); if (!ret) { save_last_sdp(ml, &sdp_in, &parsed, &streams); bencode_buffer_destroy_add(output->buffer, g_free, sdp_out.s); @@ -3771,7 +3771,7 @@ const char *call_subscribe_request_ng(bencode_item_t *input, bencode_item_t *out return "Failed to rewrite SDP"; } else { /* create new SDP */ - ret = sdp_create(&sdp_out, dest_ml, NULL, &flags); + ret = sdp_create(&sdp_out, dest_ml, &flags); } /* place return output SDP */ diff --git a/daemon/janus.c b/daemon/janus.c index 76230b5a8..426ae8257 100644 --- a/daemon/janus.c +++ b/daemon/janus.c @@ -644,7 +644,7 @@ static const char *janus_videoroom_join(struct websocket_message *wm, struct jan return "Subscribe error"; /* create SDP */ - ret = sdp_create(jsep_sdp_out, dest_ml, NULL, &flags); + ret = sdp_create(jsep_sdp_out, dest_ml, &flags); if (!dest_ml->janus_session) dest_ml->janus_session = obj_get(session); @@ -874,7 +874,7 @@ static const char *janus_videoroom_configure(struct websocket_message *wm, struc // XXX check there's only one audio and one video stream? g_auto(str) sdp_out = STR_NULL; - ret = sdp_create(&sdp_out, ml, &streams, &flags); + ret = sdp_create(&sdp_out, ml, &flags); if (ret) return "Publish error"; diff --git a/daemon/sdp.c b/daemon/sdp.c index 98069ab05..871e2f753 100644 --- a/daemon/sdp.c +++ b/daemon/sdp.c @@ -1837,6 +1837,9 @@ int sdp_streams(const sdp_sessions_q *sessions, sdp_streams_q *streams, sdp_ng_f flags->trust_address = 1; } + /* + * pass important context parameters: sdp_media -> stream_params + */ sp->consecutive_ports = media->port_count; sp->num_ports = sp->consecutive_ports * 2; // only do *=2 for RTP streams? sp->protocol_str = media->transport; @@ -1850,7 +1853,7 @@ int sdp_streams(const sdp_sessions_q *sessions, sdp_streams_q *streams, sdp_ng_f bf_set_clear(&sp->sp_flags, SP_FLAG_STRICT_SOURCE, flags->strict_source); bf_set_clear(&sp->sp_flags, SP_FLAG_MEDIA_HANDOVER, flags->media_handover); - /* b= (bandwidth) */ + /* b= (bandwidth), is parsed in sdp_parse() */ sp->media_session_as = media->as; sp->media_session_rr = media->rr; sp->media_session_rs = media->rs; @@ -3472,7 +3475,7 @@ static void sdp_out_add_timing(GString *out, struct call_monologue *monologue) } static void sdp_out_add_bandwidth(GString *out, struct call_monologue *monologue, - struct stream_params *sp, struct call_media *media) + struct call_media *media) { struct call_monologue *ml = monologue; struct media_subscription *ms = call_get_top_media_subscription(monologue); @@ -3481,12 +3484,12 @@ static void sdp_out_add_bandwidth(GString *out, struct call_monologue *monologue /* sdp bandwidth per media level */ if (media) { - if (sp && sp->media_session_as >= 0) - g_string_append_printf(out, "b=AS:%d\r\n", sp->media_session_as); - if (sp && sp->media_session_rr >= 0) - g_string_append_printf(out, "b=RR:%d\r\n", sp->media_session_rr); - if (sp && sp->media_session_rs >= 0) - g_string_append_printf(out, "b=RS:%d\r\n", sp->media_session_rs); + if (media->desired_bandwidth_as >= 0) + g_string_append_printf(out, "b=AS:%d\r\n", media->desired_bandwidth_as); + if (media->desired_bandwidth_rr >= 0) + g_string_append_printf(out, "b=RR:%d\r\n", media->desired_bandwidth_rr); + if (media->desired_bandwidth_rs >= 0) + g_string_append_printf(out, "b=RS:%d\r\n", media->desired_bandwidth_rs); } else { /* sdp bandwidth per session/media level @@ -3499,17 +3502,16 @@ static void sdp_out_add_bandwidth(GString *out, struct call_monologue *monologue } static void sdp_out_add_media_connection(GString *out, struct call_media *media, - struct packet_stream *rtp_ps, struct stream_params *sp, - sdp_ng_flags *flags) + struct packet_stream *rtp_ps, sdp_ng_flags *flags) { const char *media_conn_address = NULL; const char *media_conn_address_type = rtp_ps->selected_sfd->local_intf->advertised_address.addr.family->rfc_name; /* we want to keep an original media connection for message / force relay */ - if (sp && (media->type_id == MT_MESSAGE || flags->ice_option == ICE_FORCE_RELAY)) + if (media->desired_address && (media->type_id == MT_MESSAGE || flags->ice_option == ICE_FORCE_RELAY)) { - media_conn_address = sockaddr_print_buf(&sp->rtp_endpoint.address); - media_conn_address_type = sp->rtp_endpoint.address.family->rfc_name; + media_conn_address = sockaddr_print_buf(media->desired_address); + media_conn_address_type = media->desired_family->rfc_name; } else { media_conn_address = sockaddr_print_buf(&rtp_ps->selected_sfd->local_intf->advertised_address.addr); @@ -3529,12 +3531,10 @@ static void sdp_out_add_media_connection(GString *out, struct call_media *media, * For the rest of cases (publish, subscribe, janus etc.) this works as usual: * given monologue is a monologue which is being processed. */ -int sdp_create(str *out, struct call_monologue *monologue, - sdp_streams_q *streams, sdp_ng_flags *flags) +int sdp_create(str *out, struct call_monologue *monologue, sdp_ng_flags *flags) { const char *err = NULL; GString *s = NULL; - struct stream_params *sp; err = "Need at least one media"; if (!monologue->medias->len) @@ -3563,7 +3563,7 @@ int sdp_create(str *out, struct call_monologue *monologue, * but instead per media, below */ /* add bandwidth control per session level */ - sdp_out_add_bandwidth(s, monologue, NULL, NULL); + sdp_out_add_bandwidth(s, monologue, NULL); /* set timing to always be: 0 0 */ sdp_out_add_timing(s, monologue); @@ -3575,11 +3575,6 @@ int sdp_create(str *out, struct call_monologue *monologue, { media = monologue->medias->pdata[i]; - /* take corresponding stream to get per media flags */ - sp = NULL; - if (streams) - sp = t_queue_peek_nth(streams, i); - /* check call media existence */ err = "Empty media stream"; if (!media) @@ -3618,10 +3613,10 @@ int sdp_create(str *out, struct call_monologue *monologue, g_string_append_printf(s, "\r\n"); /* add actual media connection */ - sdp_out_add_media_connection(s, media, rtp_ps, sp, flags); + sdp_out_add_media_connection(s, media, rtp_ps, flags); /* add per media bandwidth */ - sdp_out_add_bandwidth(s, monologue, sp, media); + sdp_out_add_bandwidth(s, monologue, media); /* print media level attributes */ print_sdp_media_section(s, media, NULL, flags, rtp_ps_link, true, false); diff --git a/include/call.h b/include/call.h index b259e77b9..95c5ac2cc 100644 --- a/include/call.h +++ b/include/call.h @@ -505,6 +505,7 @@ struct call_media { const struct transport_protocol *protocol; str format_str; sockfamily_t *desired_family; + sockaddr_t *desired_address; const struct logical_intf *logical_intf; struct ice_agent *ice_agent; @@ -550,6 +551,9 @@ struct call_media { dtmf_event_q dtmf_send; int media_sdp_id; + /* bandwidth */ + int desired_bandwidth_as, desired_bandwidth_rr, desired_bandwidth_rs; + #ifdef WITH_TRANSCODING encoder_callback_t encoder_callback; #endif diff --git a/include/sdp.h b/include/sdp.h index 80dde31e3..c66bdff23 100644 --- a/include/sdp.h +++ b/include/sdp.h @@ -81,7 +81,7 @@ void sdp_streams_clear(sdp_streams_q *); void sdp_sessions_clear(sdp_sessions_q *sessions); int sdp_replace(struct sdp_chopper *, sdp_sessions_q *, struct call_monologue *, sdp_ng_flags *); int sdp_is_duplicate(sdp_sessions_q *sessions); -int sdp_create(str *out, struct call_monologue *, sdp_streams_q *streams, sdp_ng_flags *flags); +int sdp_create(str *out, struct call_monologue *, sdp_ng_flags *flags); const char *sdp_get_sendrecv(struct call_media *media); int sdp_parse_candidate(struct ice_candidate *cand, const str *s); // returns -1, 0, 1 diff --git a/t/auto-daemon-tests.pl b/t/auto-daemon-tests.pl index 91505105e..6b18dd76d 100755 --- a/t/auto-daemon-tests.pl +++ b/t/auto-daemon-tests.pl @@ -1559,6 +1559,8 @@ s=- t=0 0 m=audio PORT RTP/AVP 96 98 c=IN IP4 203.0.113.1 +b=RR:1087 +b=RS:362 a=label:0 a=rtpmap:96 AMR/8000 a=fmtp:96 mode-set=0,2,4,7;mode-change-period=2;mode-change-capability=2;mode-change-neighbor=1;max-red=0 @@ -1569,6 +1571,9 @@ a=rtcp:PORT a=ptime:20 m=audio PORT RTP/AVP 118 98 c=IN IP4 203.0.113.1 +b=AS:80 +b=RR:1087 +b=RS:362 a=label:1 a=rtpmap:118 AMR/8000 a=fmtp:118 mode-set=0,2,4,7;mode-change-period=2;mode-change-capability=2;mode-change-neighbor=1;max-red=0