[asterisk-dev] [Code Review] Revert contantssrc change in favor of a more focused fix

[Kevin Fleming]

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


Well... there is, in fact, a way to do exactly what you want. ast_rtp_read() can return a linked-list of two frames, the first being a control frame of AST_CONTROL_SRCUPDATE, and the second being the actual media frame it was going to return initially. ast_read() will then queue the media frame and return the SRCUPDATE frame to its caller.

Now as far as when this should be done: the ONLY time we should send SRCUPDATE is when the media source has actually changed. Just because we got a re-INVITE (on or off hold) that is not an indication of a source change. We should send SRCUPDATE when the RTP stack notices an SSRC change in the incoming media, and when we change the channel that a channel is bridged to. I can't really think of any other times we should send SRCUPDATE, although I'm sure there could be some.

- Kevin


On 2010-03-04 17:29:00, Terry Wilson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/540/
> -----------------------------------------------------------
> 
> (Updated 2010-03-04 17:29:00)
> 
> 
> Review request for Asterisk Developers and Russell Bryant.
> 
> 
> Summary
> -------
> 
> This change basically reverts the change reviewed in https://reviewboard.asterisk.org/r/374/ and instead limits the updating of the RTP synchronization source to only those times when we detect that the other side of the conversation has changed the ssrc.
> 
> The problem is that SRCUPDATE control frames are sent many times where we don't want a new ssrc, including whenever Asterisk has to send DTMF in a normal bridge. This is also not the first time that this mistake has been made. The initial implementation of the ast_rtp_new_source function also changed the ssrc--and then it was removed because of this same issue. Then, we put it back in again to fix a different issue. Instead, I think we should make a very limited change and just update the ssrc when we see it change.
> 
> This issue affects mantis issues 5413 (srtp doesn't like the ssrc changing), 15642 (sonus dtmf), 16292 (Exchange UM dtmf), and probably 16438 and 16714.
> 
> 
> Diffs
> -----
> 
>   /trunk/channels/chan_sip.c 244504 
>   /trunk/include/asterisk/rtp_engine.h 244504 
>   /trunk/res/res_rtp_asterisk.c 244504 
> 
> Diff: https://reviewboard.asterisk.org/r/540/diff
> 
> 
> Testing
> -------
> 
> So far...it compiles. I'm trying to devise a simple way to replicate the original issue which was a reinvite sent to asterisk which results in an ssrc change in the rtp sent to asterisk.
> 
> 
> Thanks,
> 
> Terry
> 
>