[asterisk-dev] [Code Review] 3379: ARI: Make bridges/{bridgeId}/play queue sound files if sounds are already playing on the bridge instead of playing them simultaneously as they are called

[Mark Michelson]

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



/branches/12/res/ari/resource_bridges.h
<https://reviewboard.asterisk.org/r/3379/#comment21314>

    There's no need to put \brief for one-line doxygen comments. Doxygen automatically treats these as brief summaries.



/branches/12/res/ari/resource_bridges.c
<https://reviewboard.asterisk.org/r/3379/#comment21315>

    The pointer check for playback_url here is incorrect. According to the man page for asprintf:
    
    "If memory allocation wasn't possible, or some other error occurs, these functions will return -1,  and the contents of strp is undefined."
    
    srtp, in that sentence, is the first parameter to asprintf. The proper way to check for allocation failure with asprintf is to see if it returns -1 since the pointer you passed in will be in an undefined state after a failure.



/branches/12/res/res_stasis.c
<https://reviewboard.asterisk.org/r/3379/#comment21312>

    Since you're not passing any flags in, may as well just call ao2_link() here.


- Mark Michelson


On March 28, 2014, 8:14 p.m., Jonathan Rose wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3379/
> -----------------------------------------------------------
> 
> (Updated March 28, 2014, 8:14 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-22677
>     https://issues.asterisk.org/jira/browse/ASTERISK-22677
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Previously, if you played an audio file and then played another before the first finished, the second audio file would start playing immediately as it was called overlapping the previous sound. Apparently people don't like that. This patch changes that behavior so that the sound will be queued at the end of any existing controls if they are running.
> 
> 
> Diffs
> -----
> 
>   /branches/12/rest-api/api-docs/bridges.json 411187 
>   /branches/12/res/stasis/control.c 411187 
>   /branches/12/res/stasis/control.h 411187 
>   /branches/12/res/res_stasis_playback.c 411187 
>   /branches/12/res/res_stasis.c 411187 
>   /branches/12/res/res_ari_bridges.c 411187 
>   /branches/12/res/ari/resource_bridges.c 411187 
>   /branches/12/res/ari/resource_bridges.h 411187 
>   /branches/12/include/asterisk/stasis_app.h 411187 
>   /branches/12/CHANGES 411187 
> 
> Diff: https://reviewboard.asterisk.org/r/3379/diff/
> 
> 
> Testing
> -------
> 
> Tested for playback channel wrapper leaks, tested to make sure control objects were being destroyed when they fell out of use.  Tested playing of a single file. Tested playing of multiple files in a row. Tested playing of multiple files in a row and then after a sequence finished, playing additional files so that new channels would have to be created. Tested playing sounds right as other sounds were concluding. I wasn't able to break it (although I wouldn't be surprised if there is a possible condition where you can grab a control as it is finishing up its queue and then attempting to add a sound to a finished queue causing the playback to fail. I don't think this would break things in a profound way, it just might possibly make one sound fail to queue under extremely unlikely conditions).
> 
> 
> Thanks,
> 
> Jonathan Rose
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140410/3ee53163/attachment-0001.html>