[asterisk-dev] [Code Review] 2668: ARI/bridges: Implement POST /bridge/{id}/play

[opticron]

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



/trunk/res/stasis_http/resource_bridges.c
<https://reviewboard.asterisk.org/r/2668/#comment18004>

    This is redundant and is handled by find_bridge() above.



/trunk/res/stasis_http/resource_bridges.c
<https://reviewboard.asterisk.org/r/2668/#comment18005>

    You should use stasis_http_response_alloc_failed() here.



/trunk/res/stasis_http/resource_bridges.c
<https://reviewboard.asterisk.org/r/2668/#comment18006>

    Input validation should be handled by the auto-generated code. See maxDurationSeconds in channel.json for an example.



/trunk/res/stasis_http/resource_bridges.c
<https://reviewboard.asterisk.org/r/2668/#comment18011>

    Given the checks above that will be moved to auto-generated input validation, this can only be an allocation error and may as well use stasis_http_response_alloc_failed().



/trunk/res/stasis_http/resource_bridges.c
<https://reviewboard.asterisk.org/r/2668/#comment18007>

    You should use stasis_http_response_alloc_failed() here.



/trunk/res/stasis_http/resource_bridges.c
<https://reviewboard.asterisk.org/r/2668/#comment18008>

    Idem.



/trunk/res/stasis_http/resource_bridges.c
<https://reviewboard.asterisk.org/r/2668/#comment18009>

    Idem.



/trunk/res/stasis_http/resource_bridges.c
<https://reviewboard.asterisk.org/r/2668/#comment18010>

    Idem.


- opticron


On July 10, 2013, 5:12 p.m., jrose wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2668/
> -----------------------------------------------------------
> 
> (Updated July 10, 2013, 5:12 p.m.)
> 
> 
> Review request for Asterisk Developers, David Lee, kmoore, Matt Jordan, and rmudgett.
> 
> 
> Bugs: ASTERISK-21592
>     https://issues.asterisk.org/jira/browse/ASTERISK-21592
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> This works in the following manner:
> 
> On receiving the request, we first try to grab a reference to the bridge being played to in stasis. If it exists, we proceed.
> 
> From here, a new type of unreal channel (core_announcer) is created. The ;2 part of this channel is then put into the bridge while the ;1 part is left dangling for a little while. For the rest of the playback starting operations, if anything fails this channel will be hungup.
> 
> Next a control structure is created for the new channel. This is one place where I think there might be contention on how things should be done...
> 
> Most of the rest of this process work in the same way as playback on a channel with the ;1 of the announcer channel being the target of playback. Once the playback is successfully queued, things start to deviate again.
> 
> With all of that in place, a new thread is spun off which will take care of executing the stasis_app_control pool until it is empty. If the thread is created successfully, we will succeed and put out a reply with a playback control object which can then be used to manipulate the playback as it runs.
> 
> Back on the other thread, the control queue will have all of its operations run continuously until the queue is empty at which point the channel will be hung up and the control object disposed of.
> 
> 
> Diffs
> -----
> 
>   /trunk/apps/confbridge/conf_chan_announce.c 393998 
>   /trunk/include/asterisk/_private.h 393998 
>   /trunk/include/asterisk/bridging.h 393998 
>   /trunk/include/asterisk/channel.h 393998 
>   /trunk/include/asterisk/core_announcer.h PRE-CREATION 
>   /trunk/include/asterisk/stasis_app.h 393998 
>   /trunk/include/asterisk/stasis_app_playback.h 393998 
>   /trunk/main/asterisk.c 393998 
>   /trunk/main/bridging.c 393998 
>   /trunk/main/channel.c 393998 
>   /trunk/main/core_announcer.c PRE-CREATION 
>   /trunk/res/res_stasis.c 393998 
>   /trunk/res/res_stasis_http_bridges.c 393998 
>   /trunk/res/res_stasis_http_playback.c 393998 
>   /trunk/res/res_stasis_playback.c 393998 
>   /trunk/res/stasis/control.c 393998 
>   /trunk/res/stasis_http/resource_bridges.h 393998 
>   /trunk/res/stasis_http/resource_bridges.c 393998 
>   /trunk/res/stasis_http/resource_channels.c 393998 
>   /trunk/rest-api/api-docs/bridges.json 393998 
>   /trunk/rest-api/api-docs/playback.json 393998 
> 
> Diff: https://reviewboard.asterisk.org/r/2668/diff/
> 
> 
> Testing
> -------
> 
> Tested playback with channels put into a stasis bridge by a stasis application.  In the process I hammered out a few validation bugs. I also made sure no allocations were left over in the bridging resource or the announcer and unreal channel drivers. That is hardly exhaustive though, as the actual places where objects are being allocated for this process are numerous.
> 
> 
> Thanks,
> 
> jrose
> 
>

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