[asterisk-dev] [Code Review] 2500: Fix The Payload Being Set On CN Packets And Do Not Set Marker Bit

[Matt Jordan]

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

Ship it!


RFC 3389, section 4 states:

"The RTP header for the comfort noise packet SHOULD be constructed as
   if the comfort noise were an independent codec.  ...  The RTP packet SHOULD NOT
   have the marker bit set."

Nice work!

- Matt Jordan


On May 6, 2013, 3:39 p.m., Michael Young wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2500/
> -----------------------------------------------------------
> 
> (Updated May 6, 2013, 3:39 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-21246
>     https://issues.asterisk.org/jira/browse/ASTERISK-21246
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> When we send out a CN packet (for instance, in the case of using rtpkeepalives), we are not setting the payload code properly.  Also, we are setting the marker bit when we shouldn't be.
> 
> AST_RTP_CN is not defined by AST_FORMAT codes.  Therefore, we should be using ast_rtp_codecs_payload_code() rather than ast_rtp_codecs_payload_lookup().
> 
> 11 and trunk already use the appropriate function.
> 
> If I am understanding the RFC correctly, the marker bit should not be set to on when sending a CN packet.  This part will be fixed in 11 and trunk.
> 
> One last thing, the seqno in the debug message did not correspond with the actual seqno of the packet.  Fixed up that debug message to help with debugging by incrementing the seqno after the debug message is sent.
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/res/res_rtp_asterisk.c 386529 
> 
> Diff: https://reviewboard.asterisk.org/r/2500/diff/
> 
> 
> Testing
> -------
> 
> The reporter tested this patch, providing pcaps and debugging to confirm that the packets are being set correctly.  This also resolved his audio distortion problems since the payload that was being set on the packet was ULAW (instead of CN).   He was using the ALAW codec and unless there was a re-negotiation of codecs, the user would experience audio problems after one of these rtpkeepalives were sent by Asterisk.
> 
> I also tested by looking at pcaps of packets being sent properly on dev machine.
> 
> 
> Thanks,
> 
> Michael Young
> 
>

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