From 8c521d1ba35ca55aa828b7dd498d58f261ccd04f Mon Sep 17 00:00:00 2001 From: Simon Morlat Date: Fri, 29 Apr 2016 15:12:23 +0200 Subject: [PATCH] implicit avpf bugfixes: do not configure encoders to use avpf if avpf is not accepted or offered by remote. --- coreapi/bellesip_sal/sal_sdp.c | 13 ++++++------- coreapi/linphonecall.c | 25 +++++++++++++++---------- coreapi/offeranswer.c | 6 ++++-- coreapi/sal.c | 5 +---- gtk/incall_view.c | 4 ++-- gtk/main.c | 2 +- mediastreamer2 | 2 +- 7 files changed, 30 insertions(+), 27 deletions(-) diff --git a/coreapi/bellesip_sal/sal_sdp.c b/coreapi/bellesip_sal/sal_sdp.c index 3f355aee5..2398fde30 100644 --- a/coreapi/bellesip_sal/sal_sdp.c +++ b/coreapi/bellesip_sal/sal_sdp.c @@ -126,7 +126,7 @@ static void add_rtcp_fb_attributes(belle_sdp_media_description_t *media_desc, co uint16_t trr_int = 0; general_trr_int = is_rtcp_fb_trr_int_the_same_for_all_payloads(stream, &trr_int); - if (general_trr_int == TRUE && trr_int != 0) { + if (general_trr_int == TRUE && trr_int != 0) { add_rtcp_fb_trr_int_attribute(media_desc, -1, trr_int); } if (stream->rtcp_fb.generic_nack_enabled == TRUE) { @@ -737,7 +737,7 @@ static SalStreamDescription * sdp_to_stream_description(SalMediaDescription *md, belle_sip_list_t *custom_attribute_it; const char* value; const char *mtype,*proto; - bool_t has_avpf_attributes; + bool_t has_avpf_attributes; stream=&md->streams[md->nb_streams]; media=belle_sdp_media_description_get_media ( media_desc ); @@ -850,16 +850,15 @@ static SalStreamDescription * sdp_to_stream_description(SalMediaDescription *md, /* Get ICE candidate attributes if any */ sdp_parse_media_ice_parameters(media_desc, stream); - has_avpf_attributes = sdp_parse_rtcp_fb_parameters(media_desc, stream); + has_avpf_attributes = sdp_parse_rtcp_fb_parameters(media_desc, stream); /* Get RTCP-FB attributes if any */ if (sal_stream_description_has_avpf(stream)) { enable_avpf_for_stream(stream); + }else if (has_avpf_attributes ){ + enable_avpf_for_stream(stream); + stream->implicit_rtcp_fb = TRUE; } - else if (has_avpf_attributes ){ - - stream->implicit_rtcp_fb = TRUE; - } /* Get RTCP-XR attributes if any */ stream->rtcp_xr = md->rtcp_xr; // Use session parameters if no stream parameters are defined diff --git a/coreapi/linphonecall.c b/coreapi/linphonecall.c index 7626ac9f9..8cc3aba68 100644 --- a/coreapi/linphonecall.c +++ b/coreapi/linphonecall.c @@ -735,10 +735,10 @@ void linphone_call_make_local_media_description(LinphoneCall *call) { /*set audio capabilities */ - codec_hints.bandwidth_limit=params->audio_bw; - codec_hints.max_codecs=-1; - codec_hints.previously_used=old_md ? old_md->streams[call->main_audio_stream_index].already_assigned_payloads : NULL; - l=make_codec_list(lc, &codec_hints, SalAudio, lc->codecs_conf.audio_codecs); + codec_hints.bandwidth_limit=params->audio_bw; + codec_hints.max_codecs=-1; + codec_hints.previously_used=old_md ? old_md->streams[call->main_audio_stream_index].already_assigned_payloads : NULL; + l=make_codec_list(lc, &codec_hints, SalAudio, lc->codecs_conf.audio_codecs); if (params->has_audio && l != NULL) { strncpy(md->streams[call->main_audio_stream_index].rtp_addr,linphone_call_get_public_ip_for_stream(call,call->main_audio_stream_index),sizeof(md->streams[call->main_audio_stream_index].rtp_addr)); @@ -1503,7 +1503,14 @@ void linphone_call_fix_call_parameters(LinphoneCall *call, SalMediaDescription * if (rmd) { linphone_call_compute_streams_indexes(call, rmd); linphone_call_update_biggest_desc(call, rmd); - call->params->implicit_rtcp_fb &= sal_media_description_has_implicit_avpf(rmd); + /* Why disabling implicit_rtcp_fb ? It is a local policy choice actually. It doesn't disturb to propose it again and again + * even if the other end apparently doesn't support it. + * The following line of code is causing trouble, while for example making an audio call, then adding video. + * Due to the 200Ok response of the audio-only offer where no rtcp-fb attribute is present, implicit_rtcp_fb is set to + * FALSE, which is then preventing it to be eventually used when video is later added to the call. + * I did the choice of commenting it out. + */ + /*call->params->implicit_rtcp_fb &= sal_media_description_has_implicit_avpf(rmd);*/ } rcp = linphone_call_get_remote_params(call); if (rcp){ @@ -1827,9 +1834,9 @@ const LinphoneCallParams * linphone_call_get_current_params(LinphoneCall *call){ } else { call->current_params->media_encryption=LinphoneMediaEncryptionNone; } - break; + break; } - call->current_params->avpf_enabled = linphone_call_all_streams_avpf_enabled(call) && sal_media_description_has_avpf(md); + call->current_params->avpf_enabled = linphone_call_all_streams_avpf_enabled(call) && sal_media_description_has_avpf(md); if (call->current_params->avpf_enabled == TRUE) { call->current_params->avpf_rr_interval = linphone_call_get_avpf_rr_interval(call); } else { @@ -1848,7 +1855,7 @@ const LinphoneCallParams * linphone_call_get_current_params(LinphoneCall *call){ call->current_params->audio_multicast_enabled = FALSE; sd=sal_media_description_find_best_stream(md,SalVideo); - call->current_params->implicit_rtcp_fb = sd ? sal_stream_description_has_implicit_avpf(sd): FALSE; + call->current_params->implicit_rtcp_fb = sd ? sal_stream_description_has_implicit_avpf(sd): FALSE; call->current_params->video_dir=sd ? media_direction_from_sal_stream_dir(sd->dir) : LinphoneMediaDirectionInactive; if (call->current_params->video_dir != LinphoneMediaDirectionInactive) { rtp_addr = sd->rtp_addr[0]!='\0' ? sd->rtp_addr : call->resultdesc->addr; @@ -1856,8 +1863,6 @@ const LinphoneCallParams * linphone_call_get_current_params(LinphoneCall *call){ } else call->current_params->video_multicast_enabled = FALSE; - - } return call->current_params; diff --git a/coreapi/offeranswer.c b/coreapi/offeranswer.c index 0676e2a74..81136fc73 100644 --- a/coreapi/offeranswer.c +++ b/coreapi/offeranswer.c @@ -198,7 +198,7 @@ static MSList *match_payloads(MSFactory *factory, const MSList *local, const MSL } matched->flags|=PAYLOAD_TYPE_FLAG_CAN_RECV|PAYLOAD_TYPE_FLAG_CAN_SEND; if (p2->flags & PAYLOAD_TYPE_RTCP_FEEDBACK_ENABLED) { - matched->flags |= PAYLOAD_TYPE_RTCP_FEEDBACK_ENABLED; + payload_type_set_flag(matched, PAYLOAD_TYPE_RTCP_FEEDBACK_ENABLED); /* Negotiation of AVPF features (keep common features) */ matched->avpf.features &= p2->avpf.features; matched->avpf.rpsi_compatibility = p2->avpf.rpsi_compatibility; @@ -206,6 +206,8 @@ static MSList *match_payloads(MSFactory *factory, const MSList *local, const MSL if (p2->avpf.trr_interval < matched->avpf.trr_interval) { matched->avpf.trr_interval = matched->avpf.trr_interval; } + }else{ + payload_type_unset_flag(matched, PAYLOAD_TYPE_RTCP_FEEDBACK_ENABLED); } res=ms_list_append(res,matched); /* we should use the remote numbering even when parsing a response */ @@ -518,7 +520,7 @@ static void initiate_incoming(MSFactory *factory, const SalStreamDescription *lo result->dtls_role = SalDtlsRoleInvalid; } result->rtcp_mux = remote_offer->rtcp_mux && local_cap->rtcp_mux; - result->implicit_rtcp_fb = local_cap->implicit_rtcp_fb && remote_offer->implicit_rtcp_fb; + result->implicit_rtcp_fb = local_cap->implicit_rtcp_fb && remote_offer->implicit_rtcp_fb; } diff --git a/coreapi/sal.c b/coreapi/sal.c index a1620d7c0..6a53b61d7 100644 --- a/coreapi/sal.c +++ b/coreapi/sal.c @@ -221,10 +221,7 @@ bool_t sal_stream_description_has_avpf(const SalStreamDescription *sd) { } bool_t sal_stream_description_has_implicit_avpf(const SalStreamDescription *sd){ - if (sd->implicit_rtcp_fb){ - return TRUE; - } - return FALSE; + return sd->implicit_rtcp_fb; } /*these are switch case, so that when a new proto is added we can't forget to modify this function*/ bool_t sal_stream_description_has_srtp(const SalStreamDescription *sd) { diff --git a/gtk/incall_view.c b/gtk/incall_view.c index cf97b5d53..ba90f7560 100644 --- a/gtk/incall_view.c +++ b/gtk/incall_view.c @@ -461,9 +461,9 @@ void linphone_gtk_create_in_call_view(LinphoneCall *call){ static void video_button_clicked(GtkWidget *button, LinphoneCall *call){ gboolean adding=GPOINTER_TO_INT(g_object_get_data(G_OBJECT(button),"adding_video")); LinphoneCore *lc=linphone_call_get_core(call); - LinphoneCallParams *params=linphone_call_params_copy(linphone_call_get_current_params(call)); + LinphoneCallParams *params = linphone_core_create_call_params(lc, call); gtk_widget_set_sensitive(button,FALSE); - linphone_call_params_enable_video(params,adding); + linphone_call_params_enable_video(params, adding); linphone_core_update_call(lc,call,params); linphone_call_params_destroy(params); } diff --git a/gtk/main.c b/gtk/main.c index 014c733f7..0a145bc58 100644 --- a/gtk/main.c +++ b/gtk/main.c @@ -1277,7 +1277,7 @@ static void on_call_updated_response(GtkWidget *dialog, gint responseid, gpointe LinphoneCall *call = (LinphoneCall *)g_object_get_data(G_OBJECT(dialog), "call"); if (linphone_call_get_state(call)==LinphoneCallUpdatedByRemote){ LinphoneCore *lc=linphone_call_get_core(call); - LinphoneCallParams *params=linphone_call_params_copy(linphone_call_get_current_params(call)); + LinphoneCallParams *params = linphone_core_create_call_params(lc, call); linphone_call_params_enable_video(params,responseid==GTK_RESPONSE_YES); linphone_core_accept_call_update(lc,call,params); linphone_call_params_destroy(params); diff --git a/mediastreamer2 b/mediastreamer2 index 43d32a444..26cf83eee 160000 --- a/mediastreamer2 +++ b/mediastreamer2 @@ -1 +1 @@ -Subproject commit 43d32a4441e27af0e39906e0712e1c5c809a1ab6 +Subproject commit 26cf83eee5d0a20818a0044b85a27fff5f602d7f