[asterisk-dev] [Code Review] 1.4 implementation of fix for issue 12434

[Mark Michelson]

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



/branches/1.4/channels/chan_sip.c
<https://reviewboard.asterisk.org/r/311/#comment2339>

    As I was writing the trunk version of this patch, I noticed that the codecs variable gets set twice in the when parsing a video offer. It's not actually harming anything, but it is redundant. I plan to remove the redundant assignment, but I don't feel it's worth uploading a new review just for this.


- Mark


On 2009-07-15 16:40:05, Mark Michelson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/311/
> -----------------------------------------------------------
> 
> (Updated 2009-07-15 16:40:05)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> In issue 12434, the reporter describes a situation in which audio and video is offered on the call, but because videosupport is disabled in sip.conf, Asterisk gives no response at all to the video offer. According to RFC 3264, all media offers should have a corresponding answer. For offers we do not intend to actually reply to with meaningful values, we should still reply with the port for the media stream set to 0.
> 
> In this patch, we take note of what types of media have been offered and save the information on the sip_pvt. The SDP in the response will take into account whether media was offered. If we are not otherwise going to answer a media offer, we will insert an appropriate m= line with the port set to 0.
> 
> It is important to note that this patch is pretty much a bandage being applied to a broken bone. The patch *only* helps for situations where video is offered but videosupport is disabled and when udptl_pt is disabled but T.38 is offered. Asterisk is not guaranteed to respond to every media offer. Notable cases are when multiple streams of the same type are offered. The 2 media stream limit is still present with this patch, too.
> 
> In trunk and the 1.6.X branches, things will be a bit different since Asterisk also supports text in SDPs as well. Also, since T.38 negotiation is different in those branches, further testing of T.38-specific code will be needed there. I will post a trunk version of these changes to reviewboard after I have written the code.
> 
> 
> This addresses bug 12434.
>     https://issues.asterisk.org/view.php?id=12434
> 
> 
> Diffs
> -----
> 
>   /branches/1.4/channels/chan_sip.c 206706 
> 
> Diff: https://reviewboard.asterisk.org/r/311/diff
> 
> 
> Testing
> -------
> 
> I set up 2 sipp scenarios. One offered audio and video, and the other offered audio and T.38. I verified that Asterisk gave always gave an appropriate answer to the video and T.38 offers.
> 
> 
> Thanks,
> 
> Mark
> 
>