flac dev - Sep 2004 - Re: 0.9 problems

Matt Zimmerman <mdz@debian.org> wrote:
> 0.9.  As I said, I was using an 8-bit sample,
Ah, that didn't quite register with me.  I'm using a CD-style
44.1kHz/stereo/16-bit test file.
> to avoid dealing with endian issues in the file format.  I don't
> know whether any of those exist or not.
I don't think so.  0.9 works fine on i386 (little) and sparc (big),
and flac's output is identical on both architectures.  FWIW, alpha
is little endian.

-- 
Christian "naddy" Weisgerber                         
naddy@mips.inka.de

On Sat, May 19, 2001 at 08:03:54PM +0000, Christian Weisgerber wrote:
> Matt Zimmerman <mdz@debian.org> wrote:
> 
> > 0.9.  As I said, I was using an 8-bit sample,
> 
> Ah, that didn't quite register with me.  I'm using a CD-style
> 44.1kHz/stereo/16-bit test file.
I repeated my test with a 44.1kHz/stereo/16bit file.  I get a floating point
exception when encoding at fixed.c:84.  Decoding works fine for me.

The floating point exception occurs here:

84              residual_bits_per_sample[0] = (real)((data_len > 0) ?
log(M_LN2 * (real)total_error_0  / (real) data_len) / M_LN2 : 0.0);

At the point of failure, the part inside the log() is zero, so the result of
the log is -inf.  On i386, dividing -inf / M_LN2 seems to evaluate to zero,
while on alpha it gives a floating point exception.  Here's a small test
program:

#include <math.h>

int main() {
  double foo = log((double)0);
  printf("%e\n", foo);
  printf("%e\n", foo / M_LN2);
  return 0;
}

I think this could be fixed by changing the (data_len > 0) test to be
(data_len
> 0 && total_error_X > 0).
-- 
 - mdz

On Sat, May 19, 2001 at 06:42:22PM -0400, Matt Zimmerman wrote:
> I think this could be fixed by changing the (data_len > 0) test to be
> (data_len > 0 && total_error_X > 0).
Yes, this appears to have worked.  I can now correctly encode and decode both
my 8kHz/8-bit/mono sample, and a 44.1kHz/16-bit/stereo sample on Debian/alpha.
Attached is a patch which combines this fix with my earlier patch.

-- 
 - mdz
-------------- next part --------------
--- fixed.c.orig	Sat May 19 18:28:08 2001
+++ fixed.c	Sat May 19 18:26:50 2001
@@ -81,11 +81,11 @@
 	residual_bits_per_sample[3] = (real)((data_len > 0) ? log(M_LN2 *
(real)(int64)total_error_3  / (real) data_len) / M_LN2 : 0.0);
 	residual_bits_per_sample[4] = (real)((data_len > 0) ? log(M_LN2 *
(real)(int64)total_error_4  / (real) data_len) / M_LN2 : 0.0);
 #else
-	residual_bits_per_sample[0] = (real)((data_len > 0) ? log(M_LN2 *
(real)total_error_0  / (real) data_len) / M_LN2 : 0.0);
-	residual_bits_per_sample[1] = (real)((data_len > 0) ? log(M_LN2 *
(real)total_error_1  / (real) data_len) / M_LN2 : 0.0);
-	residual_bits_per_sample[2] = (real)((data_len > 0) ? log(M_LN2 *
(real)total_error_2  / (real) data_len) / M_LN2 : 0.0);
-	residual_bits_per_sample[3] = (real)((data_len > 0) ? log(M_LN2 *
(real)total_error_3  / (real) data_len) / M_LN2 : 0.0);
-	residual_bits_per_sample[4] = (real)((data_len > 0) ? log(M_LN2 *
(real)total_error_4  / (real) data_len) / M_LN2 : 0.0);
+	residual_bits_per_sample[0] = (real)((data_len > 0 && total_error_0
> 0) ? log(M_LN2 * (real)total_error_0  / (real) data_len) / M_LN2 : 0.0);
+	residual_bits_per_sample[1] = (real)((data_len > 0 && total_error_1
> 0) ? log(M_LN2 * (real)total_error_1  / (real) data_len) / M_LN2 : 0.0);
+	residual_bits_per_sample[2] = (real)((data_len > 0 && total_error_2
> 0) ? log(M_LN2 * (real)total_error_2  / (real) data_len) / M_LN2 : 0.0);
+	residual_bits_per_sample[3] = (real)((data_len > 0 && total_error_3
> 0) ? log(M_LN2 * (real)total_error_3  / (real) data_len) / M_LN2 : 0.0);
+	residual_bits_per_sample[4] = (real)((data_len > 0 && total_error_4
> 0) ? log(M_LN2 * (real)total_error_4  / (real) data_len) / M_LN2 : 0.0);
 #endif
 
 	return order;
@@ -152,7 +152,7 @@
 
 void FLAC__fixed_compute_residual(const int32 data[], unsigned data_len,
unsigned order, int32 residual[])
 {
-	unsigned i;
+	int i;
 
 	switch(order) {
 		case 0:
@@ -190,7 +190,7 @@
 
 void FLAC__fixed_restore_signal(const int32 residual[], unsigned data_len,
unsigned order, int32 data[])
 {
-	unsigned i;
+	int i;
 
 	switch(order) {
 		case 0: