[asterisk-commits] mjordan: branch group/media_formats-reviewed-trunk r418248 - in /team/group/m...

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Wed Jul 9 12:51:47 CDT 2014


Author: mjordan
Date: Wed Jul  9 12:51:43 2014
New Revision: 418248

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=418248
Log:
media format attributes: Clean up modules, make pass through work in chan_sip

Josh did the bulk of this work on r3703 (thanks!). This patch includes that
review, as well as the work done in chan_sip in r3722 to make attributes
pass through. This included some additional changes to format_cap.

The major changes to format_cap are:
* format_cap now has a method to get the preferred codec out of a capabilities
  structure by some type. This prevents the situation where the
  'preferred codec' is not an audio codec (which can happen when format
   capabilities structures are copied, moved around, and perturbed).
* format_cap now explicitly prevents duplicate formats from being appended. A
  format is considered a duplicate if it has the same codec ID. This is a
  'strict' interpretation of equivalence, as it means that a format that
  contains an attribute cannot coexist with another format of the same type.
  This was needed, however, to prevent capabilities structures from having
  duplicate video formats (some with attributes, some without), which the
  channel drivers cannot yet understand or process (and which is not strictly
  needed right now).

The chan_sip channel driver has been tweaked up a bit to make sure that when a
capability is built out from an SDP, that capability (which is the jointcaps
structure) is given preference when constructing an outgoing SDP. That helps
ensure that adding a format to an outbound SDP gets the appropriate format w/
attributes, as opposed to the peer formats which would not have any attributes.

Note that since the jointcaps capability is guaranteed to be a subset of the
peer's capabilities, this ends up being generally the same.

Review: https://reviewboard.asterisk.org/r/3722/
Review: https://reviewboard.asterisk.org/r/3703/

ASTERISK-23957 #close

Added:
    team/group/media_formats-reviewed-trunk/UPGRADE-MF.txt   (with props)
Modified:
    team/group/media_formats-reviewed-trunk/channels/chan_sip.c
    team/group/media_formats-reviewed-trunk/include/asterisk/format.h
    team/group/media_formats-reviewed-trunk/include/asterisk/format_cap.h
    team/group/media_formats-reviewed-trunk/include/asterisk/rtp_engine.h
    team/group/media_formats-reviewed-trunk/main/format.c
    team/group/media_formats-reviewed-trunk/main/format_cap.c
    team/group/media_formats-reviewed-trunk/main/rtp_engine.c
    team/group/media_formats-reviewed-trunk/res/res_format_attr_celt.c
    team/group/media_formats-reviewed-trunk/res/res_format_attr_h263.c
    team/group/media_formats-reviewed-trunk/res/res_format_attr_h264.c
    team/group/media_formats-reviewed-trunk/res/res_format_attr_opus.c
    team/group/media_formats-reviewed-trunk/res/res_format_attr_silk.c
    team/group/media_formats-reviewed-trunk/res/res_pjsip_sdp_rtp.c

Added: team/group/media_formats-reviewed-trunk/UPGRADE-MF.txt
URL: http://svnview.digium.com/svn/asterisk/team/group/media_formats-reviewed-trunk/UPGRADE-MF.txt?view=auto&rev=418248
==============================================================================
--- team/group/media_formats-reviewed-trunk/UPGRADE-MF.txt (added)
+++ team/group/media_formats-reviewed-trunk/UPGRADE-MF.txt Wed Jul  9 12:51:43 2014
@@ -1,0 +1,16 @@
+Notes on what external behavior has changed due to the media format work
+
+* chan_sip
+  - Offers will be slightly different. The preference order of codecs used
+    to be:
+    (1) Our preferred codec
+    (2) Our configured codecs
+    (3) Any non-audio joint codecs
+
+    Since a media format with an attribute is a different object than a cacahed
+    media format on a peer (even if they are the "same" format), we can't quite
+    do that - we want to offer the joint format versions, not the ones on the
+    peer. This means our offer list will now be:
+    (1) Our preferred codec
+    (2) Any joint codecs
+    (3) All other codecs that are not preferred and not joint

Propchange: team/group/media_formats-reviewed-trunk/UPGRADE-MF.txt
------------------------------------------------------------------------------
    svn:eol-style = native

Propchange: team/group/media_formats-reviewed-trunk/UPGRADE-MF.txt
------------------------------------------------------------------------------
    svn:keywords = Author Date Id Revision

Propchange: team/group/media_formats-reviewed-trunk/UPGRADE-MF.txt
------------------------------------------------------------------------------
    svn:mime-type = text/plain

Modified: team/group/media_formats-reviewed-trunk/channels/chan_sip.c
URL: http://svnview.digium.com/svn/asterisk/team/group/media_formats-reviewed-trunk/channels/chan_sip.c?view=diff&rev=418248&r1=418247&r2=418248
==============================================================================
--- team/group/media_formats-reviewed-trunk/channels/chan_sip.c (original)
+++ team/group/media_formats-reviewed-trunk/channels/chan_sip.c Wed Jul  9 12:51:43 2014
@@ -7352,6 +7352,7 @@
 				first_codec = 0;
 			} else {
 				ast_verb(4, "Add codec to '%s' for this call because of ${SIP_CODEC*} variable\n", codec);
+				/* Add the format to the capabilities structure */
 				ast_format_cap_append(p->jointcaps, fmt, 0);
 				ast_format_cap_append(p->caps, fmt, 0);
 			}
@@ -8098,9 +8099,12 @@
 	/* Set the native formats */
 	ast_format_cap_append_from_cap(caps, what, AST_MEDIA_TYPE_UNKNOWN);
 	/* Use only the preferred audio format, which is stored at the '0' index */
-	fmt = ast_format_cap_get_format(what, 0); /* get the best audio format */
-	ast_format_cap_remove_by_type(caps, AST_MEDIA_TYPE_AUDIO); /* remove only the other audio formats */
-	ast_format_cap_append(caps, fmt, 0); /* add our best choice back */
+	fmt = ast_format_cap_get_best_by_type(what, AST_MEDIA_TYPE_AUDIO); /* get the best audio format */
+	if (fmt) {
+		ast_format_cap_remove_by_type(caps, AST_MEDIA_TYPE_AUDIO); /* remove only the other audio formats */
+		ast_format_cap_append(caps, fmt, 0); /* add our best choice back */
+		ao2_ref(fmt, -1);
+	}
 	ast_channel_nativeformats_set(tmp, caps);
 	ao2_ref(caps, -1);
 
@@ -11270,8 +11274,13 @@
 
 		if ((format = ast_rtp_codecs_get_payload_format(newaudiortp, codec))) {
 			unsigned int bit_rate;
-
-			if (!ast_format_parse_sdp_fmtp(format, fmtp_string)) {
+			struct ast_format *format_parsed;
+
+			format_parsed = ast_format_parse_sdp_fmtp(format, fmtp_string);
+			if (format_parsed) {
+				ast_rtp_codecs_payload_replace_format(newaudiortp, codec, format_parsed);
+				ao2_replace(format, format_parsed);
+				ao2_ref(format_parsed, -1);
 				found = TRUE;
 			} else {
 				ast_rtp_codecs_payloads_unset(newaudiortp, NULL, codec);
@@ -11347,7 +11356,14 @@
 		struct ast_format *format;
 
 		if ((format = ast_rtp_codecs_get_payload_format(newvideortp, codec))) {
-			if (!ast_format_parse_sdp_fmtp(format, fmtp_string)) {
+			struct ast_format *format_parsed;
+
+			format_parsed = ast_format_parse_sdp_fmtp(format, fmtp_string);
+
+			if (format_parsed) {
+				ast_rtp_codecs_payload_replace_format(newvideortp, codec, format_parsed);
+				ao2_replace(format, format_parsed);
+				ao2_ref(format_parsed, -1);
 				found = TRUE;
 			} else {
 				ast_rtp_codecs_payloads_unset(newvideortp, NULL, codec);
@@ -13285,9 +13301,22 @@
 
 	if (add_audio) {
 		doing_directmedia = (!ast_sockaddr_isnull(&p->redirip) && (ast_format_cap_count(p->redircaps))) ? TRUE : FALSE;
+
+		if (doing_directmedia) {
+			ast_format_cap_get_compatible(p->jointcaps, p->redircaps, tmpcap);
+			ast_debug(1, "** Our native-bridge filtered capablity: %s\n", ast_format_cap_get_names(tmpcap, &codec_buf));
+		} else {
+			ast_format_cap_append_from_cap(tmpcap, p->jointcaps, AST_MEDIA_TYPE_UNKNOWN);
+		}
+
+		/* Check if we need audio */
+		if (ast_format_cap_has_type(tmpcap, AST_MEDIA_TYPE_AUDIO)
+			|| ast_format_cap_has_type(p->caps, AST_MEDIA_TYPE_AUDIO)) {
+			needaudio = TRUE;
+		}
+
 		/* Check if we need video in this call */
-		if ((ast_format_cap_has_type(p->jointcaps, AST_MEDIA_TYPE_VIDEO)) && !p->novideo) {
-			ast_format_cap_get_compatible(p->jointcaps, p->redircaps, tmpcap);
+		if ((ast_format_cap_has_type(tmpcap, AST_MEDIA_TYPE_VIDEO)) && !p->novideo) {
 			if (doing_directmedia && !ast_format_cap_has_type(tmpcap, AST_MEDIA_TYPE_VIDEO)) {
 				ast_debug(2, "This call needs video offers, but caller probably did not offer it!\n");
 			} else if (p->vrtp) {
@@ -13297,6 +13326,7 @@
 				ast_debug(2, "This call needs video offers, but there's no video support enabled!\n");
 			}
 		}
+
 		/* Check if we need text in this call */
 		if ((ast_format_cap_has_type(p->jointcaps, AST_MEDIA_TYPE_TEXT)) && !p->notext) {
 			if (sipdebug_text)
@@ -13311,6 +13341,12 @@
 				ast_debug(2, "This call needs text offers, but there's no text support enabled ! \n");
 			}
 		}
+
+		/* XXX note, Video and Text are negated - 'true' means 'no' */
+		ast_debug(1, "** Our capability: %s Video flag: %s Text flag: %s\n",
+			  ast_format_cap_get_names(tmpcap, &codec_buf),
+			  p->novideo ? "True" : "False", p->notext ? "True" : "False");
+		ast_debug(1, "** Our prefcodec: %s \n", ast_format_cap_get_names(p->prefcaps, &codec_buf));
 	}
 
 	get_our_media_address(p, needvideo, needtext, &addr, &vaddr, &taddr, &dest, &vdest, &tdest);
@@ -13337,23 +13373,6 @@
 		} else {
 			hold = "a=sendrecv\r\n";
 		}
-
-		ast_format_cap_append_from_cap(tmpcap, p->jointcaps, AST_MEDIA_TYPE_UNKNOWN);
-
-		/* XXX note, Video and Text are negated - 'true' means 'no' */
-		ast_debug(1, "** Our capability: %s Video flag: %s Text flag: %s\n",
-			  ast_format_cap_get_names(tmpcap, &codec_buf),
-			  p->novideo ? "True" : "False", p->notext ? "True" : "False");
-		ast_debug(1, "** Our prefcodec: %s \n", ast_format_cap_get_names(p->prefcaps, &codec_buf));
-
-		if (doing_directmedia) {
-			ast_format_cap_get_compatible(p->jointcaps, p->redircaps, tmpcap);
-			ast_debug(1, "** Our native-bridge filtered capablity: %s\n", ast_format_cap_get_names(tmpcap, &codec_buf));
-		}
-
-		/* Check if we need audio */
-		if (ast_format_cap_has_type(tmpcap, AST_MEDIA_TYPE_AUDIO))
-			needaudio = TRUE;
 
 		if (debug) {
 			ast_verbose("Audio is at %s\n", ast_sockaddr_stringify_port(&addr));
@@ -13423,8 +13442,8 @@
 
 		/* Now, start adding audio codecs. These are added in this order:
 		   - First what was requested by the calling channel
+		   - Then our mutually shared capabilities, determined previous in tmpcap
 		   - Then preferences in order from sip.conf device config for this peer/user
-		   - Then other codecs in capabilities, including video
 		*/
 
 
@@ -13447,9 +13466,9 @@
 			}
 		}
 
-		/* Start by sending our preferred audio/video codecs */
-		for (x = 0; x < ast_format_cap_count(p->caps); x++) {
-			tmp_fmt = ast_format_cap_get_format(p->caps, x);
+		/* Now send any other common codecs */
+		for (x = 0; x < ast_format_cap_count(tmpcap); x++) {
+			tmp_fmt = ast_format_cap_get_format(tmpcap, x);
 
 			if (ast_format_cap_iscompatible_format(alreadysent, tmp_fmt) != AST_FORMAT_CMP_NOT_EQUAL) {
 				ao2_ref(tmp_fmt, -1);
@@ -13464,13 +13483,12 @@
 				add_tcodec_to_sdp(p, tmp_fmt, &m_text, &a_text, debug, &min_text_packet_size);
 			}
 
-			ast_format_cap_append(alreadysent, tmp_fmt, 0);
 			ao2_ref(tmp_fmt, -1);
 		}
 
-		/* Now send any other common audio and video codecs, and non-codec formats: */
-		for (x = 0; x < ast_format_cap_count(tmpcap); x++) {
-			tmp_fmt = ast_format_cap_get_format(tmpcap, x);
+		/* Finally our remaining audio/video codecs */
+		for (x = 0; x < ast_format_cap_count(p->caps); x++) {
+			tmp_fmt = ast_format_cap_get_format(p->caps, x);
 
 			if (ast_format_cap_iscompatible_format(alreadysent, tmp_fmt) != AST_FORMAT_CMP_NOT_EQUAL) {
 				ao2_ref(tmp_fmt, -1);
@@ -13485,6 +13503,7 @@
 				add_tcodec_to_sdp(p, tmp_fmt, &m_text, &a_text, debug, &min_text_packet_size);
 			}
 
+			ast_format_cap_append(alreadysent, tmp_fmt, 0);
 			ao2_ref(tmp_fmt, -1);
 		}
 

Modified: team/group/media_formats-reviewed-trunk/include/asterisk/format.h
URL: http://svnview.digium.com/svn/asterisk/team/group/media_formats-reviewed-trunk/include/asterisk/format.h?view=diff&rev=418248&r1=418247&r2=418248
==============================================================================
--- team/group/media_formats-reviewed-trunk/include/asterisk/format.h (original)
+++ team/group/media_formats-reviewed-trunk/include/asterisk/format.h Wed Jul  9 12:51:43 2014
@@ -48,6 +48,17 @@
 	 */
 	void (*const format_destroy)(struct ast_format *format);
 
+	/**
+	 * \brief Callback for when the format is cloned, used to clone attributes
+	 *
+	 * \param src Source format of attributes
+	 * \param dst Destination format for attributes
+	 *
+	 * \retval 0 success
+	 * \retval -1 failure
+	 */
+	int (*const format_clone)(const struct ast_format *src, struct ast_format *dst);
+
 	/*!
 	 * \brief Determine if format 1 is a subset of format 2.
 	 *
@@ -80,10 +91,11 @@
 	 * \param name The name of the attribute
 	 * \param value The value of the attribute
 	 *
-	 * \retval 0 success
-	 * \retval -1 failure
-	 */
-	int (* const format_attribute_set)(struct ast_format *format, const char *name, const char *value);
+	 * \retval non-NULL success
+	 * \retval NULL failure
+	 */
+	struct ast_format *(* const format_attribute_set)(const struct ast_format *format,
+		const char *name, const char *value);
 
 	/*
 	 * \brief Parse SDP attribute information, interpret it, and store it in the format structure.
@@ -91,10 +103,10 @@
 	 * \param format Format to set attributes on
 	 * \param attributes A string containing only the attributes from the fmtp line
 	 *
-	 * \retval 0 Success, values were valid
-	 * \retval -1 Failure, some values were not acceptable
-	 */
-	int (* const format_parse_sdp_fmtp)(struct ast_format *format, const char *attributes);
+	 * \retval non-NULL Success, values were valid
+	 * \retval NULL Failure, some values were not acceptable
+	 */
+	struct ast_format *(* const format_parse_sdp_fmtp)(const struct ast_format *format, const char *attributes);
 
 	/*!
 	 * \brief Generate SDP attribute information from an ast_format_attr structure.
@@ -149,6 +161,18 @@
 struct ast_format *ast_format_create_named(const char *format_name, struct ast_codec *codec);
 
 /*!
+ * \brief Clone an existing media format so it can be modified
+ *
+ * \param format The existing media format
+ *
+ * \note The returned format is a new ao2 object. It must be released using ao2_cleanup.
+ *
+ * \retval non-NULL success
+ * \retval NULL failure
+ */
+struct ast_format *ast_format_clone(const struct ast_format *format);
+
+/*!
  * \brief Compare two formats
  *
  * \retval ast_format_cmp_res representing the result of comparing format1 and format2.
@@ -172,10 +196,11 @@
  * \param name Attribute name
  * \param value Attribute value
  *
- * \retval 0 success
- * \retval -1 failure
- */
-int ast_format_attribute_set(struct ast_format *format, const char *name, const char *value);
+ * \retval non-NULL success
+ * \retval NULL failure
+ */
+struct ast_format *ast_format_attribute_set(const struct ast_format *format, const char *name,
+	const char *value);
 
 /*!
  * \brief This function is used to have a media format aware module parse and interpret
@@ -185,10 +210,10 @@
  * \param format to set
  * \param attributes string containing the fmtp line from the SDP
  *
- * \retval 0 success, attribute values were valid
- * \retval -1 failure, values were not acceptable
- */
-int ast_format_parse_sdp_fmtp(struct ast_format *format, const char *attributes);
+ * \retval non-NULL success, attribute values were valid
+ * \retval NULL failure, values were not acceptable
+ */
+struct ast_format *ast_format_parse_sdp_fmtp(const struct ast_format *format, const char *attributes);
 
 /*!
  * \brief This function is used to produce an fmtp SDP line for an Asterisk format. The
@@ -210,7 +235,7 @@
  * \retval 0 success
  * \retval -1 failure
  */
-int __ast_format_interface_register(const char *codec, struct ast_format_interface *interface, struct ast_module *mod);
+int __ast_format_interface_register(const char *codec, const struct ast_format_interface *interface, struct ast_module *mod);
 
 /*!
  * \brief Register a format interface for use with the provided codec
@@ -224,6 +249,23 @@
 #define ast_format_interface_register(codec, interface) __ast_format_interface_register(codec, interface, ast_module_info->self)
 
 /*!
+ * \brief Get the attribute data on a format
+ *
+ * \param format The media format
+ *
+ * \return Currently set attribute data
+ */
+void *ast_format_get_attribute_data(const struct ast_format *format);
+
+/*!
+ * \brief Set the attribute data on a format
+ *
+ * \param format The media format
+ * \param attribute_data The attribute data
+ */
+void ast_format_set_attribute_data(struct ast_format *format, void *attribute_data);
+
+/*!
  * \brief Get the name associated with a format
  *
  * \param format The media format

Modified: team/group/media_formats-reviewed-trunk/include/asterisk/format_cap.h
URL: http://svnview.digium.com/svn/asterisk/team/group/media_formats-reviewed-trunk/include/asterisk/format_cap.h?view=diff&rev=418248&r1=418247&r2=418248
==============================================================================
--- team/group/media_formats-reviewed-trunk/include/asterisk/format_cap.h (original)
+++ team/group/media_formats-reviewed-trunk/include/asterisk/format_cap.h Wed Jul  9 12:51:43 2014
@@ -189,6 +189,20 @@
 struct ast_format *ast_format_cap_get_format(const struct ast_format_cap *cap, int position);
 
 /*!
+ * \brief Get the most preferred format for a particular media type
+ *
+ * \param cap The capabilities structure
+ * \param type The type of media to get
+ *
+ * \retval non-NULL the preferred format
+ * \retval NULL no media of \c type present
+ *
+ * \note The reference count of the returned format is increased. It must be released using ao2_ref
+ * or ao2_cleanup.
+ */
+struct ast_format *ast_format_cap_get_best_by_type(const struct ast_format_cap *cap, enum ast_media_type type);
+
+/*!
  * \brief Get the framing for a format
  *
  * \param cap The capabilities structure

Modified: team/group/media_formats-reviewed-trunk/include/asterisk/rtp_engine.h
URL: http://svnview.digium.com/svn/asterisk/team/group/media_formats-reviewed-trunk/include/asterisk/rtp_engine.h?view=diff&rev=418248&r1=418247&r2=418248
==============================================================================
--- team/group/media_formats-reviewed-trunk/include/asterisk/rtp_engine.h (original)
+++ team/group/media_formats-reviewed-trunk/include/asterisk/rtp_engine.h Wed Jul  9 12:51:43 2014
@@ -1,4 +1,4 @@
-/*
+ /*
  * Asterisk -- An open source telephony toolkit.
  *
  * Copyright (C) 1999 - 2009, Digium, Inc.
@@ -1265,6 +1265,20 @@
 struct ast_rtp_payload_type *ast_rtp_codecs_get_payload(struct ast_rtp_codecs *codecs, int payload);
 
 /*!
+ * \brief Update the format associated with a payload in a codecs structure
+ *
+ * \param codecs Codecs structure to operate on
+ * \param payload Numerical payload to look up
+ * \param format The format to replace the existing one
+ *
+ * \retval 0 success
+ * \retval -1 failure
+ *
+ * \since 13
+ */
+int ast_rtp_codecs_payload_replace_format(struct ast_rtp_codecs *codecs, int payload, struct ast_format *format);
+
+/*!
  * \brief Retrieve the actual ast_format stored on the codecs structure for a specific payload
  *
  * \param codecs Codecs structure to look in

Modified: team/group/media_formats-reviewed-trunk/main/format.c
URL: http://svnview.digium.com/svn/asterisk/team/group/media_formats-reviewed-trunk/main/format.c?view=diff&rev=418248&r1=418247&r2=418248
==============================================================================
--- team/group/media_formats-reviewed-trunk/main/format.c (original)
+++ team/group/media_formats-reviewed-trunk/main/format.c Wed Jul  9 12:51:43 2014
@@ -49,13 +49,13 @@
 	/*! \brief Attribute specific data, implementation specific */
 	void *attribute_data;
 	/*! \brief Pointer to the optional format interface */
-	struct ast_format_interface *interface;
+	const struct ast_format_interface *interface;
 };
 
 /*! \brief Structure used when registering a format interface */
 struct format_interface {
 	/*! \brief Pointer to the format interface itself */
-	struct ast_format_interface *interface;
+	const struct ast_format_interface *interface;
 	/*! \brief Name of the codec the interface is for */
 	char codec[0];
 };
@@ -122,38 +122,46 @@
 	return 0;
 }
 
-/*! \brief Destructor for format interface */
-static void format_interface_destroy(void *obj)
-{
-	struct format_interface *format_interface = obj;
-
-	ao2_cleanup(format_interface->interface);
-}
-
-int __ast_format_interface_register(const char *codec, struct ast_format_interface *interface, struct ast_module *mod)
+int __ast_format_interface_register(const char *codec, const struct ast_format_interface *interface, struct ast_module *mod)
 {
 	SCOPED_AO2WRLOCK(lock, interfaces);
 	struct format_interface *format_interface;
 
-	format_interface = ao2_find(interfaces, codec, OBJ_SEARCH_KEY);
+	if (!interface->format_clone || !interface->format_destroy) {
+		ast_log(LOG_ERROR, "Format interface for codec '%s' does not implement required callbacks\n", codec);
+		return -1;
+	}
+
+	format_interface = ao2_find(interfaces, codec, OBJ_SEARCH_KEY | OBJ_NOLOCK);
 	if (format_interface) {
 		ast_log(LOG_ERROR, "A format interface is already present for codec '%s'\n", codec);
 		ao2_ref(format_interface, -1);
 		return -1;
 	}
 
-	format_interface = ao2_alloc(sizeof(*format_interface) + strlen(codec) + 1, format_interface_destroy);
+	format_interface = ao2_alloc(sizeof(*format_interface) + strlen(codec) + 1, NULL);
 	if (!format_interface) {
 		return -1;
 	}
 
 	format_interface->interface = interface;
-	ao2_link_flags(interfaces, interface, OBJ_NOLOCK);
+	strcpy(format_interface->codec, codec);
+	ao2_link_flags(interfaces, format_interface, OBJ_NOLOCK);
 	ao2_ref(format_interface, -1);
 
 	ast_verb(2, "Registered format interface for codec '%s'\n", codec);
 
 	return 0;
+}
+
+void *ast_format_get_attribute_data(const struct ast_format *format)
+{
+	return format->attribute_data;
+}
+
+void ast_format_set_attribute_data(struct ast_format *format, void *attribute_data)
+{
+	format->attribute_data = attribute_data;
 }
 
 /*! \brief Destructor for media formats */
@@ -163,7 +171,6 @@
 
 	if (format->interface) {
 		format->interface->format_destroy(format);
-		ao2_ref(format->interface, -1);
 	}
 
 	ao2_cleanup(format->codec);
@@ -183,13 +190,29 @@
 
 	format_interface = ao2_find(interfaces, codec->name, OBJ_SEARCH_KEY);
 	if (format_interface) {
-		format->interface = ao2_bump(format_interface->interface);
+		format->interface = format_interface->interface;
 		ao2_ref(format_interface, -1);
 	}
 
 	return format;
 }
 
+struct ast_format *ast_format_clone(const struct ast_format *format)
+{
+	struct ast_format *cloned = ast_format_create_named(format->name, format->codec);
+
+	if (!cloned) {
+		return NULL;
+	}
+
+	if (cloned->interface && cloned->interface->format_clone(format, cloned)) {
+		ao2_ref(cloned, -1);
+		return NULL;
+	}
+
+	return cloned;
+}
+
 struct ast_format *ast_format_create(struct ast_codec *codec)
 {
 	return ast_format_create_named(codec->name, codec);
@@ -197,6 +220,8 @@
 
 enum ast_format_cmp_res ast_format_cmp(const struct ast_format *format1, const struct ast_format *format2)
 {
+	const struct ast_format_interface *interface;
+
 	if (format1 == NULL || format2 == NULL) {
 		return AST_FORMAT_CMP_NOT_EQUAL;
 	}
@@ -209,8 +234,10 @@
 		return AST_FORMAT_CMP_NOT_EQUAL;
 	}
 
-	if (format1->interface) {
-		return format1->interface->format_cmp(format1, format2);
+	interface = format1->interface ? format1->interface : format2->interface;
+
+	if (interface) {
+		return interface->format_cmp(format1, format2);
 	}
 
 	return AST_FORMAT_CMP_EQUAL;
@@ -218,6 +245,8 @@
 
 struct ast_format *ast_format_joint(const struct ast_format *format1, const struct ast_format *format2)
 {
+	const struct ast_format_interface *interface;
+
 	if (format1->codec != format2->codec) {
 		return NULL;
 	}
@@ -226,30 +255,52 @@
 	 * exist we can immediately return a format with reference count bumped up, since they are
 	 * the same.
 	 */
-	if ((format1 == format2) || (!format1->attribute_data && !format2->attribute_data)) {
+	if ((ast_format_cmp(format1, format2) == AST_FORMAT_CMP_EQUAL && !format1->attribute_data && !format2->attribute_data)) {
 		return ao2_bump((struct ast_format*)format1);
 	}
 
+	interface = format1->interface ? format1->interface : format2->interface;
+
 	/* If there is attribute data on either there has to be an interface */
-	return format1->interface->format_get_joint(format1, format2);
-}
-
-int ast_format_attribute_set(struct ast_format *format, const char *name, const char *value)
-{
-	if (!format->interface || !format->interface->format_attribute_set) {
-		return -1;
-	}
-
-	return format->interface->format_attribute_set(format, name, value);
-}
-
-int ast_format_parse_sdp_fmtp(struct ast_format *format, const char *attributes)
-{
-	if (!format->interface || !format->interface->format_parse_sdp_fmtp) {
-		return -1;
-	}
-
-	return format->interface->format_parse_sdp_fmtp(format, attributes);
+	return interface->format_get_joint(format1, format2);
+}
+
+struct ast_format *ast_format_attribute_set(const struct ast_format *format, const char *name, const char *value)
+{
+	const struct ast_format_interface *interface = format->interface;
+
+	if (!interface) {
+		struct format_interface *format_interface = ao2_find(interfaces, format->codec->name, OBJ_SEARCH_KEY);
+		if (format_interface) {
+			interface = format_interface->interface;
+			ao2_ref(format_interface, -1);
+		}
+	}
+
+	if (!interface || !interface->format_attribute_set) {
+		return ao2_bump((struct ast_format*)format);
+	}
+
+	return interface->format_attribute_set(format, name, value);
+}
+
+struct ast_format *ast_format_parse_sdp_fmtp(const struct ast_format *format, const char *attributes)
+{
+	const struct ast_format_interface *interface = format->interface;
+
+	if (!interface) {
+		struct format_interface *format_interface = ao2_find(interfaces, format->codec->name, OBJ_SEARCH_KEY);
+		if (format_interface) {
+			interface = format_interface->interface;
+			ao2_ref(format_interface, -1);
+		}
+	}
+
+	if (!interface || !interface->format_parse_sdp_fmtp) {
+		return ao2_bump((struct ast_format*)format);
+	}
+
+	return interface->format_parse_sdp_fmtp(format, attributes);
 }
 
 void ast_format_generate_sdp_fmtp(const struct ast_format *format, unsigned int payload, struct ast_str **str)

Modified: team/group/media_formats-reviewed-trunk/main/format_cap.c
URL: http://svnview.digium.com/svn/asterisk/team/group/media_formats-reviewed-trunk/main/format_cap.c?view=diff&rev=418248&r1=418247&r2=418248
==============================================================================
--- team/group/media_formats-reviewed-trunk/main/format_cap.c (original)
+++ team/group/media_formats-reviewed-trunk/main/format_cap.c Wed Jul  9 12:51:43 2014
@@ -170,15 +170,35 @@
 	return 0;
 }
 
+/*! \internal \brief Determine if \c format is in \c cap */
+static int format_in_format_cap(struct ast_format_cap *cap, struct ast_format *format)
+{
+	struct format_cap_framed *framed;
+	int i;
+
+	for (i = 0; i < AST_VECTOR_SIZE(&cap->preference_order); i++) {
+		framed = AST_VECTOR_GET(&cap->preference_order, i);
+
+		if (ast_format_get_codec_id(format) == ast_format_get_codec_id(framed->format)) {
+			return 1;
+		}
+	}
+
+	return 0;
+}
+
 int __ast_format_cap_append(struct ast_format_cap *cap, struct ast_format *format, unsigned int framing)
 {
 	struct format_cap_framed *framed;
+
+	if (format_in_format_cap(cap, format)) {
+		return 0;
+	}
 
 	framed = ao2_alloc_options(sizeof(*framed), format_cap_framed_destroy, AO2_ALLOC_OPT_LOCK_NOLOCK);
 	if (!framed) {
 		return -1;
 	}
-
 	framed->format = ao2_bump(format);
 
 	return format_cap_framed_init(framed, cap, format, framing);
@@ -187,6 +207,10 @@
 int __ast_format_cap_append_debug(struct ast_format_cap *cap, struct ast_format *format, unsigned int framing, const char *tag, const char *file, int line, const char *func)
 {
 	struct format_cap_framed *framed;
+
+	if (format_in_format_cap(cap, format)) {
+		return 0;
+	}
 
 	framed = ao2_alloc_options(sizeof(*framed), format_cap_framed_destroy, AO2_ALLOC_OPT_LOCK_NOLOCK);
 	if (!framed) {
@@ -327,6 +351,26 @@
 	return framed->format;
 }
 
+struct ast_format *ast_format_cap_get_best_by_type(const struct ast_format_cap *cap, enum ast_media_type type)
+{
+	int i;
+
+	if (type == AST_MEDIA_TYPE_UNKNOWN) {
+		return ast_format_cap_get_format(cap, 0);
+	}
+
+	for (i = 0; i < AST_VECTOR_SIZE(&cap->preference_order); i++) {
+		struct format_cap_framed *framed = AST_VECTOR_GET(&cap->preference_order, i);
+
+		if (ast_format_get_type(framed->format) == type) {
+			ao2_ref(framed->format, +1);
+			return framed->format;
+		}
+	}
+
+	return NULL;
+}
+
 unsigned int ast_format_cap_get_framing(const struct ast_format_cap *cap)
 {
 	return (cap->framing != UINT_MAX) ? cap->framing : 0;

Modified: team/group/media_formats-reviewed-trunk/main/rtp_engine.c
URL: http://svnview.digium.com/svn/asterisk/team/group/media_formats-reviewed-trunk/main/rtp_engine.c?view=diff&rev=418248&r1=418247&r2=418248
==============================================================================
--- team/group/media_formats-reviewed-trunk/main/rtp_engine.c (original)
+++ team/group/media_formats-reviewed-trunk/main/rtp_engine.c Wed Jul  9 12:51:43 2014
@@ -790,6 +790,25 @@
 	return type;
 }
 
+int ast_rtp_codecs_payload_replace_format(struct ast_rtp_codecs *codecs, int payload, struct ast_format *format)
+{
+	struct ast_rtp_payload_type *type;
+
+	if (payload < 0 || payload >= AST_RTP_MAX_PT) {
+		return -1;
+	}
+
+	ast_rwlock_wrlock(&codecs->codecs_lock);
+	if (payload < AST_VECTOR_SIZE(&codecs->payloads)) {
+		type = AST_VECTOR_GET(&codecs->payloads, payload);
+		if (type && type->asterisk_format) {
+			ao2_replace(type->format, format);
+		}
+	}
+	ast_rwlock_unlock(&codecs->codecs_lock);
+
+	return 0;
+}
 
 struct ast_format *ast_rtp_codecs_get_payload_format(struct ast_rtp_codecs *codecs, int payload)
 {

Modified: team/group/media_formats-reviewed-trunk/res/res_format_attr_celt.c
URL: http://svnview.digium.com/svn/asterisk/team/group/media_formats-reviewed-trunk/res/res_format_attr_celt.c?view=diff&rev=418248&r1=418247&r2=418248
==============================================================================
--- team/group/media_formats-reviewed-trunk/res/res_format_attr_celt.c (original)
+++ team/group/media_formats-reviewed-trunk/res/res_format_attr_celt.c Wed Jul  9 12:51:43 2014
@@ -45,152 +45,148 @@
 	unsigned int framesize;
 };
 
-static int celt_sdp_parse(struct ast_format_attr *format_attr, const char *attributes)
-{
-	struct celt_attr *attr = (struct celt_attr *) format_attr;
+static void celt_destroy(struct ast_format *format)
+{
+	struct celt_attr *attr = ast_format_get_attribute_data(format);
+
+	ast_free(attr);
+}
+
+static int celt_clone(const struct ast_format *src, struct ast_format *dst)
+{
+	struct celt_attr *original = ast_format_get_attribute_data(src);
+	struct celt_attr *attr = ast_calloc(1, sizeof(*attr));
+
+	if (!attr) {
+		return -1;
+	}
+
+	if (original) {
+		*attr = *original;
+	}
+
+	ast_format_set_attribute_data(dst, attr);
+
+	return 0;
+}
+
+static struct ast_format *celt_parse_sdp_fmtp(const struct ast_format *format, const char *attributes)
+{
+	struct ast_format *cloned;
+	struct celt_attr *attr;
 	unsigned int val;
+
+	cloned = ast_format_clone(format);
+	if (!cloned) {
+		return NULL;
+	}
+	attr = ast_format_get_attribute_data(cloned);
 
 	if (sscanf(attributes, "framesize=%30u", &val) == 1) {
 		attr->framesize = val;
 	}
 
-	return 0;
-}
-
-static void celt_sdp_generate(const struct ast_format_attr *format_attr, unsigned int payload, struct ast_str **str)
-{
-	struct celt_attr *attr = (struct celt_attr *) format_attr;
-
-	if (!attr->framesize) {
+	return cloned;
+}
+
+static void celt_generate_sdp_fmtp(const struct ast_format *format, unsigned int payload, struct ast_str **str)
+{
+	struct celt_attr *attr = ast_format_get_attribute_data(format);
+
+	if (!attr || !attr->framesize) {
 		return;
 	}
 
-	ast_str_append(str, 0, "a=fmtp:%u framesize=%u\r\n", payload, attr->framesize);
-}
-
-static enum ast_format_cmp_res celt_cmp(const struct ast_format_attr *fattr1, const struct ast_format_attr *fattr2)
-{
-	struct celt_attr *attr1 = (struct celt_attr *) fattr1;
-	struct celt_attr *attr2 = (struct celt_attr *) fattr2;
-
-	if (attr1->samplerate == attr2->samplerate) {
+	ast_str_append(str, 0, "a=fmtp:%d framesize=%d\r\n", payload, attr->framesize);
+}
+
+static enum ast_format_cmp_res celt_cmp(const struct ast_format *format1, const struct ast_format *format2)
+{
+	struct celt_attr *attr1 = ast_format_get_attribute_data(format1);
+	struct celt_attr *attr2 = ast_format_get_attribute_data(format2);
+
+	if (((!attr1 || !attr1->samplerate) && (!attr2 || !attr2->samplerate)) ||
+		(attr1->samplerate == attr2->samplerate)) {
 		return AST_FORMAT_CMP_EQUAL;
 	}
+
 	return AST_FORMAT_CMP_NOT_EQUAL;
 }
 
-static int celt_get_val(const struct ast_format_attr *fattr, int key, void *result)
-{
-	const struct celt_attr *attr = (struct celt_attr *) fattr;
-	int *val = result;
-
-	switch (key) {
-	case CELT_ATTR_KEY_SAMP_RATE:
-		*val = attr->samplerate;
-		break;
-	case CELT_ATTR_KEY_MAX_BITRATE:
-		*val = attr->maxbitrate;
-		break;
-	case CELT_ATTR_KEY_FRAME_SIZE:
-		*val = attr->framesize;
-		break;
-	default:
-		ast_log(LOG_WARNING, "unknown attribute type %d\n", key);
-		return -1;
-	}
-	return 0;
-}
-
-static int celt_isset(const struct ast_format_attr *fattr, va_list ap)
-{
-	enum celt_attr_keys key;
-	const struct celt_attr *attr = (struct celt_attr *) fattr;
-
-	for (key = va_arg(ap, int);
-		key != AST_FORMAT_ATTR_END;
-		key = va_arg(ap, int))
-	{
-		switch (key) {
-		case CELT_ATTR_KEY_SAMP_RATE:
-			if (attr->samplerate != (va_arg(ap, int))) {
-				return -1;
-			}
-			break;
-		case CELT_ATTR_KEY_MAX_BITRATE:
-			if (attr->maxbitrate != (va_arg(ap, int))) {
-				return -1;
-			}
-			break;
-		case CELT_ATTR_KEY_FRAME_SIZE:
-			if (attr->framesize != (va_arg(ap, int))) {
-				return -1;
-			}
-			break;
-		default:
-			ast_log(LOG_WARNING, "unknown attribute type %u\n", key);
-			return -1;
-		}
-	}
-	return 0;
-}
-static int celt_getjoint(const struct ast_format_attr *fattr1, const struct ast_format_attr *fattr2, struct ast_format_attr *result)
-{
-	struct celt_attr *attr1 = (struct celt_attr *) fattr1;
-	struct celt_attr *attr2 = (struct celt_attr *) fattr2;
-	struct celt_attr *attr_res = (struct celt_attr *) result;
-
-	/* sample rate is the only attribute that has any bearing on if joint capabilities exist or not */
-	if (attr1->samplerate != attr2->samplerate) {
-		return -1;
-	}
+static struct ast_format *celt_getjoint(const struct ast_format *format1, const struct ast_format *format2)
+{
+	struct celt_attr *attr1 = ast_format_get_attribute_data(format1);
+	struct celt_attr *attr2 = ast_format_get_attribute_data(format2);
+	struct ast_format *jointformat;
+	struct celt_attr *jointattr;
+
+	if (attr1 && attr2 && (attr1->samplerate != attr2->samplerate)) {
+		return NULL;
+	}
+
+	jointformat = ast_format_clone(format1);
+	if (!jointformat) {
+		return NULL;
+	}
+	jointattr = ast_format_get_attribute_data(jointformat);
+
 	/* either would work, they are guaranteed the same at this point. */
-	attr_res->samplerate = attr1->samplerate;
+	jointattr->samplerate = attr1->samplerate;
 	/* Take the lowest max bitrate */
-	attr_res->maxbitrate = MIN(attr1->maxbitrate, attr2->maxbitrate);
-
-	attr_res->framesize = attr2->framesize; /* TODO figure out what joint framesize means */
-	return 0;
-}
-
-static void celt_set(struct ast_format_attr *fattr, va_list ap)
-{
-	enum celt_attr_keys key;
-	struct celt_attr *attr = (struct celt_attr *) fattr;
-
-	for (key = va_arg(ap, int);
-		key != AST_FORMAT_ATTR_END;
-		key = va_arg(ap, int))
-	{
-		switch (key) {
-		case CELT_ATTR_KEY_SAMP_RATE:
-			attr->samplerate = (va_arg(ap, int));
-			break;
-		case CELT_ATTR_KEY_MAX_BITRATE:
-			attr->maxbitrate = (va_arg(ap, int));
-			break;
-		case CELT_ATTR_KEY_FRAME_SIZE:
-			attr->framesize = (va_arg(ap, int));
-			break;
-		default:
-			ast_log(LOG_WARNING, "unknown attribute type %u\n", key);
-		}
-	}
-}
-
-static struct ast_format_attr_interface celt_interface = {
-	.id = AST_FORMAT_CELT,
-	.format_attr_cmp = celt_cmp,
-	.format_attr_get_joint = celt_getjoint,
-	.format_attr_set = celt_set,
-	.format_attr_isset = celt_isset,
-	.format_attr_get_val = celt_get_val,
-	.format_attr_sdp_parse = celt_sdp_parse,
-	.format_attr_sdp_generate = celt_sdp_generate,
+	jointattr->maxbitrate = MIN(attr1->maxbitrate, attr2->maxbitrate);
+
+	jointattr->framesize = attr2->framesize; /* TODO figure out what joint framesize means */
+
+	return jointformat;
+}
+
+static struct ast_format *celt_set(const struct ast_format *format, const char *name, const char *value)
+{
+	struct ast_format *cloned;
+	struct celt_attr *attr;
+	unsigned int val;
+
+	cloned = ast_format_clone(format);
+	if (!cloned) {
+		return NULL;
+	}
+	attr = ast_format_get_attribute_data(cloned);
+
+	if (sscanf(value, "%30u", &val) != 1) {
+		ast_log(LOG_WARNING, "Unknown value '%s' for attribute type '%s'\n",
+			value, name);
+		ao2_ref(cloned, -1);
+		return NULL;
+	}
+
+	if (!strcasecmp(name, "sample_rate")) {
+		attr->samplerate = val;
+	} else if (!strcasecmp(name, "max_bitrate")) {
+		attr->maxbitrate = val;
+	} else if (!strcasecmp(name, "frame_size")) {
+		attr->framesize = val;
+	} else {
+		ast_log(LOG_WARNING, "Unknown attribute type '%s'\n", name);
+		ao2_ref(cloned, -1);
+		return NULL;
+	}
+
+	return cloned;
+}
+
+static struct ast_format_interface celt_interface = {
+	.format_destroy = celt_destroy,
+	.format_clone = celt_clone,
+	.format_cmp = celt_cmp,
+	.format_get_joint = celt_getjoint,
+	.format_attribute_set = celt_set,
+	.format_parse_sdp_fmtp = celt_parse_sdp_fmtp,
+	.format_generate_sdp_fmtp = celt_generate_sdp_fmtp,
 };
 
 static int load_module(void)
 {
-	if (ast_format_attr_reg_interface(&celt_interface)) {
+	if (ast_format_interface_register("celt", &celt_interface)) {
 		return AST_MODULE_LOAD_DECLINE;
 	}
 
@@ -199,7 +195,6 @@
 
 static int unload_module(void)
 {
-	ast_format_attr_unreg_interface(&celt_interface);
 	return 0;
 }
 

Modified: team/group/media_formats-reviewed-trunk/res/res_format_attr_h263.c
URL: http://svnview.digium.com/svn/asterisk/team/group/media_formats-reviewed-trunk/res/res_format_attr_h263.c?view=diff&rev=418248&r1=418247&r2=418248
==============================================================================
--- team/group/media_formats-reviewed-trunk/res/res_format_attr_h263.c (original)
+++ team/group/media_formats-reviewed-trunk/res/res_format_attr_h263.c Wed Jul  9 12:51:43 2014
@@ -38,250 +38,231 @@
 #include "asterisk/module.h"
 #include "asterisk/format.h"
 
-enum h263_attr_keys {
-	H263_ATTR_KEY_SQCIF,       /*!< Minimum picture interval for SQCIF resolution */
-	H263_ATTR_KEY_QCIF,        /*!< Minimum picture interval for QCIF resolution */
-	H263_ATTR_KEY_CIF,         /*!< Minimum picture interval for CIF resolution */
-	H263_ATTR_KEY_CIF4,        /*!< Minimum picture interval for CIF4 resolution */
-	H263_ATTR_KEY_CIF16,       /*!< Minimum picture interval for CIF16 resolution */
-	H263_ATTR_KEY_VGA,         /*!< Minimum picture interval for VGA resolution */
-	H263_ATTR_KEY_CUSTOM_XMAX, /*!< Custom resolution (Xmax) */
-	H263_ATTR_KEY_CUSTOM_YMAX, /*!< Custom resolution (Ymax) */
-	H263_ATTR_KEY_CUSTOM_MPI,  /*!< Custom resolution (MPI) */

[... 1593 lines stripped ...]



More information about the asterisk-commits mailing list