[asterisk-dev] I want to make my first patch but have run into a problem and don't know how to progress.

[John Kiniston]

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 <mmichelson at 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20161011/27422636/attachment.html>