[Asterisk-code-review] res pjsip session: Reduce (and improve) SDP renegotiation. (asterisk[15])

[Jenkins2]

Jenkins2 has submitted this change and it was merged. ( https://gerrit.asterisk.org/6574 )

Change subject: res_pjsip_session: Reduce (and improve) SDP renegotiation.
......................................................................

res_pjsip_session: Reduce (and improve) SDP renegotiation.

When pruning a request to change the topology of a channel be
more intelligent about the resulting topology that is actually
used for SDP renegotiation.

In a case where a stream has not already been negotiated we
don't need to renegotiate and offer a declined stream. This can
occur if something in Asterisk (such as ConfBridge) requests
to add video to a PJSIP channel that has no video codecs configured.
In this case since the stream did not already exist we can safely
remove the stream from the requested topology, resulting in no
renegotiation occurring.

In a case where a renegotiation is requested with a codec that is
not supported we can reuse the formats of the existing stream if
it exists to ensure that the stream continues to flow, instead of
removing it.

Change-Id: I636540798d55922377318fe619c510fb6ed125fb
---
M res/res_pjsip_session.c
1 file changed, 29 insertions(+), 2 deletions(-)

Approvals:
  George Joseph: Looks good to me, but someone else must approve
  Kevin Harwell: Looks good to me, approved
  Jenkins2: Approved for Submit



diff --git a/res/res_pjsip_session.c b/res/res_pjsip_session.c
index 2beb6dc..d61799b 100644
--- a/res/res_pjsip_session.c
+++ b/res/res_pjsip_session.c
@@ -1482,7 +1482,14 @@
 			 * are configurable on the endpoint.
 			 */
 			for (index = 0; index < ast_stream_topology_get_count(media_state->topology); ++index) {
+				struct ast_stream *existing_stream = NULL;
+
 				stream = ast_stream_topology_get_stream(media_state->topology, index);
+
+				if (session->active_media_state->topology &&
+					index < ast_stream_topology_get_count(session->active_media_state->topology)) {
+					existing_stream = ast_stream_topology_get_stream(session->active_media_state->topology, index);
+				}
 
 				if (is_stream_limitation_reached(ast_stream_get_type(stream), session->endpoint, type_streams)) {
 					if (index < AST_VECTOR_SIZE(&media_state->sessions)) {
@@ -1516,8 +1523,28 @@
 					ast_format_cap_get_compatible(ast_stream_get_formats(stream), session->endpoint->media.codecs, joint_cap);
 					if (!ast_format_cap_count(joint_cap)) {
 						ao2_ref(joint_cap, -1);
-						ast_stream_set_state(stream, AST_STREAM_STATE_REMOVED);
-						continue;
+
+						if (!existing_stream) {
+							/* If there is no existing stream we can just not have this stream in the topology
+							 * at all.
+							 */
+							ast_stream_topology_del_stream(media_state->topology, index);
+							index -= 1;
+							continue;
+						} else if (ast_stream_get_state(stream) != ast_stream_get_state(existing_stream) ||
+								strcmp(ast_stream_get_name(stream), ast_stream_get_name(existing_stream))) {
+							/* If the underlying stream is a different type or different name then we have to
+							 * mark it as removed, as it is replacing an existing stream. We do this so order
+							 * is preserved.
+							 */
+							ast_stream_set_state(stream, AST_STREAM_STATE_REMOVED);
+							continue;
+						} else {
+							/* However if the stream is otherwise remaining the same we can keep the formats
+							 * that exist on it already which allows media to continue to flow.
+							 */
+							joint_cap = ao2_bump(ast_stream_get_formats(existing_stream));
+						}
 					}
 					ast_stream_set_formats(stream, joint_cap);
 				}

-- 
To view, visit https://gerrit.asterisk.org/6574
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 15
Gerrit-MessageType: merged
Gerrit-Change-Id: I636540798d55922377318fe619c510fb6ed125fb
Gerrit-Change-Number: 6574
Gerrit-PatchSet: 1
Gerrit-Owner: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20170925/dcabf333/attachment.html>