From c31a01bd751745c5c203525e3359e6076cd86f92 Mon Sep 17 00:00:00 2001 From: Ben Ford Date: Tue, 7 Aug 2018 10:57:29 -0500 Subject: [PATCH] res_pjsip/rtp: No joint capabilities between streams. When a conference contained a mixture of audio/video and audio-only users, a NOTICE message would pop up stating there are no joint capabilities between streams. This happens because streams can never be removed, but they can be in a REMOVED state. If we have the scenario where user A joins with audio/video, user B joins with audio-only, and user C joins with audio/video, then user A leaves, the message would be triggered. That removed stream is still in the SDP, but Asterisk would pass it through, causing it to be seen as a ulaw stream. A check has been added for removed streams, setting their status to REMOVED when handling negotiated SDPs. Also addressed an issue where user A joins, then user B joins but does not receive video until much later. Full frames were not being sent, causing some PLI from the browser. Because the video was flowing in one direction, the browser sets the SSRC to 1, but Asterisk was dropping the PLI because of that. Added a check to see if the SSRC is 1 or not, which sends full frames and allows video to flow between user A and user B. This should only happen when dealing with PSFB or FUR, and in the case of PSFB, only for PLI. ASTERISK-27398 Change-Id: I26e7c6f101bc119549eeca406b5bcd25ad8ebc5e --- res/res_pjsip_session.c | 16 ++++++++++++---- res/res_rtp_asterisk.c | 12 +++++++++++- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/res/res_pjsip_session.c b/res/res_pjsip_session.c index 1300850ed6..3bb1ef4e99 100644 --- a/res/res_pjsip_session.c +++ b/res/res_pjsip_session.c @@ -930,10 +930,18 @@ static int handle_negotiated_sdp(struct ast_sip_session *session, const pjmedia_ session_media = AST_VECTOR_GET(&session->pending_media_state->sessions, i); stream = ast_stream_topology_get_stream(session->pending_media_state->topology, i); - /* The stream state will have already been set to removed when either we - * negotiate the incoming SDP stream or when we produce our own local SDP. - * This can occur if an internal thing has requested it to be removed, or if - * we remove it as a result of the stream limit being reached. + /* Make sure that this stream is in the correct state. If we need to change + * the state to REMOVED, then our work here is done, so go ahead and move on + * to the next stream. + */ + if (!remote->media[i]->desc.port) { + ast_stream_set_state(stream, AST_STREAM_STATE_REMOVED); + continue; + } + + /* If the stream state is REMOVED, nothing needs to be done, so move on to the + * next stream. This can occur if an internal thing has requested it to be + * removed, or if we remove it as a result of the stream limit being reached. */ if (ast_stream_get_state(stream) == AST_STREAM_STATE_REMOVED) { /* diff --git a/res/res_rtp_asterisk.c b/res/res_rtp_asterisk.c index 1327d710a6..1d1d66e8b6 100644 --- a/res/res_rtp_asterisk.c +++ b/res/res_rtp_asterisk.c @@ -5802,7 +5802,17 @@ static struct ast_frame *ast_rtcp_interpret(struct ast_rtp_instance *instance, s } if (ssrc_valid && rtp->themssrc_valid) { - if (ssrc != rtp->themssrc && use_packet_source) { + /* + * If the SSRC is 1, we still need to handle RTCP since this could be a + * special case. For example, if we have a unidirectional video stream, the + * SSRC may be set to 1 by the browser (in the case of chromium), and requests + * will still need to be processed so that video can flow as expected. This + * should only be done for PLI and FUR, since there is not a way to get the + * appropriate rtp instance when the SSRC is 1. + */ + int exception = (ssrc == 1 && !((pt == RTCP_PT_PSFB && rc == AST_RTP_RTCP_FMT_PLI) || pt == RTCP_PT_FUR)); + if ((ssrc != rtp->themssrc && use_packet_source && ssrc != 1) + || exception) { /* * Skip over this RTCP record as it does not contain the * correct SSRC. We should not act upon RTCP records