[asterisk-dev] [Code Review] 3721: Masquerades: Framehook and Audiohook fixup

[Jonathan Rose]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3721/#review12507
-----------------------------------------------------------



/branches/12/res/res_fax.c
<https://reviewboard.asterisk.org/r/3721/#comment22774>

    Unnecessary remnant of something I tried.  Removed.


- Jonathan Rose


On July 7, 2014, 5:21 p.m., Jonathan Rose wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3721/
> -----------------------------------------------------------
> 
> (Updated July 7, 2014, 5:21 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 417692 
>   /branches/12/res/res_fax.c 417692 
>   /branches/12/main/framehook.c 417692 
>   /branches/12/main/channel.c 417692 
>   /branches/12/main/bridge_basic.c 417692 
>   /branches/12/main/audiohook.c 417692 
>   /branches/12/include/asterisk/res_fax.h 417692 
>   /branches/12/include/asterisk/framehook.h 417692 
>   /branches/12/include/asterisk/audiohook.h 417692 
>   /branches/12/funcs/func_audiohookinherit.c 417692 
>   /branches/12/bridges/bridge_native_rtp.c 417692 
>   /branches/12/CHANGES 417692 
> 
> 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/20140707/d73c73a8/attachment.html>