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

[jrose]

> On July 18, 2013, 7:43 p.m., rmudgett wrote:
> > /trunk/main/core_unreal.c, lines 704-707
> > <https://reviewboard.asterisk.org/r/2670/diff/2-3/?file=41376#file41376line704>
> >
> >     chan_callid and owner_callid are not initialized.


> On July 18, 2013, 7:43 p.m., rmudgett wrote:
> > /trunk/main/core_unreal.c, lines 691-700
> > <https://reviewboard.asterisk.org/r/2670/diff/2-3/?file=41376#file41376line691>
> >
> >     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.

So if I have this right... I should have the following:

ast is locked. The pvt is obtained and referenced. ast is unlocked.

With the pvt referenced, I lock it and grab both owner and chan out of the pvt and then unlock the pvt.

Then if I have the bridge call ID, I lock one channel, check for a call ID, if it doesn't have one I set it to the bridge call ID, then unlock that channel and do the same with the other channel.


- jrose


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


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/f3c2bb40/attachment-0001.htm>