Thank you Mark, That was very helpful and using the ast_monitor_setjoinfiles function does exactly what I want and it's only a single extra line to add to bridge_builtin_features.c to invoke it.
On Mon, Oct 10, 2016 at 11:12 AM, Mark Michelson <mmichel...@digium.com> wrote: > On 10/06/2016 12:52 PM, John Kiniston wrote: > > I got all fired up on getting up to date with the current LTS version of > Asterisk while at Astricon and I'd like to make my first patch. > > > Awesome! Hopefully this will be the first of many to come. > > I want to modify bridge_builtin_features.c to honor the"MONITOR_EXEC" and > "MONITOR_EXEC_ARGS" variables like Monitor() does so that my one touch > recordings using automon get post processed. > > I'm no C programmer but I took a class at the local community college uh I > guess it's been two decades ago now so forgive me. > > I found the code in res/res_monitor.c that handles setting the execute > application and running it and I've copied what looked like the part I > needed into bridges/bridge_builtin_ > features.c and with a few minor changes it almost works. > > The part where it's breaking down is getting the format of the recorded > file which I think should be touch_format I'm getting (null) so I reckon > set_touch_variables doesn't do what I hoped it did. > > Would someone please help me get the file-name extension or point me in a > better direction if this doesn't look like a good idea? > > > I'm going to answer your question in two ways here: first, from the > perspective of helping to understand the code you've written, and second > addressing the "if this doesn't look like a good idea" angle. > > Let's examine what set_touch_variables does. set_touch_variables first > determines which channel variables it needs to be looking at. Assuming > you're using monitor and not mixmonitor, that means that it will look up > the TOUCH_MONITOR_FORMAT, TOUCH_MONITOR, and TOUCH_MONITOR_PREFIX channel > variables. It will place the values of each into the touch_monitor_format, > touch_monitor, and touch_monitor_prefix local variables, respectively. In > your case, it appears that touch_mointor_format ends up NULL, which most > likely stems from the fact that the TOUCH_MONITOR_FORMAT variable is not > set on the channel which was passed into set_touch_variables. So later when > you start outputting your debug, since touch_monitor_format is NULL, the > glibc print routines turn that into "(null)". > > Why don't you see the same thing happen in the res_monitor.c code? In that > code, the ast_monitor_start() function requires a file format to be passed > to it, and that format ends up getting set on the ast_monitor structure > that gets created. That structure then gets set on the channel, and it can > be later accessed by calling ast_channel_monitor(chan). When > ast_monitor_stop() is called, it is able to get that saved format off the > channel and use that for its post-processing. You could potentially do the > same thing to get the file format by getting the value of > ast_channel_monitor(chan)->format in your code. > > So now let's talk about whether your approach is the right one to take. > One thing you may have noticed when doing your experimental coding is that > the code that you copy/pasted comes from ast_monitor_stop(). > ast_monitor_stop() is a C-level API call that is called by several places > in the code, including in bridge_builtin_features.c when one-touch > monitoring is stopped. This means, at least the way I see it, that the use > of MONITOR_EXEC and MONITOR_EXEC_ARGS should already be in a position where > they could "just work" with builtin automon. We can take a closer look at > ast_monitor_stop() and see that the guard for determining if MONITOR_EXEC > and MONITOR_EXEC_ARGS is used is: > > if (ast_channel_monitor(chan)->joinfiles && > !ast_strlen_zero(ast_channel_monitor(chan)->filename_base)) > > If MONITOR_EXEC and MONITOR_EXEC_ARGS is currently not working, it is > likely because one or both of these conditions are not met. Let's examine > them one by one. > > ast_channel_monitor(chan)->joinfiles is a boolean that indicates if files > should be joined together once recording is done. With the Monitor() > application, this gets set true by setting the 'm' option (see > start_monitor_exec() if you want to follow how that is done, specifically > looking at MON_FLAG_MIX). Otherwise, though, this value is false. When > ast_monitor_start() is called, it does not automatically set joinfiles to > true. There is a function, though, called ast_monitor_setjoinfiles() you > can use to set this value to true. > > ast_channel_monitor(chan)->filename_base is the base filename being used > for monitor recordings. "base filename" means the filename minus the extra > bits indicating the audio direction. The only reason this would not be set > is if there were some catastrophic error. ast_strlen_zero() is a utility > function that returns true if the input string is either NULL or > zero-length. So since filename_base is almost certainly more than zero > characters long, it's likely that ast_strlen_zero() of that is false, and > negating that makes it true. > > Summary: > This probably means that you could get the functionality you desire by > calling ast_monitor_setjoinfiles(chan, 1) after ast_monitor_start() in > start_automonitor() of bridge_builtin_features.c. This approach would be > better than copying/pasting because it keeps the code in one spot and makes > use of already tested code. > > Hope this was helpful, > Mark Michelson > > -- > _____________________________________________________________________ > -- Bandwidth and Colocation Provided by http://www.api-digital.com -- > > asterisk-dev mailing list > To UNSUBSCRIBE or update options visit: > http://lists.digium.com/mailman/listinfo/asterisk-dev > -- A human being should be able to change a diaper, plan an invasion, butcher a hog, conn a ship, design a building, write a sonnet, balance accounts, build a wall, set a bone, comfort the dying, take orders, give orders, cooperate, act alone, solve equations, analyze a new problem, pitch manure, program a computer, cook a tasty meal, fight efficiently, die gallantly. Specialization is for insects. ---Heinlein
-- _____________________________________________________________________ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev