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

Ship it!


Ship It!

- Joshua Colp


On Nov. 7, 2014, 3:55 p.m., Matt Jordan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4158/
> -----------------------------------------------------------
> 
> (Updated Nov. 7, 2014, 3:55 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-24489 and ASTERISK-24498
>     https://issues.asterisk.org/jira/browse/ASTERISK-24489
>     https://issues.asterisk.org/jira/browse/ASTERISK-24498
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Asterisk - in res_rtp_asterisk - only understands a single RTCP report info 
> block. When the RTCP information was refactored in the RTP Engine to be 
> pushed over the Stasis message bus, I put in the hooks into the engine to 
> handle multiple RTCP report info blocks, in the hope that a future RTP 
> implementation would be able to provide that data. Unfortunately, 
> res_rtp_asterisk has a tendency to "lie":
> (1) It will send RTCP reports with a reception_report_count greater than 1 
> (which is pulled directly from the RTCP packet itself, so that part is 
> correct)
> (2) It will only provide a single report block
> 
> When the rtp_engine goes to convert this to a JSON blob, hilarity ensues as 
> it looks for a report block that doesn't exist.
> 
> This patch updates the rtp_engine to be a bit more skeptical about what it is 
> presented with. While this could also be fixed in res_rtp_asterisk, I prefer 
> to fix it in the engine for two reasons:
> (1) The engine is designed to work with multiple RTP implementation, and 
> hence having it be more robust is a good thing (tm)
> (2) res_rtp_asterisk's handling of RTCP information is "fun". It should 
> report the correct reception_report_count; ideally it should also be giving 
> us all of the blocks - but it is *definitely* not designed to do that. Going 
> down that road is a non-trivial effort.
> 
> 
> Diffs
> -----
> 
>   /branches/12/main/rtp_engine.c 427539 
> 
> Diff: https://reviewboard.asterisk.org/r/4158/diff/
> 
> 
> Testing
> -------
> 
> Issue reporter tested and verified that they no longer crash.
> 
> 
> Thanks,
> 
> Matt Jordan
> 
>

-- 
_____________________________________________________________________
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Reply via email to