[asterisk-dev] [Code Review] 2670: ARI: Bridge Playback and Record functions

[rmudgett]

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



/trunk/main/core_unreal.c
<https://reviewboard.asterisk.org/r/2670/#comment18090>

    Please get the callid from the bridge with the bridge locked.



/trunk/main/core_unreal.c
<https://reviewboard.asterisk.org/r/2670/#comment18091>

    Move this code out of the block.  It does not need to be done while ast is locked.  It should be done while p is locked.



/trunk/main/core_unreal.c
<https://reviewboard.asterisk.org/r/2670/#comment18094>

    chan_callid and owner_callid are not initialized.



/trunk/include/asterisk/logger.h
<https://reviewboard.asterisk.org/r/2670/#comment18093>

    Remove this cast.  It prevents any type safety if c is used in an immediate assignment.


- rmudgett


On July 17, 2013, 8:43 p.m., jrose wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2670/
> -----------------------------------------------------------
> 
> (Updated July 17, 2013, 8:43 p.m.)
> 
> 
> Review request for Asterisk Developers, David Lee, kmoore, Matt Jordan, and rmudgett.
> 
> 
> Bugs: ASTERISK-21592 and ASTERISK-21593
>     https://issues.asterisk.org/jira/browse/ASTERISK-21592
>     https://issues.asterisk.org/jira/browse/ASTERISK-21593
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> The diff has become somewhat wide reaching, but this review primarily takes care of the following:
> 
> 1. Provides a new channel driver for creating unreal channels with specific roles already applied to them and a mechanism for shoving the ;2 end of that channel into a bridge
> 2. Provides Playback and Record on Bridge ARI functions which make use of that channel
> 
> In the process of working on this a few bugs got in the way and this review takes care of those as well.
> 
> 1. Confbridge announcer channels have a reference leak and don't get destroyed on leaving the bridge
> 2. Numerous ARI function documentation items were flawed and others didn't match the intended implementation and had to be tweaked.
> 
> 
> Diffs
> -----
> 
>   /trunk/channels/chan_bridge_media.c PRE-CREATION 
>   /trunk/include/asterisk/channel.h 394203 
>   /trunk/include/asterisk/core_unreal.h 394203 
>   /trunk/include/asterisk/logger.h 394203 
>   /trunk/include/asterisk/stasis_app.h 394203 
>   /trunk/include/asterisk/stasis_app_playback.h 394203 
>   /trunk/main/channel.c 394203 
>   /trunk/main/core_unreal.c 394203 
>   /trunk/res/res_stasis.c 394203 
>   /trunk/res/res_stasis_http_bridges.c 394203 
>   /trunk/res/res_stasis_http_channels.c 394203 
>   /trunk/res/res_stasis_http_playback.c 394203 
>   /trunk/res/res_stasis_playback.c 394203 
>   /trunk/res/stasis/control.c 394203 
>   /trunk/res/stasis_http/ari_model_validators.h 394203 
>   /trunk/res/stasis_http/ari_model_validators.c 394203 
>   /trunk/res/stasis_http/resource_bridges.h 394203 
>   /trunk/res/stasis_http/resource_bridges.c 394203 
>   /trunk/res/stasis_http/resource_channels.c 394203 
>   /trunk/rest-api/api-docs/bridges.json 394203 
>   /trunk/rest-api/api-docs/channels.json 394203 
>   /trunk/rest-api/api-docs/playback.json 394203 
>   /trunk/rest-api/api-docs/recordings.json 394203 
> 
> Diff: https://reviewboard.asterisk.org/r/2670/diff/
> 
> 
> Testing
> -------
> 
> All ARI playback and recording functions on channels were tested to make sure they pass validation after the changes I've made
> All ARI playback and recording functions on bridges were tested for functionality in softmix bridges
> 
> I also did some rudimentary tests to see that memory wasn't leaking in the files I touched under these normal conditions. It's hardly exhaustive, but it's a start.
> 
> 
> Thanks,
> 
> jrose
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20130718/8ea1d22e/attachment-0001.htm>