thr3ads.net - opus - [opus] Make check failure on clone from 31 January [Feb 2014]

If this information is useful, please help other people find it:
Share via:

Marcello Caramma (mcaramma)

2014-Feb-05 16:05 UTC

[opus] Make check failure on clone from 31 January

Hi,

Apologies if this is a known issue, but running make on revision
e3187444692195957eb66989622c7b1ad8448b06 fails one of the tests when using fixed
point configuration (floating point is ok) on my linux x86.
Note that libopus1.1, as extracted from the tar ball, is OK.

Specifically, the tests that fail are in celt/tests/test_unit_mdct:
nfft=32 inverse=0,snr = 85.341197
nfft=32 inverse=1,snr = 96.285703
nfft=256 inverse=0,snr = 86.726515
nfft=256 inverse=1,snr = 87.308386
nfft=512 inverse=0,snr = 82.577935
nfft=512 inverse=1,snr = 87.103849
nfft=1024 inverse=0,snr = 84.950837
nfft=1024 inverse=1,snr = 87.105776
nfft=2048 inverse=0,snr = 84.749139
nfft=2048 inverse=1,snr = 87.243518
nfft=36 inverse=0,snr = 84.424292
nfft=36 inverse=1,snr = 0.217968
** poor snr: 0.217968 **
nfft=40 inverse=0,snr = 83.806898
nfft=40 inverse=1,snr = 87.400185
nfft=60 inverse=0,snr = 84.981677
nfft=60 inverse=1,snr = 12.874762
** poor snr: 12.874762 **
nfft=120 inverse=0,snr = 82.344266
nfft=120 inverse=1,snr = 84.955920
nfft=240 inverse=0,snr = 83.273247
nfft=240 inverse=1,snr = 85.040479
nfft=480 inverse=0,snr = 83.702884
nfft=480 inverse=1,snr = 84.168293
nfft=960 inverse=0,snr = 84.481005
nfft=960 inverse=1,snr = 83.929637
nfft=1920 inverse=0,snr = 83.974087
nfft=1920 inverse=1,snr = 84.177282
FAIL: celt/tests/test_unit_mdct

Running bisect on the repository gives: 

e43a0abe0a908603a71b0c35e8c2307a77a7211f is the first bad commit
commit e43a0abe0a908603a71b0c35e8c2307a77a7211f
Author: Jean-Marc Valin <jmvalin at jmvalin.ca>
Date:   Fri Dec 27 00:10:54 2013 -0500

    Removes the separate 1/8N rotation in the (I)MDCT and unmerges the MDCT
sizes
    
    Undoes commits f7547a4e and 72513f3c


This is what I used to configure:
../configure --prefix=$HOME/opus_install  --enable-fixed-point

These are the details of my system:

uname -a
Linux mcdebian 2.6.32-5-686 #1 SMP Mon Feb 25 01:04:36 UTC 2013 i686 GNU/Linux

Best regards,

Marcello Caramma

Tristan Matthews

2014-Feb-05 18:40 UTC

head link

[opus] Make check failure on clone from 31 January

On Wed, Feb 5, 2014 at 11:05 AM, Marcello Caramma (mcaramma)
<mcaramma at cisco.com> wrote:> Hi,
>
> Apologies if this is a known issue, but running make on revision
e3187444692195957eb66989622c7b1ad8448b06 fails one of the tests when using fixed
point configuration (floating point is ok) on my linux x86.
> Note that libopus1.1, as extracted from the tar ball, is OK.
>
> Specifically, the tests that fail are in celt/tests/test_unit_mdct:
> nfft=32 inverse=0,snr = 85.341197
> nfft=32 inverse=1,snr = 96.285703
> nfft=256 inverse=0,snr = 86.726515
> nfft=256 inverse=1,snr = 87.308386
> nfft=512 inverse=0,snr = 82.577935
> nfft=512 inverse=1,snr = 87.103849
> nfft=1024 inverse=0,snr = 84.950837
> nfft=1024 inverse=1,snr = 87.105776
> nfft=2048 inverse=0,snr = 84.749139
> nfft=2048 inverse=1,snr = 87.243518
> nfft=36 inverse=0,snr = 84.424292
> nfft=36 inverse=1,snr = 0.217968
> ** poor snr: 0.217968 **
> nfft=40 inverse=0,snr = 83.806898
> nfft=40 inverse=1,snr = 87.400185
> nfft=60 inverse=0,snr = 84.981677
> nfft=60 inverse=1,snr = 12.874762
> ** poor snr: 12.874762 **
> nfft=120 inverse=0,snr = 82.344266
> nfft=120 inverse=1,snr = 84.955920
> nfft=240 inverse=0,snr = 83.273247
> nfft=240 inverse=1,snr = 85.040479
> nfft=480 inverse=0,snr = 83.702884
> nfft=480 inverse=1,snr = 84.168293
> nfft=960 inverse=0,snr = 84.481005
> nfft=960 inverse=1,snr = 83.929637
> nfft=1920 inverse=0,snr = 83.974087
> nfft=1920 inverse=1,snr = 84.177282
> FAIL: celt/tests/test_unit_mdct
>
> Running bisect on the repository gives:
>
> e43a0abe0a908603a71b0c35e8c2307a77a7211f is the first bad commit
> commit e43a0abe0a908603a71b0c35e8c2307a77a7211f
> Author: Jean-Marc Valin <jmvalin at jmvalin.ca>
> Date:   Fri Dec 27 00:10:54 2013 -0500
>
>     Removes the separate 1/8N rotation in the (I)MDCT and unmerges the MDCT
sizes
>
>     Undoes commits f7547a4e and 72513f3c
>
>
> This is what I used to configure:
> ../configure --prefix=$HOME/opus_install  --enable-fixed-point
>
> These are the details of my system:
>
> uname -a
> Linux mcdebian 2.6.32-5-686 #1 SMP Mon Feb 25 01:04:36 UTC 2013 i686
GNU/Linux
What compiler are you using?

Best,
Tristan

Gregory Maxwell

2014-Feb-05 18:46 UTC

head link

[opus] Make check failure on clone from 31 January

On Wed, Feb 5, 2014 at 8:05 AM, Marcello Caramma (mcaramma)
<mcaramma at cisco.com> wrote:> Hi,
>
> Apologies if this is a known issue, but running make on revision
e3187444692195957eb66989622c7b1ad8448b06 fails one of the tests when using fixed
point configuration (floating point is ok) on my linux x86.
> Note that libopus1.1, as extracted from the tar ball, is OK.
I can't reproduce with Fedora 19, gcc version 4.8.2 20131212 (Red Hat
4.8.2-7) (GCC) compiled with
CFLAGS='-m32 -O2 -g'  ./configure --enable-fixed-point ; make clean ;
make celt/tests/test_unit_mdct

or with Clang 3.5 on the same system.

None of the fixed point builds on or ci system are throwing errors on
this either.

Can you try compiling without optimizations? Smells like a compiler bug.

Marcello Caramma (mcaramma)

2014-Feb-06 12:40 UTC

head link

[opus] Make check failure on clone from 31 January

gcc-4.4.real (Debian 4.4.5-8) 4.4.5

If I compile with -O1 the test is succesful
[...]
nfft=36 inverse=1,snr = 86.619364
[...]
nfft=60 inverse=1,snr = 88.728534
[...]
If I compile with -O2 the test fails

[...]
nfft=36 inverse=1,snr = 0.217968
** poor snr: 0.217968 **
[...]
nfft=60 inverse=1,snr = 12.874762
** poor snr: 12.874762 **
[...]

Marcello


________________________________________
From: le.businessman at gmail.com [le.businessman at gmail.com] on behalf of
Tristan Matthews [tristan at sat.qc.ca]
Sent: 05 February 2014 18:40
To: Marcello Caramma (mcaramma)
Cc: opus at xiph.org
Subject: Re: [opus] Make check failure on clone from 31 January

On Wed, Feb 5, 2014 at 11:05 AM, Marcello Caramma (mcaramma)
<mcaramma at cisco.com> wrote:> Hi,
>
> Apologies if this is a known issue, but running make on revision
e3187444692195957eb66989622c7b1ad8448b06 fails one of the tests when using fixed
point configuration (floating point is ok) on my linux x86.
> Note that libopus1.1, as extracted from the tar ball, is OK.
>
> Specifically, the tests that fail are in celt/tests/test_unit_mdct:
> nfft=32 inverse=0,snr = 85.341197
> nfft=32 inverse=1,snr = 96.285703
> nfft=256 inverse=0,snr = 86.726515
> nfft=256 inverse=1,snr = 87.308386
> nfft=512 inverse=0,snr = 82.577935
> nfft=512 inverse=1,snr = 87.103849
> nfft=1024 inverse=0,snr = 84.950837
> nfft=1024 inverse=1,snr = 87.105776
> nfft=2048 inverse=0,snr = 84.749139
> nfft=2048 inverse=1,snr = 87.243518
> nfft=36 inverse=0,snr = 84.424292
> nfft=36 inverse=1,snr = 0.217968
> ** poor snr: 0.217968 **
> nfft=40 inverse=0,snr = 83.806898
> nfft=40 inverse=1,snr = 87.400185
> nfft=60 inverse=0,snr = 84.981677
> nfft=60 inverse=1,snr = 12.874762
> ** poor snr: 12.874762 **
> nfft=120 inverse=0,snr = 82.344266
> nfft=120 inverse=1,snr = 84.955920
> nfft=240 inverse=0,snr = 83.273247
> nfft=240 inverse=1,snr = 85.040479
> nfft=480 inverse=0,snr = 83.702884
> nfft=480 inverse=1,snr = 84.168293
> nfft=960 inverse=0,snr = 84.481005
> nfft=960 inverse=1,snr = 83.929637
> nfft=1920 inverse=0,snr = 83.974087
> nfft=1920 inverse=1,snr = 84.177282
> FAIL: celt/tests/test_unit_mdct
>
> Running bisect on the repository gives:
>
> e43a0abe0a908603a71b0c35e8c2307a77a7211f is the first bad commit
> commit e43a0abe0a908603a71b0c35e8c2307a77a7211f
> Author: Jean-Marc Valin <jmvalin at jmvalin.ca>
> Date:   Fri Dec 27 00:10:54 2013 -0500
>
>     Removes the separate 1/8N rotation in the (I)MDCT and unmerges the MDCT
sizes
>
>     Undoes commits f7547a4e and 72513f3c
>
>
> This is what I used to configure:
> ../configure --prefix=$HOME/opus_install  --enable-fixed-point
>
> These are the details of my system:
>
> uname -a
> Linux mcdebian 2.6.32-5-686 #1 SMP Mon Feb 25 01:04:36 UTC 2013 i686
GNU/Linux
What compiler are you using?

Best,
Tristan

Marcello Caramma (mcaramma)

2014-Feb-21 22:54 UTC

head link

[opus] Make check failure on clone from 31 January

I tracked down the bug to an incorrect use of restrict.

I would not consider this a compiler bug: we are lying to the optimizer by
telling it that a pointer is restrict when in fact it isn't.

This can be fixed like so:


diff --git a/celt/mdct.c b/celt/mdct.c
index 1634e8e..fa5098c 100644
--- a/celt/mdct.c
+++ b/celt/mdct.c
@@ -276,8 +276,8 @@ void clt_mdct_backward(const mdct_lookup *l,
kiss_fft_scalar *in, kiss_fft_scala
    /* Post-rotate and de-shuffle from both ends of the buffer at once to
make
       it in-place. */
    {
-      kiss_fft_scalar * OPUS_RESTRICT yp0 = out+(overlap>>1);
-      kiss_fft_scalar * OPUS_RESTRICT yp1 = out+(overlap>>1)+N2-2;
+      kiss_fft_scalar * yp0 = out+(overlap>>1);
+      kiss_fft_scalar * yp1 = out+(overlap>>1)+N2-2;
       const kiss_twiddle_scalar *t = &trig[0];
       /* Loop to (N4+1)>>1 to handle odd N4. When N4 is odd, the
          middle pair will be computed twice. */


Or like so:


diff --git a/celt/mdct.c b/celt/mdct.c
index 1634e8e..cdd053f 100644
--- a/celt/mdct.c
+++ b/celt/mdct.c
@@ -279,9 +279,8 @@ void clt_mdct_backward(const mdct_lookup *l,
kiss_fft_scalar *in, kiss_fft_scala
       kiss_fft_scalar * OPUS_RESTRICT yp0 = out+(overlap>>1);
       kiss_fft_scalar * OPUS_RESTRICT yp1 = out+(overlap>>1)+N2-2;
       const kiss_twiddle_scalar *t = &trig[0];
-      /* Loop to (N4+1)>>1 to handle odd N4. When N4 is odd, the
-         middle pair will be computed twice. */
-      for(i=0;i<(N4+1)>>1;i++)
+      /* Loop to N4>>1 to make sure pointers never overlap */
+      for(i=0;i<N4>>1;i++)
       {
          kiss_fft_scalar re, im, yr, yi;
          kiss_twiddle_scalar t0, t1;
@@ -309,6 +308,20 @@ void clt_mdct_backward(const mdct_lookup *l,
kiss_fft_scalar *in, kiss_fft_scala
          yp0 += 2;
          yp1 -= 2;
       }
+      // Handle the odd case
+      if(N4 & 1)
+      {
+         kiss_fft_scalar re, im;
+         kiss_twiddle_scalar t0, t1;
+         /* We swap real and imag because we're using an FFT instead of
an IFFT. */
+         re = yp0[1];
+         im = yp0[0];
+         t0 = t[i];
+         t1 = t[N4+i];
+         /* We'd scale up by 2 here, but instead it's done when mixing
the windows */
+         yp0[0] = S_MUL(re,t0) + S_MUL(im,t1);
+         yp0[1] = S_MUL(re,t1) - S_MUL(im,t0);
+      }
    }
 
    /* Mirror on both sides for TDAC */


Regards,

Marcello



On 05/02/2014 18:46, "Gregory Maxwell" <gmaxwell at gmail.com>
wrote:
>On Wed, Feb 5, 2014 at 8:05 AM, Marcello Caramma (mcaramma)
><mcaramma at cisco.com> wrote:
>> Hi,
>>
>> Apologies if this is a known issue, but running make on revision
>>e3187444692195957eb66989622c7b1ad8448b06 fails one of the tests when
>>using fixed point configuration (floating point is ok) on my linux x86.
>> Note that libopus1.1, as extracted from the tar ball, is OK.
>
>I can't reproduce with Fedora 19, gcc version 4.8.2 20131212 (Red Hat
>4.8.2-7) (GCC) compiled with
>CFLAGS='-m32 -O2 -g'  ./configure --enable-fixed-point ; make clean
;
>make celt/tests/test_unit_mdct
>
>or with Clang 3.5 on the same system.
>
>None of the fixed point builds on or ci system are throwing errors on
>this either.
>
>Can you try compiling without optimizations? Smells like a compiler bug.

Reasonably Related Threads

Search for more possibly parallel threads

opus - Feb 2014 - Make check failure on clone from 31 January

[opus] Make check failure on clone from 31 January

[opus] Make check failure on clone from 31 January

[opus] Make check failure on clone from 31 January

[opus] Make check failure on clone from 31 January

[opus] Make check failure on clone from 31 January

Reasonably Related Threads