[asterisk-dev] [Code Review] app_swift - streaming text-to-speech interface for the Cepstral Swift engine

[Darren Sessions]

> On 2010-06-03 13:44:39, pabelanger wrote:
> > Good work, plan to give this a try.  Some formatting comments below.  Specifically, the 'red blobs' should be removed.

Red blobs are gone.

:)


> On 2010-06-03 13:44:39, pabelanger wrote:
> > /trunk/apps/app_swift.c, line 37
> > <https://reviewboard.asterisk.org/r/691/diff/1/?file=10562#file10562line37>
> >
> >     Path should not be hardcoded.

fixed.


> On 2010-06-03 13:44:39, pabelanger wrote:
> > /trunk/apps/app_swift.c, lines 164-166
> > <https://reviewboard.asterisk.org/r/691/diff/1/?file=10562#file10562line164>
> >
> >     C++ comments (//) should be changed to /* */

done


> On 2010-06-03 13:44:39, pabelanger wrote:
> > /trunk/apps/app_swift.c, line 168
> > <https://reviewboard.asterisk.org/r/691/diff/1/?file=10562#file10562line168>
> >
> >     same as above.

done


> On 2010-06-03 13:44:39, pabelanger wrote:
> > /trunk/apps/app_swift.c, line 174
> > <https://reviewboard.asterisk.org/r/691/diff/1/?file=10562#file10562line174>
> >
> >     Would you not need to ASTOBJ_UNLOCK(ps) here?

added


> On 2010-06-03 13:44:39, pabelanger wrote:
> > /trunk/apps/app_swift.c, line 225
> > <https://reviewboard.asterisk.org/r/691/diff/1/?file=10562#file10562line225>
> >
> >     dtmf_search_counter += 1;

done


> On 2010-06-03 13:44:39, pabelanger wrote:
> > /trunk/apps/app_swift.c, line 245
> > <https://reviewboard.asterisk.org/r/691/diff/1/?file=10562#file10562line245>
> >
> >     i += 1;

done


> On 2010-06-03 13:44:39, pabelanger wrote:
> > /trunk/apps/app_swift.c, line 247
> > <https://reviewboard.asterisk.org/r/691/diff/1/?file=10562#file10562line247>
> >
> >     wouldn't break; be more appropriate?

added break and removed the loop variable


> On 2010-06-03 13:44:39, pabelanger wrote:
> > /trunk/apps/app_swift.c, line 373
> > <https://reviewboard.asterisk.org/r/691/diff/1/?file=10562#file10562line373>
> >
> >     /* */ comments, or to be removed?

done


> On 2010-06-03 13:44:39, pabelanger wrote:
> > /trunk/apps/app_swift.c, line 378
> > <https://reviewboard.asterisk.org/r/691/diff/1/?file=10562#file10562line378>
> >
> >     same as above.

done


> On 2010-06-03 13:44:39, pabelanger wrote:
> > /trunk/apps/app_swift.c, line 420
> > <https://reviewboard.asterisk.org/r/691/diff/1/?file=10562#file10562line420>
> >
> >     space before and after /

done


> On 2010-06-03 13:44:39, pabelanger wrote:
> > /trunk/apps/app_swift.c, line 449
> > <https://reviewboard.asterisk.org/r/691/diff/1/?file=10562#file10562line449>
> >
> >     Same as above.

done


> On 2010-06-03 13:44:39, pabelanger wrote:
> > /trunk/apps/app_swift.c, lines 462-466
> > <https://reviewboard.asterisk.org/r/691/diff/1/?file=10562#file10562line462>
> >
> >     You should be able to change this code to:
> >     
> >          }
> >     ast_ffree(f);

done


> On 2010-06-03 13:44:39, pabelanger wrote:
> > /trunk/apps/app_swift.c, lines 489-490
> > <https://reviewboard.asterisk.org/r/691/diff/1/?file=10562#file10562line489>
> >
> >     Same as above.

removed


> On 2010-06-03 13:44:39, pabelanger wrote:
> > /trunk/apps/app_swift.c, line 476
> > <https://reviewboard.asterisk.org/r/691/diff/1/?file=10562#file10562line476>
> >
> >     Seems unnecessary.

removed


> On 2010-06-03 13:44:39, pabelanger wrote:
> > /trunk/apps/app_swift.c, line 516
> > <https://reviewboard.asterisk.org/r/691/diff/1/?file=10562#file10562line516>
> >
> >     ast_free()

changed


> On 2010-06-03 13:44:39, pabelanger wrote:
> > /trunk/apps/app_swift.c, line 520
> > <https://reviewboard.asterisk.org/r/691/diff/1/?file=10562#file10562line520>
> >
> >     Same as above

changed


- Darren


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


On 2010-06-03 16:00:28, Darren Sessions wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/691/
> -----------------------------------------------------------
> 
> (Updated 2010-06-03 16:00:28)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> app_swift uses the Cepstral text-to-speech engine and copies audio frames straight from the Cepstral libs to the call channel which avoids the need to write the generated audio to a file first and then read it. Additionally, DTMF recognition code was integrated into the module to prevent delays in previous usage scenarios where the app was called and dtmf listened for afterward.
> 
> 
> Diffs
> -----
> 
>   /trunk/apps/app_swift.c PRE-CREATION 
>   /trunk/autoconf/ast_ext_lib.m4 267714 
>   /trunk/build_tools/menuselect-deps.in 267714 
>   /trunk/configs/swift.conf.sample PRE-CREATION 
>   /trunk/configure.ac 267714 
>   /trunk/include/asterisk/autoconfig.h.in 267714 
>   /trunk/makeopts.in 267714 
> 
> Diff: https://reviewboard.asterisk.org/r/691/diff
> 
> 
> Testing
> -------
> 
> I've built against trunk successfully now (thanks to Tilghman for getting the build tweaks squared away) without any errors and made dozens of test calls using different usage scenarios.
> 
> 
> Thanks,
> 
> Darren
> 
>