[asterisk-dev] [Code Review] 2507: Add support for indexing installed sounds and for Stasis-HTTP sounds queries

[opticron]

> On May 17, 2013, 5:12 p.m., Matt Jordan wrote:
> > /trunk/res/res_sounds.c, lines 656-661
> > <https://reviewboard.asterisk.org/r/2507/diff/1/?file=37312#file37312line656>
> >
> >     I'm not really sure what benefit we get here by having this be a resource module.
> >     
> >     (1) Sounds are a relatively 'core' concept
> >     (2) The ability to show the loaded sounds on the system feels like a core ability, and would be nice to always have available
> >     (3) You already have a sounds indexer API in core.
> >     
> >     Other than 'not loading it if you don't need it', why not have this in core?

This was pulled out of core because it needs to load after format modules to know what formats to index.  Otherwise, there will be no available mapping between file extensions and encodings when the indexing occurs and it currently only indexes files for known formats.


> On May 17, 2013, 5:12 p.m., Matt Jordan wrote:
> > /trunk/res/res_sounds.c, lines 159-185
> > <https://reviewboard.asterisk.org/r/2507/diff/1/?file=37312#file37312line159>
> >
> >     This is a fairly costly operation. Imagine someone writing something like this in python:
> >     
> >     foreach (sound_file in sounds):
> >       if (language in asterisk.get_sound_languages()):
> >         ...
> >     
> >     What they don't know is that each call to 'get_sound_languages()' is iterating over the entire directory of sound files. That can be a pretty big hit.
> >     
> >     Since this is a costly operation, you should at a minimum cache the result of this after the first call. Alternatively, you could get all the language on module load and return that container.
> >     
> >     On a module reload, destroy the cached version and either reprocess on the next call, or rebuild the cached version.

This is most definitely a costly operation which is why it is only called once per indexing and is not available from the API.  The get_languages variant that is available from the API requires a sound file name and uses only indexed data.


- opticron


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


On May 7, 2013, 9:16 a.m., opticron wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2507/
> -----------------------------------------------------------
> 
> (Updated May 7, 2013, 9:16 a.m.)
> 
> 
> Review request for Asterisk Developers and David Lee.
> 
> 
> Bugs: ASTERISK-21584 and ASTERISK-21585
>     https://issues.asterisk.org/jira/browse/ASTERISK-21584
>     https://issues.asterisk.org/jira/browse/ASTERISK-21585
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> This adds support for stasis/sounds and stasis/sounds/{ID} queries via the Stasis-HTTP interface.
> 
> The following changes have been made to accomplish this:
> * A modular indexer was created for local sound files.
> * A new function to get an ast_format associated with a file extension was added.  
> * Modifications were made to the built-in HTTP server so that URI decoding could be deferred to the URI handler when necessary.
> * The Stasis-HTTP sounds JSON documentation was modified to handle cases where multiple languages are installed in different formats.
> 
> This requires the additions to astobj2.c/h present in https://reviewboard.asterisk.org/r/2428/
> 
> 
> Diffs
> -----
> 
>   /trunk/main/sounds.c PRE-CREATION 
>   /trunk/main/http.c 387798 
>   /trunk/main/file.c 387798 
>   /trunk/include/asterisk/sounds.h PRE-CREATION 
>   /trunk/include/asterisk/http.h 387798 
>   /trunk/include/asterisk/file.h 387798 
>   /trunk/res/res_sounds.c PRE-CREATION 
>   /trunk/res/res_stasis_http.c 387798 
>   /trunk/res/stasis_http/resource_sounds.h 387798 
>   /trunk/res/stasis_http/resource_sounds.c 387798 
>   /trunk/rest-api/api-docs/sounds.json 387798 
> 
> Diff: https://reviewboard.asterisk.org/r/2507/diff/
> 
> 
> Testing
> -------
> 
> Tested using a browser and HTTP GET requests using the Stasis-HTTP API.  Currently contemplating how to create automated tests.
> 
> 
> Thanks,
> 
> opticron
> 
>

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