[asterisk-dev] [Code Review] improved translation paths for wideband codecs

Russell Bryant russell at digium.com
Tue Aug 10 17:46:32 CDT 2010


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

Ship it!


Awesome work, batman.


/trunk/include/asterisk/translate.h
<https://reviewboard.asterisk.org/r/842/#comment5596>

    You can use \param[out] here



/trunk/include/asterisk/translate.h
<https://reviewboard.asterisk.org/r/842/#comment5595>

    use \return here based on how you have it written.  \retval is used like:
    
    \retval <value> <description>



/trunk/main/translate.c
<https://reviewboard.asterisk.org/r/842/#comment5597>

    trailing whitespace



/trunk/main/translate.c
<https://reviewboard.asterisk.org/r/842/#comment5598>

    This is an odd way to write this.  Why not just a for loop?
    
    for (; p; p = p->next) {
       ast_str_append(...);
    }


- Russell


On 2010-08-04 10:26:55, David Vossel wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/842/
> -----------------------------------------------------------
> 
> (Updated 2010-08-04 10:26:55)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> The problem I'm addressing is that Asterisk's current method of building the least cost translation paths between codecs does not take into account sample rate.  For instance, it was possible for siren14 (a 32khz codec), to contain the a translation path to siren7 (a 16khz audio codec) that goes through slin at 8khz.  In this case Asterisk takes a 32khz codec, down samples it to 8khz and then up samples it to 16khz which is terrible regardless if it is computationally less expensive.  This patch now builds translation paths that give priority to maintaining the best possible sample rate before taking into consideration computational cost.  This patch also adds cli commands to expose what translation paths are actually being used.
> 
> Changes:
> 1. Translation paths will never contain a step that changes the sample rate unless absolutely necessary.
> 2. When choosing the best codec to make two channels compatible.  Shared codecs with the highest sample rate are given priority.
> 3. A new cli command to show all translation paths available for a specific codec 'core show translation paths [codec name]' has been added.
> 4. 'core show translation' which displays the translation matrix now includes the new higher bit audio codecs in the table.
> 5. 'core show channel [channel name]'  now displays the translation paths if translation is used.
> 
> 
> This addresses bug 16841.
>     https://issues.asterisk.org/view.php?id=16841
> 
> 
> Diffs
> -----
> 
>   /trunk/include/asterisk/translate.h 280911 
>   /trunk/main/cli.c 280911 
>   /trunk/main/translate.c 280911 
> 
> Diff: https://reviewboard.asterisk.org/r/842/diff
> 
> 
> Testing
> -------
> 
> I tested this patch using the following wide band codecs; siren7, siren14, g722, and speex16.  I verified that the translation paths between each of these codecs are now built in the most optimal way.
> 
> 
> Thanks,
> 
> David
> 
>




More information about the asterisk-dev mailing list