[Asterisk-code-review] codecs: Fix ABI incompatibility created by adding format na... (asterisk[13])

George Joseph asteriskteam at digium.com
Tue Jun 28 12:35:07 CDT 2016


George Joseph has uploaded a new change for review.

  https://gerrit.asterisk.org/3100

Change subject: codecs:  Fix ABI incompatibility created by adding format_name to ast_codec
......................................................................

codecs:  Fix ABI incompatibility created by adding format_name to ast_codec

Adding format_name even to the end of ast_codec caused issued with
binary codec modules because the pointer would be garbage in asterisk
when they registered.  So, the ast_codec structure was reverted and an
internal_ast_codec structure was created just for use in codec.c.  A new
internal-only API was also added (__ast_codec_register_with_format) so
that codec_builtin could register codecs with the format_name in a
separate parameter rather than in the ast_codec structure.

ASTERISK-26144 #close
Reported-by: Alexei Gradinari

Change-Id: I6df1b08f6a6ae089db23adfe1ebc8636330265ba
---
M include/asterisk/codec.h
M main/codec.c
M main/codec_builtin.c
3 files changed, 75 insertions(+), 10 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/00/3100/1

diff --git a/include/asterisk/codec.h b/include/asterisk/codec.h
index fb2b7da..28befec 100644
--- a/include/asterisk/codec.h
+++ b/include/asterisk/codec.h
@@ -77,8 +77,6 @@
 	unsigned int smooth;
 	/*! \brief The module that registered this codec */
 	struct ast_module *mod;
-	/*! \brief A format name for a default sane format using this codec */
-	const char *format_name;
 };
 
 /*!
diff --git a/main/codec.c b/main/codec.c
index c8644fd..f22a0db 100644
--- a/main/codec.c
+++ b/main/codec.c
@@ -49,6 +49,66 @@
 /*! \brief Registered codecs */
 static struct ao2_container *codecs;
 
+/*!
+ * \brief Internal codec structure
+ *
+ * External codecs won't know about the format_name field so the public
+ * ast_codec structure has to leave it out.  This structure will be used
+ * for the internal codecs.
+ *
+ */
+struct internal_ast_codec {
+	/*! \brief Internal unique identifier for this codec, set at registration time (starts at 1) */
+	unsigned int id;
+	/*! \brief Name for this codec */
+	const char *name;
+	/*! \brief Brief description */
+	const char *description;
+	/*! \brief Type of media this codec contains */
+	enum ast_media_type type;
+	/*! \brief Sample rate (number of samples carried in a second) */
+	unsigned int sample_rate;
+	/*! \brief Minimum length of media that can be carried (in milliseconds) in a frame */
+	unsigned int minimum_ms;
+	/*! \brief Maximum length of media that can be carried (in milliseconds) in a frame */
+	unsigned int maximum_ms;
+	/*! \brief Default length of media carried (in milliseconds) in a frame */
+	unsigned int default_ms;
+	/*! \brief Length in bytes of the data payload of a minimum_ms frame */
+	unsigned int minimum_bytes;
+	/*!
+	 * \brief Retrieve the number of samples in a frame
+	 *
+	 * \param frame The frame to examine
+	 *
+	 * \return the number of samples
+	 */
+	int (*samples_count)(struct ast_frame *frame);
+	/*!
+	 * \brief Retrieve the length of media from number of samples
+	 *
+	 * \param samples The number of samples
+	 *
+	 * \return The length of media in milliseconds
+	 */
+	int (*get_length)(unsigned int samples);
+	/*! \brief Whether the media can be smoothed or not */
+	unsigned int smooth;
+	/*! \brief The module that registered this codec */
+	struct ast_module *mod;
+	/*! \brief A format name for a default sane format using this codec */
+	const char *format_name;
+};
+
+/*!
+ * \brief Internal function for registration with format name
+ *
+ * This function is only used by codec.c and codec_builtin.c and
+ * will be removed in Asterisk 14
+ */
+int __ast_codec_register_with_format(struct ast_codec *codec, const char *format_name,
+	struct ast_module *mod);
+
 static int codec_hash(const void *obj, int flags)
 {
 	const struct ast_codec *codec;
@@ -113,7 +173,7 @@
 static char *show_codecs(struct ast_cli_entry *e, int cmd, struct ast_cli_args *a)
 {
 	struct ao2_iterator i;
-	struct ast_codec *codec;
+	struct internal_ast_codec *codec;
 
 	switch (cmd) {
 	case CLI_INIT:
@@ -190,7 +250,7 @@
 static char *show_codec(struct ast_cli_entry *e, int cmd, struct ast_cli_args *a)
 {
 	int type_punned_codec;
-	struct ast_codec *codec;
+	struct internal_ast_codec *codec;
 
 	switch (cmd) {
 	case CLI_INIT:
@@ -263,8 +323,13 @@
 
 int __ast_codec_register(struct ast_codec *codec, struct ast_module *mod)
 {
+	return __ast_codec_register_with_format(codec, NULL, mod);
+}
+
+int __ast_codec_register_with_format(struct ast_codec *codec, const char *format_name, struct ast_module *mod)
+{
 	SCOPED_AO2WRLOCK(lock, codecs);
-	struct ast_codec *codec_new;
+	struct internal_ast_codec *codec_new;
 
 	/* Some types have specific requirements */
 	if (codec->type == AST_MEDIA_TYPE_UNKNOWN) {
@@ -293,7 +358,8 @@
 			codec->name, ast_codec_media_type2str(codec->type), codec->sample_rate);
 		return -1;
 	}
-	*codec_new = *codec;
+	memcpy(codec_new, codec, sizeof(*codec));
+	codec_new->format_name = format_name;
 	codec_new->id = codec_id++;
 
 	ao2_link_flags(codecs, codec_new, OBJ_NOLOCK);
diff --git a/main/codec_builtin.c b/main/codec_builtin.c
index d7d253a..d3f6517 100644
--- a/main/codec_builtin.c
+++ b/main/codec_builtin.c
@@ -38,6 +38,9 @@
 #include "asterisk/format_cache.h"
 #include "asterisk/frame.h"
 
+int __ast_codec_register_with_format(struct ast_codec *codec, const char *format_name,
+	struct ast_module *mod);
+
 enum frame_type {
 	TYPE_HIGH,     /* 0x0 */
 	TYPE_LOW,      /* 0x1 */
@@ -774,8 +777,7 @@
 		int __res_ ## __LINE__ = 0; \
 		struct ast_format *__fmt_ ## __LINE__; \
 		struct ast_codec *__codec_ ## __LINE__; \
-		codec.format_name = (codec).name; \
-		res |= __ast_codec_register(&(codec), NULL); \
+		res |= __ast_codec_register_with_format(&(codec), (codec).name, NULL); \
 		__codec_ ## __LINE__ = ast_codec_get((codec).name, (codec).type, (codec).sample_rate); \
 		__fmt_ ## __LINE__ = __codec_ ## __LINE__ ? ast_format_create(__codec_ ## __LINE__) : NULL; \
 		res |= ast_format_cache_set(__fmt_ ## __LINE__); \
@@ -789,8 +791,7 @@
 		int __res_ ## __LINE__ = 0; \
 		struct ast_format *__fmt_ ## __LINE__; \
 		struct ast_codec *__codec_ ## __LINE__; \
-		codec.format_name = fmt_name; \
-		res |= __ast_codec_register(&(codec), NULL); \
+		res |= __ast_codec_register_with_format(&(codec), fmt_name, NULL); \
 		__codec_ ## __LINE__ = ast_codec_get((codec).name, (codec).type, (codec).sample_rate); \
 		__fmt_ ## __LINE__ = ast_format_create_named((fmt_name), __codec_ ## __LINE__); \
 		res |= ast_format_cache_set(__fmt_ ## __LINE__); \

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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I6df1b08f6a6ae089db23adfe1ebc8636330265ba
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: George Joseph <gjoseph at digium.com>



More information about the asterisk-code-review mailing list