[asterisk-dev] [Code Review] 3721: Masquerades: Framehook and Audiohook fixup
Jonathan Rose
reviewboard at asterisk.org
Thu Jul 17 16:02:31 CDT 2014
> On July 17, 2014, 3:33 p.m., rmudgett wrote:
> > /branches/12/funcs/func_audiohookinherit.c, lines 95-96
> > <https://reviewboard.asterisk.org/r/3721/diff/3/?file=64757#file64757line95>
> >
> > ... result of a small ...
> >
> > Extra space between "the original"
It's a trap! It's actually a tab and in my editor (4 spaces per tab), it just happens to have occupied just the right position for it to be visually identical to a space. :(
- Jonathan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3721/#review12729
-----------------------------------------------------------
On July 17, 2014, 3:10 p.m., Jonathan Rose wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3721/
> -----------------------------------------------------------
>
> (Updated July 17, 2014, 3:10 p.m.)
>
>
> Review request for Asterisk Developers, Joshua Colp, Matt Jordan, Mark Michelson, and rmudgett.
>
>
> Repository: Asterisk
>
>
> Description
> -------
>
> It involves masquerades, so you know it's perilous. It involves FAX, so you know it's maddening.
>
> The idea is fairly fairly simple at its core. During a masquerade, audiohooks and framehooks will now be moved automatically (unless explicitly set not to via a value in the framehook interface). Framehooks are a little more complicated and some are simply not inheritable while others may require some kind of fixup (none currently use this. The framehooks in res_fax ended up getting a little too wierd to work well with the framehook fixup, so its fixup had to be handled as part of the datastore fixup function.)
>
> It might be more appropriate to do this as an Asterisk 13 patch. I'm uncertain right now. There are some crashes resolved by this (including https://reviewboard.asterisk.org/r/3603/ which Corey Farrell is currently trying to solve for 11 and maybe for earlier versions as well), but it's a moderately big change and it likely has some potential to break things.
>
>
> Diffs
> -----
>
> /branches/12/res/res_pjsip_refer.c 418716
> /branches/12/res/res_fax.c 418716
> /branches/12/main/framehook.c 418716
> /branches/12/main/channel.c 418716
> /branches/12/main/bridge_basic.c 418716
> /branches/12/main/audiohook.c 418716
> /branches/12/include/asterisk/res_fax.h 418716
> /branches/12/include/asterisk/framehook.h 418716
> /branches/12/include/asterisk/audiohook.h 418716
> /branches/12/funcs/func_audiohookinherit.c 418716
> /branches/12/bridges/bridge_native_rtp.c 418716
> /branches/12/CHANGES 418716
>
> Diff: https://reviewboard.asterisk.org/r/3721/diff/
>
>
> Testing
> -------
>
> Tested masquerades on channels containing every different type of framehook. Tested masquerades on channels containing every type of audiohook except for app_jack (extended support and I haven't every used it, so I would need to dig into its dependencies... plus none of the audiohooks so far have needed any special attention anyway and app_jack looks less likely too than some of the others).
>
> For everything with obvious audio impact (volume gain, mixmonitors, pitch shift, chanspy etc), I also checked that audio kept going through as you would expect it to. I also tested multiple audiohooks and multiple framehooks for inheritance. Audiohooks were tested for multiple inheritance of the same type (previously unavailable with the AUDIOHOOK_INHERIT function)
>
>
> Thanks,
>
> Jonathan Rose
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140717/ace3b7b5/attachment-0001.html>
More information about the asterisk-dev
mailing list