[asterisk-dev] [Code Review] 3427: ARI: Add tones playback resource

[Jonathan Rose]

> On April 9, 2014, 4:20 p.m., Matt Jordan wrote:
> > /trunk/main/app.c, lines 1056-1060
> > <https://reviewboard.asterisk.org/r/3427/diff/2/?file=57141#file57141line1056>
> >
> >     This would be an appropriate place for a ternary operator:
> >     
> >     ast_playtones_start(chan, 0, ts ? ts->data : tone, 0);
> >     
> >     You also need to check for success/failure here. If the start function fails, bail immediately.

Also added bails on failure for restart and suspend (resume). I think that's appropriate anyway.


> On April 9, 2014, 4:20 p.m., Matt Jordan wrote:
> > /trunk/res/res_stasis_playback.c, lines 50-66
> > <https://reviewboard.asterisk.org/r/3427/diff/2/?file=57142#file57142line50>
> >
> >     Update the play operation documentation in channels and bridges json files with the new URI schema that is supported.

I've added tone: to the list of supported URIs shown on the play actions docs. There doesn't appear to be anything on those that actually describes how each URI works in detail unfortunately. If there is somewhere this is documented and I'm just not aware of it, I can add notes on how tone URIs work to that. If not, it might be prudent to get a wiki page on media URIs started.


> On April 9, 2014, 4:20 p.m., Matt Jordan wrote:
> > /trunk/main/app.c, lines 1067-1069
> > <https://reviewboard.asterisk.org/r/3427/diff/2/?file=57141#file57141line1067>
> >
> >     I'm not sure you should be specifying a full second here. If I send the channel a control stream stop/restart/etc. frame, I don't want to wait a second before you process it.
> >     
> >     This should probably use ast_waitfor(chan, -1) and respond to any frame.

So from my experimenting, the use of playback controls on the tones appeared to trigger the effects fairly instantaneously including when I did several restart operations in the course of less than a second. If I'm not mistaken, a control frame being queued on the channel will actually interrupt the ast_waitfor function returning the remaining time that it is intended to wait for... which is why we just continue if it's 0 since that means it was an uninterrupted wait.

That said, I guess there isn't anything wrong with an indefinite wait.


- Jonathan


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


On April 8, 2014, 5:46 p.m., Jonathan Rose wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3427/
> -----------------------------------------------------------
> 
> (Updated April 8, 2014, 5:46 p.m.)
> 
> 
> Review request for Asterisk Developers, David Lee, Joshua Colp, and Matt Jordan.
> 
> 
> Bugs: ASTERISK-23433
>     https://issues.asterisk.org/jira/browse/ASTERISK-23433
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Adds a tones URI type to the playback resource. The tone can be specified by name (from indications.conf) or by a tone pattern (comma separate pitch/duration list). Tones aren't like regular sounds in that they must be canceled manually before the control can move on to the next item in the queue.
> 
> Tones are capable of being paused and resumed (although they will always resumed from the beginning of the tone), restarted, and stopped.  Tones are not capable of being fastforwarded, skipped into by a duration, or rewound by a small amount. Those operations unfortunately report success rather than a lack of availability right now due to how control on playbacks is defined (a playback is either completely controllable or not). I could probably add a little more granularity to that if we want it.
> 
> 
> Diffs
> -----
> 
>   /trunk/res/res_stasis_playback.c 411884 
>   /trunk/main/app.c 411884 
>   /trunk/include/asterisk/app.h 411884 
>   /trunk/CHANGES 411884 
> 
> Diff: https://reviewboard.asterisk.org/r/3427/diff/
> 
> 
> Testing
> -------
> 
> I've written two testsuite tests (one for channels, one for bridges) which queue and stop tones with playback. I'll be posting them before too long.  I've also performed all the basic control operations by hand.
> 
> 
> Thanks,
> 
> Jonathan Rose
> 
>

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