[asterisk-bugs] [Asterisk 0008126]: [patch] G.711 codec woes

[noreply at bugs.digium.com]

A NOTE has been added to this issue. 
====================================================================== 
http://bugs.digium.com/view.php?id=8126 
====================================================================== 
Reported By:                fossil
Assigned To:                murf
====================================================================== 
Project:                    Asterisk
Issue ID:                   8126
Category:                   Core/CodecInterface
Reproducibility:            always
Severity:                   minor
Priority:                   normal
Status:                     ready for testing
Asterisk Version:            SVN 
SVN Branch (only for SVN checkouts, not tarball releases):  1.2 
SVN Revision (number only!): 44743 
Disclaimer on File?:        Yes 
Request Review:              
====================================================================== 
Date Submitted:             10-09-2006 20:21 CDT
Last Modified:              08-18-2007 17:08 CDT
====================================================================== 
Summary:                    [patch] G.711 codec woes
Description: 
There is a *number* of problems in the a-law and u-law core transcoders
(most severe first):

1. a-Law decoder does not add the rounding error to the linear samples
output;
This results in a stable amplitude drop in the decoded signal overall, but
the negative phase portion of the signal is even more adversely affected:
the amplitude drop actually accumulates with consequtive transcodings (see
attached test patch). If the call encounters 127 tandem a-law transcodings
(a-alaw -> slin -> a-law -> slin -> ...), the entire negative portion will
be reduced to http://bugs.digium.com/view.php?id=#0.

2. Lookup table-driven slin->law coding rounds the negative values the
wrong way;
The breaks in linear value sequences do not happen where the table-driven
slin->law system expect them to. This results in certain negative linear
values to be encoded incorrectly (see attached test patch), which isn't
such a *big* problem, but a problem nonetheless.
There is no one-liner fix for this issue. To fix this, for example, we
could generate only half the slin->law table, for positive values only.
This table would contain half-cooked law bytes, so that the sign could be
added later to the values, along with the post-coding transform (NOT for
u-law and XOR 0x55 for a-law). In this case, AST_LIN2MU() would look
something like this:

inline unsigned char AST_LIN2MU(short sample)
{
  unsigned sign = ((unsigned)sample & 0x8000) >> 8;
  unsigned char law = __ast_lin2mu[(sample & 0x7fff) >> 2];
  return ~(law | sign);
}

3. slin->a-law and slin->u-law functions handle value -32768 incorrectly;
This is not really a problem when using a lookup table system because the
slot of -32768 is overwritten later, but for the sake of correctness...

4. alaw.c:linear2alaw() is less than optimal;

5. slin->law lookup table generation code is less than optimal;
There is no reason to enumerate all the possible values between -32768 and
32767 when most of the results are overwritten later.

====================================================================== 

---------------------------------------------------------------------- 
 murf - 08-18-07 17:08  
---------------------------------------------------------------------- 
OK, I've done this: Matt Fredrickson felt that the new algorithm should be
a compile time option, especially since it may negatively impact the speed
of the codec. So, I have made it so. The new and old algorithms coexist in
the same files wrapped in #ifdefs to allow them to be compiled either way.

I have added the entries to the menuselect stuff, so you can turn on and
off the options. I did this in trunk, which is a clear, no-brainer type
solution. 1.4 is a little trickier. By default, the New Algorithm is turned
off.  I don't know if it will be all right to add the same options to 1.4's
menuselect. I'll ask Monday.

Now, I've done some more timing and comparing based on your last patch.

I see this

ulaw > slin  137    82/100    125/103/99    82/84/85
alaw > slin  83     82/82     82/95/95      82/111/81/300

slin > ulaw  59     64/65     203/202/243   202/203/287
slin > alaw  59     62        242/295/236   223/223/311
             OLD-A   OLD-B    NEW-noreduce   NEW-reducedbran


OLD-A is with the test using just zeros. OLD-B is the more realistic
test with values that vary across the test range. My patch includes the
test input files so modified.

Since the REDUCED_BRANCHING option seems to yield, on the average, faster
times, I make it the default. There's a bit of variance in the results,
but
it seems to average out.

I've also included the changes I made to the translate.c stuff to make it
show microseconds instead of milliseconds, and display 5 digits instead of
3.
I see that the new converter -> u/alaw is 3.1 to 3.6 x slower, using best
times.
This seems really bad, but it's still very fast (in comparison to other
converters), and it's more complete and correct. When multiple transcodings
are possible, this algorithm should better results.

Please, double check my code and let me know if it's all correct. I am
slightly concerned about having the table/transcode tests available in both
old and new algorithms. (but, then, it would just say that the old has
problems, and the new doesn't, wouldn't it?) So, for the moment, I only
allow the tests if you select the new algorithm. 

Issue History 
Date Modified   Username       Field                    Change               
====================================================================== 
08-18-07 17:08  murf           Note Added: 0069051                          
======================================================================