[Asterisk-code-review] media index.c: Refactored so it doesn't cache the index (asterisk[13])

George Joseph asteriskteam at digium.com
Thu Jan 24 12:12:04 CST 2019


George Joseph has uploaded this change for review. ( https://gerrit.asterisk.org/10922


Change subject: media_index.c: Refactored so it doesn't cache the index
......................................................................

media_index.c: Refactored so it doesn't cache the index

Testing revealed that the cache added no benefit but that it could
consume excessive memory.

Two new index related functions were created:
ast_sounds_get_index_for_file() and ast_media_index_update_for_file()
which restrict index updating to specific sound files.

The original ast_sounds_get_index() and ast_media_index_update()
calls are still available but since they no longer cache the results
internally, developers should re-use an index they may already have
instead of calling ast_sounds_get_index() repeatedly.  If information
for only a single file is needed, ast_sounds_get_index_for_file()
should be called instead of ast_sounds_get_index().

The media_index directory scan code was elimininated in favor of
using the existing ast_file_read_dirs() function.

Since there's no more cache, ast_sounds_index_init now only
registers the sounds cli commands instead of generating the
initial index and subscribing to stasis format register/unregister
messages.

ast_sounds_reindex() is now a no-op but left for backwards
compatibility.

loader.c no longer registers "sounds" as a special reload target.

Both the sounds cli commands and the sounds ari resources were
refactored to only call ast_sounds_get_index() once per invocation
and to use ast_sounds_get_index_for_file() when a specific sound
file is requested.

Change-Id: I1cef327ba1b0648d85d218b70ce469ad07f4aa8d
---
M CHANGES
M include/asterisk/media_index.h
M include/asterisk/sounds_index.h
M main/loader.c
M main/media_index.c
M main/sounds.c
M res/ari/resource_sounds.c
7 files changed, 279 insertions(+), 207 deletions(-)



  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/22/10922/1

diff --git a/CHANGES b/CHANGES
index da58f29..5cd3d4c 100644
--- a/CHANGES
+++ b/CHANGES
@@ -17,6 +17,19 @@
  * Added "send_contact_status_on_update_registration" global configuration option
    to enable sending AMI ContactStatus event when a device refreshes its registration.
 
+Core
+------------------
+ * Reworked the media indexer so it doesn't cache the index.  Testing revealed
+   that the cache added no benefit but that it could consume excessive memory.
+   Two new index related functions were created: ast_sounds_get_index_for_file()
+   and ast_media_index_update_for_file() which restrict index updating to
+   specific sound files.  The original ast_sounds_get_index() and
+   ast_media_index_update() calls are still available but since they no longer
+   cache the results internally, developers should re-use an index they may
+   already have instead of calling ast_sounds_get_index() repeatedly.  If
+   information for only a single file is needed, ast_sounds_get_index_for_file()
+   should be called instead of ast_sounds_get_index().
+
 Features
 ------------------
  * Before Asterisk 12, when using the automon or automixmon features defined
diff --git a/include/asterisk/media_index.h b/include/asterisk/media_index.h
index 40fb721..f3a95a6 100644
--- a/include/asterisk/media_index.h
+++ b/include/asterisk/media_index.h
@@ -101,6 +101,26 @@
  */
 int ast_media_index_update(struct ast_media_index *index,
 	const char *variant);
+
+/*!
+ * \brief Update a media index for a specific sound file
+ *
+ * \since 13.25.0
+ * \since 16.2.0
+ *
+ * \param index Media index in which to query information
+ * \param variant Media variant for which to get the description
+ * \param filename Sound file name without extension
+ *
+ * \note If filename is NULL, this function will act as
+ * \ref ast_media_index_update and add all sound files to the index.
+ *
+ * \retval non-zero on error
+ * \return zero on success
+ */
+int ast_media_index_update_for_file(struct ast_media_index *index,
+	const char *variant, const char *filename);
+
 #if defined(__cplusplus) || defined(c_plusplus)
 }
 #endif
diff --git a/include/asterisk/sounds_index.h b/include/asterisk/sounds_index.h
index d7eb053..dad54a0 100644
--- a/include/asterisk/sounds_index.h
+++ b/include/asterisk/sounds_index.h
@@ -34,9 +34,11 @@
 
 /*!
  * \brief Reload the sounds index
+ * \deprecated
  *
- * \retval zero on success
- * \retval non-zero on failure
+ * This function is a NoOp and will be removed in a future release.
+ *
+ * \retval zero
  */
 int ast_sounds_reindex(void);
 
@@ -48,6 +50,19 @@
  */
 struct ast_media_index *ast_sounds_get_index(void);
 
+/*!
+ * \brief Get the index for a specific sound file
+ * \since 13.25.0
+ * \since 16.2.0
+ *
+ * \param filename Sound file name without extension
+ *
+ * \retval sounds index (must be ao2_cleanup()'ed)
+ * \retval NULL on failure
+ */
+struct ast_media_index *ast_sounds_get_index_for_file(const char *filename);
+
+
 #if defined(__cplusplus) || defined(c_plusplus)
 }
 #endif
diff --git a/main/loader.c b/main/loader.c
index ff6b282..da8fcf2 100644
--- a/main/loader.c
+++ b/main/loader.c
@@ -56,7 +56,6 @@
 #include "asterisk/vector.h"
 #include "asterisk/app.h"
 #include "asterisk/test.h"
-#include "asterisk/sounds_index.h"
 #include "asterisk/cli.h"
 
 #include <dlfcn.h>
@@ -376,7 +375,6 @@
 	{ "logger",      logger_reload },
 	{ "manager",     reload_manager },
 	{ "plc",         ast_plc_reload },
-	{ "sounds",      ast_sounds_reindex },
 	{ "udptl",       ast_udptl_reload },
 	{ NULL,          NULL }
 };
diff --git a/main/media_index.c b/main/media_index.c
index 2d1bc6b..67396e7 100644
--- a/main/media_index.c
+++ b/main/media_index.c
@@ -380,7 +380,8 @@
 static int process_description_file(struct ast_media_index *index,
 	const char *subdir,
 	const char *variant_str,
-	const char *filename)
+	const char *filename,
+	const char *match_filename)
 {
 	RAII_VAR(struct ast_str *, description_file_path, ast_str_create(64), ast_free);
 	RAII_VAR(struct ast_str *, cumulative_description, ast_str_create(64), ast_free);
@@ -450,16 +451,21 @@
 			if (file_id_persist && !ast_strlen_zero(ast_str_buffer(cumulative_description))) {
 				struct media_variant *variant;
 
-				variant = alloc_variant(index, file_id_persist, variant_str);
-				if (!variant) {
-					res = -1;
-					break;
+				/*
+				 * If we were only searching for a specific sound filename
+				 * don't include others.
+				 */
+				if (ast_strlen_zero(match_filename) || strcmp(match_filename, file_id_persist) == 0) {
+					variant = alloc_variant(index, file_id_persist, variant_str);
+					if (!variant) {
+						res = -1;
+						break;
+					}
+					ast_string_field_set(variant, description, ast_str_buffer(cumulative_description));
+					ao2_ref(variant, -1);
 				}
 
-				ast_string_field_set(variant, description, ast_str_buffer(cumulative_description));
-
 				ast_str_reset(cumulative_description);
-				ao2_ref(variant, -1);
 			}
 
 			ast_free(file_id_persist);
@@ -473,12 +479,18 @@
 	if (file_id_persist && !ast_strlen_zero(ast_str_buffer(cumulative_description))) {
 		struct media_variant *variant;
 
-		variant = alloc_variant(index, file_id_persist, variant_str);
-		if (variant) {
-			ast_string_field_set(variant, description, ast_str_buffer(cumulative_description));
-			ao2_ref(variant, -1);
-		} else {
-			res = -1;
+		/*
+		 * If we were only searching for a specific sound filename
+		 * don't include others.
+		 */
+		if (ast_strlen_zero(match_filename) || strcmp(match_filename, file_id_persist) == 0) {
+			variant = alloc_variant(index, file_id_persist, variant_str);
+			if (variant) {
+				ast_string_field_set(variant, description, ast_str_buffer(cumulative_description));
+				ao2_ref(variant, -1);
+			} else {
+				res = -1;
+			}
 		}
 	}
 
@@ -487,110 +499,121 @@
 	return res;
 }
 
-/*! \brief process an individual file listing */
-static int process_file(struct ast_media_index *index, const char *variant_str, const char *subdir, const char *filename)
-{
-	RAII_VAR(char *, filename_stripped, ast_strdup(filename), ast_free);
-	char *ext;
+struct read_dirs_data {
+	const char *search_filename;
+	size_t search_filename_len;
+	const char *search_variant;
+	struct ast_media_index *index;
+	size_t dirname_len;
+};
 
-	if (!filename_stripped) {
-		return -1;
+static int read_dirs_cb(const char *dir_name, const char *filename, void *obj)
+{
+	struct read_dirs_data *data = obj;
+	char *ext;
+	size_t match_len;
+	char *match;
+	size_t match_base_len;
+	char *subdirs = (char *)dir_name + data->dirname_len;
+
+	/*
+	 * Example:
+	 * From the filesystem:
+	 * 	  index's base_dir = "/var/lib/asterisk/sounds"
+	 * 	  search_variant = "en"
+	 * 	  search directory base = "/var/lib/asterisk/sounds/en"
+	 * 	  dirname_len = 27
+	 *    current dir_name = "/var/lib/asterisk/sounds/en/digits"
+	 *    subdirs =                                     "/digits"
+	 *    filename = "1.ulaw"
+	 *
+	 * From the search criteria:
+	 *    search_filename = "digits/1"
+	 *    search_filename_len = 8
+	 */
+
+	if (*subdirs == '/') {
+		subdirs++;
 	}
 
-	ext = strrchr(filename_stripped, '.');
+	/* subdirs = "digits" */
+
+	match_len = strlen(subdirs) + strlen(filename) + 2;
+	match = ast_alloca(match_len);
+	snprintf(match, match_len, "%s%s%s", subdirs,
+		ast_strlen_zero(subdirs) ? "" : "/", filename);
+
+	/* match = discovered filename relative to language = "digits/1.ulaw" */
+
+	ext = strrchr(match, '.');
 	if (!ext) {
-		/* file has no extension */
 		return 0;
 	}
 
-	*ext++ = '\0';
-	if (!strcmp(ext, "txt")) {
-		if (process_description_file(index, subdir, variant_str, filename)) {
-			return -1;
-		}
-	} else {
-		if (process_media_file(index, variant_str, subdir, filename_stripped, ext)) {
-			return -1;
+	/* ext = ".ulaw" */
+
+	if (data->search_filename_len > 0) {
+		match_base_len = ext - match;
+		/*
+		 * match_base_len = length of "digits/1" = 8 which
+		 * happens to match the length of search_filename.
+		 * However if the discovered filename was 11.ulaw
+		 * it would be length of "digits/11" = 9.
+		 * We need to use the larger during the compare to
+		 * make sure we don't match just search_filename
+		 * as a substring of the discovered filename.
+		 */
+		if (data->search_filename_len > match_base_len) {
+			match_base_len = data->search_filename_len;
 		}
 	}
+
+	/* We always process txt files because they should contain description. */
+	if (strcmp(ext, ".txt") == 0) {
+		if (process_description_file(data->index, NULL, data->search_variant,
+			match, data->search_filename)) {
+			return -1;
+		}
+	} else if (data->search_filename_len == 0
+		|| strncmp(data->search_filename, match, match_base_len	) == 0) {
+		*ext = '\0';
+		ext++;
+		process_media_file(data->index, data->search_variant, NULL, match, ext);
+	}
+
 	return 0;
 }
 
-/*! \brief internal function for updating the index, recursive */
-static int media_index_update(struct ast_media_index *index,
-	const char *variant,
-	const char *subdir)
+int ast_media_index_update_for_file(struct ast_media_index *index,
+	const char *variant, const char *filename)
 {
-	struct dirent* dent;
-	DIR* srcdir;
-	RAII_VAR(struct ast_str *, index_dir, ast_str_create(64), ast_free);
-	RAII_VAR(struct ast_str *, statfile, ast_str_create(64), ast_free);
-	int res = 0;
+	struct timeval start;
+	struct timeval end;
+	int64_t elapsed;
+	int rc;
+	size_t dirname_len = strlen(index->base_dir) + strlen(S_OR(variant, "")) + 1;
+	struct read_dirs_data data = {
+		.search_filename = S_OR(filename, ""),
+		.search_filename_len = strlen(S_OR(filename, "")),
+		.search_variant = S_OR(variant, ""),
+		.index = index,
+		.dirname_len = dirname_len,
+	};
+	char *search_dir = ast_alloca(dirname_len + 1);
 
-	if (!index_dir) {
-		return 0;
-	}
+	sprintf(search_dir, "%s%s%s", index->base_dir, ast_strlen_zero(variant) ? "" : "/",
+		data.search_variant);
 
-	ast_str_set(&index_dir, 0, "%s", index->base_dir);
-	if (!ast_strlen_zero(variant)) {
-		ast_str_append(&index_dir, 0, "/%s", variant);
-	}
-	if (!ast_strlen_zero(subdir)) {
-		ast_str_append(&index_dir, 0, "/%s", subdir);
-	}
+	gettimeofday(&start, NULL);
+	rc = ast_file_read_dirs(search_dir, read_dirs_cb, &data, -1);
+	gettimeofday(&end, NULL);
+	elapsed = ast_tvdiff_us(end, start);
+	ast_debug(1, "Media for language '%s' indexed in %8.6f seconds\n", data.search_variant, elapsed / 1E6);
 
-	srcdir = opendir(ast_str_buffer(index_dir));
-	if (srcdir == NULL) {
-		ast_log(LOG_ERROR, "Failed to open %s: %s\n", ast_str_buffer(index_dir), strerror(errno));
-		return -1;
-	}
-
-	while((dent = readdir(srcdir)) != NULL) {
-		struct stat st;
-
-		if(!strcmp(dent->d_name, ".") || !strcmp(dent->d_name, "..")) {
-			continue;
-		}
-
-		ast_str_reset(statfile);
-		ast_str_set(&statfile, 0, "%s/%s", ast_str_buffer(index_dir), dent->d_name);
-
-		if (stat(ast_str_buffer(statfile), &st) < 0) {
-			ast_log(LOG_WARNING, "Failed to stat %s: %s\n", ast_str_buffer(statfile), strerror(errno));
-			continue;
-		}
-
-		if (S_ISDIR(st.st_mode)) {
-			if (ast_strlen_zero(subdir)) {
-				res = media_index_update(index, variant, dent->d_name);
-			} else {
-				RAII_VAR(struct ast_str *, new_subdir, ast_str_create(64), ast_free);
-				ast_str_set(&new_subdir, 0, "%s/%s", subdir, dent->d_name);
-				res = media_index_update(index, variant, ast_str_buffer(new_subdir));
-			}
-
-			if (res) {
-				break;
-			}
-			continue;
-		}
-
-		if (!S_ISREG(st.st_mode)) {
-			continue;
-		}
-
-		if (process_file(index, variant, subdir, dent->d_name)) {
-			res = -1;
-			break;
-		}
-	}
-
-	closedir(srcdir);
-	return res;
+	return rc;
 }
 
-int ast_media_index_update(struct ast_media_index *index,
-	const char *variant)
+int ast_media_index_update(struct ast_media_index *index, const char *variant)
 {
-	return media_index_update(index, variant, NULL);
+	return ast_media_index_update_for_file(index, variant, NULL);
 }
diff --git a/main/sounds.c b/main/sounds.c
index 6fec48e..c3666ed 100644
--- a/main/sounds.c
+++ b/main/sounds.c
@@ -45,10 +45,6 @@
 /*! \brief The number of buckets to be used for storing language-keyed objects */
 #define LANGUAGE_BUCKETS 7
 
-static struct ast_media_index *sounds_index;
-
-static struct stasis_message_router *sounds_system_router;
-
 /*! \brief Get the languages in which sound files are available */
 static struct ao2_container *get_languages(void)
 {
@@ -97,52 +93,8 @@
 	return lang_dirs;
 }
 
-/*! \brief Callback to process an individual language directory or subdirectory */
-static int update_index_cb(void *obj, void *arg, int flags)
-{
-	char *lang = obj;
-	struct ast_media_index *index = arg;
-
-	if (ast_media_index_update(index, lang)) {
-		return CMP_MATCH;
-	}
-	return 0;
-}
-
-AST_MUTEX_DEFINE_STATIC(reload_lock);
-
 int ast_sounds_reindex(void)
 {
-	RAII_VAR(struct ast_str *, sounds_dir, NULL, ast_free);
-	RAII_VAR(struct ao2_container *, languages, NULL, ao2_cleanup);
-	RAII_VAR(char *, failed_index, NULL, ao2_cleanup);
-	RAII_VAR(struct ast_media_index *, new_index, NULL, ao2_cleanup);
-	struct ast_media_index *old_index;
-
-	SCOPED_MUTEX(lock, &reload_lock);
-
-	old_index = sounds_index;
-	languages = get_languages();
-	sounds_dir = ast_str_create(64);
-
-	if (!languages || !sounds_dir) {
-		return -1;
-	}
-
-	ast_str_set(&sounds_dir, 0, "%s/sounds", ast_config_AST_DATA_DIR);
-	new_index = ast_media_index_create(ast_str_buffer(sounds_dir));
-	if (!new_index) {
-		return -1;
-	}
-
-	failed_index = ao2_callback(languages, 0, update_index_cb, new_index);
-	if (failed_index) {
-		return -1;
-	}
-
-	ao2_ref(new_index, +1);
-	sounds_index = new_index;
-	ao2_cleanup(old_index);
 	return 0;
 }
 
@@ -154,13 +106,13 @@
 	return 0;
 }
 
-static int show_sound_info_cb(void *obj, void *arg, int flags)
+static int show_sound_info_cb(void *obj, void *arg, void *data, int flags)
 {
 	char *language = obj;
 	struct ast_cli_args *a = arg;
 	struct ast_format *format;
 	int formats_shown = 0;
-	RAII_VAR(struct ast_media_index *, local_index, ast_sounds_get_index(), ao2_cleanup);
+	struct ast_media_index *local_index = data;
 	struct ast_format_cap *cap;
 	const char *description = ast_media_get_description(local_index, a->argv[3], language);
 
@@ -203,13 +155,23 @@
 	}
 
 	if (a->argc == 3) {
-		RAII_VAR(struct ao2_container *, sound_files, ast_media_get_media(sounds_index), ao2_cleanup);
+		struct ast_media_index *sounds_index = ast_sounds_get_index();
+		struct ao2_container *sound_files;
+
+		if (!sounds_index) {
+			return CLI_FAILURE;
+		}
+
+		sound_files = ast_media_get_media(sounds_index);
+		ao2_ref(sounds_index, -1);
 		if (!sound_files) {
 			return CLI_FAILURE;
 		}
 
 		ast_cli(a->fd, "Available audio files:\n");
 		ao2_callback(sound_files, OBJ_MULTIPLE | OBJ_NODATA, show_sounds_cb, a);
+		ao2_ref(sound_files, -1);
+
 		return CLI_SUCCESS;
 	}
 
@@ -222,6 +184,7 @@
 	int length;
 	struct ao2_iterator it_sounds;
 	char *filename;
+	struct ast_media_index *sounds_index;
 	struct ao2_container *sound_files;
 
 	switch (cmd) {
@@ -236,7 +199,13 @@
 			return NULL;
 		}
 
+		sounds_index = ast_sounds_get_index();
+		if (!sounds_index) {
+			return NULL;
+		}
+
 		sound_files = ast_media_get_media(sounds_index);
+		ao2_ref(sounds_index, -1);
 		if (!sound_files) {
 			return NULL;
 		}
@@ -259,14 +228,26 @@
 	}
 
 	if (a->argc == 4) {
-		RAII_VAR(struct ao2_container *, variants, ast_media_get_variants(sounds_index, a->argv[3]), ao2_cleanup);
+		struct ao2_container *variants;
+
+		sounds_index = ast_sounds_get_index_for_file(a->argv[3]);
+		if (!sounds_index) {
+			return NULL;
+		}
+
+		variants = ast_media_get_variants(sounds_index, a->argv[3]);
+
 		if (!variants || !ao2_container_count(variants)) {
+			ao2_ref(sounds_index, -1);
 			ast_cli(a->fd, "ERROR: File %s not found in index\n", a->argv[3]);
 			return CLI_FAILURE;
 		}
 
 		ast_cli(a->fd, "Indexed Information for %s:\n", a->argv[3]);
-		ao2_callback(variants, OBJ_MULTIPLE | OBJ_NODATA, show_sound_info_cb, a);
+		ao2_callback_data(variants, OBJ_MULTIPLE | OBJ_NODATA, show_sound_info_cb, a, sounds_index);
+		ao2_ref(sounds_index, -1);
+		ao2_ref(variants, -1);
+
 		return CLI_SUCCESS;
 	}
 
@@ -281,60 +262,72 @@
 
 static void sounds_cleanup(void)
 {
-	stasis_message_router_unsubscribe_and_join(sounds_system_router);
-	sounds_system_router = NULL;
 	ast_cli_unregister_multiple(cli_sounds, ARRAY_LEN(cli_sounds));
-	ao2_cleanup(sounds_index);
-	sounds_index = NULL;
-}
-
-static void format_update_cb(void *data, struct stasis_subscription *sub,
-	struct stasis_message *message)
-{
-	/* Reindexing during shutdown is pointless. */
-	if (!ast_shutting_down()) {
-		ast_sounds_reindex();
-	}
 }
 
 int ast_sounds_index_init(void)
 {
 	int res = 0;
-	if (ast_sounds_reindex()) {
-		return -1;
-	}
-	res |= ast_cli_register_multiple(cli_sounds, ARRAY_LEN(cli_sounds));
 
-	sounds_system_router = stasis_message_router_create(ast_system_topic());
-	if (!sounds_system_router) {
-		return -1;
-	}
-
-	if (ast_format_register_type()) {
-		res |= stasis_message_router_add(
-			sounds_system_router,
-			ast_format_register_type(),
-			format_update_cb,
-			NULL);
-	}
-
-	if (ast_format_unregister_type()) {
-		res |= stasis_message_router_add(
-			sounds_system_router,
-			ast_format_unregister_type(),
-			format_update_cb,
-			NULL);
-	}
-
+	res = ast_cli_register_multiple(cli_sounds, ARRAY_LEN(cli_sounds));
 	if (res) {
-		return -1;
+		return 1;
 	}
 
 	ast_register_cleanup(sounds_cleanup);
 	return 0;
 }
 
+/*! \brief Callback to process an individual language directory or subdirectory */
+static int update_index_cb(void *obj, void *arg, void *data, int flags)
+{
+	char *lang = obj;
+	char *filename = data;
+	struct ast_media_index *index = arg;
+
+	if (ast_media_index_update_for_file(index, lang, filename)) {
+		return CMP_MATCH;
+	}
+
+	return 0;
+}
+
 struct ast_media_index *ast_sounds_get_index(void)
 {
-	return ao2_bump(sounds_index);
+	return ast_sounds_get_index_for_file(NULL);
+}
+
+struct ast_media_index *ast_sounds_get_index_for_file(const char *filename)
+{
+	struct ast_str *sounds_dir = ast_str_create(64);
+	struct ao2_container *languages;
+	char *failed_index;
+	struct ast_media_index *new_index;
+
+	if (!sounds_dir) {
+		return NULL;
+	}
+
+	ast_str_set(&sounds_dir, 0, "%s/sounds", ast_config_AST_DATA_DIR);
+	new_index = ast_media_index_create(ast_str_buffer(sounds_dir));
+	ast_free(sounds_dir);
+	if (!new_index) {
+		return NULL;
+	}
+
+	languages = get_languages();
+	if (!languages) {
+		ao2_ref(new_index, -1);
+		return NULL;
+	}
+
+	failed_index = ao2_callback_data(languages, 0, update_index_cb, new_index, (void *)filename);
+	ao2_ref(languages, -1);
+	if (failed_index) {
+		ao2_ref(new_index, -1);
+		new_index = NULL;
+	}
+	ao2_cleanup(failed_index);
+
+	return new_index;
 }
diff --git a/res/ari/resource_sounds.c b/res/ari/resource_sounds.c
index 76a4d94..0f52a22 100644
--- a/res/ari/resource_sounds.c
+++ b/res/ari/resource_sounds.c
@@ -42,13 +42,13 @@
 };
 
 /*! \brief Add format/lang pairs to the array embedded in the sound object */
-static int add_format_information_cb(void *obj, void *arg, int flags)
+static int add_format_information_cb(void *obj, void *arg, void *data, int flags)
 {
 	char *language = obj;
 	struct lang_format_info *args = arg;
 	int idx;
 	RAII_VAR(struct ast_format_cap *, cap, NULL, ao2_cleanup);
-	RAII_VAR(struct ast_media_index *, sounds_index, ast_sounds_get_index(), ao2_cleanup);
+	struct ast_media_index *sounds_index = data;
 
 	if (!sounds_index) {
 		return CMP_STOP;
@@ -97,14 +97,13 @@
 
 /*! \brief Generate a Sound structure as documented in sounds.json for the specified filename */
 static struct ast_json *create_sound_blob(const char *filename,
-	struct ast_ari_sounds_list_args *args)
+	struct ast_ari_sounds_list_args *args, struct ast_media_index *sounds_index)
 {
 	RAII_VAR(struct ast_json *, sound, NULL, ast_json_unref);
 	RAII_VAR(struct ao2_container *, languages, NULL, ao2_cleanup);
 	const char *description;
 	struct ast_json *format_lang_list;
 	struct lang_format_info info;
-	RAII_VAR(struct ast_media_index *, sounds_index, ast_sounds_get_index(), ao2_cleanup);
 
 	if (!sounds_index) {
 		return NULL;
@@ -150,7 +149,7 @@
 	if (args) {
 		info.format_filter = args->format;
 	}
-	ao2_callback(languages, OBJ_NODATA, add_format_information_cb, &info);
+	ao2_callback_data(languages, OBJ_NODATA, add_format_information_cb, &info, sounds_index);
 
 	/* no format/lang pairs for this sound so nothing to return */
 	if (!ast_json_array_size(format_lang_list)) {
@@ -160,13 +159,18 @@
 	return ast_json_ref(sound);
 }
 
+struct sounds_cb_data {
+	struct ast_ari_sounds_list_args *args;
+	struct ast_media_index *index;
+};
+
 /*! \brief Generate a Sound structure and append it to the output blob */
 static int append_sound_cb(void *obj, void *arg, void *data, int flags)
 {
 	struct ast_json *sounds_array = arg;
 	char *filename = obj;
-	struct ast_ari_sounds_list_args *args = data;
-	struct ast_json *sound_blob = create_sound_blob(filename, args);
+	struct sounds_cb_data *cb_data = data;
+	struct ast_json *sound_blob = create_sound_blob(filename, cb_data->args, cb_data->index);
 	if (!sound_blob) {
 		return 0;
 	}
@@ -182,6 +186,10 @@
 	RAII_VAR(struct ao2_container *, sound_files, NULL, ao2_cleanup);
 	struct ast_json *sounds_blob;
 	RAII_VAR(struct ast_media_index *, sounds_index, ast_sounds_get_index(), ao2_cleanup);
+	struct sounds_cb_data cb_data = {
+		.args = args,
+		.index = sounds_index,
+	};
 
 	if (!sounds_index) {
 		ast_ari_response_error(response, 500, "Internal Error", "Sounds index not available");
@@ -200,7 +208,7 @@
 		return;
 	}
 
-	ao2_callback_data(sound_files, OBJ_NODATA, append_sound_cb, sounds_blob, args);
+	ao2_callback_data(sound_files, OBJ_NODATA, append_sound_cb, sounds_blob, &cb_data);
 
 	if (!ast_json_array_size(sounds_blob)) {
 		ast_ari_response_error(response, 404, "Not Found", "No sounds found that matched the query");
@@ -216,8 +224,10 @@
 	struct ast_ari_response *response)
 {
 	struct ast_json *sound_blob;
+	struct ast_media_index *sounds_index = ast_sounds_get_index_for_file(args->sound_id);
 
-	sound_blob = create_sound_blob(args->sound_id, NULL);
+	sound_blob = create_sound_blob(args->sound_id, NULL, sounds_index);
+	ao2_cleanup(sounds_index);
 	if (!sound_blob) {
 		ast_ari_response_error(response, 404, "Not Found", "Sound not found");
 		return;

-- 
To view, visit https://gerrit.asterisk.org/10922
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-MessageType: newchange
Gerrit-Change-Id: I1cef327ba1b0648d85d218b70ce469ad07f4aa8d
Gerrit-Change-Number: 10922
Gerrit-PatchSet: 1
Gerrit-Owner: George Joseph <gjoseph at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20190124/c0d4533e/attachment-0001.html>


More information about the asterisk-code-review mailing list