[asterisk-dev] [RFC] Audiohooks

[Joshua Colp]

----- "Kaloyan Kovachev" <kkovachev at varna.net> wrote:

> On Sun, 1 Mar 2009 18:58:12 -0600, Tilghman Lesher wrote
> > On Sunday 01 March 2009 06:35:52 Kaloyan Kovachev wrote:
> > > Hello list,
> > >  there are two volume audiohooks: inside the audiohooks.c which is
> still
> > > unused (i think) and another one in function volume. The
> difference is that
> > > the function allows the volume to be adjusted during the call
> (not
> > > specified in the description) with '*' and '#'.
> > >  I am about to try and remove the duplication, but i would like
> get some
> > > comments on the direction:

The volume audiohook common API is actually there for the upcoming app_confbridge. It needed to be
able to adjust the volume and making this code common and available for other purposes seemed to make sense.

> > >   1. Just remove the audiohook from main as it is not used
> > >   2. Leave the audiohook in main making it available to all users
> of
> > > audiohooks.h and just call it from the function
> > >   3. Convert the function to res_volume together with the
> audiohook and add
> > > cli and manager commands
> > >
> > > My personal vote is for 3. so it can be used as optional from
> other modules
> > > too, but as it will take more time and changes, wouldn't want to
> do it, if
> > > it is unacceptable/wrong.
> > >
> > > If 3. is the way to go, the same can be done for the wishper_file
> too, by
> > > leaving in the audiohooks.c only the API and not adding a bunch of
> hooks
> > > there that someone may (not) need
> > 
> > I'd go with 4:  add an API to manipulate those values to
> audiohook.h
> > implemented in audiohook.c and adjust func_volume to point to that
> API.
> This is what i meant with 2. actually - sorry if i wasn't clear.
> 
> > If you wanted to add manager commands, I'd do so directly in
> audiohook.c,
> > with no need for a resource module.  I'd also avoid the CLI command,
> as it is
> > of limited usefulness.
> > 
> 
>  The disadvantage here is that audiohook.c will contain all the
> audiohooks
> (not talking for just volume here, but also about possibly many
> future
> audiohooks) and will create all the manager commands even if one does
> not
> need/want them, while with a resource module they can be optional
> (thinking of
> like AGI is for app_stack) or mandatory for other modules.

I'm actually fine with having them be in the core EXCEPT I would want to draw the boundary
on lower level implementation being in the core and then additional modules using that lower
level API to do things. For example for manager commands I would want to see those be in a
loadable module.

> May be an example of 3. will make it clearer what i am thinking of:
>   res_volume - implements the audiohook API only (and optionaly AMI
> command)
>   func_volume - only calls that api for a _simple_ volume
> increase/decrease or
> status.
>   app_volume - in addition to what func_volume does, allows setting
> the keys
> to adjust the volume during the call separately for caller or callee
> via
> additional options.
> 
> but instead of having 3 files i meant to put everything in res_volume
> 

I don't think you need a second module to do the more advanced DTMF changing of volume. It should
just be possible to attach a second audiohook to just get DTMF and use the audiohook_volume_* stuff
to increase/decrease it. That can also continue to live in func_volume.

-- 
Joshua Colp
Digium, Inc. | Software Developer
445 Jan Davis Drive NW - Huntsville, AL 35806 - USA
Check us out at:  www.digium.com  & www.asterisk.org