[Asterisk-code-review] Add the ability to read the media file type from HTTP header... (asterisk[master])

Jenkins2 asteriskteam at digium.com
Thu May 3 10:50:52 CDT 2018


Jenkins2 has submitted this change and it was merged. ( https://gerrit.asterisk.org/8882 )

Change subject: Add the ability to read the media file type from HTTP header for playback
......................................................................

Add the ability to read the media file type from HTTP header for playback

How it works today:
media_cache tries to parse out the extension of the media file to be played
from the URI provided to Asterisk while caching the file.

What's expected:
Better will be to have Asterisk get extension from other ways too. One of the
common ways is to get the type of content from the CONTENT-TYPE header in the
HTTP response for fetching the media file using the URI provided.

Steps to Reproduce:
Provide a URL of the form: http://host/media/1234 to Asterisk for media
playback. It fails to play and logs show the following error line:

[Sep 15 15:48:05] WARNING [29148] [C-00000092] file.c:
File http://host/media/1234 does not exist in any format

Scenario this issue is blocking:
In the case where the media files are stored in some cloud object store,
following can block the media being played via Asterisk:

Cloud storage generally needs authenticated access to the storage. The way
to do that is by using signed URIs. With the signed URIs there's no way to
preserve the name of the file.
In most cases Cloud storage returns a key to access the object and preserving
file name is also not a thing there

ASTERISK-27286

 Reporter: Gaurav Khurana

Change-Id: I1b14692a49b2c1ac67688f58757184122e92ba89
---
M formats/format_pcm.c
M formats/format_vox.c
M formats/format_wav.c
M include/asterisk/file.h
M include/asterisk/mod_format.h
M main/file.c
M main/media_cache.c
M res/res_http_media_cache.c
8 files changed, 87 insertions(+), 6 deletions(-)

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



diff --git a/formats/format_pcm.c b/formats/format_pcm.c
index 4e846d7..674c8b5 100644
--- a/formats/format_pcm.c
+++ b/formats/format_pcm.c
@@ -507,6 +507,7 @@
 static struct ast_format_def pcm_f = {
 	.name = "pcm",
 	.exts = "pcm|ulaw|ul|mu|ulw",
+	.mime_types = "audio/basic",
 	.write = pcm_write,
 	.seek = pcm_seek,
 	.trunc = pcm_trunc,
diff --git a/formats/format_vox.c b/formats/format_vox.c
index b63e225..023c409 100644
--- a/formats/format_vox.c
+++ b/formats/format_vox.c
@@ -128,6 +128,7 @@
 static struct ast_format_def vox_f = {
 	.name = "vox",
 	.exts = "vox",
+	.mime_types = "audio/x-vox",
 	.write = vox_write,
 	.seek = vox_seek,
 	.trunc = vox_trunc,
diff --git a/formats/format_wav.c b/formats/format_wav.c
index 81a686e..ec7e3d3 100644
--- a/formats/format_wav.c
+++ b/formats/format_wav.c
@@ -532,6 +532,7 @@
 static struct ast_format_def wav_f = {
 	.name = "wav",
 	.exts = "wav",
+	.mime_types = "audio/wav|audio/x-wav",
 	.open =	wav_open,
 	.rewrite = wav_rewrite,
 	.write = wav_write,
diff --git a/include/asterisk/file.h b/include/asterisk/file.h
index c17cb32..1c2c7a8 100644
--- a/include/asterisk/file.h
+++ b/include/asterisk/file.h
@@ -427,6 +427,18 @@
  */
 struct ast_format *ast_get_format_for_file_ext(const char *file_ext);
 
+/*!
+ * \brief Get a suitable filename extension for the given MIME type
+ *
+ * \param mime_type The MIME type for which to find extensions
+ * \param buffer A pointer to a buffer to receive the extension
+ * \param capacity The size of 'buffer' in bytes
+ *
+ * \retval 1 if an extension was found for the provided MIME type
+ * \retval 0 if the MIME type was not found
+ */
+int ast_get_extension_for_mime_type(const char *mime_type, char *buffer, size_t capacity);
+
 #if defined(__cplusplus) || defined(c_plusplus)
 }
 #endif
diff --git a/include/asterisk/mod_format.h b/include/asterisk/mod_format.h
index 5f93ea4..6e772f0 100644
--- a/include/asterisk/mod_format.h
+++ b/include/asterisk/mod_format.h
@@ -44,6 +44,7 @@
 	char name[80];		/*!< Name of format */
 	char exts[80];		/*!< Extensions (separated by | if more than one)
 						 * this format can read.  First is assumed for writing (e.g. .mp3) */
+	char mime_types[80]; /*!< MIME Types related to the format (separated by | if more than one)*/
 	struct ast_format *format;	/*!< Format of frames it uses/provides (one only) */
 	/*!
 	 * \brief Prepare an input stream for playback.
diff --git a/main/file.c b/main/file.c
index 8dded81..3ee58b1 100644
--- a/main/file.c
+++ b/main/file.c
@@ -333,19 +333,20 @@
 
 /* compare type against the list 'exts' */
 /* XXX need a better algorithm */
-static int exts_compare(const char *exts, const char *type)
+static int type_in_list(const char *list, const char *type, int (*cmp)(const char *s1, const char *s2))
 {
-	char tmp[256];
-	char *stringp = tmp, *ext;
+	char *stringp = ast_strdupa(list), *item;
 
-	ast_copy_string(tmp, exts, sizeof(tmp));
-	while ((ext = strsep(&stringp, "|"))) {
-		if (!strcmp(ext, type))
+	while ((item = strsep(&stringp, "|"))) {
+		if (!cmp(item, type)) {
 			return 1;
+		}
 	}
 
 	return 0;
 }
+
+#define exts_compare(list, type) (type_in_list((list), (type), strcmp))
 
 /*!
  * \internal
@@ -1926,6 +1927,27 @@
 	return NULL;
 }
 
+int ast_get_extension_for_mime_type(const char *mime_type, char *buffer, size_t capacity)
+{
+	struct ast_format_def *f;
+	SCOPED_RDLOCK(lock, &formats.lock);
+
+	ast_assert(buffer && capacity);
+
+	AST_RWLIST_TRAVERSE(&formats, f, list) {
+		if (type_in_list(f->mime_types, mime_type, strcasecmp)) {
+			size_t item_len = strcspn(f->exts, "|");
+			size_t bytes_written = snprintf(buffer, capacity, ".%.*s", (int) item_len, f->exts);
+			if (bytes_written < capacity) {
+				/* Only return success if we didn't truncate */
+				return 1;
+			}
+		}
+	}
+
+	return 0;
+}
+
 static struct ast_cli_entry cli_file[] = {
 	AST_CLI_DEFINE(handle_cli_core_show_file_formats, "Displays file formats")
 };
diff --git a/main/media_cache.c b/main/media_cache.c
index e93d1a0..e0a6dbb 100644
--- a/main/media_cache.c
+++ b/main/media_cache.c
@@ -35,6 +35,7 @@
 #include "asterisk/bucket.h"
 #include "asterisk/astdb.h"
 #include "asterisk/cli.h"
+#include "asterisk/file.h"
 #include "asterisk/media_cache.h"
 
 /*! The name of the AstDB family holding items in the cache. */
@@ -125,6 +126,24 @@
 
 /*!
  * \internal
+ * \brief Normalize the value of a Content-Type header
+ *
+ * This will trim off any optional parameters after the type/subtype.
+ */
+static void normalize_content_type_header(char *content_type)
+{
+	char *params = strchr(content_type, ';');
+
+	if (params) {
+		*params-- = 0;
+		while (params > content_type && (*params == ' ' || *params == '\t')) {
+			*params-- = 0;
+		}
+	}
+}
+
+/*!
+ * \internal
  * \brief Update the name of the file backing a \c bucket_file
  * \param preferred_file_name The preferred name of the backing file
  */
@@ -142,9 +161,32 @@
 	} else if (!strchr(bucket_file->path, '.') && (ext = strrchr(ast_sorcery_object_get_id(bucket_file), '.'))) {
 		/* If we don't have a file extension and were provided one in the URI, use it */
 		char new_path[PATH_MAX];
+		char found_ext[PATH_MAX];
 
 		ast_bucket_file_metadata_set(bucket_file, "ext", ext);
 
+		/* Don't pass '.' while checking for supported extension */
+		if (!ast_get_format_for_file_ext(ext + 1)) {
+			/* If the file extension passed in the URI isn't supported check for the
+			 * extension based on the MIME type passed in the Content-Type header before
+			 * giving up.
+			 * If a match is found then retrieve the extension from the supported list
+			 * corresponding to the mime-type and use that to rename the file */
+			struct ast_bucket_metadata *header = ast_bucket_file_metadata_get(bucket_file, "content-type");
+			if (header) {
+				char *mime_type = ast_strdup(header->value);
+				if (mime_type) {
+					normalize_content_type_header(mime_type);
+					if (!ast_strlen_zero(mime_type)) {
+						if (ast_get_extension_for_mime_type(mime_type, found_ext, sizeof(found_ext))) {
+							ext = found_ext;
+						}
+					}
+					ast_free(mime_type);
+				}
+			}
+		}
+
 		snprintf(new_path, sizeof(new_path), "%s%s", bucket_file->path, ext);
 		rename(bucket_file->path, new_path);
 		ast_copy_string(bucket_file->path, new_path, sizeof(bucket_file->path));
diff --git a/res/res_http_media_cache.c b/res/res_http_media_cache.c
index eba7ecc..bca5763 100644
--- a/res/res_http_media_cache.c
+++ b/res/res_http_media_cache.c
@@ -84,6 +84,7 @@
 	if (strcasecmp(header, "ETag")
 		&& strcasecmp(header, "Cache-Control")
 		&& strcasecmp(header, "Last-Modified")
+		&& strcasecmp(header, "Content-Type")
 		&& strcasecmp(header, "Expires")) {
 		return realsize;
 	}

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

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I1b14692a49b2c1ac67688f58757184122e92ba89
Gerrit-Change-Number: 8882
Gerrit-PatchSet: 2
Gerrit-Owner: Sean Bright <sean.bright at gmail.com>
Gerrit-Reviewer: Gaurav Khurana <gkhurana at godaddy.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180503/75e02764/attachment-0001.html>


More information about the asterisk-code-review mailing list