[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