[Mplayer-dev-eng] clicks/noise from AC3 audio decoder using 3dnowex instructions (AMD K7)

[Juergen Keil]

Hi,

there's a problem in libac3/imdct.c and libac3/mmx/imdct_3dnow.c, function
imdct_do_512_nol().  In both files the routine ends with:

	...
	
        data_ptr = data;
        delay_ptr = delay;
        window_ptr = window;

        /* Window and convert to real valued signal, no overlap here*/
[1]     for(i=0; i< 64; i++) {
               *data_ptr++   = -buf[64+i].im   * *window_ptr++;
               *data_ptr++   =  buf[64-i-1].re * *window_ptr++;
        }
[2]
	/* The trailing edge of the window goes into the delay line */

   	delay_ptr = delay;

[3]     for(i=0; i< 64; i++) {
               *delay_ptr++  = -buf[64+i].re   * *--window_ptr;
               *delay_ptr++  =  buf[64-i-1].im * *--window_ptr;
        }

[4]     for(i=0; i<64; i++) {
               *delay_ptr++  =  buf[i].im       * *--window_ptr;
               *delay_ptr++  = -buf[128-i-1].re * *--window_ptr;
        }
}


'window' is an array with 256 float elements.  The loop at [1] uses the
elements 0 .. 127 (via window_ptr).  The loop at [3] use the same elements
in reverse order, 127 .. 0.

And finally the loop at [4] continues looping backwards, using elements 
-1 .. -128 .  Oops.  It access out-of-bound array elements,  which obviouly
is not ok.


Looking at the imdct_do_256*() variant of the code, I have the impression
that a second loop accessing the array elements 128 .. 255 is missing here [2].
With such a second loop added, [3] would access the elements 255 .. 128 and
[4] 127 .. 0.  The appended patch adds the missing second loop.  DCT experts
out there,  does that fix look OK?



The patch fixes some stange audio clicks I get on dvd video playback;
these clicks 'appeared' a few days ago with the mplayer cvs version due
to an unrelated change in mp3lib/decode_k7.s which triggered this noise
problem for me.


--
Jürgen Keil          		jk at tools.de
Tools GmbH			+49 (228) 9858011
-------------- next part --------------
diff -x config.mak -x config.h -ru /tmp/mplayer/main/libac3/imdct.c mplayer/main/libac3/imdct.c
--- /tmp/mplayer/main/libac3/imdct.c	Sat Jun  9 16:33:28 2001
+++ mplayer/main/libac3/imdct.c	Wed Jun 13 01:01:36 2001
@@ -192,12 +192,12 @@
 // Window and convert to real valued signal
 	for (i=0; i< 64; i++) {
 		*data_ptr++   = -buf[64+i].im   * *window_ptr++ + *delay_ptr++;
-		*data_ptr++   = buf[64-i-1].re * *window_ptr++ + *delay_ptr++;
+		*data_ptr++   =  buf[64-i-1].re * *window_ptr++ + *delay_ptr++;
 	}
 
 	for(i=0; i< 64; i++) {
 		*data_ptr++  = -buf[i].re       * *window_ptr++ + *delay_ptr++;
-		*data_ptr++  = buf[128-i-1].im * *window_ptr++ + *delay_ptr++;
+		*data_ptr++  =  buf[128-i-1].im * *window_ptr++ + *delay_ptr++;
 	}
 
 // The trailing edge of the window goes into the delay line
@@ -269,9 +269,15 @@
 
        /* Window and convert to real valued signal, no overlap here*/
         for(i=0; i< 64; i++) {
-               *data_ptr++   = -buf[64+i].im   * *window_ptr++;
-               *data_ptr++   = buf[64-i-1].re * *window_ptr++;
+		*data_ptr++   = -buf[64+i].im   * *window_ptr++;
+		*data_ptr++   =  buf[64-i-1].re * *window_ptr++;
         }
+
+	for(i=0; i< 64; i++) {
+		*data_ptr++  = -buf[i].re       * *window_ptr++;
+		*data_ptr++  =  buf[128-i-1].im * *window_ptr++;
+	}
+
 
         /* The trailing edge of the window goes into the delay line */
 
diff -x config.mak -x config.h -ru /tmp/mplayer/main/libac3/mmx/imdct_3dnow.c mplayer/main/libac3/mmx/imdct_3dnow.c
--- /tmp/mplayer/main/libac3/mmx/imdct_3dnow.c	Sat Jun  9 16:33:28 2001
+++ mplayer/main/libac3/mmx/imdct_3dnow.c	Wed Jun 13 01:08:07 2001
@@ -250,22 +250,27 @@
 
        /* Window and convert to real valued signal, no overlap here*/
         for(i=0; i< 64; i++) {
-               *data_ptr++   = -buf[64+i].im   * *window_ptr++;
-               *data_ptr++   = buf[64-i-1].re * *window_ptr++;
+		*data_ptr++   = -buf[64+i].im   * *window_ptr++;
+		*data_ptr++   =  buf[64-i-1].re * *window_ptr++;
         }
 
+	for(i=0; i< 64; i++) {
+		*data_ptr++  = -buf[i].re       * *window_ptr++;
+		*data_ptr++  =  buf[128-i-1].im * *window_ptr++;
+	}
+
         /* The trailing edge of the window goes into the delay line */
 
-   delay_ptr = delay;
+	delay_ptr = delay;
 
         for(i=0; i< 64; i++) {
-               *delay_ptr++  = -buf[64+i].re   * *--window_ptr;
-               *delay_ptr++  =  buf[64-i-1].im * *--window_ptr;
+		*delay_ptr++  = -buf[64+i].re   * *--window_ptr;
+		*delay_ptr++  =  buf[64-i-1].im * *--window_ptr;
         }
 
         for(i=0; i<64; i++) {
-               *delay_ptr++  =  buf[i].im       * *--window_ptr;
-               *delay_ptr++  = -buf[128-i-1].re * *--window_ptr;
+		*delay_ptr++  =  buf[i].im       * *--window_ptr;
+		*delay_ptr++  = -buf[128-i-1].re * *--window_ptr;
         }
 }
 
@@ -345,12 +350,12 @@
 	/* Window and convert to real valued signal */
 	for(i=0; i< 64; i++) { 
 		*data_ptr++  = -buf1[i].im      * *window_ptr++ + *delay_ptr++;
-		*data_ptr++  = buf1[64-i-1].re * *window_ptr++ + *delay_ptr++;
+		*data_ptr++  =  buf1[64-i-1].re * *window_ptr++ + *delay_ptr++;
 	}
 
 	for(i=0; i< 64; i++) {
 		*data_ptr++  = -buf1[i].re      * *window_ptr++ + *delay_ptr++;
-		*data_ptr++  = buf1[64-i-1].im * *window_ptr++ + *delay_ptr++;
+		*data_ptr++  =  buf1[64-i-1].im * *window_ptr++ + *delay_ptr++;
 	}
 	
 	delay_ptr = delay;
@@ -443,12 +448,12 @@
        /* Window and convert to real valued signal, no overlap */
        for(i=0; i< 64; i++) {
                *data_ptr++  = -buf1[i].im      * *window_ptr++;
-               *data_ptr++  = buf1[64-i-1].re * *window_ptr++;
+               *data_ptr++  =  buf1[64-i-1].re * *window_ptr++;
        }
 
        for(i=0; i< 64; i++) {
                *data_ptr++  = -buf1[i].re      * *window_ptr++ + *delay_ptr++;
-               *data_ptr++  = buf1[64-i-1].im * *window_ptr++ + *delay_ptr++;
+               *data_ptr++  =  buf1[64-i-1].im * *window_ptr++ + *delay_ptr++;
        }
 
        delay_ptr = delay;