[asterisk-commits] rmudgett: branch 12 r414749 - in /branches/12: include/asterisk/ res/

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Wed May 28 11:54:35 CDT 2014


Author: rmudgett
Date: Wed May 28 11:54:31 2014
New Revision: 414749

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=414749
Log:
res_pjsip_session: Fix leaked video RTP ports.

Simply enabling PJSIP to negotiage a video codec (e.g., h264) would leak
video RTP ports if the codec were not negotiated by an incoming call.

* Made add_sdp_streams() associate the handler with the media stream if
the handler handled the media stream.  Otherwise, when the
ast_sip_session_media object was destroyed it didn't know how to clean up
the RTP resources.

* Fixed sdp_requires_deferral() associating the handler with the media
stream when deciding if the SDP processing needs to be deferred for T.38.
Like the leaked video RTP ports, the T.38 handler needs to clean up
allocated resources from deciding if SDP processing needs to be deffered.

* Cleaned up some dead code in handle_incoming_sdp() and
sdp_requires_deferral().

ASTERISK-23721 #close
Reported by: cervajs

Review: https://reviewboard.asterisk.org/r/3571/

Modified:
    branches/12/include/asterisk/res_pjsip_session.h
    branches/12/res/res_pjsip_session.c
    branches/12/res/res_pjsip_t38.c

Modified: branches/12/include/asterisk/res_pjsip_session.h
URL: http://svnview.digium.com/svn/asterisk/branches/12/include/asterisk/res_pjsip_session.h?view=diff&rev=414749&r1=414748&r2=414749
==============================================================================
--- branches/12/include/asterisk/res_pjsip_session.h (original)
+++ branches/12/include/asterisk/res_pjsip_session.h Wed May 28 11:54:31 2014
@@ -216,6 +216,17 @@
 	AST_LIST_ENTRY(ast_sip_session_supplement) next;
 };
 
+enum ast_sip_session_sdp_stream_defer {
+	/*! The stream was not handled by this handler. If there are other registered handlers for this stream type, they will be called. */
+	AST_SIP_SESSION_SDP_DEFER_NOT_HANDLED,
+	/*! There was an error encountered. No further operations will take place and the current negotiation will be abandoned. */
+	AST_SIP_SESSION_SDP_DEFER_ERROR,
+	/*! Re-invite is not needed */
+	AST_SIP_SESSION_SDP_DEFER_NOT_NEEDED,
+	/*! Re-invite should be deferred and will be resumed later. No further operations will take place. */
+	AST_SIP_SESSION_SDP_DEFER_NEEDED,
+};
+
 /*!
  * \brief A handler for SDPs in SIP sessions
  *
@@ -230,14 +241,17 @@
 	 * If a stream can not be immediately negotiated the re-invite can be deferred and
 	 * resumed at a later time. It is up to the handler which caused deferral to occur
 	 * to resume it.
+	 *
 	 * \param session The session for which the media is being re-invited
 	 * \param session_media The media being reinvited
-	 * \param sdp The entire SDP.
-	 * \retval 0 The stream was unhandled or does not need the re-invite to be deferred.
-	 * \retval 1 Re-invite should be deferred and will be resumed later. No further operations will take place.
+	 * \param sdp The entire SDP. Useful for getting "global" information, such as connections or attributes
+	 * \param stream PJSIP incoming SDP media lines to parse by handler.
+	 *
+	 * \return enum ast_sip_session_defer_stream
+	 *
 	 * \note This is optional, if not implemented the stream is assumed to not be deferred.
 	 */
-	int (*defer_incoming_sdp_stream)(struct ast_sip_session *session, struct ast_sip_session_media *session_media, const struct pjmedia_sdp_session *sdp, const struct pjmedia_sdp_media *stream);
+	enum ast_sip_session_sdp_stream_defer (*defer_incoming_sdp_stream)(struct ast_sip_session *session, struct ast_sip_session_media *session_media, const struct pjmedia_sdp_session *sdp, const struct pjmedia_sdp_media *stream);
 	/*!
 	 * \brief Set session details based on a stream in an incoming SDP offer or answer
 	 * \param session The session for which the media is being negotiated

Modified: branches/12/res/res_pjsip_session.c
URL: http://svnview.digium.com/svn/asterisk/branches/12/res/res_pjsip_session.c?view=diff&rev=414749&r1=414748&r2=414749
==============================================================================
--- branches/12/res/res_pjsip_session.c (original)
+++ branches/12/res/res_pjsip_session.c Wed May 28 11:54:31 2014
@@ -337,6 +337,7 @@
 static int handle_incoming_sdp(struct ast_sip_session *session, const pjmedia_sdp_session *sdp)
 {
 	int i;
+
 	if (validate_incoming_sdp(sdp)) {
 		return -1;
 	}
@@ -347,6 +348,7 @@
 		struct ast_sip_session_sdp_handler *handler;
 		RAII_VAR(struct sdp_handler_list *, handler_list, NULL, ao2_cleanup);
 		RAII_VAR(struct ast_sip_session_media *, session_media, NULL, ao2_cleanup);
+		int res;
 
 		/* We need a null-terminated version of the media string */
 		ast_copy_pj_str(media, &sdp->media[i]->desc.media, sizeof(media));
@@ -359,18 +361,15 @@
 		}
 
 		if (session_media->handler) {
-			int res;
 			handler = session_media->handler;
-			res = handler->negotiate_incoming_sdp_stream(
-				session, session_media, sdp, sdp->media[i]);
+			res = handler->negotiate_incoming_sdp_stream(session, session_media, sdp,
+				sdp->media[i]);
 			if (res <= 0) {
 				/* Catastrophic failure or ignored by assigned handler. Abort! */
 				return -1;
 			}
-			if (res > 0) {
-				/* Handled by this handler. Move to the next stream */
-				continue;
-			}
+			/* Handled by this handler. Move to the next stream */
+			continue;
 		}
 
 		handler_list = ao2_find(sdp_handlers, media, OBJ_KEY);
@@ -379,13 +378,8 @@
 			continue;
 		}
 		AST_LIST_TRAVERSE(&handler_list->list, handler, next) {
-			int res;
-			if (session_media->handler) {
-				/* There is only one slot for this stream type and it has already been claimed
-				 * so it will go unhandled */
-				break;
-			}
-			res = handler->negotiate_incoming_sdp_stream(session, session_media, sdp, sdp->media[i]);
+			res = handler->negotiate_incoming_sdp_stream(session, session_media, sdp,
+				sdp->media[i]);
 			if (res < 0) {
 				/* Catastrophic failure. Abort! */
 				return -1;
@@ -420,6 +414,7 @@
 		char media[20];
 		struct ast_sip_session_sdp_handler *handler;
 		RAII_VAR(struct sdp_handler_list *, handler_list, NULL, ao2_cleanup);
+		int res;
 
 		if (!remote->media[i]) {
 			continue;
@@ -435,7 +430,8 @@
 
 		handler = session_media->handler;
 		if (handler) {
-			int res = handler->apply_negotiated_sdp_stream(session, session_media, local, local->media[i], remote, remote->media[i]);
+			res = handler->apply_negotiated_sdp_stream(session, session_media, local,
+				local->media[i], remote, remote->media[i]);
 			if (res >= 0) {
 				return CMP_MATCH;
 			}
@@ -448,7 +444,8 @@
 			continue;
 		}
 		AST_LIST_TRAVERSE(&handler_list->list, handler, next) {
-			int res = handler->apply_negotiated_sdp_stream(session, session_media, local, local->media[i], remote, remote->media[i]);
+			res = handler->apply_negotiated_sdp_stream(session, session_media, local,
+				local->media[i], remote, remote->media[i]);
 			if (res < 0) {
 				/* Catastrophic failure. Abort! */
 				return 0;
@@ -807,6 +804,7 @@
 static int sdp_requires_deferral(struct ast_sip_session *session, const pjmedia_sdp_session *sdp)
 {
 	int i;
+
 	if (validate_incoming_sdp(sdp)) {
 		return 0;
 	}
@@ -817,6 +815,7 @@
 		struct ast_sip_session_sdp_handler *handler;
 		RAII_VAR(struct sdp_handler_list *, handler_list, NULL, ao2_cleanup);
 		RAII_VAR(struct ast_sip_session_media *, session_media, NULL, ao2_cleanup);
+		enum ast_sip_session_sdp_stream_defer res;
 
 		/* We need a null-terminated version of the media string */
 		ast_copy_pj_str(media, &sdp->media[i]->desc.media, sizeof(media));
@@ -828,14 +827,24 @@
 			continue;
 		}
 
-		if (session_media->handler && session_media->handler->defer_incoming_sdp_stream) {
-			int res;
+		if (session_media->handler) {
 			handler = session_media->handler;
-			res = handler->defer_incoming_sdp_stream(
-				session, session_media, sdp, sdp->media[i]);
-			if (res) {
-				return 1;
+			if (handler->defer_incoming_sdp_stream) {
+				res = handler->defer_incoming_sdp_stream(session, session_media, sdp,
+					sdp->media[i]);
+				switch (res) {
+				case AST_SIP_SESSION_SDP_DEFER_NOT_HANDLED:
+					break;
+				case AST_SIP_SESSION_SDP_DEFER_ERROR:
+					return 0;
+				case AST_SIP_SESSION_SDP_DEFER_NOT_NEEDED:
+					break;
+				case AST_SIP_SESSION_SDP_DEFER_NEEDED:
+					return 1;
+				}
 			}
+			/* Handled by this handler. Move to the next stream */
+			continue;
 		}
 
 		handler_list = ao2_find(sdp_handlers, media, OBJ_KEY);
@@ -844,19 +853,28 @@
 			continue;
 		}
 		AST_LIST_TRAVERSE(&handler_list->list, handler, next) {
-			int res;
-			if (session_media->handler) {
-				/* There is only one slot for this stream type and it has already been claimed
-				 * so it will go unhandled */
-				break;
-			}
 			if (!handler->defer_incoming_sdp_stream) {
 				continue;
 			}
-			res = handler->defer_incoming_sdp_stream(session, session_media, sdp, sdp->media[i]);
-			if (res) {
+			res = handler->defer_incoming_sdp_stream(session, session_media, sdp,
+				sdp->media[i]);
+			switch (res) {
+			case AST_SIP_SESSION_SDP_DEFER_NOT_HANDLED:
+				continue;
+			case AST_SIP_SESSION_SDP_DEFER_ERROR:
+				session_media->handler = handler;
+				return 0;
+			case AST_SIP_SESSION_SDP_DEFER_NOT_NEEDED:
+				/* Handled by this handler. */
+				session_media->handler = handler;
+				break;
+			case AST_SIP_SESSION_SDP_DEFER_NEEDED:
+				/* Handled by this handler. */
+				session_media->handler = handler;
 				return 1;
 			}
+			/* Move to the next stream */
+			break;
 		}
 	}
 	return 0;
@@ -2003,10 +2021,12 @@
 	struct ast_sip_session *session = data;
 	struct ast_sip_session_sdp_handler *handler = session_media->handler;
 	RAII_VAR(struct sdp_handler_list *, handler_list, NULL, ao2_cleanup);
+	int res;
 
 	if (handler) {
 		/* if an already assigned handler does not handle the session_media or reports a catastrophic error, fail */
-		if (handler->create_outgoing_sdp_stream(session, session_media, answer) <= 0) {
+		res = handler->create_outgoing_sdp_stream(session, session_media, answer);
+		if (res <= 0) {
 			return 0;
 		}
 		return CMP_MATCH;
@@ -2019,13 +2039,14 @@
 
 	/* no handler for this stream type and we have a list to search */
 	AST_LIST_TRAVERSE(&handler_list->list, handler, next) {
-		int res = handler->create_outgoing_sdp_stream(session, session_media, answer);
+		res = handler->create_outgoing_sdp_stream(session, session_media, answer);
 		if (res < 0) {
 			/* catastrophic error */
 			return 0;
 		}
 		if (res > 0) {
-			/* handled */
+			/* Handled by this handler. Move to the next stream */
+			session_media->handler = handler;
 			return CMP_MATCH;
 		}
 	}

Modified: branches/12/res/res_pjsip_t38.c
URL: http://svnview.digium.com/svn/asterisk/branches/12/res/res_pjsip_t38.c?view=diff&rev=414749&r1=414748&r2=414749
==============================================================================
--- branches/12/res/res_pjsip_t38.c (original)
+++ branches/12/res/res_pjsip_t38.c Wed May 28 11:54:31 2014
@@ -569,21 +569,22 @@
 }
 
 /*! \brief Function which defers an incoming media stream */
-static int defer_incoming_sdp_stream(struct ast_sip_session *session, struct ast_sip_session_media *session_media,
-					 const struct pjmedia_sdp_session *sdp, const struct pjmedia_sdp_media *stream)
+static enum ast_sip_session_sdp_stream_defer defer_incoming_sdp_stream(
+	struct ast_sip_session *session, struct ast_sip_session_media *session_media,
+	const struct pjmedia_sdp_session *sdp, const struct pjmedia_sdp_media *stream)
 {
 	struct t38_state *state;
 
 	if (!session->endpoint->media.t38.enabled) {
-		return 0;
+		return AST_SIP_SESSION_SDP_DEFER_NOT_HANDLED;
 	}
 
 	if (t38_initialize_session(session, session_media)) {
-		return 0;
+		return AST_SIP_SESSION_SDP_DEFER_ERROR;
 	}
 
 	if (!(state = t38_state_get_or_alloc(session))) {
-		return 0;
+		return AST_SIP_SESSION_SDP_DEFER_ERROR;
 	}
 
 	t38_interpret_sdp(state, session, session_media, stream);
@@ -591,10 +592,10 @@
 	/* If they are initiating the re-invite we need to defer responding until later */
 	if (session->t38state == T38_DISABLED) {
 		t38_change_state(session, session_media, state, T38_PEER_REINVITE);
-		return 1;
-	}
-
-	return 0;
+		return AST_SIP_SESSION_SDP_DEFER_NEEDED;
+	}
+
+	return AST_SIP_SESSION_SDP_DEFER_NOT_NEEDED;
 }
 
 /*! \brief Function which negotiates an incoming media stream */




More information about the asterisk-commits mailing list