[asterisk-dev] [Code Review] 2531: Implement REST API for playback to chamnel

[Matt Jordan]

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



/team/dlee/res-stasis-cleanup/include/asterisk/stasis_channels.h
<https://reviewboard.asterisk.org/r/2531/#comment16759>

    I'm probably going to lose this merge race, but I have a version of this same function with a slightly different name in the "Refactor random AMI events" review.
    
    (Yay find and replace)



/team/dlee/res-stasis-cleanup/main/stasis_channels.c
<https://reviewboard.asterisk.org/r/2531/#comment16760>

    This error message shouldn't be needed



/team/dlee/res-stasis-cleanup/res/res_stasis_http_events.c
<https://reviewboard.asterisk.org/r/2531/#comment16761>

    I had comments here, then I realized this was auto-generated.
    
    Do we need an issue opened up for validation of passed in parameters?
    
    If we aren't going to validate a parameter (as in this case, where validation doesn't need to happen as no parameters really drive the response result), can we remove the 'validation' portion from the template?



/team/dlee/res-stasis-cleanup/res/res_stasis_playback.c
<https://reviewboard.asterisk.org/r/2531/#comment16762>

    So, I know why you picked this function, but this won't do all that you want to do.
    
    ast_streamfile will support stop (which permanently stops the playback), fastforward, and rewind. It won't support pause or restart, which are treated as stop. Since you have PAUSE as one of the supported operations, that means you probably want the user to be able to toggle the paused state of a playback operation. This limitation of ast_waitstream is essentially because it has nothing in its looping construct to remember where a stream has been stopped.
    
    ast_control_streamfile, on the other hand, provides all of the mechanisms to control a playback operation, including pause/restart. It also allows for direct DTMF control of a channel, although those parameters are optional (pass NULL). You can find its usage in app_controlplayback.


- Matt Jordan


On May 10, 2013, 6:52 p.m., David Lee wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2531/
> -----------------------------------------------------------
> 
> (Updated May 10, 2013, 6:52 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-21283 and ASTERISK-21586
>     https://issues.asterisk.org/jira/browse/ASTERISK-21283
>     https://issues.asterisk.org/jira/browse/ASTERISK-21586
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> (depends on https://reviewboard.asterisk.org/r/2530)
> 
> This patch implements the REST API's for POST /channels/{channelId}/play
> and GET /playback/{playbackId}.
> 
> This allows an external application to initiate playback of a sound on a
> channel while the channel is in the Stasis application.
> 
> /play commands are issued asynchronously, and return immediately with
> the URL of the associated /playback resource. Playback commands queue up,
> playing in succession. The /playback resource shows the state of a
> playback operation as enqueued, playing or complete. (Although the
> operation will only be in the 'complete' state for a very short time,
> since it is almost immediately freed up).
> 
> (closes issue ASTERISK-21283)
> (closes issue ASTERISK-21586)
> Review: https://reviewboard.asterisk.org/r/2531/
> 
> 
> Diffs
> -----
> 
>   /team/dlee/res-stasis-cleanup/include/asterisk/stasis_app_playback.h PRE-CREATION 
>   /team/dlee/res-stasis-cleanup/include/asterisk/stasis_channels.h 388370 
>   /team/dlee/res-stasis-cleanup/include/asterisk/stasis_http.h 388370 
>   /team/dlee/res-stasis-cleanup/main/channel.c 388370 
>   /team/dlee/res-stasis-cleanup/main/channel_internal_api.c 388370 
>   /team/dlee/res-stasis-cleanup/main/stasis_channels.c 388370 
>   /team/dlee/res-stasis-cleanup/res/res_stasis_http.c 388370 
>   /team/dlee/res-stasis-cleanup/res/res_stasis_http_channels.c 388370 
>   /team/dlee/res-stasis-cleanup/res/res_stasis_http_events.c 388370 
>   /team/dlee/res-stasis-cleanup/res/res_stasis_playback.c PRE-CREATION 
>   /team/dlee/res-stasis-cleanup/res/res_stasis_playback.exports.in PRE-CREATION 
>   /team/dlee/res-stasis-cleanup/res/stasis_http/resource_channels.h 388370 
>   /team/dlee/res-stasis-cleanup/res/stasis_http/resource_channels.c 388370 
>   /team/dlee/res-stasis-cleanup/res/stasis_http/resource_events.h 388370 
>   /team/dlee/res-stasis-cleanup/res/stasis_http/resource_playback.c 388370 
>   /team/dlee/res-stasis-cleanup/rest-api/api-docs/channels.json 388370 
>   /team/dlee/res-stasis-cleanup/rest-api/api-docs/events.json 388370 
> 
> Diff: https://reviewboard.asterisk.org/r/2531/diff/
> 
> 
> Testing
> -------
> 
> Poked the playback API manually.
> 
> 
> Thanks,
> 
> David Lee
> 
>

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