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

[Joshua Colp]

Joshua Colp has uploaded this change for review. ( 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(-)



  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/74/6574/1

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: newchange
Gerrit-Change-Id: I636540798d55922377318fe619c510fb6ed125fb
Gerrit-Change-Number: 6574
Gerrit-PatchSet: 1
Gerrit-Owner: Joshua Colp <jcolp at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20170922/76d16374/attachment-0001.html>