[asterisk-dev] [Code Review] sip codec negotiation of dynamic rtp payloads error fix

[Terry Wilson]

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

Ship it!


Commit this puppy.


/branches/1.8/include/asterisk/rtp_engine.h
<https://reviewboard.asterisk.org/r/1169/#comment6920>

    What is a numbe?


- Terry


On 2011-04-12 09:32:38, David Vossel wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1169/
> -----------------------------------------------------------
> 
> (Updated 2011-04-12 09:32:38)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> This patch fixes how chan_sip handles dynamic rtp payload types it does not understand.  At the moment if a dynamic payload's mime type does not match one we understand, the payload does not get removed from our payload table.  As a result of this, the payload is set to whatever dynamic codec we use internally for that payload number on outgoing INVITES.  This is incorrect.
> 
> For example.  Internally we advertise speex at payload 117 on outgoing INVITES.  If unknown codec BLAH is in a incoming INVITE's SDP at dynamic payload 117, we mark in our payload table that 117 exists, and by default we set that to type speex in our internal list.  Later we go back through and set the format for that payload to what it actually is when we parse the "rtpmap" part.  When we don't understand a format type, it never gets removed and often an incorrect format type is sent in response for that payload.
> 
> This patch fixes this by properly checking the rtpmap set function's return code to make sure it was found.  The function can return both -1 and -2 depending on the source of the mismatch.  We were just checking -1 explicitly.
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/channels/chan_sip.c 313187 
>   /branches/1.8/include/asterisk/rtp_engine.h 313187 
>   /branches/1.8/main/rtp_engine.c 313187 
> 
> Diff: https://reviewboard.asterisk.org/r/1169/diff
> 
> 
> Testing
> -------
> 
> Tested patch with Ekiga supporting g726 variants we do not support.  I verified Asterisk now responds correctly.
> 
> 
> Thanks,
> 
> David
> 
>

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