[asterisk-dev] [Code Review]: Allow chan_sip to decline unwanted media streams

[opticron]

> On June 13, 2012, 8:52 a.m., Matt Jordan wrote:
> > This seems like a fairly intrusive change for a release branch.  If we're going to make this change, then the SDP_offer_answer test should be passing with no modifications to the test.
> > 
> > If its failing because Asterisk no longer rejects offers in the same fashion, then I'm not sure we should make this change in a release branch.
> 
> Matt Jordan wrote:
>     So, I probably should have read where you were making this change - disregard this comment :-)

I should have stated explicitly that this was a trunk-only change targeted for Asterisk 11.  I'll change the description with the new patch.


> On June 13, 2012, 8:52 a.m., Matt Jordan wrote:
> > trunk/channels/chan_sip.c, line 9325
> > <https://reviewboard.asterisk.org/r/1988/diff/2/?file=28886#file28886line9325>
> >
> >     I'm not a fan of comments directly in a malloc statement.

Fixed.


> On June 13, 2012, 8:52 a.m., Matt Jordan wrote:
> > trunk/channels/chan_sip.c, lines 9323-9328
> > <https://reviewboard.asterisk.org/r/1988/diff/2/?file=28886#file28886line9323>
> >
> >     I'm not sure we should be building decline information every time we have an offer.  I would expect that we would only build decline information for an offer if Asterisk can't handle that offer.
> >     
> >     This finding applies to the other media types as well.

This is particularly hard to predict with only the information available in process_sdp().  The relevant logic is in add_sdp() and depends on two parameters (add_audio and add_t38) as well as information from the pvt which has not yet been determined in process_sdp().  Video also depends on directmedia-related information.


- opticron


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


On June 12, 2012, 9:56 p.m., opticron wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1988/
> -----------------------------------------------------------
> 
> (Updated June 12, 2012, 9:56 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> This modification stores known SDP streams in an AST_LIST in the sip pvt, replacing the static offered_media array so that port-zeroed version of each stream can be stored and used when rebuilding the SDP response.  This also makes messages and comments more consistent.
> 
> 
> Diffs
> -----
> 
>   trunk/channels/chan_sip.c 368849 
>   trunk/channels/sip/include/sip.h 368849 
> 
> Diff: https://reviewboard.asterisk.org/r/1988/diff
> 
> 
> Testing
> -------
> 
> Ran this through the testsuite and the only thing that failed was SDP_offer_answer (which was expected).
> 
> 
> Thanks,
> 
> opticron
> 
>

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