From 28a7d3dbbe15d674f265ab3bab19a4e8358331a5 Mon Sep 17 00:00:00 2001 From: Simon Morlat Date: Wed, 23 Sep 2015 18:10:13 +0200 Subject: [PATCH] fix bug where a pause reINVITE was having a video stream while the original INVITE (made by other party) wasn't proposing video at all. --- coreapi/callbacks.c | 1 + coreapi/linphonecall.c | 20 +++++++++---- coreapi/private.h | 11 +++---- tester/call_tester.c | 65 +++++++++++++++++++++++++++++++++++++++--- 4 files changed, 82 insertions(+), 15 deletions(-) diff --git a/coreapi/callbacks.c b/coreapi/callbacks.c index 1824ba905..c3abebc53 100644 --- a/coreapi/callbacks.c +++ b/coreapi/callbacks.c @@ -702,6 +702,7 @@ static void call_updating(SalOp *op, bool_t is_update){ ms_error("call_updating(): call doesn't exist anymore"); return ; } + linphone_call_fix_call_parameters(call); if (call->state!=LinphoneCallPaused){ /*Refresh the local description, but in paused state, we don't change anything.*/ linphone_call_make_local_media_description(call); diff --git a/coreapi/linphonecall.c b/coreapi/linphonecall.c index 0ed8d1970..9d60cff50 100644 --- a/coreapi/linphonecall.c +++ b/coreapi/linphonecall.c @@ -691,7 +691,7 @@ void linphone_call_make_local_media_description(LinphoneCall *call) { ms_warning("Cannot get audio local ssrc for call [%p]",call); nb_active_streams++; - if (params->has_video && (!params->internal_call_update || !call->current_params->video_declined)){ + if (params->has_video){ strncpy(md->streams[call->main_video_stream_index].rtp_addr,linphone_call_get_public_ip_for_stream(call,call->main_video_stream_index),sizeof(md->streams[call->main_video_stream_index].rtp_addr)); strncpy(md->streams[call->main_video_stream_index].rtcp_addr,linphone_call_get_public_ip_for_stream(call,call->main_video_stream_index),sizeof(md->streams[call->main_video_stream_index].rtcp_addr)); strncpy(md->streams[call->main_video_stream_index].name,"Video",sizeof(md->streams[call->main_video_stream_index].name)-1); @@ -1083,7 +1083,7 @@ void linphone_call_set_compatible_incoming_call_parameters(LinphoneCall *call, c }else if (call->params->media_encryption != LinphoneMediaEncryptionZRTP){ call->params->media_encryption = LinphoneMediaEncryptionNone; } - + linphone_call_fix_call_parameters(call); } static void linphone_call_compute_streams_indexes(LinphoneCall *call, SalMediaDescription *md) { @@ -1385,11 +1385,19 @@ static void linphone_call_set_terminated(LinphoneCall *call){ } } +/*function to be called at each incoming reINVITE, in order to adjust the video enablement parameter according to what is offered + * and our local policy. Fixing the call->params to proper values avoid request video by accident during internal call updates, pauses and resumes*/ void linphone_call_fix_call_parameters(LinphoneCall *call){ - if (sal_call_is_offerer(call->op)) { - /*get remote params*/ - const LinphoneCallParams* rcp = linphone_call_get_remote_params(call); - call->current_params->video_declined = call->params->has_video && !rcp->has_video; + const LinphoneCallParams* rcp = linphone_call_get_remote_params(call); + if (rcp){ + if (call->params->has_video && !rcp->has_video){ + ms_message("Call [%p]: disabling video in our call params because the remote doesn't want it.", call); + call->params->has_video = FALSE; + } + if (rcp->has_video && call->core->video_policy.automatically_accept && !call->params->has_video){ + ms_message("Call [%p]: re-enabling video in our call params because the remote wants it and the policy allows to automatically accept.", call); + call->params->has_video = TRUE; + } } } diff --git a/coreapi/private.h b/coreapi/private.h index 390c3b716..dcedc3e2e 100644 --- a/coreapi/private.h +++ b/coreapi/private.h @@ -128,18 +128,19 @@ struct _LinphoneCallParams{ char *record_file; char *session_name; SalCustomHeader *custom_headers; + LinphonePrivacyMask privacy; + LinphoneMediaDirection audio_dir; + LinphoneMediaDirection video_dir; bool_t has_video; bool_t avpf_enabled; /* RTCP feedback messages are enabled */ bool_t real_early_media; /*send real media even during early media (for outgoing calls)*/ bool_t in_conference; /*in conference mode */ + bool_t low_bandwidth; bool_t no_user_consent;/*when set to TRUE an UPDATE request will be used instead of reINVITE*/ uint16_t avpf_rr_interval; /*in milliseconds*/ - LinphonePrivacyMask privacy; - LinphoneMediaDirection audio_dir; - LinphoneMediaDirection video_dir; - bool_t video_declined; /*use to keep traces of declined video to avoid to re-offer video in case of automatic RE-INVITE*/ - bool_t internal_call_update; /*use mark that call update was requested internally (might be by ice)*/ + + bool_t internal_call_update; /*use mark that call update was requested internally (might be by ice) - unused for the moment*/ bool_t video_multicast_enabled; bool_t audio_multicast_enabled; bool_t realtimetext_enabled; diff --git a/tester/call_tester.c b/tester/call_tester.c index c6eba9c69..283e3984a 100644 --- a/tester/call_tester.c +++ b/tester/call_tester.c @@ -1215,10 +1215,7 @@ void call_paused_resumed_base(bool_t multicast) { stats = rtp_session_get_stats(call_pauline->sessions->rtp_session); BC_ASSERT_EQUAL(stats->cum_packet_loss, 0, int, "%d"); - 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_call(pauline, marie); end: linphone_core_manager_destroy(marie); linphone_core_manager_destroy(pauline); @@ -1293,6 +1290,65 @@ end: ms_list_free(lcs); } +/*this test makes sure that pause/resume will not bring up video by accident*/ +static void call_paused_resumed_with_video(){ + LinphoneCoreManager* marie = linphone_core_manager_new("marie_rc"); + LinphoneCoreManager* pauline = linphone_core_manager_new(transport_supported(LinphoneTransportTls) ? "pauline_rc" : "pauline_tcp_rc"); + LinphoneCall* call_pauline, *call_marie; + MSList *lcs = NULL; + LinphoneVideoPolicy vpol; + bool_t call_ok; + + lcs = ms_list_append(lcs, pauline->lc); + lcs = ms_list_append(lcs, marie->lc); + + vpol.automatically_accept = FALSE; + vpol.automatically_initiate = FALSE; + + linphone_core_set_video_policy(marie->lc, &vpol); + linphone_core_enable_video(marie->lc, TRUE, TRUE); + + vpol.automatically_accept = TRUE; + vpol.automatically_initiate = TRUE; + + linphone_core_set_video_policy(pauline->lc, &vpol); + linphone_core_enable_video(pauline->lc, TRUE, TRUE); + + BC_ASSERT_TRUE((call_ok=call(marie, pauline))); + + if (!call_ok) goto end; + + call_pauline = linphone_core_get_current_call(pauline->lc); + call_marie = linphone_core_get_current_call(marie->lc); + + wait_for_until(pauline->lc, marie->lc, NULL, 5, 2000); + + linphone_core_pause_call(pauline->lc,call_pauline); + BC_ASSERT_TRUE(wait_for(pauline->lc,marie->lc,&pauline->stat.number_of_LinphoneCallPausing,1)); + BC_ASSERT_TRUE(wait_for(pauline->lc,marie->lc,&marie->stat.number_of_LinphoneCallPausedByRemote,1)); + BC_ASSERT_FALSE(linphone_call_params_video_enabled(linphone_call_get_remote_params(call_marie))); + BC_ASSERT_TRUE(wait_for(pauline->lc,marie->lc,&pauline->stat.number_of_LinphoneCallPaused,1)); + + /*stay in pause a little while in order to generate traffic*/ + wait_for_until(pauline->lc, marie->lc, NULL, 5, 2000); + + /*now pauline wants to resume*/ + linphone_core_resume_call(pauline->lc, call_pauline); + BC_ASSERT_TRUE(wait_for(pauline->lc,marie->lc,&pauline->stat.number_of_LinphoneCallResuming,1)); + 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)); + + BC_ASSERT_FALSE(linphone_call_params_video_enabled(linphone_call_get_current_params(call_pauline))); + BC_ASSERT_FALSE(linphone_call_params_video_enabled(linphone_call_get_current_params(call_marie))); + + end_call(marie, pauline); + +end: + linphone_core_manager_destroy(marie); + linphone_core_manager_destroy(pauline); + ms_list_free(lcs); +} + #define CHECK_CURRENT_LOSS_RATE() \ rtcp_count_current = pauline->stat.number_of_rtcp_sent; \ /*wait for an RTCP packet to have an accurate cumulative lost value*/ \ @@ -4981,6 +5037,7 @@ test_t call_tests[] = { { "Call without SDP", call_with_no_sdp}, { "Call without SDP and ACK without SDP", call_with_no_sdp_ack_without_sdp}, { "Call paused resumed", call_paused_resumed }, + { "Call paused resumed with video", call_paused_resumed_with_video }, { "Call paused by both parties", call_paused_by_both }, { "Call paused resumed with loss", call_paused_resumed_with_loss }, { "Call paused resumed from callee", call_paused_resumed_from_callee },