[Asterisk-code-review] SDP: Update ast get topology from sdp() to keep RTP map. (asterisk[master])

George Joseph asteriskteam at digium.com
Fri May 12 12:29:40 CDT 2017


George Joseph has submitted this change and it was merged. ( https://gerrit.asterisk.org/5596 )

Change subject: SDP: Update ast_get_topology_from_sdp() to keep RTP map.
......................................................................


SDP: Update ast_get_topology_from_sdp() to keep RTP map.

* Add failure exits to ast_get_topology_from_sdp().

Change-Id: I4cc85c1ede8d712766ed20f544dbcef04c8c1049
---
M include/asterisk/sdp.h
M include/asterisk/stream.h
M main/sdp.c
M main/sdp_state.c
M tests/test_sdp.c
5 files changed, 93 insertions(+), 53 deletions(-)

Approvals:
  Mark Michelson: Looks good to me, approved
  Jenkins2: Approved for Submit



diff --git a/include/asterisk/sdp.h b/include/asterisk/sdp.h
index 06470c4..8aa9e3b 100644
--- a/include/asterisk/sdp.h
+++ b/include/asterisk/sdp.h
@@ -638,11 +638,12 @@
  * each m-line corresponding to a stream in the created topology.
  *
  * \param sdp The SDP to convert
+ * \param g726_non_standard Non-zero if G.726 is non-standard
  *
  * \retval NULL An error occurred when converting
  * \retval non-NULL The generated stream topology
  *
  * \since 15.0.0
  */
-struct ast_stream_topology *ast_get_topology_from_sdp(const struct ast_sdp *sdp);
+struct ast_stream_topology *ast_get_topology_from_sdp(const struct ast_sdp *sdp, int g726_non_standard);
 #endif /* _SDP_PRIV_H */
diff --git a/include/asterisk/stream.h b/include/asterisk/stream.h
index 821ecec..526aee1 100644
--- a/include/asterisk/stream.h
+++ b/include/asterisk/stream.h
@@ -55,39 +55,39 @@
  * \brief States that a stream may be in
  */
 enum ast_stream_state {
-    /*!
-     * \brief Set when the stream has been removed
-     */
-    AST_STREAM_STATE_REMOVED = 0,
-    /*!
-     * \brief Set when the stream is sending and receiving media
-     */
-    AST_STREAM_STATE_SENDRECV,
-    /*!
-     * \brief Set when the stream is sending media only
-     */
-    AST_STREAM_STATE_SENDONLY,
-    /*!
-     * \brief Set when the stream is receiving media only
-     */
-    AST_STREAM_STATE_RECVONLY,
-    /*!
-     * \brief Set when the stream is not sending OR receiving media
-     */
-    AST_STREAM_STATE_INACTIVE,
+	/*!
+	 * \brief Set when the stream has been removed
+	 */
+	AST_STREAM_STATE_REMOVED = 0,
+	/*!
+	 * \brief Set when the stream is sending and receiving media
+	 */
+	AST_STREAM_STATE_SENDRECV,
+	/*!
+	 * \brief Set when the stream is sending media only
+	 */
+	AST_STREAM_STATE_SENDONLY,
+	/*!
+	 * \brief Set when the stream is receiving media only
+	 */
+	AST_STREAM_STATE_RECVONLY,
+	/*!
+	 * \brief Set when the stream is not sending OR receiving media
+	 */
+	AST_STREAM_STATE_INACTIVE,
 };
 
 /*!
  * \brief Stream data slots
  */
 enum ast_stream_data_slot {
-    /*!
-     * \brief Data slot for RTP instance
-     */
-	AST_STREAM_DATA_RTP_INSTANCE = 0,
-    /*!
-     * \brief Controls the size of the data pointer array
-     */
+	/*!
+	 * \brief Data slot for RTP instance
+	 */
+	AST_STREAM_DATA_RTP_CODECS = 0,
+	/*!
+	 * \brief Controls the size of the data pointer array
+	 */
 	AST_STREAM_DATA_SLOT_MAX
 };
 
@@ -386,15 +386,15 @@
  *
  * \param topology The topology of streams
  *
-  * \retval non-NULL success
-  * \retval NULL failure
-  *
-  * \note The stream topology is NOT altered by this function.
-  *
-  * \since 15
-  */
+ * \retval non-NULL success
+ * \retval NULL failure
+ *
+ * \note The stream topology is NOT altered by this function.
+ *
+ * \since 15
+ */
 struct ast_format_cap *ast_format_cap_from_stream_topology(
-    struct ast_stream_topology *topology);
+	struct ast_stream_topology *topology);
 
 /*!
  * \brief Gets the first stream of a specific type from the topology
diff --git a/main/sdp.c b/main/sdp.c
index 62acdd3..019c669 100644
--- a/main/sdp.c
+++ b/main/sdp.c
@@ -693,6 +693,17 @@
 	ao2_ref(format, -1);
 }
 
+/*
+ * Needed so we don't have an external function referenced as data.
+ * The dynamic linker doesn't handle that very well.
+ */
+static void rtp_codecs_free(struct ast_rtp_codecs *codecs)
+{
+	if (codecs) {
+		ast_rtp_codecs_payloads_destroy(codecs);
+	}
+}
+
 /*!
  * \brief Convert an SDP stream into an Asterisk stream
  *
@@ -700,16 +711,19 @@
  * This takes formats, as well as clock-rate and fmtp attributes into account.
  *
  * \param m_line The SDP media section to convert
+ * \param g726_non_standard Non-zero if G.726 is non-standard
+ *
  * \retval NULL An error occurred
  * \retval non-NULL The converted stream
  */
-static struct ast_stream *get_stream_from_m(const struct ast_sdp_m_line *m_line)
+static struct ast_stream *get_stream_from_m(const struct ast_sdp_m_line *m_line, int g726_non_standard)
 {
 	int i;
 	int non_ast_fmts;
-	struct ast_rtp_codecs codecs;
+	struct ast_rtp_codecs *codecs;
 	struct ast_format_cap *caps;
 	struct ast_stream *stream;
+	enum ast_rtp_options options;
 
 	caps = ast_format_cap_alloc(AST_FORMAT_CAP_FLAG_DEFAULT);
 	if (!caps) {
@@ -724,8 +738,15 @@
 	switch (ast_stream_get_type(stream)) {
 	case AST_MEDIA_TYPE_AUDIO:
 	case AST_MEDIA_TYPE_VIDEO:
-		ast_rtp_codecs_payloads_initialize(&codecs);
+		codecs = ast_calloc(1, sizeof(*codecs));
+		if (!codecs || ast_rtp_codecs_payloads_initialize(codecs)) {
+			rtp_codecs_free(codecs);
+			ast_stream_free(stream);
+			ao2_ref(caps, -1);
+			return NULL;
+		}
 
+		options = g726_non_standard ? AST_RTP_OPT_G726_NONSTANDARD : 0;
 		for (i = 0; i < ast_sdp_m_get_payload_count(m_line); ++i) {
 			struct ast_sdp_payload *payload_s;
 			struct ast_sdp_rtpmap *rtpmap;
@@ -733,22 +754,25 @@
 
 			payload_s = ast_sdp_m_get_payload(m_line, i);
 			sscanf(payload_s->fmt, "%30d", &payload);
-			ast_rtp_codecs_payloads_set_m_type(&codecs, NULL, payload);
 
 			rtpmap = sdp_payload_get_rtpmap(m_line, payload);
 			if (!rtpmap) {
+				/* No rtpmap attribute.  Try static payload type format assignment */
+				ast_rtp_codecs_payloads_set_m_type(codecs, NULL, payload);
 				continue;
 			}
-			ast_rtp_codecs_payloads_set_rtpmap_type_rate(&codecs, NULL,
-				payload, m_line->type, rtpmap->encoding_name, 0,
-				rtpmap->clock_rate);
-			ast_sdp_rtpmap_free(rtpmap);
 
-			process_fmtp(m_line, payload, &codecs);
+			if (!ast_rtp_codecs_payloads_set_rtpmap_type_rate(codecs, NULL, payload,
+				m_line->type, rtpmap->encoding_name, options, rtpmap->clock_rate)) {
+				/* Successfully mapped the payload type to format */
+				process_fmtp(m_line, payload, codecs);
+			}
+			ast_sdp_rtpmap_free(rtpmap);
 		}
 
-		ast_rtp_codecs_payload_formats(&codecs, caps, &non_ast_fmts);
-		ast_rtp_codecs_payloads_destroy(&codecs);
+		ast_rtp_codecs_payload_formats(codecs, caps, &non_ast_fmts);
+		ast_stream_set_data(stream, AST_STREAM_DATA_RTP_CODECS, codecs,
+			(ast_stream_data_free_fn) rtp_codecs_free);
 		break;
 	case AST_MEDIA_TYPE_IMAGE:
 		for (i = 0; i < ast_sdp_m_get_payload_count(m_line); ++i) {
@@ -773,7 +797,7 @@
 	return stream;
 }
 
-struct ast_stream_topology *ast_get_topology_from_sdp(const struct ast_sdp *sdp)
+struct ast_stream_topology *ast_get_topology_from_sdp(const struct ast_sdp *sdp, int g726_non_standard)
 {
 	struct ast_stream_topology *topology;
 	int i;
@@ -786,11 +810,21 @@
 	for (i = 0; i < ast_sdp_get_m_count(sdp); ++i) {
 		struct ast_stream *stream;
 
-		stream = get_stream_from_m(ast_sdp_get_m(sdp, i));
+		stream = get_stream_from_m(ast_sdp_get_m(sdp, i), g726_non_standard);
 		if (!stream) {
-			continue;
+			/*
+			 * The topology cannot match the SDP because
+			 * we failed to create a corresponding stream.
+			 */
+			ast_stream_topology_free(topology);
+			return NULL;
 		}
-		ast_stream_topology_append_stream(topology, stream);
+		if (ast_stream_topology_append_stream(topology, stream) < 0) {
+			/* Failed to add stream to topology */
+			ast_stream_free(stream);
+			ast_stream_topology_free(topology);
+			return NULL;
+		}
 	}
 
 	return topology;
diff --git a/main/sdp_state.c b/main/sdp_state.c
index a9979eb..6ce06ee 100644
--- a/main/sdp_state.c
+++ b/main/sdp_state.c
@@ -1007,7 +1007,8 @@
 	struct ast_stream_topology *remote_capabilities;
 	int i;
 
-	remote_capabilities = ast_get_topology_from_sdp(remote_sdp);
+	remote_capabilities = ast_get_topology_from_sdp(remote_sdp,
+		sdp_state->options->g726_non_standard);
 	if (!remote_capabilities) {
 		return -1;
 	}
diff --git a/tests/test_sdp.c b/tests/test_sdp.c
index a5d3710..408888f 100644
--- a/tests/test_sdp.c
+++ b/tests/test_sdp.c
@@ -623,7 +623,11 @@
 		goto end;
 	}
 
-	topology = ast_get_topology_from_sdp(sdp);
+	topology = ast_get_topology_from_sdp(sdp, 0);
+	if (!topology) {
+		res = AST_TEST_FAIL;
+		goto end;
+	}
 
 	if (ast_stream_topology_get_count(topology) != 3) {
 		ast_test_status_update(test, "Unexpected topology count '%d'. Expecting 2\n",

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

Gerrit-MessageType: merged
Gerrit-Change-Id: I4cc85c1ede8d712766ed20f544dbcef04c8c1049
Gerrit-PatchSet: 2
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>



More information about the asterisk-code-review mailing list