[Asterisk-code-review] sdp: Add support for T.38 (asterisk[master])

Joshua Colp asteriskteam at digium.com
Thu Apr 27 05:38:14 CDT 2017


Joshua Colp has submitted this change and it was merged. ( https://gerrit.asterisk.org/5496 )

Change subject: sdp: Add support for T.38
......................................................................


sdp: Add support for T.38

This change adds a T.38 format which can be used in a stream
topology to specify that a UDPTL stream needs to be created.
The SDP API has been changed to understand T.38 and create
the UDPTL session, add the attributes, and parse the attributes.

This change does not change the boundary of the T.38 state
machine. It is still up to the channel driver to implement and
act on it (such as queueing control frames or reacting to them).

ASTERISK-26949

Change-Id: If28956762ccb8ead562ac6c03d162d3d6014f2c7
---
M include/asterisk/format_cache.h
M include/asterisk/sdp_options.h
M include/asterisk/sdp_state.h
M main/codec_builtin.c
M main/format_cache.c
M main/sdp.c
M main/sdp_options.c
M main/sdp_private.h
M main/sdp_state.c
M tests/test_sdp.c
10 files changed, 623 insertions(+), 47 deletions(-)

Approvals:
  Mark Michelson: Looks good to me, approved
  Richard Mudgett: Looks good to me, but someone else must approve
  Jenkins2: Approved for Submit



diff --git a/include/asterisk/format_cache.h b/include/asterisk/format_cache.h
index 6099c59..92272e8 100644
--- a/include/asterisk/format_cache.h
+++ b/include/asterisk/format_cache.h
@@ -224,6 +224,11 @@
 extern struct ast_format *ast_format_t140_red;
 
 /*!
+ * \brief Built-in cached T.38 format.
+ */
+extern struct ast_format *ast_format_t38;
+
+/*!
  * \brief Built-in "null" format.
  */
 extern struct ast_format *ast_format_none;
diff --git a/include/asterisk/sdp_options.h b/include/asterisk/sdp_options.h
index 4b411c7..af694cd 100644
--- a/include/asterisk/sdp_options.h
+++ b/include/asterisk/sdp_options.h
@@ -19,6 +19,8 @@
 #ifndef _ASTERISK_SDP_OPTIONS_H
 #define _ASTERISK_SDP_OPTIONS_H
 
+#include "asterisk/udptl.h"
+
 struct ast_sdp_options;
 
 /*!
@@ -427,4 +429,84 @@
  */
 void ast_sdp_options_set_rtcp_mux(struct ast_sdp_options *options, unsigned int value);
 
+/*!
+ * \since 15.0.0
+ * \brief Set SDP Options udptl_symmetric
+ *
+ * \param options SDP Options
+ * \param udptl_symmetric
+ */
+void ast_sdp_options_set_udptl_symmetric(struct ast_sdp_options *options,
+	unsigned int udptl_symmetric);
+
+/*!
+ * \since 15.0.0
+ * \brief Get SDP Options udptl_symmetric
+ *
+ * \param options SDP Options
+ *
+ * \returns udptl_symmetric
+ */
+unsigned int ast_sdp_options_get_udptl_symmetric(const struct ast_sdp_options *options);
+
+/*!
+ * \since 15.0.0
+ * \brief Set SDP Options udptl_error_correction
+ *
+ * \param options SDP Options
+ * \param error_correction
+ */
+void ast_sdp_options_set_udptl_error_correction(struct ast_sdp_options *options,
+	enum ast_t38_ec_modes error_correction);
+
+/*!
+ * \since 15.0.0
+ * \brief Get SDP Options udptl_error_correction
+ *
+ * \param options SDP Options
+ *
+ * \returns udptl_error_correction
+ */
+enum ast_t38_ec_modes ast_sdp_options_get_udptl_error_correction(const struct ast_sdp_options *options);
+
+/*!
+ * \since 15.0.0
+ * \brief Set SDP Options udptl_far_max_datagram
+ *
+ * \param options SDP Options
+ * \param far_max_datagram
+ */
+void ast_sdp_options_set_udptl_far_max_datagram(struct ast_sdp_options *options,
+	unsigned int far_max_datagram);
+
+/*!
+ * \since 15.0.0
+ * \brief Get SDP Options udptl_far_max_datagram
+ *
+ * \param options SDP Options
+ *
+ * \returns udptl_far_max_datagram
+ */
+unsigned int ast_sdp_options_get_udptl_far_max_datagram(const struct ast_sdp_options *options);
+
+/*!
+ * \since 15.0.0
+ * \brief Set SDP Options bind_udptl_to_media_address
+ *
+ * \param options SDP Options
+ * \param bind_udptl_to_media_address
+ */
+void ast_sdp_options_set_bind_udptl_to_media_address(struct ast_sdp_options *options,
+	unsigned int bind_udptl_to_media_address);
+
+/*!
+ * \since 15.0.0
+ * \brief Get SDP Options bind_udptl_to_media_address
+ *
+ * \param options SDP Options
+ *
+ * \returns bind_udptl_to_media_address
+ */
+unsigned int ast_sdp_options_get_bind_udptl_to_media_address(const struct ast_sdp_options *options);
+
 #endif /* _ASTERISK_SDP_OPTIONS_H */
diff --git a/include/asterisk/sdp_state.h b/include/asterisk/sdp_state.h
index 7f25c25..1382ed6 100644
--- a/include/asterisk/sdp_state.h
+++ b/include/asterisk/sdp_state.h
@@ -24,6 +24,8 @@
 
 struct ast_sdp_state;
 struct ast_sockaddr;
+struct ast_udptl;
+struct ast_control_t38_parameters;
 
 /*!
  * \brief Allocate a new SDP state
@@ -49,6 +51,14 @@
  * Stream numbers correspond to the streams in the topology of the associated channel
  */
 struct ast_rtp_instance *ast_sdp_state_get_rtp_instance(const struct ast_sdp_state *sdp_state,
+	int stream_index);
+
+/*!
+ * \brief Get the associated UDPTL instance for a particular stream on the SDP state.
+ *
+ * Stream numbers correspond to the streams in the topology of the associated channel
+ */
+struct ast_udptl *ast_sdp_state_get_udptl_instance(const struct ast_sdp_state *sdp_state,
 	int stream_index);
 
 /*!
@@ -225,6 +235,17 @@
 
 /*!
  * \since 15.0.0
+ * \brief Set the UDPTL session parameters
+ *
+ * \param sdp_state
+ * \param stream_index The stream to set the UDPTL session parameters for
+ * \param params
+ */
+void ast_sdp_state_set_t38_parameters(struct ast_sdp_state *sdp_state,
+	int stream_index, struct ast_control_t38_parameters *params);
+
+/*!
+ * \since 15.0.0
  * \brief Get whether a stream is held or not
  *
  * \param sdp_state
diff --git a/main/codec_builtin.c b/main/codec_builtin.c
index f622c91..3320900 100644
--- a/main/codec_builtin.c
+++ b/main/codec_builtin.c
@@ -822,6 +822,12 @@
 	.type = AST_MEDIA_TYPE_TEXT,
 };
 
+static struct ast_codec t38 = {
+	.name = "t38",
+	.description = "T.38 UDPTL Fax",
+	.type = AST_MEDIA_TYPE_IMAGE,
+};
+
 static int silk_samples(struct ast_frame *frame)
 {
 	/* XXX This is likely not at all what's intended from this callback. However,
@@ -952,6 +958,7 @@
 	res |= CODEC_REGISTER_AND_CACHE(vp8);
 	res |= CODEC_REGISTER_AND_CACHE(t140red);
 	res |= CODEC_REGISTER_AND_CACHE(t140);
+	res |= CODEC_REGISTER_AND_CACHE(t38);
 	res |= CODEC_REGISTER_AND_CACHE(none);
 	res |= CODEC_REGISTER_AND_CACHE_NAMED("silk8", silk8);
 	res |= CODEC_REGISTER_AND_CACHE_NAMED("silk12", silk12);
diff --git a/main/format_cache.c b/main/format_cache.c
index d0ae32e..302bbf8 100644
--- a/main/format_cache.c
+++ b/main/format_cache.c
@@ -231,6 +231,11 @@
 struct ast_format *ast_format_t140_red;
 
 /*!
+ * \brief Built-in cached T.38 format.
+ */
+struct ast_format *ast_format_t38;
+
+/*!
  * \brief Built-in "null" format.
  */
 struct ast_format *ast_format_none;
@@ -342,6 +347,7 @@
 	ao2_replace(ast_format_vp8, NULL);
 	ao2_replace(ast_format_t140_red, NULL);
 	ao2_replace(ast_format_t140, NULL);
+	ao2_replace(ast_format_t38, NULL);
 	ao2_replace(ast_format_none, NULL);
 	ao2_replace(ast_format_silk8, NULL);
 	ao2_replace(ast_format_silk12, NULL);
@@ -442,6 +448,8 @@
 		ao2_replace(ast_format_t140_red, format);
 	} else if (!strcmp(name, "t140")) {
 		ao2_replace(ast_format_t140, format);
+	} else if (!strcmp(name, "t38")) {
+		ao2_replace(ast_format_t38, format);
 	} else if (!strcmp(name, "none")) {
 		ao2_replace(ast_format_none, format);
 	} else if (!strcmp(name, "silk8")) {
diff --git a/main/sdp.c b/main/sdp.c
index 75a9da9..dc6afe7 100644
--- a/main/sdp.c
+++ b/main/sdp.c
@@ -23,6 +23,7 @@
 #include "asterisk/codec.h"
 #include "asterisk/format.h"
 #include "asterisk/format_cap.h"
+#include "asterisk/format_cache.h"
 #include "asterisk/rtp_engine.h"
 #include "asterisk/sdp_state.h"
 #include "asterisk/sdp_options.h"
@@ -712,34 +713,56 @@
 		ao2_ref(caps, -1);
 		return NULL;
 	}
-	ast_rtp_codecs_payloads_initialize(&codecs);
 
-	for (i = 0; i < ast_sdp_m_get_payload_count(m_line); ++i) {
-		struct ast_sdp_payload *payload_s;
-		struct ast_sdp_rtpmap *rtpmap;
-		int payload;
+	switch (ast_stream_get_type(stream)) {
+	case AST_MEDIA_TYPE_AUDIO:
+	case AST_MEDIA_TYPE_VIDEO:
+		ast_rtp_codecs_payloads_initialize(&codecs);
 
-		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);
+		for (i = 0; i < ast_sdp_m_get_payload_count(m_line); ++i) {
+			struct ast_sdp_payload *payload_s;
+			struct ast_sdp_rtpmap *rtpmap;
+			int payload;
 
-		rtpmap = sdp_payload_get_rtpmap(m_line, payload);
-		if (!rtpmap) {
-			continue;
+			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) {
+				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);
 		}
-		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);
+		ast_rtp_codecs_payload_formats(&codecs, caps, &non_ast_fmts);
+		ast_rtp_codecs_payloads_destroy(&codecs);
+		break;
+	case AST_MEDIA_TYPE_IMAGE:
+		for (i = 0; i < ast_sdp_m_get_payload_count(m_line); ++i) {
+			struct ast_sdp_payload *payload;
+
+			/* As we don't carry T.38 over RTP we do our own format check */
+			payload = ast_sdp_m_get_payload(m_line, i);
+			if (!strcasecmp(payload->fmt, "t38")) {
+				ast_format_cap_append(caps, ast_format_t38, 0);
+			}
+		}
+		break;
+	case AST_MEDIA_TYPE_UNKNOWN:
+	case AST_MEDIA_TYPE_TEXT:
+	case AST_MEDIA_TYPE_END:
+		break;
 	}
 
-	ast_rtp_codecs_payload_formats(&codecs, caps, &non_ast_fmts);
 	ast_stream_set_formats(stream, caps);
-
 	ao2_ref(caps, -1);
-	ast_rtp_codecs_payloads_destroy(&codecs);
+
 	return stream;
 }
 
diff --git a/main/sdp_options.c b/main/sdp_options.c
index 1162f69..ef056a1 100644
--- a/main/sdp_options.c
+++ b/main/sdp_options.c
@@ -60,7 +60,11 @@
 DEFINE_STRINGFIELD_GETTERS_SETTERS_FOR(rtp_engine, 0);
 
 DEFINE_GETTERS_SETTERS_FOR(unsigned int, bind_rtp_to_media_address);
+DEFINE_GETTERS_SETTERS_FOR(unsigned int, bind_udptl_to_media_address);
 DEFINE_GETTERS_SETTERS_FOR(unsigned int, rtp_symmetric);
+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);
diff --git a/main/sdp_private.h b/main/sdp_private.h
index 15b9d1e..f2efe86 100644
--- a/main/sdp_private.h
+++ b/main/sdp_private.h
@@ -35,7 +35,9 @@
 	);
 	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;
@@ -47,10 +49,12 @@
 		unsigned int cos_audio;
 		unsigned int tos_video;
 		unsigned int cos_video;
+		unsigned int udptl_far_max_datagram;
 	};
 	enum ast_sdp_options_ice ice;
 	enum ast_sdp_options_impl impl;
 	enum ast_sdp_options_encryption encryption;
+	enum ast_t38_ec_modes udptl_error_correction;
 };
 
 #endif /* _MAIN_SDP_PRIVATE_H */
diff --git a/main/sdp_state.c b/main/sdp_state.c
index fc7fff4..5aee567 100644
--- a/main/sdp_state.c
+++ b/main/sdp_state.c
@@ -28,6 +28,7 @@
 #include "asterisk/format_cap.h"
 #include "asterisk/config.h"
 #include "asterisk/codec.h"
+#include "asterisk/udptl.h"
 
 #include "../include/asterisk/sdp.h"
 #include "asterisk/stream.h"
@@ -63,21 +64,53 @@
 
 typedef int (*state_fn)(struct ast_sdp_state *state);
 
+struct sdp_state_udptl {
+	/*! The underlying UDPTL instance */
+	struct ast_udptl *instance;
+};
+
 struct sdp_state_stream {
+	/*! Type of the stream */
+	enum ast_media_type type;
 	union {
 		/*! The underlying RTP instance */
 		struct ast_rtp_instance *instance;
+		/*! The underlying UDPTL instance */
+		struct sdp_state_udptl *udptl;
 	};
 	/*! An explicit connection address for this stream */
 	struct ast_sockaddr connection_address;
 	/*! Whether this stream is held or not */
 	unsigned int locally_held;
+	/*! UDPTL session parameters */
+	struct ast_control_t38_parameters t38_local_params;
 };
+
+static void sdp_state_udptl_destroy(void *obj)
+{
+	struct sdp_state_udptl *udptl = obj;
+
+	if (udptl->instance) {
+		ast_udptl_destroy(udptl->instance);
+	}
+}
 
 static void sdp_state_stream_free(struct sdp_state_stream *state_stream)
 {
-	if (state_stream->instance) {
-		ast_rtp_instance_destroy(state_stream->instance);
+	switch (state_stream->type) {
+	case AST_MEDIA_TYPE_AUDIO:
+	case AST_MEDIA_TYPE_VIDEO:
+		if (state_stream->instance) {
+			ast_rtp_instance_destroy(state_stream->instance);
+		}
+		break;
+	case AST_MEDIA_TYPE_IMAGE:
+		ao2_cleanup(state_stream->udptl);
+		break;
+	case AST_MEDIA_TYPE_UNKNOWN:
+	case AST_MEDIA_TYPE_TEXT:
+	case AST_MEDIA_TYPE_END:
+		break;
 	}
 	ast_free(state_stream);
 }
@@ -165,6 +198,43 @@
 	return rtp;
 }
 
+/*! \brief Internal function which creates a UDPTL instance */
+static struct sdp_state_udptl *create_udptl(const struct ast_sdp_options *options)
+{
+	struct sdp_state_udptl *udptl;
+	struct ast_sockaddr temp_media_address;
+	static struct ast_sockaddr address_udptl;
+	struct ast_sockaddr *media_address =  &address_udptl;
+
+	if (options->bind_udptl_to_media_address && !ast_strlen_zero(options->media_address)) {
+		ast_sockaddr_parse(&temp_media_address, options->media_address, 0);
+		media_address = &temp_media_address;
+	} else {
+		if (ast_check_ipv6()) {
+			ast_sockaddr_parse(&address_udptl, "::", 0);
+		} else {
+			ast_sockaddr_parse(&address_udptl, "0.0.0.0", 0);
+		}
+	}
+
+	udptl = ao2_alloc_options(sizeof(*udptl), sdp_state_udptl_destroy, AO2_ALLOC_OPT_LOCK_NOLOCK);
+	if (!udptl) {
+		return NULL;
+	}
+
+	udptl->instance = ast_udptl_new_with_bindaddr(NULL, NULL, 0, media_address);
+	if (!udptl->instance) {
+		ao2_ref(udptl, -1);
+		return NULL;
+	}
+
+	ast_udptl_set_error_correction_scheme(udptl->instance, ast_sdp_options_get_udptl_error_correction(options));
+	ast_udptl_setnat(udptl->instance, ast_sdp_options_get_udptl_symmetric(options));
+	ast_udptl_set_far_max_datagram(udptl->instance, ast_sdp_options_get_udptl_far_max_datagram(options));
+
+	return udptl;
+}
+
 static struct sdp_state_capabilities *sdp_initialize_state_capabilities(const struct ast_stream_topology *topology,
 	const struct ast_sdp_options *options)
 {
@@ -191,22 +261,34 @@
 
 	for (i = 0; i < ast_stream_topology_get_count(topology); ++i) {
 		struct sdp_state_stream *state_stream;
-		enum ast_media_type stream_type;
 
 		state_stream = ast_calloc(1, sizeof(*state_stream));
 		if (!state_stream) {
 			return NULL;
 		}
 
-		stream_type = ast_stream_get_type(ast_stream_topology_get_stream(topology, i));
+		state_stream->type = ast_stream_get_type(ast_stream_topology_get_stream(topology, i));
 
-		if (stream_type == AST_MEDIA_TYPE_AUDIO || stream_type == AST_MEDIA_TYPE_VIDEO) {
-			state_stream->instance = create_rtp(options, stream_type);
-		}
-
-		if (!state_stream->instance) {
-			sdp_state_stream_free(state_stream);
-			return NULL;
+		switch (state_stream->type) {
+		case AST_MEDIA_TYPE_AUDIO:
+		case AST_MEDIA_TYPE_VIDEO:
+			state_stream->instance = create_rtp(options, state_stream->type);
+			if (!state_stream->instance) {
+				sdp_state_stream_free(state_stream);
+				return NULL;
+			}
+			break;
+		case AST_MEDIA_TYPE_IMAGE:
+			state_stream->udptl = create_udptl(options);
+			if (!state_stream->udptl) {
+				sdp_state_stream_free(state_stream);
+				return NULL;
+			}
+			break;
+		case AST_MEDIA_TYPE_UNKNOWN:
+		case AST_MEDIA_TYPE_TEXT:
+		case AST_MEDIA_TYPE_END:
+			break;
 		}
 
 		AST_VECTOR_APPEND(&capabilities->streams, state_stream);
@@ -314,6 +396,9 @@
 	struct sdp_state_stream *stream_state;
 
 	ast_assert(sdp_state != NULL);
+	ast_assert(ast_stream_get_type(ast_stream_topology_get_stream(sdp_state->proposed_capabilities->topology,
+		stream_index)) == AST_MEDIA_TYPE_AUDIO || ast_stream_get_type(ast_stream_topology_get_stream(
+			sdp_state->proposed_capabilities->topology, stream_index)) == AST_MEDIA_TYPE_VIDEO);
 
 	stream_state = sdp_state_get_stream(sdp_state, stream_index);
 	if (!stream_state) {
@@ -321,6 +406,23 @@
 	}
 
 	return stream_state->instance;
+}
+
+struct ast_udptl *ast_sdp_state_get_udptl_instance(
+	const struct ast_sdp_state *sdp_state, int stream_index)
+{
+	struct sdp_state_stream *stream_state;
+
+	ast_assert(sdp_state != NULL);
+	ast_assert(ast_stream_get_type(ast_stream_topology_get_stream(sdp_state->proposed_capabilities->topology,
+		stream_index)) == AST_MEDIA_TYPE_IMAGE);
+
+	stream_state = sdp_state_get_stream(sdp_state, stream_index);
+	if (!stream_state || !stream_state->udptl) {
+		return NULL;
+	}
+
+	return stream_state->udptl->instance;
 }
 
 const struct ast_sockaddr *ast_sdp_state_get_connection_address(const struct ast_sdp_state *sdp_state)
@@ -334,7 +436,6 @@
 	int stream_index, struct ast_sockaddr *address)
 {
 	struct sdp_state_stream *stream_state;
-	enum ast_media_type type;
 
 	ast_assert(sdp_state != NULL);
 	ast_assert(address != NULL);
@@ -350,12 +451,18 @@
 		return 0;
 	}
 
-	type = ast_stream_get_type(ast_stream_topology_get_stream(sdp_state->proposed_capabilities->topology,
-		stream_index));
-
-	if (type == AST_MEDIA_TYPE_AUDIO || type == AST_MEDIA_TYPE_VIDEO) {
+	switch (ast_stream_get_type(ast_stream_topology_get_stream(sdp_state->proposed_capabilities->topology,
+		stream_index))) {
+	case AST_MEDIA_TYPE_AUDIO:
+	case AST_MEDIA_TYPE_VIDEO:
 		ast_rtp_instance_get_local_address(stream_state->instance, address);
-	} else {
+		break;
+	case AST_MEDIA_TYPE_IMAGE:
+		ast_udptl_get_us(stream_state->udptl->instance, address);
+		break;
+	case AST_MEDIA_TYPE_UNKNOWN:
+	case AST_MEDIA_TYPE_TEXT:
+	case AST_MEDIA_TYPE_END:
 		return -1;
 	}
 
@@ -556,7 +663,22 @@
 			}
 
 			current_state_stream = AST_VECTOR_GET(&current->streams, current_index);
-			joint_state_stream->instance = ao2_bump(current_state_stream->instance);
+			joint_state_stream->type = current_state_stream->type;
+
+			switch (joint_state_stream->type) {
+			case AST_MEDIA_TYPE_AUDIO:
+			case AST_MEDIA_TYPE_VIDEO:
+				joint_state_stream->instance = ao2_bump(current_state_stream->instance);
+				break;
+			case AST_MEDIA_TYPE_IMAGE:
+				joint_state_stream->udptl = ao2_bump(current_state_stream->udptl);
+				joint_state_stream->t38_local_params = current_state_stream->t38_local_params;
+				break;
+			case AST_MEDIA_TYPE_UNKNOWN:
+			case AST_MEDIA_TYPE_TEXT:
+			case AST_MEDIA_TYPE_END:
+				break;
+			}
 
 			if (!ast_sockaddr_isnull(&current_state_stream->connection_address)) {
 				ast_sockaddr_copy(&joint_state_stream->connection_address, &current_state_stream->connection_address);
@@ -572,11 +694,25 @@
 			if (!joint_stream) {
 				goto fail;
 			}
-			if (new_stream_type == AST_MEDIA_TYPE_AUDIO || new_stream_type == AST_MEDIA_TYPE_VIDEO) {
+
+			switch (new_stream_type) {
+			case AST_MEDIA_TYPE_AUDIO:
+			case AST_MEDIA_TYPE_VIDEO:
 				joint_state_stream->instance = create_rtp(options, new_stream_type);
 				if (!joint_state_stream->instance) {
 					goto fail;
 				}
+				break;
+			case AST_MEDIA_TYPE_IMAGE:
+				joint_state_stream->udptl = create_udptl(options);
+				if (!joint_state_stream->udptl) {
+					goto fail;
+				}
+				break;
+			case AST_MEDIA_TYPE_UNKNOWN:
+			case AST_MEDIA_TYPE_TEXT:
+			case AST_MEDIA_TYPE_END:
+				break;
 			}
 			ast_sockaddr_setnull(&joint_state_stream->connection_address);
 			joint_state_stream->locally_held = 0;
@@ -747,6 +883,63 @@
 	}
 }
 
+/*!
+ * \brief Update UDPTL instances based on merged SDPs
+ *
+ * UDPTL instances, when first allocated, cannot make assumptions about what the other
+ * side supports and thus has to go with some default behaviors. This function gets
+ * called after we know both what we support and what the remote endpoint supports.
+ * This way, we can update the UDPTL instance to reflect what is supported by both
+ * sides.
+ *
+ * \param state The SDP state in which SDPs have been negotiated
+ * \param udptl The UDPTL instance that is being updated
+ * \param options Our locally-supported SDP options
+ * \param remote_sdp The SDP we most recently received
+ * \param remote_m_line The remote SDP stream that corresponds to the RTP instance we are modifying
+ */
+static void update_udptl_after_merge(const struct ast_sdp_state *state, struct sdp_state_udptl *udptl,
+    const struct ast_sdp_options *options,
+	const struct ast_sdp *remote_sdp,
+	const struct ast_sdp_m_line *remote_m_line)
+{
+	struct ast_sdp_a_line *a_line;
+	struct ast_sdp_c_line *c_line;
+	unsigned int fax_max_datagram;
+	struct ast_sockaddr *addrs;
+
+	a_line = ast_sdp_m_find_attribute(remote_m_line, "t38faxmaxdatagram", -1);
+	if (!a_line) {
+		a_line = ast_sdp_m_find_attribute(remote_m_line, "t38maxdatagram", -1);
+	}
+	if (a_line && !ast_sdp_options_get_udptl_far_max_datagram(options) &&
+		(sscanf(a_line->value, "%30u", &fax_max_datagram) == 1)) {
+		ast_udptl_set_far_max_datagram(udptl->instance, fax_max_datagram);
+	}
+
+	a_line = ast_sdp_m_find_attribute(remote_m_line, "t38faxudpec", -1);
+	if (a_line) {
+		if (!strcasecmp(a_line->value, "t38UDPRedundancy")) {
+			ast_udptl_set_error_correction_scheme(udptl->instance, UDPTL_ERROR_CORRECTION_REDUNDANCY);
+		} else if (!strcasecmp(a_line->value, "t38UDPFEC")) {
+			ast_udptl_set_error_correction_scheme(udptl->instance, UDPTL_ERROR_CORRECTION_FEC);
+		} else {
+			ast_udptl_set_error_correction_scheme(udptl->instance, UDPTL_ERROR_CORRECTION_NONE);
+		}
+	}
+
+	c_line = remote_sdp->c_line;
+	if (remote_m_line->c_line) {
+		c_line = remote_m_line->c_line;
+	}
+
+	if (ast_sockaddr_resolve(&addrs, c_line->address, PARSE_PORT_FORBID, AST_AF_UNSPEC) > 0) {
+		ast_sockaddr_set_port(addrs, remote_m_line->port);
+		ast_udptl_set_peer(udptl->instance, addrs);
+		ast_free(addrs);
+	}
+}
+
 static void set_negotiated_capabilities(struct ast_sdp_state *sdp_state,
 	struct sdp_state_capabilities *new_capabilities)
 {
@@ -811,14 +1004,23 @@
 
 	for (i = 0; i < AST_VECTOR_SIZE(&joint_capabilities->streams); ++i) {
 		struct sdp_state_stream *state_stream;
-		enum ast_media_type stream_type;
-
-		stream_type = ast_stream_get_type(ast_stream_topology_get_stream(joint_capabilities->topology, i));
 
 		state_stream = AST_VECTOR_GET(&joint_capabilities->streams, i);
-		if ((stream_type == AST_MEDIA_TYPE_AUDIO || stream_type == AST_MEDIA_TYPE_VIDEO) && state_stream->instance) {
+
+		switch (ast_stream_get_type(ast_stream_topology_get_stream(joint_capabilities->topology, i))) {
+		case AST_MEDIA_TYPE_AUDIO:
+		case AST_MEDIA_TYPE_VIDEO:
 			update_rtp_after_merge(sdp_state, state_stream->instance, sdp_state->options,
 				remote_sdp, ast_sdp_get_m(remote_sdp, i));
+			break;
+		case AST_MEDIA_TYPE_IMAGE:
+			update_udptl_after_merge(sdp_state, state_stream->udptl, sdp_state->options,
+				remote_sdp, ast_sdp_get_m(remote_sdp, i));
+			break;
+		case AST_MEDIA_TYPE_UNKNOWN:
+		case AST_MEDIA_TYPE_TEXT:
+		case AST_MEDIA_TYPE_END:
+			break;
 		}
 	}
 
@@ -966,6 +1168,20 @@
 	return stream_state->locally_held;
 }
 
+void ast_sdp_state_set_t38_parameters(struct ast_sdp_state *sdp_state,
+	int stream_index, struct ast_control_t38_parameters *params)
+{
+	struct sdp_state_stream *stream_state;
+	ast_assert(sdp_state != NULL && params != NULL);
+
+	stream_state = sdp_state_get_stream(sdp_state, stream_index);
+	if (!stream_state) {
+		return;
+	}
+
+	stream_state->t38_local_params = *params;
+}
+
 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)
 {
@@ -1104,6 +1320,167 @@
 	return 0;
 }
 
+/*! \brief Get Max T.38 Transmission rate from T38 capabilities */
+static unsigned int t38_get_rate(enum ast_control_t38_rate rate)
+{
+	switch (rate) {
+	case AST_T38_RATE_2400:
+		return 2400;
+	case AST_T38_RATE_4800:
+		return 4800;
+	case AST_T38_RATE_7200:
+		return 7200;
+	case AST_T38_RATE_9600:
+		return 9600;
+	case AST_T38_RATE_12000:
+		return 12000;
+	case AST_T38_RATE_14400:
+		return 14400;
+	default:
+		return 0;
+	}
+}
+
+static int sdp_add_m_from_udptl_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)
+{
+	struct ast_stream *stream;
+	struct ast_sdp_m_line *m_line;
+	struct ast_sdp_payload *payload;
+	char tmp[64];
+	struct ast_sockaddr address_udptl;
+	struct sdp_state_udptl *udptl;
+	struct ast_sdp_a_line *a_line;
+	struct sdp_state_stream *stream_state;
+
+	stream = ast_stream_topology_get_stream(capabilities->topology, stream_index);
+	udptl = AST_VECTOR_GET(&capabilities->streams, stream_index)->udptl;
+
+	ast_assert(sdp && options && stream);
+
+	if (udptl) {
+		if (ast_sdp_state_get_stream_connection_address(sdp_state, 0, &address_udptl)) {
+			return -1;
+		}
+	} else {
+		ast_sockaddr_setnull(&address_udptl);
+	}
+
+	m_line = ast_sdp_m_alloc(
+		ast_codec_media_type2str(ast_stream_get_type(stream)),
+		ast_sockaddr_port(&address_udptl), 1, "udptl", NULL);
+	if (!m_line) {
+		return -1;
+	}
+
+	payload = ast_sdp_payload_alloc("t38");
+	if (!payload || ast_sdp_m_add_payload(m_line, payload)) {
+		ast_sdp_payload_free(payload);
+		ast_sdp_m_free(m_line);
+		return -1;
+	}
+
+	stream_state = sdp_state_get_stream(sdp_state, stream_index);
+
+	snprintf(tmp, sizeof(tmp), "%u", stream_state->t38_local_params.version);
+	a_line = ast_sdp_a_alloc("T38FaxVersion", 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), "%u", t38_get_rate(stream_state->t38_local_params.rate));
+	a_line = ast_sdp_a_alloc("T38FaxMaxBitRate", 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;
+	}
+
+	if (stream_state->t38_local_params.fill_bit_removal) {
+		a_line = ast_sdp_a_alloc("T38FaxFillBitRemoval", "");
+		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;
+		}
+	}
+
+	if (stream_state->t38_local_params.transcoding_mmr) {
+		a_line = ast_sdp_a_alloc("T38FaxTranscodingMMR", "");
+		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;
+		}
+	}
+
+	if (stream_state->t38_local_params.transcoding_jbig) {
+		a_line = ast_sdp_a_alloc("T38FaxTranscodingJBIG", "");
+		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;
+		}
+	}
+
+	switch (stream_state->t38_local_params.rate_management) {
+	case AST_T38_RATE_MANAGEMENT_TRANSFERRED_TCF:
+		a_line = ast_sdp_a_alloc("T38FaxRateManagement", "transferredTCF");
+		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;
+		}
+		break;
+	case AST_T38_RATE_MANAGEMENT_LOCAL_TCF:
+		a_line = ast_sdp_a_alloc("T38FaxRateManagement", "localTCF");
+		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;
+		}
+		break;
+	}
+
+	snprintf(tmp, sizeof(tmp), "%u", ast_udptl_get_local_max_datagram(udptl->instance));
+	a_line = ast_sdp_a_alloc("T38FaxMaxDatagram", 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;
+	}
+
+	switch (ast_udptl_get_error_correction_scheme(udptl->instance)) {
+	case UDPTL_ERROR_CORRECTION_NONE:
+		break;
+	case UDPTL_ERROR_CORRECTION_FEC:
+		a_line = ast_sdp_a_alloc("T38FaxUdpEC", "t38UDPFEC");
+		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;
+		}
+		break;
+	case UDPTL_ERROR_CORRECTION_REDUNDANCY:
+		a_line = ast_sdp_a_alloc("T38FaxUdpEC", "t38UDPRedundancy");
+		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;
+		}
+		break;
+	}
+
+	if (ast_sdp_add_m(sdp, m_line)) {
+		ast_sdp_m_free(m_line);
+		return -1;
+	}
+
+	return 0;
+}
+
 /*!
  * \brief Create an SDP based on current SDP state
  *
@@ -1155,12 +1532,22 @@
 	stream_count = ast_stream_topology_get_count(topology);
 
 	for (stream_num = 0; stream_num < stream_count; stream_num++) {
-		enum ast_media_type type = ast_stream_get_type(ast_stream_topology_get_stream(topology, stream_num));
-
-		if (type == AST_MEDIA_TYPE_AUDIO || type == AST_MEDIA_TYPE_VIDEO) {
+		switch (ast_stream_get_type(ast_stream_topology_get_stream(topology, stream_num))) {
+		case AST_MEDIA_TYPE_AUDIO:
+		case AST_MEDIA_TYPE_VIDEO:
 			if (sdp_add_m_from_rtp_stream(sdp, sdp_state, options, capabilities, stream_num)) {
 				goto error;
 			}
+			break;
+		case AST_MEDIA_TYPE_IMAGE:
+			if (sdp_add_m_from_udptl_stream(sdp, sdp_state, options, capabilities, stream_num)) {
+				goto error;
+			}
+			break;
+		case AST_MEDIA_TYPE_UNKNOWN:
+		case AST_MEDIA_TYPE_TEXT:
+		case AST_MEDIA_TYPE_END:
+			break;
 		}
 	}
 
diff --git a/tests/test_sdp.c b/tests/test_sdp.c
index 38968aa..33a6a28 100644
--- a/tests/test_sdp.c
+++ b/tests/test_sdp.c
@@ -130,6 +130,22 @@
 	return -1;
 }
 
+static enum ast_test_result_state validate_t38(struct ast_test *test, const struct ast_sdp_m_line *m_line)
+{
+	struct ast_sdp_a_line *a_line;
+
+	a_line = ast_sdp_m_find_attribute(m_line, "T38FaxVersion", -1);
+	ast_test_validate(test, a_line && !strcmp(a_line->value, "0"));
+
+	a_line = ast_sdp_m_find_attribute(m_line, "T38FaxMaxBitRate", -1);
+	ast_test_validate(test, a_line && !strcmp(a_line->value, "14400"));
+
+	a_line = ast_sdp_m_find_attribute(m_line, "T38FaxRateManagement", -1);
+	ast_test_validate(test, a_line && !strcmp(a_line->value, "transferredTCF"));
+
+	return AST_TEST_PASS;
+}
+
 AST_TEST_DEFINE(invalid_rtpmap)
 {
 	/* a=rtpmap: is already assumed. This is the part after that */
@@ -405,6 +421,7 @@
 	struct sdp_format formats[] = {
 		{ AST_MEDIA_TYPE_AUDIO, "ulaw,alaw,g722,opus" },
 		{ AST_MEDIA_TYPE_VIDEO, "h264,vp8" },
+		{ AST_MEDIA_TYPE_IMAGE, "t38" },
 	};
 
 	switch(cmd) {
@@ -437,7 +454,7 @@
 		goto end;
 	}
 
-	if (ast_sdp_get_m_count(sdp) != 2) {
+	if (ast_sdp_get_m_count(sdp) != 3) {
 		ast_test_status_update(test, "Unexpected number of streams in generated SDP: %d\n",
 			ast_sdp_get_m_count(sdp));
 		goto end;
@@ -475,6 +492,14 @@
 	}
 
 	if (validate_rtpmap(test, m_line, "H264")) {
+		goto end;
+	}
+
+	m_line = ast_sdp_get_m(sdp, 2);
+	if (validate_m_line(test, m_line, "image", 1)) {
+		goto end;
+	}
+	if (validate_t38(test, m_line) != AST_TEST_PASS) {
 		goto end;
 	}
 
@@ -527,6 +552,7 @@
 	struct sdp_format sdp_formats[] = {
 		{ AST_MEDIA_TYPE_AUDIO, "ulaw,alaw,g722,opus" },
 		{ AST_MEDIA_TYPE_VIDEO, "h264,vp8" },
+		{ AST_MEDIA_TYPE_IMAGE, "t38" },
 	};
 	static const char *expected_audio_formats[] = {
 		"ulaw",
@@ -537,6 +563,9 @@
 	static const char *expected_video_formats[] = {
 		"h264",
 		"vp8",
+	};
+	static const char *expected_image_formats[] = {
+		"t38",
 	};
 
 	switch(cmd) {
@@ -565,7 +594,7 @@
 
 	topology = ast_get_topology_from_sdp(sdp);
 
-	if (ast_stream_topology_get_count(topology) != 2) {
+	if (ast_stream_topology_get_count(topology) != 3) {
 		ast_test_status_update(test, "Unexpected topology count '%d'. Expecting 2\n",
 			ast_stream_topology_get_count(topology));
 		res = AST_TEST_FAIL;
@@ -584,6 +613,12 @@
 		goto end;
 	}
 
+	if (validate_formats(test, topology, 2, AST_MEDIA_TYPE_IMAGE,
+			ARRAY_LEN(expected_image_formats), expected_image_formats)) {
+		res = AST_TEST_FAIL;
+		goto end;
+	}
+
 end:
 	ast_sdp_state_free(sdp_state);
 	ast_stream_topology_free(topology);

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

Gerrit-MessageType: merged
Gerrit-Change-Id: If28956762ccb8ead562ac6c03d162d3d6014f2c7
Gerrit-PatchSet: 4
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Joshua Colp <jcolp 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