From 53ccb2c5641dbe3cede40940352c6ac0eebd222c Mon Sep 17 00:00:00 2001 From: Ghislain MARY Date: Mon, 3 Aug 2015 12:41:17 +0200 Subject: [PATCH] Handle and add test for AVPF generic NACK. --- coreapi/bellesip_sal/sal_sdp.c | 17 +++++++++-- coreapi/linphonecall.c | 26 ++++++++++++++++ coreapi/offeranswer.c | 5 ++++ include/sal/sal.h | 1 + mediastreamer2 | 2 +- oRTP | 2 +- tester/call_tester.c | 55 +++++++++++++++++++++++++++++++++- tester/liblinphone_tester.h | 1 + 8 files changed, 103 insertions(+), 6 deletions(-) diff --git a/coreapi/bellesip_sal/sal_sdp.c b/coreapi/bellesip_sal/sal_sdp.c index 2bc6afb5c..b9273f5fb 100644 --- a/coreapi/bellesip_sal/sal_sdp.c +++ b/coreapi/bellesip_sal/sal_sdp.c @@ -128,6 +128,12 @@ static void add_rtcp_fb_attributes(belle_sdp_media_description_t *media_desc, co if (general_trr_int == TRUE) { add_rtcp_fb_trr_int_attribute(media_desc, -1, trr_int); } + if (stream->rtcp_fb.generic_nack_enabled == TRUE) { + add_rtcp_fb_nack_attribute(media_desc, -1, BELLE_SDP_RTCP_FB_NONE); + } + if (stream->rtcp_fb.tmmbr_enabled == TRUE) { + add_rtcp_fb_ccm_attribute(media_desc, -1, BELLE_SDP_RTCP_FB_TMMBR); + } for (pt_it = stream->payloads; pt_it != NULL; pt_it = pt_it->next) { pt = (PayloadType *)pt_it->data; @@ -550,7 +556,7 @@ static void enable_avpf_for_stream(SalStreamDescription *stream) { } } -static void apply_rtcp_fb_attribute_to_payload(belle_sdp_rtcp_fb_attribute_t *fb_attribute, PayloadType *pt) { +static void apply_rtcp_fb_attribute_to_payload(belle_sdp_rtcp_fb_attribute_t *fb_attribute, SalStreamDescription *stream, PayloadType *pt) { PayloadTypeAvpfParams avpf_params = payload_type_get_avpf_params(pt); switch (belle_sdp_rtcp_fb_attribute_get_type(fb_attribute)) { case BELLE_SDP_RTCP_FB_ACK: @@ -574,6 +580,8 @@ static void apply_rtcp_fb_attribute_to_payload(belle_sdp_rtcp_fb_attribute_t *fb avpf_params.rpsi_compatibility = TRUE; break; case BELLE_SDP_RTCP_FB_NONE: + stream->rtcp_fb.generic_nack_enabled = TRUE; + break; default: break; } @@ -586,6 +594,9 @@ static void apply_rtcp_fb_attribute_to_payload(belle_sdp_rtcp_fb_attribute_t *fb case BELLE_SDP_RTCP_FB_FIR: avpf_params.features |= PAYLOAD_TYPE_AVPF_FIR; break; + case BELLE_SDP_RTCP_FB_TMMBR: + stream->rtcp_fb.tmmbr_enabled = TRUE; + break; default: break; } @@ -612,7 +623,7 @@ static void sdp_parse_rtcp_fb_parameters(belle_sdp_media_description_t *media_de if (belle_sdp_rtcp_fb_attribute_get_id(fb_attribute) == -1) { for (pt_it = stream->payloads; pt_it != NULL; pt_it = pt_it->next) { pt = (PayloadType *)pt_it->data; - apply_rtcp_fb_attribute_to_payload(fb_attribute, pt); + apply_rtcp_fb_attribute_to_payload(fb_attribute, stream, pt); } } } @@ -627,7 +638,7 @@ static void sdp_parse_rtcp_fb_parameters(belle_sdp_media_description_t *media_de for (pt_it = stream->payloads; pt_it != NULL; pt_it = pt_it->next) { pt = (PayloadType *)pt_it->data; if (payload_type_get_number(pt) == (int)pt_num) { - apply_rtcp_fb_attribute_to_payload(fb_attribute, pt); + apply_rtcp_fb_attribute_to_payload(fb_attribute, stream, pt); } } } diff --git a/coreapi/linphonecall.c b/coreapi/linphonecall.c index f92680316..46c301c44 100644 --- a/coreapi/linphonecall.c +++ b/coreapi/linphonecall.c @@ -485,10 +485,13 @@ static void setup_rtcp_fb(LinphoneCall *call, SalMediaDescription *md) { MSList *pt_it; PayloadType *pt; PayloadTypeAvpfParams avpf_params; + LinphoneCore *lc = call->core; int i; for (i = 0; i < md->nb_streams; i++) { if (!sal_stream_description_active(&md->streams[i])) continue; + md->streams[i].rtcp_fb.generic_nack_enabled = lp_config_get_int(lc->config, "rtp", "rtcp_fb_generic_nack_enabled", 0); + md->streams[i].rtcp_fb.tmmbr_enabled = lp_config_get_int(lc->config, "rtp", "rtcp_fb_tmmbr_enabled", 0); for (pt_it = md->streams[i].payloads; pt_it != NULL; pt_it = pt_it->next) { pt = (PayloadType *)pt_it->data; if (call->params->avpf_enabled == TRUE) { @@ -2392,6 +2395,27 @@ static int find_crypto_index_from_tag(const SalSrtpCryptoAlgo crypto[],unsigned return -1; } +static void configure_rtp_session_for_rtcp_fb(LinphoneCall *call, SalStreamDescription *stream) { + RtpSession *session = NULL; + + if (stream->type == SalAudio) { + session = call->audiostream->ms.sessions.rtp_session; + } else if (stream->type == SalVideo) { + session = call->videostream->ms.sessions.rtp_session; + } else { + // Do nothing for streams that are not audio or video + return; + } + if (stream->rtcp_fb.generic_nack_enabled) + rtp_session_enable_avpf_feature(session, ORTP_AVPF_FEATURE_GENERIC_NACK, TRUE); + else + rtp_session_enable_avpf_feature(session, ORTP_AVPF_FEATURE_GENERIC_NACK, FALSE); + if (stream->rtcp_fb.tmmbr_enabled) + rtp_session_enable_avpf_feature(session, ORTP_AVPF_FEATURE_TMMBR, TRUE); + else + rtp_session_enable_avpf_feature(session, ORTP_AVPF_FEATURE_TMMBR, FALSE); +} + static void configure_rtp_session_for_rtcp_xr(LinphoneCore *lc, LinphoneCall *call, SalStreamType type) { RtpSession *session; const OrtpRtcpXrConfiguration *localconfig; @@ -2622,6 +2646,7 @@ static void linphone_call_start_audio_stream(LinphoneCall *call, bool_t muted, b ms_warning("Failed to find local crypto algo with tag: %d", stream->crypto_local_tag); } } + configure_rtp_session_for_rtcp_fb(call, stream); configure_rtp_session_for_rtcp_xr(lc, call, SalAudio); if (is_multicast) rtp_session_set_multicast_ttl(call->audiostream->ms.sessions.rtp_session,stream->ttl); @@ -2789,6 +2814,7 @@ static void linphone_call_start_video_stream(LinphoneCall *call, bool_t all_inpu media_stream_set_srtp_send_key_b64(&(call->videostream->ms.sessions),vstream->crypto[0].algo,local_st_desc->crypto[crypto_idx].master_key); } } + configure_rtp_session_for_rtcp_fb(call, vstream); configure_rtp_session_for_rtcp_xr(lc, call, SalVideo); call->log->video_enabled = TRUE; diff --git a/coreapi/offeranswer.c b/coreapi/offeranswer.c index 90ec2c665..0b0edb931 100644 --- a/coreapi/offeranswer.c +++ b/coreapi/offeranswer.c @@ -503,6 +503,8 @@ int offer_answer_initiate_outgoing(const SalMediaDescription *local_offer, if ((ls->rtcp_xr.enabled == TRUE) && (rs->rtcp_xr.enabled == FALSE)) { result->streams[i].rtcp_xr.enabled = FALSE; } + result->streams[i].rtcp_fb.generic_nack_enabled = ls->rtcp_fb.generic_nack_enabled & rs->rtcp_fb.generic_nack_enabled; + result->streams[i].rtcp_fb.tmmbr_enabled = ls->rtcp_fb.tmmbr_enabled & rs->rtcp_fb.tmmbr_enabled; ++j; } else ms_warning("No matching stream for %i",i); @@ -569,6 +571,9 @@ int offer_answer_initiate_incoming(const SalMediaDescription *local_capabilities }else ms_warning("Unknown protocol for mline %i, declining",i); if (ls){ initiate_incoming(ls,rs,&result->streams[i],one_matching_codec); + // Handle global RTCP FB attributes + result->streams[i].rtcp_fb.generic_nack_enabled = rs->rtcp_fb.generic_nack_enabled; + result->streams[i].rtcp_fb.tmmbr_enabled = rs->rtcp_fb.tmmbr_enabled; // Handle media RTCP XR attribute memset(&result->streams[i].rtcp_xr, 0, sizeof(result->streams[i].rtcp_xr)); if (rs->rtcp_xr.enabled == TRUE) { diff --git a/include/sal/sal.h b/include/sal/sal.h index 80f334835..12015b105 100644 --- a/include/sal/sal.h +++ b/include/sal/sal.h @@ -230,6 +230,7 @@ typedef struct SalStreamDescription{ SalSrtpCryptoAlgo crypto[SAL_CRYPTO_ALGO_MAX]; unsigned int crypto_local_tag; int max_rate; + OrtpRtcpFbConfiguration rtcp_fb; OrtpRtcpXrConfiguration rtcp_xr; SalIceCandidate ice_candidates[SAL_MEDIA_DESCRIPTION_MAX_ICE_CANDIDATES]; SalIceRemoteCandidate ice_remote_candidates[SAL_MEDIA_DESCRIPTION_MAX_ICE_REMOTE_CANDIDATES]; diff --git a/mediastreamer2 b/mediastreamer2 index 0b72fb676..dba58b24d 160000 --- a/mediastreamer2 +++ b/mediastreamer2 @@ -1 +1 @@ -Subproject commit 0b72fb676c24356a548710c5e3a1d0ee3a1dbd70 +Subproject commit dba58b24d9ae6772a17513d66aebf55f45e63030 diff --git a/oRTP b/oRTP index 5aa889bfe..79e8c769e 160000 --- a/oRTP +++ b/oRTP @@ -1 +1 @@ -Subproject commit 5aa889bfe539f6b83962334adb9f49aa76bc8303 +Subproject commit 79e8c769eebb7a60ba55f5bcada0aa200507d017 diff --git a/tester/call_tester.c b/tester/call_tester.c index 2cd398dcd..26dc31426 100644 --- a/tester/call_tester.c +++ b/tester/call_tester.c @@ -4381,6 +4381,58 @@ end: ms_free(hellopath); } +static void generic_nack_received(const OrtpEventData *evd, stats *st) { + if (rtcp_is_RTPFB(evd->packet)) { + switch (rtcp_RTPFB_get_type(evd->packet)) { + case RTCP_RTPFB_NACK: + st->number_of_rtcp_generic_nack++; + break; + default: + break; + } + } +} + +static void call_with_generic_nack_rtcp_feedback(void) { + LinphoneCoreManager *marie = linphone_core_manager_new("marie_rc"); + LinphoneCoreManager *pauline = linphone_core_manager_new(transport_supported(LinphoneTransportTls) ? "pauline_rc" : "pauline_tcp_rc"); + LpConfig *lp; + LinphoneCall *call_marie; + bool_t call_ok; + OrtpNetworkSimulatorParams params = { 0 }; + int dummy = 0; + + params.enabled = TRUE; + params.loss_rate = 10; + params.consecutive_loss_probability = 0.75; + params.mode = OrtpNetworkSimulatorOutbound; + linphone_core_set_avpf_mode(marie->lc, LinphoneAVPFEnabled); + linphone_core_set_avpf_mode(pauline->lc, LinphoneAVPFEnabled); + lp = linphone_core_get_config(pauline->lc); + lp_config_set_int(lp, "rtp", "rtcp_fb_generic_nack_enabled", 1); + + + BC_ASSERT_TRUE(call_ok = call(pauline, marie)); + if (!call_ok) goto end; + BC_ASSERT_TRUE(wait_for(pauline->lc, marie->lc, &pauline->stat.number_of_LinphoneCallStreamsRunning, 1)); + BC_ASSERT_TRUE(wait_for(pauline->lc, marie->lc, &marie->stat.number_of_LinphoneCallStreamsRunning, 1)); + call_marie = linphone_core_get_current_call(marie->lc); + if (call_marie) { + rtp_session_enable_network_simulation(call_marie->audiostream->ms.sessions.rtp_session, ¶ms); + ortp_ev_dispatcher_connect(media_stream_get_event_dispatcher(&call_marie->audiostream->ms), + ORTP_EVENT_RTCP_PACKET_RECEIVED, RTCP_RTPFB, (OrtpEvDispatcherCb)generic_nack_received, &marie->stat); + } + + BC_ASSERT_TRUE(wait_for_until(pauline->lc, marie->lc, &marie->stat.number_of_rtcp_generic_nack, 5, 5000)); + linphone_core_terminate_all_calls(pauline->lc); + BC_ASSERT_TRUE(wait_for(pauline->lc, marie->lc, &pauline->stat.number_of_LinphoneCallEnd, 1)); + BC_ASSERT_TRUE(wait_for(pauline->lc, marie->lc, &marie->stat.number_of_LinphoneCallEnd, 1)); + +end: + linphone_core_manager_destroy(marie); + linphone_core_manager_destroy(pauline); +} + test_t call_tests[] = { { "Early declined call", early_declined_call }, { "Call declined", call_declined }, @@ -4507,7 +4559,8 @@ test_t call_tests[] = { { "Simple stereo call with opus", simple_stereo_call_opus }, { "Simple mono call with opus", simple_mono_call_opus }, { "Call with FQDN in SDP", call_with_fqdn_in_sdp}, - { "Call with RTP IO mode", call_with_rtp_io_mode } + { "Call with RTP IO mode", call_with_rtp_io_mode }, + { "Call with generic NACK RTCP feedback", call_with_generic_nack_rtcp_feedback } }; test_suite_t call_test_suite = { diff --git a/tester/liblinphone_tester.h b/tester/liblinphone_tester.h index 98e2e146c..340c0887b 100644 --- a/tester/liblinphone_tester.h +++ b/tester/liblinphone_tester.h @@ -224,6 +224,7 @@ typedef struct _stats { int video_upload_bandwidth[3]; int current_bandwidth_index; + int number_of_rtcp_generic_nack; }stats;