Summary: | ASTERISK-15637: Codec translation path builder does not produce expected results with 16khz and 32khz audio | ||
Reporter: | David Vossel (dvossel) | Labels: | |
Date Opened: | 2010-02-16 12:21:20.000-0600 | Date Closed: | 2010-10-29 08:26:34 |
Priority: | Minor | Regression? | No |
Status: | Closed/Complete | Components: | Core/CodecInterface |
Versions: | Frequency of Occurrence | ||
Related Issues: | |||
Environment: | Attachments: | ||
Description: | The codec translation path builder only has the ability to distinguish what path has the least translation cost involved, but when dealing with sample rates greater 16khz the path builder should also consider the quality of the translation. For example. I have seen issues where a translation path between a 16khz codec and another 16khz codec will result in a translation path containing 8khz slin. While converting down to 8khz slin may have the least cost, it reduces quality. I am proposing that the translation path builder not consider a path containing a conversion sample rate less that the lowest sample rate involved in the call, unless there is no other way of going about it. So if one side of the call is 16khz and the other side is a different 16khz codec, the goal should be to have every translation that takes place in that path be 16khz if possible regardless of cost. We also need a way to view the translation path being used by a call. This should probably be shown during the 'core show channel' output. If this output is considered a new feature, I recommend making it at least available for debug builds in existing branches. Changes Required - The translation path builder must take into account the sampling rate used. - A channel's translation path must be viewable - A unit test verifying the translation path builder works as expected. ****** ADDITIONAL INFORMATION ****** This affects 1.6.0 and greater. *NOTE* I'm not sure what category this issue belongs in. If a bug marshal knows of a better place to put this please change it and remove this note. | ||
Comments: | By: Digium Subversion (svnbot) 2010-08-12 15:15:40 Repository: asterisk Revision: 282047 U branches/1.8/CHANGES U branches/1.8/include/asterisk/translate.h U branches/1.8/main/cli.c U branches/1.8/main/translate.c ------------------------------------------------------------------------ r282047 | dvossel | 2010-08-12 15:15:40 -0500 (Thu, 12 Aug 2010) | 35 lines improved translation paths for wideband codecs 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. (closes issue ASTERISK-15637) Reported by: dvossel Review: https://reviewboard.asterisk.org/r/842/ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=282047 By: Digium Subversion (svnbot) 2010-08-12 15:17:16 Repository: asterisk Revision: 282048 _U trunk/ U trunk/CHANGES U trunk/include/asterisk/translate.h U trunk/main/cli.c U trunk/main/translate.c ------------------------------------------------------------------------ r282048 | dvossel | 2010-08-12 15:17:16 -0500 (Thu, 12 Aug 2010) | 42 lines Merged revisions 282047 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.8 ........ r282047 | dvossel | 2010-08-12 15:15:41 -0500 (Thu, 12 Aug 2010) | 35 lines improved translation paths for wideband codecs 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. (closes issue ASTERISK-15637) Reported by: dvossel Review: https://reviewboard.asterisk.org/r/842/ ........ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=282048 |