[asterisk-dev] [Code Review] Mute / Unmute manager commands for MixMonitor

[Julian Lyndon-Smith]

> On 2010-02-12 07:36:04, Russell Bryant wrote:
> > There are some common things going on in this patch and res_mutestream.c.  We should be able to simplify everything by trying to make sure we're using as much of the common code as possible for each feature.

Mmm, way above my paygrade :)


> On 2010-02-12 07:36:04, Russell Bryant wrote:
> > trunk/main/audiohook.c, lines 166-171
> > <https://reviewboard.asterisk.org/r/487/diff/10/?file=8075#file8075line166>
> >
> >     There is a function called ast_frame_clear() in res_mutestream.c that does this same type of thing.  We should move that to main/frame.c, add it to frame.h, and use it here, as well.

Ok, I'll have a look


> On 2010-02-12 07:36:04, Russell Bryant wrote:
> > trunk/apps/app_mixmonitor.c, line 682
> > <https://reviewboard.asterisk.org/r/487/diff/10/?file=8073#file8073line682>
> >
> >     This function is defined to be able to return an error.  Should you check for it?

yup. Done.


> On 2010-02-12 07:36:04, Russell Bryant wrote:
> > trunk/main/audiohook.c, lines 1051-1059
> > <https://reviewboard.asterisk.org/r/487/diff/10/?file=8075#file8075line1051>
> >
> >     I think the channel lock should be held until you're done modifying the audiohook.

Done


- Julian


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/487/#review1501
-----------------------------------------------------------


On 2010-02-24 11:05:21, Julian Lyndon-Smith wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/487/
> -----------------------------------------------------------
> 
> (Updated 2010-02-24 11:05:21)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> For PCI-DSS compliance we are not allowed to record a credit card number in 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).
> 
> 
> This addresses bug 16740.
>     https://issues.asterisk.org/view.php?id=16740
> 
> 
> Diffs
> -----
> 
>   trunk/apps/app_mixmonitor.c 244767 
>   trunk/include/asterisk/audiohook.h 244767 
>   trunk/include/asterisk/frame.h 244767 
>   trunk/main/audiohook.c 244767 
>   trunk/main/frame.c 244767 
>   trunk/res/res_mutestream.c 244767 
> 
> Diff: https://reviewboard.asterisk.org/r/487/diff
> 
> 
> Testing
> -------
> 
> Tested on 1.4 : Tested muting each direction, and both directions, and unmuting, and then listening to the mixmonitor file. The mutes and unmutes all seemed to be in the right place at the right time
> Tested on trunk : Tested muting each direction, and both directions, and unmuting, and then listening to the mixmonitor file. The mutes and unmutes all seemed to be in the right place at the right time
> 
> 
> Thanks,
> 
> Julian
> 
>