[Home]

Summary:ASTERISK-15552: [patch] New mute feature for MixMonitor
Reporter:jmls (jmls)Labels:
Date Opened:2010-01-31 03:28:08.000-0600Date Closed:2010-04-21 07:41:08
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Applications/app_mixmonitor
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) mute.trunk.diff
( 1) mute1.4.diff
Description:for PCI-DSS compliance we are not allowed to record a credit card number is a MixMonitor file. However, we must record all conversations ....

I have added a new feature to audiohooks so that you can mute either read / write or both types of frames - this allows for MixMonitor to mute either side of the conversation without affecting the conversation itself.

MixMonitor now has two manager commands

1) manager show command MuteMixMonitor
Action: MuteMixMonitor
Synopsis: Mute a channel in MixMonitor
Privilege: <none>
Description: Mutes a Mixmonitor Channel.
Variables:
 Channel: Channel to mute.
 Direction: Which part to mute. read|write|both (from channel|to channel|both channels).

2)  manager show command unMuteMixMonitor
Action: unMuteMixMonitor
Synopsis: unMute a channel in MixMonitor
Privilege: <none>
Description: unMutes a Mixmonitor Channel.
Variables:
 Channel: Channel to unmute.
 Direction: Which part to unmute. read|write|both (from channel|to channel|both channels).




****** ADDITIONAL INFORMATION ******

please find attached 2 patches, for 1.4 svn trunk and for asterisk trunk

I have tested all three scenarios on 1.4 (read|write|both) for mute and unmute and listened to the recording, it seems to work just fine.

I have not been able to test the trunk version just yet
Comments:By: snuffy (snuffy) 2010-01-31 18:07:04.000-0600

Nice work jmls,

Probably worth posting on the reviewboard.
Just two quick things, in mute_mixmonitor on the error path you should be returning '1' not '0'.
You will need to change docs to XML style for trunk and eventual inclusion of the feature into trunk
By: jmls (jmls) 2010-02-01 04:10:41.000-0600

Thanks snuffy. Updated the documentation, but looking at the "1" vs "0" I notice that a little further up is this code snippet:

if (ast_strlen_zero(args.filename)) {
               ast_log(LOG_WARNING, "MixMonitor requires an argument (filename)\n");
               return -1;
       }

So, should I be returning -1, or 1 if there is an error ?
By: jmls (jmls) 2010-02-02 15:00:33.000-0600

do I need to submit this to code review panel, or does it find it's own way ? Just keen to see it get accepted into the main code base

;)
By: snuffy (snuffy) 2010-02-02 16:16:23.000-0600

jmls, upload the trunk version to the code review site.

Then it can be picked @ by vultures that want to improve your code :)
By: jmls (jmls) 2010-02-03 04:21:42.000-0600

uploaded new diff, after suggestions from review board
By: jmls (jmls) 2010-02-03 08:40:49.000-0600

uploaded new diff, after more suggestions from review board
By: jmls (jmls) 2010-02-03 14:10:40.000-0600

tabs instead of spaces. I think that I have it now ..
By: jmls (jmls) 2010-02-03 16:36:43.000-0600

/me just wants to cry now.
By: jmls (jmls) 2010-02-03 17:02:43.000-0600

snuff-work: i think u now apply for coding guideline completeness
By: jmls (jmls) 2010-04-21 06:39:25

latest (and last) patch attached. This is now merged into trunk
By: Digium Subversion (svnbot) 2010-04-21 07:41:07

Repository: asterisk
Revision: 258190

U   trunk/apps/app_mixmonitor.c
U   trunk/include/asterisk/audiohook.h
U   trunk/include/asterisk/frame.h
U   trunk/main/audiohook.c
U   trunk/main/frame.c
U   trunk/res/res_mutestream.c

------------------------------------------------------------------------
r258190 | jmls | 2010-04-21 06:27:27 -0500 (Wed, 21 Apr 2010) | 13 lines

Added MixMonitorMute manager command

Added a new manager command to mute/unmute MixMonitor audio on a channel.
Added a new feature to audiohooks so that you can mute either read / write
(or both) types of frames - this allows for MixMonitor to mute either side
of the conversation without affecting the conversation itself.

(closes issue ASTERISK-15552)
Reported by: jmls

Review: https://reviewboard.asterisk.org/r/487/


------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=258190