[Asterisk-code-review] res musiconhold: Remove 'special' MP3 modes (asterisk[master])
Sean Bright
asteriskteam at digium.com
Wed Jul 12 13:33:15 CDT 2017
Sean Bright has uploaded this change for review. ( https://gerrit.asterisk.org/5995
Change subject: res_musiconhold: Remove 'special' MP3 modes
......................................................................
res_musiconhold: Remove 'special' MP3 modes
The modes mp3, mp3nb, quietmp3, and quietmp3nb are specializations of
the 'custom' mode that could just as easily be configured through
musiconhold.conf.
Remove the hard-coded mpg123 checks and add examples to the sample
configuration file showing how to replicate the removed modes.
Change-Id: I865fbee781c36f9cf9f6b5076f95f60b34fa7d64
---
M UPGRADE.txt
M configs/samples/musiconhold.conf.sample
M res/res_musiconhold.c
3 files changed, 135 insertions(+), 150 deletions(-)
git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/95/5995/1
diff --git a/UPGRADE.txt b/UPGRADE.txt
index eb05b03..f9793cf 100644
--- a/UPGRADE.txt
+++ b/UPGRADE.txt
@@ -32,6 +32,13 @@
ARI. As a result, the 'DataGet' AMI action as well as the 'data get'
CLI command have been removed.
+Music On Hold:
+ - The built-in modes - mp3, mp3nb, quietmp3, and quietmp3nb - have been
+ removed in favor of the 'custom' mode. These four modes were internally
+ launching mpg123 with varying command line arguments, all of which is
+ achievable using the 'custom' mode directly. See musiconhold.conf.sample
+ for examples of how to mimic the behavior of the removed modes.
+
From 14.4.0 to 14.5.0:
Core:
diff --git a/configs/samples/musiconhold.conf.sample b/configs/samples/musiconhold.conf.sample
index 741bde6..c263ca5 100644
--- a/configs/samples/musiconhold.conf.sample
+++ b/configs/samples/musiconhold.conf.sample
@@ -13,10 +13,6 @@
; valid mode options:
; files -- read files from a directory in any Asterisk supported
; media format
-; quietmp3 -- default
-; mp3 -- loud
-; mp3nb -- unbuffered
-; quietmp3nb -- quiet unbuffered
; custom -- run a custom application (See examples below)
; =========
@@ -120,3 +116,30 @@
;kill_escalation_delay=500
; Send signals to just the child process instead of all descendants
;kill_method=process
+
+;
+; In Asterisk 15, some of the built-in modes (mp3, mp3nb, quietmp3, and
+; quietmp3nb) were removed in favor of the 'custom' mode. The following example
+; classes demonstrate how to configure a music class to mimic the same behavior
+; as the removed modes:
+;
+; [mp3] ; Loud, buffered
+; mode = custom
+; directory = /var/lib/asterisk/mohmp3
+; application = /usr/bin/mpg123 -q -s --mono -r 8000 -f 8192
+;
+; [quietmp3] ; Quiet, buffered
+; mode = custom
+; directory = /var/lib/asterisk/mohmp3
+; application = /usr/bin/mpg123 -q -s --mono -r 8000 -f 4096
+;
+; [mp3nb] ; Loud, unbuffered
+; mode = custom
+; directory = /var/lib/asterisk/mohmp3
+; application = /usr/bin/mpg123 -q -s --mono -r 8000 -b 2048 -f 8192
+;
+; [quietmp3nb] ; Quiet, unbuffered
+; mode = custom
+; directory = /var/lib/asterisk/mohmp3
+; application = /usr/bin/mpg123 -q -s --mono -r 8000 -b 2048 -f 4096
+;
diff --git a/res/res_musiconhold.c b/res/res_musiconhold.c
index e4bb7a2..fcf3563 100644
--- a/res/res_musiconhold.c
+++ b/res/res_musiconhold.c
@@ -141,17 +141,15 @@
char save_pos_filename[PATH_MAX];
};
-#define MOH_QUIET (1 << 0)
-#define MOH_SINGLE (1 << 1)
-#define MOH_CUSTOM (1 << 2)
-#define MOH_RANDOMIZE (1 << 3)
-#define MOH_SORTALPHA (1 << 4)
+#define MOH_CUSTOM (1 << 0)
+#define MOH_RANDOMIZE (1 << 1)
+#define MOH_SORTALPHA (1 << 2)
#define MOH_RANDSTART (MOH_RANDOMIZE | MOH_SORTALPHA) /*!< Sorted but start at random position */
-#define MOH_SORTMODE (3 << 3)
+#define MOH_SORTMODE (3 << 1)
-#define MOH_CACHERTCLASSES (1 << 5) /*!< Should we use a separate instance of MOH for each user or not */
-#define MOH_ANNOUNCEMENT (1 << 6) /*!< Do we play announcement files between songs on this channel? */
-#define MOH_PREFERCHANNELCLASS (1 << 7) /*!< Should queue moh override channel moh */
+#define MOH_CACHERTCLASSES (1 << 3) /*!< Should we use a separate instance of MOH for each user or not */
+#define MOH_ANNOUNCEMENT (1 << 4) /*!< Do we play announcement files between songs on this channel? */
+#define MOH_PREFERCHANNELCLASS (1 << 5) /*!< Should queue moh override channel moh */
/* Custom astobj2 flag */
#define MOH_NOTDELETED (1 << 30) /*!< Find only records that aren't deleted? */
@@ -209,9 +207,7 @@
static struct ao2_container *mohclasses;
-#define LOCAL_MPG_123 "/usr/local/bin/mpg123"
-#define MPG_123 "/usr/bin/mpg123"
-#define MAX_MP3S 256
+#define MAX_FILE_COUNT 256
static void moh_parse_options(struct ast_variable *var, struct mohclass *mohclass);
static int reload(void);
@@ -548,136 +544,106 @@
.write_format_change = moh_files_write_format_change,
};
-static int spawn_mp3(struct mohclass *class)
+#define IS_URL(x) (!(strncasecmp(x, "http://", sizeof("http://") - 1) \
+ && strncasecmp(x, "https://", sizeof("https://") - 1)))
+
+static int external_audio_proc_spawn(struct mohclass *class)
{
int fds[2];
- int files = 0;
- char fns[MAX_MP3S][80];
- char *argv[MAX_MP3S + 50];
+ char fns[MAX_FILE_COUNT][80];
+ char *argv[MAX_FILE_COUNT + 50];
char xargs[256];
char *argptr;
int argc = 0;
- DIR *dir = NULL;
- struct dirent *de;
+ int is_real_directory = 0;
-
+ /* Format arguments for argv vector */
+ ast_copy_string(xargs, class->args, sizeof(xargs));
+ argptr = xargs;
+ while (!ast_strlen_zero(argptr)) {
+ argv[argc++] = argptr;
+ strsep(&argptr, " ");
+ }
+
if (!strcasecmp(class->dir, "nodir")) {
- files = 1;
+ /* Nothing to do */
+ } else if (IS_URL(class->dir)) {
+ ast_copy_string(fns[0], class->dir, sizeof(fns[0]));
+ argv[argc++] = fns[0];
} else {
- dir = opendir(class->dir);
- if (!dir && strncasecmp(class->dir, "http://", 7)) {
- ast_log(LOG_WARNING, "%s is not a valid directory\n", class->dir);
+ int files = 0;
+ struct dirent *de;
+ DIR *dir = opendir(class->dir);
+
+ if (!dir) {
+ ast_log(LOG_WARNING, "Cannot access '%s': %s\n", class->dir, strerror(errno));
return -1;
}
- }
- if (!ast_test_flag(class, MOH_CUSTOM)) {
- argv[argc++] = "mpg123";
- argv[argc++] = "-q";
- argv[argc++] = "-s";
- argv[argc++] = "--mono";
- argv[argc++] = "-r";
- argv[argc++] = "8000";
-
- if (!ast_test_flag(class, MOH_SINGLE)) {
- argv[argc++] = "-b";
- argv[argc++] = "2048";
- }
-
- argv[argc++] = "-f";
-
- if (ast_test_flag(class, MOH_QUIET))
- argv[argc++] = "4096";
- else
- argv[argc++] = "8192";
-
- /* Look for extra arguments and add them to the list */
- ast_copy_string(xargs, class->args, sizeof(xargs));
- argptr = xargs;
- while (!ast_strlen_zero(argptr)) {
- argv[argc++] = argptr;
- strsep(&argptr, ",");
- }
- } else {
- /* Format arguments for argv vector */
- ast_copy_string(xargs, class->args, sizeof(xargs));
- argptr = xargs;
- while (!ast_strlen_zero(argptr)) {
- argv[argc++] = argptr;
- strsep(&argptr, " ");
- }
- }
-
- if (!strncasecmp(class->dir, "http://", 7)) {
- ast_copy_string(fns[files], class->dir, sizeof(fns[files]));
- argv[argc++] = fns[files];
- files++;
- } else if (dir) {
- while ((de = readdir(dir)) && (files < MAX_MP3S)) {
- if ((strlen(de->d_name) > 3) &&
- ((ast_test_flag(class, MOH_CUSTOM) &&
- (!strcasecmp(de->d_name + strlen(de->d_name) - 4, ".raw") ||
- !strcasecmp(de->d_name + strlen(de->d_name) - 4, ".sln"))) ||
- !strcasecmp(de->d_name + strlen(de->d_name) - 4, ".mp3"))) {
+ while ((de = readdir(dir)) && (files < MAX_FILE_COUNT)) {
+ if (strlen(de->d_name) > 3 &&
+ (ast_ends_with(de->d_name, ".raw") ||
+ ast_ends_with(de->d_name, ".sln") ||
+ ast_ends_with(de->d_name, ".mp3"))) {
ast_copy_string(fns[files], de->d_name, sizeof(fns[files]));
argv[argc++] = fns[files];
files++;
}
}
- }
- argv[argc] = NULL;
- if (dir) {
+
closedir(dir);
+
+ if (!files) {
+ ast_log(LOG_WARNING, "Found no files in '%s'\n", class->dir);
+ return -1;
+ }
+
+ is_real_directory = 1;
}
- if (pipe(fds)) {
- ast_log(LOG_WARNING, "Pipe failed\n");
+
+ argv[argc] = NULL;
+
+ if (pipe(fds)) {
+ ast_log(LOG_WARNING, "pipe() failed: %s\n", strerror(errno));
return -1;
}
- if (!files) {
- ast_log(LOG_WARNING, "Found no files in '%s'\n", class->dir);
- close(fds[0]);
- close(fds[1]);
- return -1;
- }
- if (!strncasecmp(class->dir, "http://", 7) && time(NULL) - class->start < respawn_time) {
+
+ if (IS_URL(class->dir) && time(NULL) - class->start < respawn_time) {
sleep(respawn_time - (time(NULL) - class->start));
}
time(&class->start);
+
class->pid = ast_safe_fork(0);
if (class->pid < 0) {
+ ast_log(LOG_WARNING, "fork() failed: %s\n", strerror(errno));
close(fds[0]);
close(fds[1]);
- ast_log(LOG_WARNING, "Fork failed: %s\n", strerror(errno));
return -1;
}
- if (!class->pid) {
- if (ast_opt_high_priority)
+
+ if (class->pid == 0) {
+ /* Child */
+ if (ast_opt_high_priority) {
ast_set_priority(0);
+ }
close(fds[0]);
- /* Stdout goes to pipe */
+
+ /* stdout goes to pipe */
dup2(fds[1], STDOUT_FILENO);
/* Close everything else */
ast_close_fds_above_n(STDERR_FILENO);
- /* Child */
- if (strncasecmp(class->dir, "http://", 7) && strcasecmp(class->dir, "nodir") && chdir(class->dir) < 0) {
+ if (is_real_directory && chdir(class->dir) < 0) {
ast_log(LOG_WARNING, "chdir() failed: %s\n", strerror(errno));
_exit(1);
}
setpgid(0, getpid());
- if (ast_test_flag(class, MOH_CUSTOM)) {
- execv(argv[0], argv);
- } else {
- /* Default install is /usr/local/bin */
- execv(LOCAL_MPG_123, argv);
- /* Many places have it in /usr/bin */
- execv(MPG_123, argv);
- /* Check PATH as a last-ditch effort */
- execvp("mpg123", argv);
- }
+
+ execv(argv[0], argv);
+
/* Can't use logger, since log FDs are closed */
fprintf(stderr, "MOH: exec failed: %s\n", strerror(errno));
close(fds[1]);
@@ -686,6 +652,7 @@
/* Parent */
close(fds[1]);
}
+
return fds[0];
}
@@ -734,7 +701,7 @@
}
}
-static void *monmp3thread(void *data)
+static void *external_audio_proc_monitor(void *data)
{
#define MOH_MS_INTERVAL 100
@@ -749,10 +716,10 @@
deadline.tv_usec = 0;
for(;/* ever */;) {
pthread_testcancel();
- /* Spawn mp3 player if it's not there */
+ /* Spawn audio player if it's not there */
if (class->srcfd < 0) {
- if ((class->srcfd = spawn_mp3(class)) < 0) {
- ast_log(LOG_WARNING, "Unable to spawn mp3player\n");
+ if ((class->srcfd = external_audio_proc_spawn(class)) < 0) {
+ ast_log(LOG_WARNING, "Unable to spawn external audio process\n");
/* Try again later */
sleep(500);
continue;
@@ -767,7 +734,7 @@
/* Pause some amount of time */
if (ast_poll(&pfd, 1, -1) > 0) {
if (ast_timer_ack(class->timer, 1) < 0) {
- ast_log(LOG_ERROR, "Failed to acknowledge timer for mp3player\n");
+ ast_log(LOG_ERROR, "Failed to acknowledge timer for external audio process\n");
return NULL;
}
/* 25 samples per second => 40ms framerate => 320 samples */
@@ -800,7 +767,7 @@
if ((strncasecmp(class->dir, "http://", 7) && strcasecmp(class->dir, "nodir")) && AST_LIST_EMPTY(&class->members))
continue;
- /* Read mp3 audio */
+ /* Read audio */
len = ast_format_determine_length(class->format, res);
if ((res2 = read(class->srcfd, sbuf, len)) != len) {
@@ -1263,31 +1230,36 @@
return 0;
}
+static struct ast_timer *timer_init(void)
+{
+ struct ast_timer *timer = ast_timer_open();
+
+ if (!timer) {
+ ast_log(LOG_WARNING, "Unable to create timer: %s\n", strerror(errno));
+ return NULL;
+ }
+
+ if (ast_timer_set_rate(timer, 25)) {
+ ast_log(LOG_WARNING, "Unable to set 40ms frame rate: %s\n", strerror(errno));
+ ast_timer_close(timer);
+ return NULL;
+ }
+
+ return timer;
+}
+
static int init_app_class(struct mohclass *class)
{
- if (!strcasecmp(class->mode, "custom")) {
- ast_set_flag(class, MOH_CUSTOM);
- } else if (!strcasecmp(class->mode, "mp3nb")) {
- ast_set_flag(class, MOH_SINGLE);
- } else if (!strcasecmp(class->mode, "quietmp3nb")) {
- ast_set_flag(class, MOH_SINGLE | MOH_QUIET);
- } else if (!strcasecmp(class->mode, "quietmp3")) {
- ast_set_flag(class, MOH_QUIET);
- }
+ ast_set_flag(class, MOH_CUSTOM);
class->srcfd = -1;
+ class->timer = timer_init();
- if (!(class->timer = ast_timer_open())) {
- ast_log(LOG_WARNING, "Unable to create timer: %s\n", strerror(errno));
+ if (!class->timer) {
return -1;
}
- if (class->timer && ast_timer_set_rate(class->timer, 25)) {
- ast_log(LOG_WARNING, "Unable to set 40ms frame rate: %s\n", strerror(errno));
- ast_timer_close(class->timer);
- class->timer = NULL;
- }
- if (ast_pthread_create_background(&class->thread, NULL, monmp3thread, class)) {
+ if (ast_pthread_create_background(&class->thread, NULL, external_audio_proc_monitor, class)) {
ast_log(LOG_WARNING, "Unable to create moh thread...\n");
if (class->timer) {
ast_timer_close(class->timer);
@@ -1331,9 +1303,7 @@
}
return -1;
}
- } else if (!strcasecmp(moh->mode, "mp3") || !strcasecmp(moh->mode, "mp3nb") ||
- !strcasecmp(moh->mode, "quietmp3") || !strcasecmp(moh->mode, "quietmp3nb") ||
- !strcasecmp(moh->mode, "httpmp3") || !strcasecmp(moh->mode, "custom")) {
+ } else if (!strcasecmp(moh->mode, "custom")) {
if (init_app_class(moh)) {
if (unref) {
moh = mohclass_unref(moh, "unreffing potential new moh class (init_app_class_failed)");
@@ -1519,26 +1489,11 @@
}
ast_set_flag(mohclass, MOH_RANDOMIZE);
}
- } else if (!strcasecmp(mohclass->mode, "mp3") || !strcasecmp(mohclass->mode, "mp3nb") || !strcasecmp(mohclass->mode, "quietmp3") || !strcasecmp(mohclass->mode, "quietmp3nb") || !strcasecmp(mohclass->mode, "httpmp3") || !strcasecmp(mohclass->mode, "custom")) {
-
- if (!strcasecmp(mohclass->mode, "custom"))
- ast_set_flag(mohclass, MOH_CUSTOM);
- else if (!strcasecmp(mohclass->mode, "mp3nb"))
- ast_set_flag(mohclass, MOH_SINGLE);
- else if (!strcasecmp(mohclass->mode, "quietmp3nb"))
- ast_set_flag(mohclass, MOH_SINGLE | MOH_QUIET);
- else if (!strcasecmp(mohclass->mode, "quietmp3"))
- ast_set_flag(mohclass, MOH_QUIET);
+ } else if (!strcasecmp(mohclass->mode, "custom")) {
+ ast_set_flag(mohclass, MOH_CUSTOM);
mohclass->srcfd = -1;
- if (!(mohclass->timer = ast_timer_open())) {
- ast_log(LOG_WARNING, "Unable to create timer: %s\n", strerror(errno));
- }
- if (mohclass->timer && ast_timer_set_rate(mohclass->timer, 25)) {
- ast_log(LOG_WARNING, "Unable to set 40ms frame rate: %s\n", strerror(errno));
- ast_timer_close(mohclass->timer);
- mohclass->timer = NULL;
- }
+ mohclass->timer = timer_init();
/* Let's check if this channel already had a moh class before */
if (state && state->class) {
@@ -1550,7 +1505,7 @@
mohclass = mohclass_ref(state->class, "using existing class from state");
}
} else {
- if (ast_pthread_create_background(&mohclass->thread, NULL, monmp3thread, mohclass)) {
+ if (ast_pthread_create_background(&mohclass->thread, NULL, external_audio_proc_monitor, mohclass)) {
ast_log(LOG_WARNING, "Unable to create moh...\n");
if (mohclass->timer) {
ast_timer_close(mohclass->timer);
@@ -1656,7 +1611,7 @@
tbytes = tbytes + bytes;
}
- ast_debug(1, "mpg123 pid %d and child died after %d bytes read\n",
+ ast_debug(1, "audio pid %d and child died after %d bytes read\n",
class->pid, tbytes);
class->pid = 0;
--
To view, visit https://gerrit.asterisk.org/5995
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I865fbee781c36f9cf9f6b5076f95f60b34fa7d64
Gerrit-Change-Number: 5995
Gerrit-PatchSet: 1
Gerrit-Owner: Sean Bright <sean.bright at gmail.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20170712/b2241c59/attachment-0001.html>
More information about the asterisk-code-review
mailing list