[Asterisk-code-review] SDP: Replace SDP telephone event option with dtmf option (asterisk[master])
Jenkins2
asteriskteam at digium.com
Thu May 4 19:17:06 CDT 2017
Jenkins2 has submitted this change and it was merged. ( https://gerrit.asterisk.org/5572 )
Change subject: SDP: Replace SDP telephone_event option with dtmf option
......................................................................
SDP: Replace SDP telephone_event option with dtmf option
The telephone_event option was used as a flag and a bit mapped value in
different places when it is a boolean. It is also inadequate to configure
the DTMF operation of the RTP instance created for the stream.
Change-Id: Ib1addeaf0ce86f07039f2f979cab29405dc5239b
---
M include/asterisk/sdp_options.h
M main/sdp_options.c
M main/sdp_private.h
M main/sdp_state.c
4 files changed, 63 insertions(+), 55 deletions(-)
Approvals:
Mark Michelson: Looks good to me, approved
Jenkins2: Approved for Submit
Joshua Colp: Looks good to me, but someone else must approve
diff --git a/include/asterisk/sdp_options.h b/include/asterisk/sdp_options.h
index 3a1add3..9c699a2 100644
--- a/include/asterisk/sdp_options.h
+++ b/include/asterisk/sdp_options.h
@@ -24,6 +24,20 @@
struct ast_sdp_options;
/*!
+ * \brief SDP DTMF mode options
+ */
+enum ast_sdp_options_dtmf {
+ /*! No DTMF to be used */
+ AST_SDP_DTMF_NONE,
+ /*! Use RFC 4733 events for DTMF */
+ AST_SDP_DTMF_RFC_4733,
+ /*! Use DTMF in the audio stream */
+ AST_SDP_DTMF_INBAND,
+ /*! Use SIP 4733 if supported by the other side or INBAND if not */
+ AST_SDP_DTMF_AUTO,
+};
+
+/*!
* \brief ICE options
*
* This is an enum because it will support a TRICKLE-ICE option
@@ -212,26 +226,6 @@
/*!
* \since 15.0.0
- * \brief Set SDP Options telephone_event
- *
- * \param options SDP Options
- * \param telephone_event
- */
-void ast_sdp_options_set_telephone_event(struct ast_sdp_options *options,
- unsigned int telephone_event);
-
-/*!
- * \since 15.0.0
- * \brief Get SDP Options telephone_event
- *
- * \param options SDP Options
- *
- * \returns telephone_event
- */
-unsigned int ast_sdp_options_get_telephone_event(const struct ast_sdp_options *options);
-
-/*!
- * \since 15.0.0
* \brief Set SDP Options rtp_ipv6
*
* \param options SDP Options
@@ -352,6 +346,26 @@
/*!
* \since 15.0.0
+ * \brief Set SDP Options dtmf
+ *
+ * \param options SDP Options
+ * \param dtmf
+ */
+void ast_sdp_options_set_dtmf(struct ast_sdp_options *options,
+ enum ast_sdp_options_dtmf dtmf);
+
+/*!
+ * \since 15.0.0
+ * \brief Get SDP Options dtmf
+ *
+ * \param options SDP Options
+ *
+ * \returns dtmf
+ */
+enum ast_sdp_options_dtmf ast_sdp_options_get_dtmf(const struct ast_sdp_options *options);
+
+/*!
+ * \since 15.0.0
* \brief Set SDP Options ice
*
* \param options SDP Options
diff --git a/main/sdp_options.c b/main/sdp_options.c
index 3f25e43..9b57e18 100644
--- a/main/sdp_options.c
+++ b/main/sdp_options.c
@@ -23,8 +23,8 @@
#include "sdp_private.h"
+#define DEFAULT_DTMF AST_SDP_DTMF_NONE
#define DEFAULT_ICE AST_SDP_ICE_DISABLED
-#define DEFAULT_TELEPHONE_EVENT 0
#define DEFAULT_IMPL AST_SDP_IMPL_STRING
#define DEFAULT_ENCRYPTION AST_SDP_ENCRYPTION_DISABLED
@@ -65,7 +65,6 @@
DEFINE_GETTERS_SETTERS_FOR(unsigned int, udptl_symmetric);
DEFINE_GETTERS_SETTERS_FOR(enum ast_t38_ec_modes, udptl_error_correction);
DEFINE_GETTERS_SETTERS_FOR(unsigned int, udptl_far_max_datagram);
-DEFINE_GETTERS_SETTERS_FOR(unsigned int, telephone_event);
DEFINE_GETTERS_SETTERS_FOR(unsigned int, rtp_ipv6);
DEFINE_GETTERS_SETTERS_FOR(unsigned int, g726_non_standard);
DEFINE_GETTERS_SETTERS_FOR(unsigned int, rtcp_mux);
@@ -73,6 +72,7 @@
DEFINE_GETTERS_SETTERS_FOR(unsigned int, cos_audio);
DEFINE_GETTERS_SETTERS_FOR(unsigned int, tos_video);
DEFINE_GETTERS_SETTERS_FOR(unsigned int, cos_video);
+DEFINE_GETTERS_SETTERS_FOR(enum ast_sdp_options_dtmf, dtmf);
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);
@@ -80,8 +80,8 @@
static void set_defaults(struct ast_sdp_options *options)
{
+ options->dtmf = DEFAULT_DTMF;
options->ice = DEFAULT_ICE;
- options->telephone_event = DEFAULT_TELEPHONE_EVENT;
options->impl = DEFAULT_IMPL;
options->encryption = DEFAULT_ENCRYPTION;
}
diff --git a/main/sdp_private.h b/main/sdp_private.h
index f80cefb..c90a574 100644
--- a/main/sdp_private.h
+++ b/main/sdp_private.h
@@ -34,16 +34,15 @@
AST_STRING_FIELD(rtp_engine);
);
struct {
- unsigned int bind_rtp_to_media_address : 1;
- unsigned int bind_udptl_to_media_address : 1;
- unsigned int rtp_symmetric : 1;
- unsigned int udptl_symmetric : 1;
- unsigned int telephone_event : 1;
- unsigned int rtp_ipv6 : 1;
- unsigned int g726_non_standard : 1;
- unsigned int locally_held : 1;
- unsigned int rtcp_mux: 1;
- unsigned int ssrc: 1;
+ unsigned int bind_rtp_to_media_address:1;
+ unsigned int bind_udptl_to_media_address:1;
+ unsigned int rtp_symmetric:1;
+ unsigned int udptl_symmetric:1;
+ unsigned int rtp_ipv6:1;
+ 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;
@@ -52,6 +51,7 @@
unsigned int cos_video;
unsigned int udptl_far_max_datagram;
};
+ enum ast_sdp_options_dtmf dtmf;
enum ast_sdp_options_ice ice;
enum ast_sdp_options_impl impl;
enum ast_sdp_options_encryption encryption;
diff --git a/main/sdp_state.c b/main/sdp_state.c
index 953f90c..3a87a81 100644
--- a/main/sdp_state.c
+++ b/main/sdp_state.c
@@ -178,9 +178,11 @@
ice->stop(rtp);
}
- if (options->telephone_event) {
+ if (options->dtmf == AST_SDP_DTMF_RFC_4733 || options->dtmf == AST_SDP_DTMF_AUTO) {
ast_rtp_instance_dtmf_mode_set(rtp, AST_RTP_DTMF_MODE_RFC2833);
ast_rtp_instance_set_prop(rtp, AST_RTP_PROPERTY_DTMF, 1);
+ } else if (options->dtmf == AST_SDP_DTMF_INBAND) {
+ ast_rtp_instance_dtmf_mode_set(rtp, AST_RTP_DTMF_MODE_INBAND);
}
if (media_type == AST_MEDIA_TYPE_AUDIO &&
@@ -1257,7 +1259,6 @@
ast_assert(sdp && options && stream);
- media_type = ast_stream_get_type(stream);
if (rtp) {
if (ast_sdp_state_get_stream_connection_address(sdp_state, 0, &address_rtp)) {
return -1;
@@ -1300,31 +1301,24 @@
ao2_ref(format, -1);
}
- if (rtp && media_type != AST_MEDIA_TYPE_VIDEO) {
- for (i = 1LL; i <= AST_RTP_MAX; i <<= 1) {
- if (!(options->telephone_event & i)) {
- continue;
- }
-
- rtp_code = ast_rtp_codecs_payload_code(
- ast_rtp_instance_get_codecs(rtp), 0, NULL, i);
- if (rtp_code == -1) {
- continue;
- }
-
+ media_type = ast_stream_get_type(stream);
+ if (rtp && media_type != AST_MEDIA_TYPE_VIDEO
+ && (options->dtmf == AST_SDP_DTMF_RFC_4733 || options->dtmf == AST_SDP_DTMF_AUTO)) {
+ i = AST_RTP_DTMF;
+ rtp_code = ast_rtp_codecs_payload_code(
+ ast_rtp_instance_get_codecs(rtp), 0, NULL, i);
+ if (-1 < rtp_code) {
if (ast_sdp_m_add_format(m_line, options, rtp_code, 0, NULL, i)) {
ast_sdp_m_free(m_line);
return -1;
}
- if (i == AST_RTP_DTMF) {
- snprintf(tmp, sizeof(tmp), "%d 0-16", rtp_code);
- a_line = ast_sdp_a_alloc("fmtp", tmp);
- if (!a_line || ast_sdp_m_add_a(m_line, a_line)) {
- ast_sdp_a_free(a_line);
- ast_sdp_m_free(m_line);
- return -1;
- }
+ snprintf(tmp, sizeof(tmp), "%d 0-16", rtp_code);
+ a_line = ast_sdp_a_alloc("fmtp", tmp);
+ if (!a_line || ast_sdp_m_add_a(m_line, a_line)) {
+ ast_sdp_a_free(a_line);
+ ast_sdp_m_free(m_line);
+ return -1;
}
}
}
--
To view, visit https://gerrit.asterisk.org/5572
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib1addeaf0ce86f07039f2f979cab29405dc5239b
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Richard Mudgett <rmudgett 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-code-review
mailing list