[asterisk-commits] SDP API: Add SSRC-level attributes (asterisk[master])
SVN commits to the Asterisk project
asterisk-commits at lists.digium.com
Mon May 1 14:16:56 CDT 2017
Jenkins2 has submitted this change and it was merged. ( https://gerrit.asterisk.org/5550 )
Change subject: SDP API: Add SSRC-level attributes
......................................................................
SDP API: Add SSRC-level attributes
RFC 5576 defines how SSRC-level attributes may be added to SDP media
descriptions. In general, this is useful for grouping related SSRCes,
indicating SSRC-level format attributes, and resolving collisions in RTP
SSRC values. These attributes are used widely by browsers during WebRTC
communications, including attributes defined by documents outside of RFC
5576.
This commit introduces the addition of SSRC-level attributes into SDPs
generated by Asterisk. Since Asterisk does not tend to use multiple
SSRCs on a media stream, the initial support is minimal. Asterisk
includes an SSRC-level CNAME attribute if configured to do so. This at
least gives browsers (and possibly others) the ability to resolve SSRC
collisions at offer-answer time.
In order to facilitate this, the RTP engine API has been enhanced to be
able to retrieve the SSRC and CNAME on a given RTP instance.
res_rtp_asterisk currently does not provide meaningful CNAME values in
its RTCP SDES items, and therefore it currently will always return an
empty string as the CNAME value. A task in the near future will result
in res_rtp_asterisk generating more meaningful CNAMEs.
Change-Id: I29e7f23e7db77524f82a3b6e8531b1195ff57789
---
M include/asterisk/rtp_engine.h
M include/asterisk/sdp_options.h
M main/rtp_engine.c
M main/sdp_options.c
M main/sdp_private.h
M main/sdp_state.c
M res/res_rtp_asterisk.c
M tests/test_sdp.c
8 files changed, 279 insertions(+), 14 deletions(-)
Approvals:
Richard Mudgett: Looks good to me, but someone else must approve
Jenkins2: Approved for Submit
Joshua Colp: Looks good to me, approved
diff --git a/include/asterisk/rtp_engine.h b/include/asterisk/rtp_engine.h
index 55acf65..5f43916 100644
--- a/include/asterisk/rtp_engine.h
+++ b/include/asterisk/rtp_engine.h
@@ -599,6 +599,10 @@
void (*available_formats)(struct ast_rtp_instance *instance, struct ast_format_cap *to_endpoint, struct ast_format_cap *to_asterisk, struct ast_format_cap *result);
/*! Callback to send CNG */
int (*sendcng)(struct ast_rtp_instance *instance, int level);
+ /*! Callback to retrieve local SSRC */
+ unsigned int (*ssrc_get)(struct ast_rtp_instance *instance);
+ /*! Callback to retrieve RTCP SDES CNAME */
+ const char *(*cname_get)(struct ast_rtp_instance *instance);
/*! Callback to pointer for optional ICE support */
struct ast_rtp_engine_ice *ice;
/*! Callback to pointer for optional DTLS SRTP support */
@@ -2389,6 +2393,24 @@
*/
void ast_rtp_instance_set_last_rx(struct ast_rtp_instance *rtp, time_t time);
+/*!
+ * \brief Retrieve the local SSRC value that we will be using
+ *
+ * \param rtp The RTP instance
+ * \return The SSRC value
+ */
+unsigned int ast_rtp_instance_get_ssrc(struct ast_rtp_instance *rtp);
+
+/*!
+ * \brief Retrieve the CNAME used in RTCP SDES items
+ *
+ * This is a pointer directly into the RTP struct, not a copy.
+ *
+ * \param rtp The RTP instance
+ * \return the CNAME
+ */
+const char *ast_rtp_instance_get_cname(struct ast_rtp_instance *rtp);
+
/*! \addtogroup StasisTopicsAndMessages
* @{
*/
diff --git a/include/asterisk/sdp_options.h b/include/asterisk/sdp_options.h
index af694cd..3a1add3 100644
--- a/include/asterisk/sdp_options.h
+++ b/include/asterisk/sdp_options.h
@@ -509,4 +509,23 @@
*/
unsigned int ast_sdp_options_get_bind_udptl_to_media_address(const struct ast_sdp_options *options);
+/*!
+ * \since 15.0.0
+ * \brief Enable setting SSRC level attributes on SDPs
+ *
+ * \param options SDP Options
+ * \param ssrc Boolean indicating if SSRC attributes should be included in generated SDPs
+ */
+void ast_sdp_options_set_ssrc(struct ast_sdp_options *options, unsigned int ssrc);
+
+/*!
+ * \since 15.0.0
+ * \brief Get SDP Options ssrc
+ *
+ * \param options SDP Options
+ *
+ * \returns Whether SSRC-level attributes will be added to our SDP.
+ */
+unsigned int ast_sdp_options_get_ssrc(const struct ast_sdp_options *options);
+
#endif /* _ASTERISK_SDP_OPTIONS_H */
diff --git a/main/rtp_engine.c b/main/rtp_engine.c
index 8229890..9cfae09 100644
--- a/main/rtp_engine.c
+++ b/main/rtp_engine.c
@@ -3340,3 +3340,29 @@
{
rtp->last_rx = time;
}
+
+unsigned int ast_rtp_instance_get_ssrc(struct ast_rtp_instance *rtp)
+{
+ unsigned int ssrc = 0;
+
+ ao2_lock(rtp);
+ if (rtp->engine->ssrc_get) {
+ ssrc = rtp->engine->ssrc_get(rtp);
+ }
+ ao2_unlock(rtp);
+
+ return ssrc;
+}
+
+const char *ast_rtp_instance_get_cname(struct ast_rtp_instance *rtp)
+{
+ const char *cname = "";
+
+ ao2_lock(rtp);
+ if (rtp->engine->cname_get) {
+ cname = rtp->engine->cname_get(rtp);
+ }
+ ao2_unlock(rtp);
+
+ return cname;
+}
diff --git a/main/sdp_options.c b/main/sdp_options.c
index ef056a1..3f25e43 100644
--- a/main/sdp_options.c
+++ b/main/sdp_options.c
@@ -76,6 +76,7 @@
DEFINE_GETTERS_SETTERS_FOR(enum ast_sdp_options_ice, ice);
DEFINE_GETTERS_SETTERS_FOR(enum ast_sdp_options_impl, impl);
DEFINE_GETTERS_SETTERS_FOR(enum ast_sdp_options_encryption, encryption);
+DEFINE_GETTERS_SETTERS_FOR(unsigned int, ssrc);
static void set_defaults(struct ast_sdp_options *options)
{
diff --git a/main/sdp_private.h b/main/sdp_private.h
index f2efe86..f80cefb 100644
--- a/main/sdp_private.h
+++ b/main/sdp_private.h
@@ -43,6 +43,7 @@
unsigned int g726_non_standard : 1;
unsigned int locally_held : 1;
unsigned int rtcp_mux: 1;
+ unsigned int ssrc: 1;
};
struct {
unsigned int tos_audio;
diff --git a/main/sdp_state.c b/main/sdp_state.c
index 5aee567..9880349 100644
--- a/main/sdp_state.c
+++ b/main/sdp_state.c
@@ -1182,6 +1182,37 @@
stream_state->t38_local_params = *params;
}
+/*!
+ * \brief Add SSRC-level attributes if appropriate.
+ *
+ * This function does nothing if the SDP options indicate not to add SSRC-level attributes.
+ *
+ * Currently, the only attribute added is cname, which is retrieved from the RTP instance.
+ *
+ * \param m_line The m_line on which to add the SSRC attributes
+ * \param options Options that indicate what, if any, SSRC attributes to add
+ * \param rtp RTP instance from which we get SSRC-level information
+ */
+static void add_ssrc_attributes(struct ast_sdp_m_line *m_line, const struct ast_sdp_options *options,
+ struct ast_rtp_instance *rtp)
+{
+ struct ast_sdp_a_line *a_line;
+ char attr_buffer[128];
+
+ if (!ast_sdp_options_get_ssrc(options)) {
+ return;
+ }
+
+ snprintf(attr_buffer, sizeof(attr_buffer), "%u cname:%s", ast_rtp_instance_get_ssrc(rtp),
+ ast_rtp_instance_get_cname(rtp));
+
+ a_line = ast_sdp_a_alloc("ssrc", attr_buffer);
+ if (!a_line) {
+ return;
+ }
+ ast_sdp_m_add_a(m_line, a_line);
+}
+
static int sdp_add_m_from_rtp_stream(struct ast_sdp *sdp, const struct ast_sdp_state *sdp_state,
const struct ast_sdp_options *options, const struct sdp_state_capabilities *capabilities, int stream_index)
{
@@ -1312,6 +1343,8 @@
return -1;
}
+ add_ssrc_attributes(m_line, options, rtp);
+
if (ast_sdp_add_m(sdp, m_line)) {
ast_sdp_m_free(m_line);
return -1;
diff --git a/res/res_rtp_asterisk.c b/res/res_rtp_asterisk.c
index d0d7959..85e2425 100644
--- a/res/res_rtp_asterisk.c
+++ b/res/res_rtp_asterisk.c
@@ -474,6 +474,8 @@
static void ast_rtp_stop(struct ast_rtp_instance *instance);
static int ast_rtp_qos_set(struct ast_rtp_instance *instance, int tos, int cos, const char* desc);
static int ast_rtp_sendcng(struct ast_rtp_instance *instance, int level);
+static unsigned int ast_rtp_get_ssrc(struct ast_rtp_instance *instance);
+static const char *ast_rtp_get_cname(struct ast_rtp_instance *instance);
#ifdef HAVE_OPENSSL_SRTP
static int ast_rtp_activate(struct ast_rtp_instance *instance);
@@ -1903,6 +1905,8 @@
.dtls = &ast_rtp_dtls,
.activate = ast_rtp_activate,
#endif
+ .ssrc_get = ast_rtp_get_ssrc,
+ .cname_get = ast_rtp_get_cname,
};
#ifdef HAVE_OPENSSL_SRTP
@@ -5815,6 +5819,27 @@
return res;
}
+/*! \pre instance is locked */
+static unsigned int ast_rtp_get_ssrc(struct ast_rtp_instance *instance)
+{
+ struct ast_rtp *rtp = ast_rtp_instance_get_data(instance);
+
+ return rtp->ssrc;
+}
+
+/*! \pre instance is locked */
+static const char *ast_rtp_get_cname(struct ast_rtp_instance *instance)
+{
+ /* XXX
+ *
+ * Asterisk currently puts a zero-length CNAME value in RTCP SDES items,
+ * meaning our CNAME will always be an empty string. In future, should
+ * Asterisk actually start using meaningful CNAMEs, this function will
+ * need to return that instead of an empty string
+ */
+ return "";
+}
+
#ifdef HAVE_OPENSSL_SRTP
static void dtls_perform_setup(struct dtls_details *dtls)
{
diff --git a/tests/test_sdp.c b/tests/test_sdp.c
index 33a6a28..a5d3710 100644
--- a/tests/test_sdp.c
+++ b/tests/test_sdp.c
@@ -29,6 +29,7 @@
#include "asterisk/format.h"
#include "asterisk/format_cache.h"
#include "asterisk/format_cap.h"
+#include "asterisk/rtp_engine.h"
static int validate_o_line(struct ast_test *test, const struct ast_sdp_o_line *o_line,
const char *sdpowner, const char *address_type, const char *address)
@@ -353,26 +354,56 @@
return res;
}
+static struct ast_sdp_options *sdp_options_common(void)
+{
+ struct ast_sdp_options *options;
+
+ options = ast_sdp_options_alloc();
+ if (!options) {
+ return NULL;
+ }
+ ast_sdp_options_set_media_address(options, "127.0.0.1");
+ ast_sdp_options_set_sdpowner(options, "me");
+ ast_sdp_options_set_rtp_engine(options, "asterisk");
+ ast_sdp_options_set_impl(options, AST_SDP_IMPL_PJMEDIA);
+
+ return options;
+}
+
struct sdp_format {
enum ast_media_type type;
const char *formats;
};
-static struct ast_sdp_state *build_sdp_state(int num_streams, const struct sdp_format *formats)
+/*!
+ * \brief Common method to build an SDP state for a test.
+ *
+ * This uses the passed-in formats to create a stream topology, which is then used to create the SDP
+ * state.
+ *
+ * There is an optional test_options field you can use if your test has specific options you need to
+ * set. If your test does not require anything special, it can just pass NULL for this parameter. If
+ * you do pass in test_options, this function steals ownership of those options.
+ *
+ * \param num_streams The number of elements in the formats array.
+ * \param formats Array of media types and formats that will be in the state.
+ * \param test_options Optional SDP options.
+ */
+static struct ast_sdp_state *build_sdp_state(int num_streams, const struct sdp_format *formats, struct ast_sdp_options *test_options)
{
struct ast_stream_topology *topology = NULL;
struct ast_sdp_state *state = NULL;
struct ast_sdp_options *options;
int i;
- options = ast_sdp_options_alloc();
- if (!options) {
- goto end;
+ if (!test_options) {
+ options = sdp_options_common();
+ if (!options) {
+ goto end;
+ }
+ } else {
+ options = test_options;
}
- ast_sdp_options_set_media_address(options, "127.0.0.1");
- ast_sdp_options_set_sdpowner(options, "me");
- ast_sdp_options_set_rtp_engine(options, "asterisk");
- ast_sdp_options_set_impl(options, AST_SDP_IMPL_PJMEDIA);
topology = ast_stream_topology_alloc();
if (!topology) {
@@ -436,7 +467,7 @@
break;
}
- sdp_state = build_sdp_state(ARRAY_LEN(formats), formats);
+ sdp_state = build_sdp_state(ARRAY_LEN(formats), formats, NULL);
if (!sdp_state) {
goto end;
}
@@ -580,7 +611,7 @@
break;
}
- sdp_state = build_sdp_state(ARRAY_LEN(sdp_formats), sdp_formats);
+ sdp_state = build_sdp_state(ARRAY_LEN(sdp_formats), sdp_formats, NULL);
if (!sdp_state) {
res = AST_TEST_FAIL;
goto end;
@@ -704,13 +735,13 @@
break;
}
- sdp_state_offerer = build_sdp_state(ARRAY_LEN(offerer_formats), offerer_formats);
+ sdp_state_offerer = build_sdp_state(ARRAY_LEN(offerer_formats), offerer_formats, NULL);
if (!sdp_state_offerer) {
res = AST_TEST_FAIL;
goto end;
}
- sdp_state_answerer = build_sdp_state(ARRAY_LEN(answerer_formats), answerer_formats);
+ sdp_state_answerer = build_sdp_state(ARRAY_LEN(answerer_formats), answerer_formats, NULL);
if (!sdp_state_answerer) {
res = AST_TEST_FAIL;
goto end;
@@ -782,13 +813,13 @@
break;
}
- sdp_state_offerer = build_sdp_state(ARRAY_LEN(offerer_formats), offerer_formats);
+ sdp_state_offerer = build_sdp_state(ARRAY_LEN(offerer_formats), offerer_formats, NULL);
if (!sdp_state_offerer) {
res = AST_TEST_FAIL;
goto end;
}
- sdp_state_answerer = build_sdp_state(ARRAY_LEN(answerer_formats), answerer_formats);
+ sdp_state_answerer = build_sdp_state(ARRAY_LEN(answerer_formats), answerer_formats, NULL);
if (!sdp_state_answerer) {
res = AST_TEST_FAIL;
goto end;
@@ -827,6 +858,111 @@
return res;
}
+static int validate_ssrc(struct ast_test *test, struct ast_sdp_m_line *m_line,
+ struct ast_rtp_instance *rtp)
+{
+ unsigned int ssrc;
+ const char *cname;
+ struct ast_sdp_a_line *a_line;
+ char attr_value[128];
+
+ ssrc = ast_rtp_instance_get_ssrc(rtp);
+ cname = ast_rtp_instance_get_cname(rtp);
+
+ snprintf(attr_value, sizeof(attr_value), "%u cname:%s", ssrc, cname);
+
+ a_line = ast_sdp_m_find_attribute(m_line, "ssrc", -1);
+ if (!a_line) {
+ ast_test_status_update(test, "Could not find 'ssrc' attribute\n");
+ return -1;
+ }
+
+ if (strcmp(a_line->value, attr_value)) {
+ ast_test_status_update(test, "SDP attribute '%s' did not match expected attribute '%s'\n",
+ a_line->value, attr_value);
+ return -1;
+ }
+
+ return 0;
+}
+
+AST_TEST_DEFINE(sdp_ssrc_attributes)
+{
+ enum ast_test_result_state res;
+ struct ast_sdp_state *test_state = NULL;
+ struct ast_sdp_options *options;
+ struct sdp_format formats[] = {
+ { AST_MEDIA_TYPE_AUDIO, "ulaw,alaw,g722,opus" },
+ };
+ const struct ast_sdp *sdp;
+ struct ast_sdp_m_line *m_line;
+ struct ast_rtp_instance *rtp;
+
+ switch(cmd) {
+ case TEST_INIT:
+ info->name = "sdp_ssrc_attributes";
+ info->category = "/main/sdp/";
+ info->summary = "Ensure SSRC-level attributes are added to local SDPs";
+ info->description =
+ "An SDP is created and is instructed to include SSRC-level attributes.\n"
+ "This test ensures that the CNAME SSRC-level attribute is present and\n"
+ "that the values match what the RTP instance reports";
+ return AST_TEST_NOT_RUN;
+ case TEST_EXECUTE:
+ break;
+ }
+
+ res = AST_TEST_FAIL;
+
+ options = sdp_options_common();
+ if (!options) {
+ ast_test_status_update(test, "Failed to allocate SDP options\n");
+ goto end;
+ }
+ ast_sdp_options_set_ssrc(options, 1);
+
+ test_state = build_sdp_state(ARRAY_LEN(formats), formats, options);
+ if (!test_state) {
+ ast_test_status_update(test, "Failed to create SDP state\n");
+ goto end;
+ }
+
+ sdp = ast_sdp_state_get_local_sdp(test_state);
+ if (!sdp) {
+ ast_test_status_update(test, "Failed to get local SDP\n");
+ goto end;
+ }
+
+ /* Need a couple of sanity checks */
+ if (ast_sdp_get_m_count(sdp) != ARRAY_LEN(formats)) {
+ ast_test_status_update(test, "SDP m count is %d instead of %zu\n",
+ ast_sdp_get_m_count(sdp), ARRAY_LEN(formats));
+ goto end;
+ }
+
+ m_line = ast_sdp_get_m(sdp, 0);
+ if (!m_line) {
+ ast_test_status_update(test, "Failed to get SDP m-line\n");
+ goto end;
+ }
+
+ rtp = ast_sdp_state_get_rtp_instance(test_state, 0);
+ if (!rtp) {
+ ast_test_status_update(test, "Failed to get the RTP instance\n");
+ goto end;
+ }
+
+ if (validate_ssrc(test, m_line, rtp)) {
+ goto end;
+ }
+
+ res = AST_TEST_PASS;
+
+end:
+ ast_sdp_state_free(test_state);
+ return res;
+}
+
static int unload_module(void)
{
AST_TEST_UNREGISTER(invalid_rtpmap);
@@ -836,6 +972,7 @@
AST_TEST_UNREGISTER(sdp_to_topology);
AST_TEST_UNREGISTER(sdp_merge_symmetric);
AST_TEST_UNREGISTER(sdp_merge_crisscross);
+ AST_TEST_UNREGISTER(sdp_ssrc_attributes);
return 0;
}
@@ -849,6 +986,7 @@
AST_TEST_REGISTER(sdp_to_topology);
AST_TEST_REGISTER(sdp_merge_symmetric);
AST_TEST_REGISTER(sdp_merge_crisscross);
+ AST_TEST_REGISTER(sdp_ssrc_attributes);
return AST_MODULE_LOAD_SUCCESS;
}
--
To view, visit https://gerrit.asterisk.org/5550
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I29e7f23e7db77524f82a3b6e8531b1195ff57789
Gerrit-PatchSet: 3
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
More information about the asterisk-commits
mailing list