[Asterisk-code-review] app_mixmonitor: Turn on synchronization by default (asterisk[master])

[Kevin Harwell]

Kevin Harwell has submitted this change. ( https://gerrit.asterisk.org/c/asterisk/+/13809 )

Change subject: app_mixmonitor: Turn on synchronization by default
......................................................................

app_mixmonitor: Turn on synchronization by default

The optional synchronization behavior created in
64906c4c9ba63e18f2c71310fdbf14450dac7b62 is now the default for
MixMonitor.

* Add a new flag 'n' that allows for this behavior to be turned off

* Add a notice when the 'S' option is used indicating that it is no
  longer necessary

Change-Id: I158987c475cda4e1ff1256dd0daccdd99df568b4
---
M apps/app_mixmonitor.c
A doc/UPGRADE-staging/app_mixmonitor_sync_default.txt
2 files changed, 26 insertions(+), 8 deletions(-)

Approvals:
  George Joseph: Looks good to me, but someone else must approve
  Joshua Colp: Looks good to me, but someone else must approve
  Kevin Harwell: Looks good to me, approved; Approved for Submit



diff --git a/apps/app_mixmonitor.c b/apps/app_mixmonitor.c
index 6746c47..16c2cd2 100644
--- a/apps/app_mixmonitor.c
+++ b/apps/app_mixmonitor.c
@@ -115,10 +115,10 @@
 						Like with the basic filename argument, if an absolute path isn't given, it will create
 						the file in the configured monitoring directory.</para>
 					</option>
-					<option name="S">
-						<para>When combined with the <replaceable>r</replaceable> or <replaceable>t</replaceable>
-						option, inserts silence when necessary to maintain synchronization between the receive
-						and transmit audio streams.</para>
+					<option name="n">
+						<para>When the <replaceable>r</replaceable> or <replaceable>t</replaceable> option is
+						used, MixMonitor will insert silence into the specified files to maintain
+						synchronization between them. Use this option to disable that behavior.</para>
 					</option>
 					<option name="i">
 						<argument name="chanvar" required="true" />
@@ -353,7 +353,8 @@
 	MUXFLAG_BEEP = (1 << 11),
 	MUXFLAG_BEEP_START = (1 << 12),
 	MUXFLAG_BEEP_STOP = (1 << 13),
-	MUXFLAG_RWSYNC = (1 << 14),
+	MUXFLAG_DEPRECATED_RWSYNC = (1 << 14),
+	MUXFLAG_NO_RWSYNC = (1 << 15),
 };
 
 enum mixmonitor_args {
@@ -365,7 +366,8 @@
 	OPT_ARG_UID,
 	OPT_ARG_VMRECIPIENTS,
 	OPT_ARG_BEEP_INTERVAL,
-	OPT_ARG_RWSYNC,
+	OPT_ARG_DEPRECATED_RWSYNC,
+	OPT_ARG_NO_RWSYNC,
 	OPT_ARG_ARRAY_SIZE,	/* Always last element of the enum */
 };
 
@@ -382,7 +384,8 @@
 	AST_APP_OPTION_ARG('t', MUXFLAG_WRITE, OPT_ARG_WRITENAME),
 	AST_APP_OPTION_ARG('i', MUXFLAG_UID, OPT_ARG_UID),
 	AST_APP_OPTION_ARG('m', MUXFLAG_VMRECIPIENTS, OPT_ARG_VMRECIPIENTS),
-	AST_APP_OPTION_ARG('S', MUXFLAG_RWSYNC, OPT_ARG_RWSYNC),
+	AST_APP_OPTION_ARG('S', MUXFLAG_DEPRECATED_RWSYNC, OPT_ARG_DEPRECATED_RWSYNC),
+	AST_APP_OPTION_ARG('n', MUXFLAG_NO_RWSYNC, OPT_ARG_NO_RWSYNC),
 });
 
 struct mixmonitor_ds {
@@ -970,7 +973,7 @@
 	}
 
 	ast_set_flag(&mixmonitor->audiohook, AST_AUDIOHOOK_TRIGGER_SYNC);
-	if ((ast_test_flag(mixmonitor, MUXFLAG_RWSYNC))) {
+	if (!ast_test_flag(mixmonitor, MUXFLAG_NO_RWSYNC)) {
 		ast_set_flag(&mixmonitor->audiohook, AST_AUDIOHOOK_SUBSTITUTE_SILENCE);
 	}
 
@@ -1049,6 +1052,11 @@
 
 		ast_app_parse_options(mixmonitor_opts, &flags, opts, args.options);
 
+		if (ast_test_flag(&flags, MUXFLAG_DEPRECATED_RWSYNC)) {
+			ast_log(LOG_NOTICE, "The synchronization behavior enabled by the 'S' option is now the default"
+				" and does not need to be specified.\n");
+		}
+
 		if (ast_test_flag(&flags, MUXFLAG_READVOLUME)) {
 			if (ast_strlen_zero(opts[OPT_ARG_READVOLUME])) {
 				ast_log(LOG_WARNING, "No volume level was provided for the heard volume ('v') option.\n");
diff --git a/doc/UPGRADE-staging/app_mixmonitor_sync_default.txt b/doc/UPGRADE-staging/app_mixmonitor_sync_default.txt
new file mode 100644
index 0000000..33a55e1
--- /dev/null
+++ b/doc/UPGRADE-staging/app_mixmonitor_sync_default.txt
@@ -0,0 +1,10 @@
+Subject: app_mixmonitor
+Master-Only: true
+
+In Asterisk 13.29, a new option flag was added to MixMonitor (the 'S'
+option) that when combined with the r() or t() options would inject
+silence into these files if audio was going to be written to one and
+not that other. This allowed the files specified by r() and t() to
+subsequently be mixed outside of Asterisk and be appropriately
+synchronized. This behavior is now the default, and a new option has
+been added to disable this behavior if desired (the 'n' option).

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

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Change-Id: I158987c475cda4e1ff1256dd0daccdd99df568b4
Gerrit-Change-Number: 13809
Gerrit-PatchSet: 2
Gerrit-Owner: Sean Bright <sean.bright at gmail.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20200227/3cde5273/attachment-0001.html>