[Asterisk-code-review] res_pjsip_session: Set stream state on created streams for incoming SDP. (asterisk[17])

[Joshua C. Colp]

Joshua C. Colp has submitted this change. ( https://gerrit.asterisk.org/c/asterisk/+/13436 )

Change subject: res_pjsip_session: Set stream state on created streams for incoming SDP.
......................................................................

res_pjsip_session: Set stream state on created streams for incoming SDP.

A previous review, 13174, made a change whereby on an incoming offer SDP
the pending topology was initialized to the configured. This caused a problem
for bundle with WebRTC where bundle could reference a stream that did not
actually exist if the configuration had both audio and video but the
offer SDP only contained audio.

This change undoes that review and instead fixes the original problem it
sought to solve by setting the state of created streams based on the
contents of the offer SDP. This way the stream state is not inactive
until negotiation later completes.

ASTERISK-28659

Change-Id: Ic5ae5a86437d3e686ac5afd91d133cc916198355
---
M res/res_pjsip_session.c
1 file changed, 19 insertions(+), 1 deletion(-)

Approvals:
  Benjamin Keith Ford: Looks good to me, but someone else must approve
  George Joseph: Looks good to me, but someone else must approve
  Joshua C. Colp: Looks good to me, approved; Approved for Submit



diff --git a/res/res_pjsip_session.c b/res/res_pjsip_session.c
index bc01548..76740c1 100644
--- a/res/res_pjsip_session.c
+++ b/res/res_pjsip_session.c
@@ -712,7 +712,7 @@
 
 	/* It is possible for SDP deferral to have already created a pending topology */
 	if (!session->pending_media_state->topology) {
-		session->pending_media_state->topology = ast_stream_topology_clone(session->endpoint->media.topology);
+		session->pending_media_state->topology = ast_stream_topology_alloc();
 		if (!session->pending_media_state->topology) {
 			return -1;
 		}
@@ -753,6 +753,24 @@
 				ast_stream_free(stream);
 				return -1;
 			}
+			/* For backwards compatibility with the core default streams are always sendrecv */
+			if (!ast_sip_session_is_pending_stream_default(session, stream)) {
+				if (pjmedia_sdp_media_find_attr2(remote_stream, "sendonly", NULL)) {
+					/* Stream state reflects our state of a stream, so in the case of
+					 * sendonly and recvonly we store the opposite since that is what ours
+					 * is.
+					 */
+					ast_stream_set_state(stream, AST_STREAM_STATE_RECVONLY);
+				} else if (pjmedia_sdp_media_find_attr2(remote_stream, "recvonly", NULL)) {
+					ast_stream_set_state(stream, AST_STREAM_STATE_SENDONLY);
+				} else if (pjmedia_sdp_media_find_attr2(remote_stream, "inactive", NULL)) {
+					ast_stream_set_state(stream, AST_STREAM_STATE_INACTIVE);
+				} else {
+					ast_stream_set_state(stream, AST_STREAM_STATE_SENDRECV);
+				}
+			} else {
+				ast_stream_set_state(stream, AST_STREAM_STATE_SENDRECV);
+			}
 		}
 
 		session_media = ast_sip_session_media_state_add(session, session->pending_media_state, ast_media_type_from_str(media), i);

-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/13436
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 17
Gerrit-Change-Id: Ic5ae5a86437d3e686ac5afd91d133cc916198355
Gerrit-Change-Number: 13436
Gerrit-PatchSet: 1
Gerrit-Owner: Joshua C. Colp <jcolp at sangoma.com>
Gerrit-Reviewer: Benjamin Keith Ford <bford at digium.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua C. Colp <jcolp at sangoma.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20191218/f2974afc/attachment.html>