Review Board 1.7.16


New dialplan function to allow for inheritance of audiohooks

Review Request #102 - Created Dec. 18, 2008 and submitted

Mark Michelson
/trunk
13538
Reviewers
asterisk-dev
Asterisk
It has been a problem for a while now that MixMonitor does not continue to record a conversation if the channel on which the MixMonitor is attached is masqueraded (the most common case being a transfer). MixMonitor is not the only audiohook source that does not propagate to new channels during a masquerade, and so I made the dialplan function generic enough to allow for any audiohook source to be transferred properly.

This patch introduces a new dialplan function called AUDIOHOOK_INHERIT. The basic mechanism is that if one declares an audiohook from a specific source as being inheritable, then the audiohook will move to the new channel structure during a masquerade. I have included example usage of the function in the XML documentation in the file.

One point to double check in this is the behavior change I made to ast_do_masquerade. I changed the datastore processing a bit by calling the fixup function only on the datastores on the clone channel and doing so before moving them onto the original channel. As far as I can tell, this actually makes more sense than how it was done previously, but I'd like to know if this could cause troubles.

I know of one limitation right now, which is that only one audiohook of a given source may be moved to another channel during a masquerade. So if someone had things set up so that there were two MixMonitors on a channel, and that channel got masqueraded into a new one, only the first MixMonitor audiohook found while traversing the list of audiohooks would be moved. At this point, the effort required to accommodate moving multiple audiohooks greatly exceeds the need to implement it, so I am holding back unless it is deemed necessary.
I have tested by using MixMonitor on a channel which gets masqueraded into a new channel as a result of transferring. I have tried both attended and blind transfers, both with the channel as the caller and callee. The result was that the recording continued across the transfer.

I also did some testing where I set a ChanSpy audiohook on a channel which is masqueraded. I specifically tested this to be sure that the datastore fixup done in app_chanspy would not interfere with the datastore fixup being done in func_audihookinherit. Things worked as expected.
Review request changed
Updated (Dec. 19, 2008, 7:36 a.m.)
Made changes suggested by Russell in his review
Ship it!
Posted (Dec. 19, 2008, 7:42 a.m.)
Aside from these minor formatting suggestions, this looks good to go.  Nice job.
/trunk/funcs/func_audiohookinherit.c (Diff revision 4)
 
 
:%s/\s\+$//g before commit
/trunk/funcs/func_audiohookinherit.c (Diff revision 4)
 
 
While this hasn't really been followed in the past, the coding guidelines mention in passing that lines should be limited to 90 columns in length.  I would really like to start following that and am encouraging people to start doing it in new code.
/trunk/funcs/func_audiohookinherit.c (Diff revision 4)
 
 
\retval 0 success
\retval non-zero failure
Posted (Dec. 19, 2008, 7:49 a.m.)
I have made the specific changes in the working copy. Now that there is a "ship it!" attached to this review, I want to pose a new question. Should this feature be backported to 1.4?

In some people's view, this fixes a bug, even though development-wise, it meant adding a new feature. Since the development here was completely external to existing code (except for the small change to ast_do_masquerade), I think that it would be easy to backport the change and would not cause problems if we did. Opinions?
  1. Yes, I think this should go in 1.4.  I do feel that it fixes a bug.  It is a reasonable expectation for MixMonitor to carry over on a masquerade, given that the long standing Monitor() functionality works this way.

https://reviewboard.asterisk.org/ runs on a server provided by Digium, Inc. and uses bandwidth donated to the open source Asterisk community by API Digital Communications in Huntsville, AL USA.
Please report problems with this site to asteriskteam@digium.com.