[asterisk-dev] [Code Review] chan_jingle2: New Jingle + Google Talk channel driver

[Leif Madsen]

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


Just some minor grammatical suggestions. One suggestion in sample configuration to not use 'default' context for end points as the example.


/trunk/configs/jingle2.conf.sample
<https://reviewboard.asterisk.org/r/1917/#comment11389>

    I think you need to s/gmail/Gmail/



/trunk/configs/jingle2.conf.sample
<https://reviewboard.asterisk.org/r/1917/#comment11390>

    s/used/uses/
    And I'd drop the word "very"



/trunk/configs/jingle2.conf.sample
<https://reviewboard.asterisk.org/r/1917/#comment11391>

    s/searched/searched for/



/trunk/configs/jingle2.conf.sample
<https://reviewboard.asterisk.org/r/1917/#comment11392>

    s/transpor/transport/



/trunk/configs/jingle2.conf.sample
<https://reviewboard.asterisk.org/r/1917/#comment11393>

    I don't like "settable" so I'd say:
    
    Outgoing caller id can *not* be set.



/trunk/configs/jingle2.conf.sample
<https://reviewboard.asterisk.org/r/1917/#comment11394>

    Looks like you perhaps meant to add a comment after disallow=all? Or maybe just a spare semi-colon.



/trunk/configs/jingle2.conf.sample
<https://reviewboard.asterisk.org/r/1917/#comment11395>

    I don't think I like that you're using the 'default' context for your end points. You're kind of directing people to allow outgoing calls in the 'default' context, which is not good. Should probably at least use something that isn't defined generally for unauthenticated calls.


- Leif


On May 13, 2012, 12:15 p.m., Joshua Colp wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1917/
> -----------------------------------------------------------
> 
> (Updated May 13, 2012, 12:15 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> This is a new channel driver written from scratch for the Jingle, Google Jingle, and Google Talk protocols. It has been written to the specs available and tested extensively.
> 
> ICE and STUN support for Jingle uses the new ICE/STUN/TURN support which is present in another review. (Please do not review any of that code in this review)
> STUN support for Google uses the existing STUN implementation, as the new support is not compatible with it.
> 
> 
> Diffs
> -----
> 
>   /trunk/channels/chan_jingle2.c PRE-CREATION 
>   /trunk/channels/chan_sip.c 365451 
>   /trunk/configs/jingle2.conf.sample PRE-CREATION 
>   /trunk/configs/rtp.conf.sample 365451 
>   /trunk/include/asterisk/jabber.h 365451 
>   /trunk/include/asterisk/jingle.h 365451 
>   /trunk/include/asterisk/rtp_engine.h 365451 
>   /trunk/main/rtp_engine.c 365451 
>   /trunk/res/Makefile 365451 
>   /trunk/res/res_jabber.c 365451 
>   /trunk/res/res_rtp_asterisk.c 365451 
> 
> Diff: https://reviewboard.asterisk.org/r/1917/diff
> 
> 
> Testing
> -------
> 
> Tested audio calls with following:
> 
> GMail Google Talk Plug-in (and video)
> Google Voice
> Jitsi (and video)
> Psi
> OneTeam
> 
> * Included varying codecs (ulaw, speex, g722, etc)
> 
> Tested ringing, hold, and unhold with following:
> 
> Jitsi
> 
> Other clients do not support this.
> 
> 
> Thanks,
> 
> Joshua
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20120514/eff61039/attachment-0001.htm>