[Asterisk-code-review] app mp3: Use correct buffer size and the same sample rate as... (asterisk[14])

Anonymous Coward asteriskteam at digium.com
Sun Sep 4 13:23:56 CDT 2016


Anonymous Coward #1000019 has submitted this change and it was merged.

Change subject: app_mp3: Use correct buffer size and the same sample rate as the channel
......................................................................


app_mp3: Use correct buffer size and the same sample rate as the channel

Previously, the buffer used for MP3 streamed from HTTP servers had a size of
1 MB. For 8 kHz mono audio at 16 bit resolution, such a buffer covers about 1
minute. Only when the buffer is full does audio start to play.
For MP3 files streamed from a server, that is usually not a big deal as long as
the connection to the server is fast enough to supply that much data within a
second or two. For MP3 live streams however, it takes 1 minute to download 1
minute of audio, so without this change, app_mp3 wasn't really usable for MP3
live streams.
This commit changes the buffer size so that it covers 6 seconds of an MP3 file
streamed from a server and 0.5 seconds of an MP3 live stream. The latter is
identified by the use of a .m3u file extension.

app_mp3 so far only supported 8 kHz audio.
Now it always runs at the sample rate of the channel.

ASTERISK-26085 #close

Change-Id: Id1ee274733cd804a0edecf7450329b72f1235af0
---
M apps/app_mp3.c
1 file changed, 37 insertions(+), 15 deletions(-)

Approvals:
  Anonymous Coward #1000019: Verified
  Matt Jordan: Looks good to me, approved



diff --git a/apps/app_mp3.c b/apps/app_mp3.c
index 5712cfe..05afe54 100644
--- a/apps/app_mp3.c
+++ b/apps/app_mp3.c
@@ -77,9 +77,10 @@
  ***/
 static char *app = "MP3Player";
 
-static int mp3play(const char *filename, int fd)
+static int mp3play(const char *filename, unsigned int sampling_rate, int fd)
 {
 	int res;
+	char sampling_rate_str[8];
 
 	res = ast_safe_fork(0);
 	if (res < 0) 
@@ -93,30 +94,44 @@
 	dup2(fd, STDOUT_FILENO);
 	ast_close_fds_above_n(STDERR_FILENO);
 
+	snprintf(sampling_rate_str, 8, "%u", sampling_rate);
+
 	/* Execute mpg123, but buffer if it's a net connection */
-	if (!strncasecmp(filename, "http://", 7)) {
+	if (!strncasecmp(filename, "http://", 7) && strstr(filename, ".m3u")) {
+	    char buffer_size_str[8];
+	    snprintf(buffer_size_str, 8, "%u", (int) 0.5*2*sampling_rate/1000); // 0.5 seconds for a live stream
 		/* Most commonly installed in /usr/local/bin */
-	    execl(LOCAL_MPG_123, "mpg123", "-q", "-s", "-b", "1024", "-f", "8192", "--mono", "-r", "8000", filename, (char *)NULL);
+	    execl(LOCAL_MPG_123, "mpg123", "-q", "-s", "-b", buffer_size_str, "-f", "8192", "--mono", "-r", sampling_rate_str, "-@", filename, (char *)NULL);
 		/* But many places has it in /usr/bin */
-	    execl(MPG_123, "mpg123", "-q", "-s", "-b", "1024","-f", "8192", "--mono", "-r", "8000", filename, (char *)NULL);
+	    execl(MPG_123, "mpg123", "-q", "-s", "-b", buffer_size_str, "-f", "8192", "--mono", "-r", sampling_rate_str, "-@", filename, (char *)NULL);
 		/* As a last-ditch effort, try to use PATH */
-	    execlp("mpg123", "mpg123", "-q", "-s", "-b", "1024",  "-f", "8192", "--mono", "-r", "8000", filename, (char *)NULL);
+	    execlp("mpg123", "mpg123", "-q", "-s", "-b", buffer_size_str, "-f", "8192", "--mono", "-r", sampling_rate_str, "-@", filename, (char *)NULL);
+	}
+	else if (!strncasecmp(filename, "http://", 7)) {
+	    char buffer_size_str[8];
+	    snprintf(buffer_size_str, 8, "%u", 6*2*sampling_rate/1000); // 6 seconds for a remote MP3 file
+		/* Most commonly installed in /usr/local/bin */
+	    execl(LOCAL_MPG_123, "mpg123", "-q", "-s", "-b", buffer_size_str, "-f", "8192", "--mono", "-r", sampling_rate_str, filename, (char *)NULL);
+		/* But many places has it in /usr/bin */
+	    execl(MPG_123, "mpg123", "-q", "-s", "-b", buffer_size_str, "-f", "8192", "--mono", "-r", sampling_rate_str, filename, (char *)NULL);
+		/* As a last-ditch effort, try to use PATH */
+	    execlp("mpg123", "mpg123", "-q", "-s", "-b", buffer_size_str, "-f", "8192", "--mono", "-r", sampling_rate_str, filename, (char *)NULL);
 	}
 	else if (strstr(filename, ".m3u")) {
 		/* Most commonly installed in /usr/local/bin */
-	    execl(LOCAL_MPG_123, "mpg123", "-q", "-z", "-s", "-b", "1024", "-f", "8192", "--mono", "-r", "8000", "-@", filename, (char *)NULL);
+	    execl(LOCAL_MPG_123, "mpg123", "-q", "-z", "-s", "-f", "8192", "--mono", "-r", sampling_rate_str, "-@", filename, (char *)NULL);
 		/* But many places has it in /usr/bin */
-	    execl(MPG_123, "mpg123", "-q", "-z", "-s", "-b", "1024","-f", "8192", "--mono", "-r", "8000", "-@", filename, (char *)NULL);
+	    execl(MPG_123, "mpg123", "-q", "-z", "-s", "-f", "8192", "--mono", "-r", sampling_rate_str, "-@", filename, (char *)NULL);
 		/* As a last-ditch effort, try to use PATH */
-	    execlp("mpg123", "mpg123", "-q", "-z", "-s", "-b", "1024",  "-f", "8192", "--mono", "-r", "8000", "-@", filename, (char *)NULL);
+	    execlp("mpg123", "mpg123", "-q", "-z", "-s",  "-f", "8192", "--mono", "-r", sampling_rate_str, "-@", filename, (char *)NULL);
 	}
 	else {
 		/* Most commonly installed in /usr/local/bin */
-	    execl(MPG_123, "mpg123", "-q", "-s", "-f", "8192", "--mono", "-r", "8000", filename, (char *)NULL);
+	    execl(MPG_123, "mpg123", "-q", "-s", "-f", "8192", "--mono", "-r", sampling_rate_str, filename, (char *)NULL);
 		/* But many places has it in /usr/bin */
-	    execl(LOCAL_MPG_123, "mpg123", "-q", "-s", "-f", "8192", "--mono", "-r", "8000", filename, (char *)NULL);
+	    execl(LOCAL_MPG_123, "mpg123", "-q", "-s", "-f", "8192", "--mono", "-r", sampling_rate_str, filename, (char *)NULL);
 		/* As a last-ditch effort, try to use PATH */
-	    execlp("mpg123", "mpg123", "-q", "-s", "-f", "8192", "--mono", "-r", "8000", filename, (char *)NULL);
+	    execlp("mpg123", "mpg123", "-q", "-s", "-f", "8192", "--mono", "-r", sampling_rate_str, filename, (char *)NULL);
 	}
 	/* Can't use ast_log since FD's are closed */
 	fprintf(stderr, "Execute of mpg123 failed\n");
@@ -155,6 +170,9 @@
 	} myf = {
 		.f = { 0, },
 	};
+	struct ast_format * native_format;
+	unsigned int sampling_rate;
+	struct ast_format * write_format;
 
 	if (ast_strlen_zero(data)) {
 		ast_log(LOG_WARNING, "MP3 Playback requires an argument (filename)\n");
@@ -168,15 +186,19 @@
 	
 	ast_stopstream(chan);
 
+	native_format = ast_format_cap_get_format(ast_channel_nativeformats(chan), 0);
+	sampling_rate = ast_format_get_sample_rate(native_format);
+	write_format = ast_format_cache_get_slin_by_rate(sampling_rate);
+
 	owriteformat = ao2_bump(ast_channel_writeformat(chan));
-	res = ast_set_write_format(chan, ast_format_slin);
+	res = ast_set_write_format(chan, write_format);
 	if (res < 0) {
 		ast_log(LOG_WARNING, "Unable to set write format to signed linear\n");
 		return -1;
 	}
 
 	myf.f.frametype = AST_FRAME_VOICE;
-	myf.f.subclass.format = ast_format_slin;
+	myf.f.subclass.format = write_format;
 	myf.f.mallocd = 0;
 	myf.f.offset = AST_FRIENDLY_OFFSET;
 	myf.f.src = __PRETTY_FUNCTION__;
@@ -184,7 +206,7 @@
 	myf.f.delivery.tv_usec = 0;
 	myf.f.data.ptr = myf.frdata;
 	
-	res = mp3play(data, fds[1]);
+	res = mp3play(data, sampling_rate, fds[1]);
 	if (!strncasecmp(data, "http://", 7)) {
 		timeout = 10000;
 	}
@@ -211,7 +233,7 @@
 					res = 0;
 					break;
 				}
-				next = ast_tvadd(next, ast_samp2tv(myf.f.samples, 8000));
+				next = ast_tvadd(next, ast_samp2tv(myf.f.samples, sampling_rate));
 			} else {
 				ms = ast_waitfor(chan, ms);
 				if (ms < 0) {

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

Gerrit-MessageType: merged
Gerrit-Change-Id: Id1ee274733cd804a0edecf7450329b72f1235af0
Gerrit-PatchSet: 3
Gerrit-Project: asterisk
Gerrit-Branch: 14
Gerrit-Owner: Michael Kuron <m.kuron at gmx.de>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>



More information about the asterisk-code-review mailing list