[asterisk-dev] [Code Review] Pimp Sip: Video support

[Joshua Colp]

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



/team/group/pimp_my_sip/channels/chan_gulp.c
<https://reviewboard.asterisk.org/r/2408/#comment15603>

    I'd rather be explicit with this instead of using division voodoo. Easier when reading too.



/team/group/pimp_my_sip/include/asterisk/res_sip_session.h
<https://reviewboard.asterisk.org/r/2408/#comment15604>

    s/negotiated/requested/



/team/group/pimp_my_sip/res/res_sip_sdp_audio.c
<https://reviewboard.asterisk.org/r/2408/#comment15605>

    Erm make this != PJ_SUCCESS



/team/group/pimp_my_sip/res/res_sip_sdp_audio.c
<https://reviewboard.asterisk.org/r/2408/#comment15606>

    It is not guaranteed for this to be NULL terminated. Use ast_copy_pj_str to put this in a temporary buffer.



/team/group/pimp_my_sip/res/res_sip_sdp_audio.c
<https://reviewboard.asterisk.org/r/2408/#comment15607>

    This changes the behavior of things. Codec preference (the order as specified within the configuration file) is no longer obeyed.



/team/group/pimp_my_sip/res/res_sip_sdp_video.c
<https://reviewboard.asterisk.org/r/2408/#comment15609>

    This should do it the way res_sip_sdp_audio does, specifically bind to either wildcard IPv4 or IPv6. ICE support should also exist.
    
    How much difference is there between video and audio besides ptime/hold/unhold? Doesn't look like that much. It may just make the most sense to combine the two, easier to maintain in the end too since they overlap so much.


- Joshua


On March 21, 2013, 11:54 a.m., Kevin Harwell wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2408/
> -----------------------------------------------------------
> 
> (Updated March 21, 2013, 11:54 a.m.)
> 
> 
> Review request for Asterisk Developers and Joshua Colp.
> 
> 
> Summary
> -------
> 
> Adds video media handling support to the new SIP work being done in Asterisk.  Similar to audio added res_sip_sdp_video.c to handle sdp media negotiations for video.  Added capabilities support to make sure the correct codecs were being offered.  Also added the ability to handle the video source update control frame and initiate a fast video update request when needed.
> 
> 
> This addresses bug ASTERISK-21077.
>     https://issues.asterisk.org/jira/browse/ASTERISK-21077
> 
> 
> Diffs
> -----
> 
>   /team/group/pimp_my_sip/channels/chan_gulp.c 383518 
>   /team/group/pimp_my_sip/include/asterisk/res_sip_session.h 383518 
>   /team/group/pimp_my_sip/res/res_sip_sdp_audio.c 383518 
>   /team/group/pimp_my_sip/res/res_sip_sdp_video.c PRE-CREATION 
>   /team/group/pimp_my_sip/res/res_sip_session.c 383518 
> 
> Diff: https://reviewboard.asterisk.org/r/2408/diff
> 
> 
> Testing
> -------
> 
> Using two softphones that support video was able to successfully connect, via asterisk, and view video.  Also monitored sip messages to make sure the appropriate media attributes were being propagated.
> 
> 
> Thanks,
> 
> Kevin
> 
>

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