> On April 10, 2014, 1:44 p.m., Mark Michelson wrote:
> > /branches/12/res/ari/resource_bridges.h, lines 282-292
> > <https://reviewboard.asterisk.org/r/3379/diff/5/?file=56855#file56855line282>
> >
> >     There's no need to put \brief for one-line doxygen comments. Doxygen 
> > automatically treats these as brief summaries.

This one's on the automatic documentation generation. We aren't supposed to 
touch these headers directly.


> On April 10, 2014, 1:44 p.m., Mark Michelson wrote:
> > /branches/12/res/ari/resource_bridges.c, lines 378-384
> > <https://reviewboard.asterisk.org/r/3379/diff/5/?file=56856#file56856line378>
> >
> >     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.

Hmmm, this is probably a problem in a fairly sizable number of places. This 
code was just moved for this update.


- Jonathan


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


On March 28, 2014, 3: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, 3: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
> 
>

-- 
_____________________________________________________________________
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Reply via email to