[asterisk-dev] [Code Review]: Allow chan_sip to decline unwanted media streams
opticron
reviewboard at asterisk.org
Wed Jun 13 13:11:28 CDT 2012
> 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>
More information about the asterisk-dev
mailing list