[9fans] mpg123

[9fans] mpg123

[lucio]

The following code from mpg123/layer3.c (around line 180, as provided
by anyrhine) could not be more intentionally broken:

    double r;
    if (i < 6) {
        double t = tan( (double) i * M_PI / 12.0 );
        r = t / (1.0 + t);
    } else
        r = 1.0;

    tan1_1[i] = r;
    tan2_1[i] = 1.0 - r;
    tan1_2[i] = M_SQRT2 * r;
    tan2_2[i] = M_SQRT2 / (1.0 - r);

Does anyone know how to intelligently stop the division by zero in
line 181?

++L

Re: [9fans] mpg123

[Aki M Nyrhinen]

terrible :)

i can't remember breaking it, but it is most certainly is me who
got it broken _that_ bad. the original code has it less broken, but it
too has division by zero. google shows that the last time this 
problem was discussed on 9fans, someone suggested just not calculating
the entry i == 9. other possibilities include using a slightly less
accurate value for pi.

i suggest giving up on mpg123 and using nemo's port of madplay instead :)
that's what i did anyway.

	Aki


On Fri, Jan 21, 2005 at 06:38:44AM +0200, lucio@proxima.alt.za wrote:
> The following code from mpg123/layer3.c (around line 180, as provided
> by anyrhine) could not be more intentionally broken:
> 
>     double r;
>     if (i < 6) {
>         double t = tan( (double) i * M_PI / 12.0 );
>         r = t / (1.0 + t);
>     } else
>         r = 1.0;
> 
>     tan1_1[i] = r;
>     tan2_1[i] = 1.0 - r;
>     tan1_2[i] = M_SQRT2 * r;
>     tan2_2[i] = M_SQRT2 / (1.0 - r);
> 
> Does anyone know how to intelligently stop the division by zero in
> line 181?
> 
> ++L
> 

Re: [9fans] mpg123

[lucio]

> i suggest giving up on mpg123 and using nemo's port of madplay instead :)
> that's what i did anyway.

Sold!  Sounds like mpg123 contains code that is hazardous to one's
health and should carry a Surgeon General's warning.

++L

Re: [9fans] mpg123

[Conor Williams]

nice 1 Nigel - that worked a peach.
There wasnt any noticable change warping wise
really...
I still like my 0.001 hack!

Tad & Russ - maybe incorporate the change
below into layer3.c on those 2 ports of mpg123.
l8r
will551
----- Original Message -----
From: <nigel@9fs.org>
To: <9fans@cse.psu.edu>
Sent: Tuesday, February 11, 2003 11:26 AM
Subject: Re: [9fans] mpg123


> > thanks Nigel
> > Heard my first plan9 mp3 late last night!
> > I ended up subtracting 0.001 from that eqation and let
> > it run the 1 -> 16 loop! doesnt seem to have warped it 2
> > much!
>
> That's almost bound to be a bad decision. It will warp
> all the values, rather than just the one which I suspect
> is not used.
>
> You forced me to get my copy of 11172-3 off the shelf.
>
> These tables are used for intensity coding, e.g. the left
> channel has the intensity of the frequency bucket,
> the right channel the pan ratio.
>
> Given that the equation is
>
> ratio = tan(pos * pi / 12)
> L' = L * ratio / (1 + ratio)
> R' = R * 1 / (1 + ratio)
>
> which can be rewritten as
>
> ratio = tan(pos * pi / 12)
> R = ratio/(1 + ratio)
> L' = L * R
> R' = L * (1 - R)
>
> R = 0 pans fully to the right, and increasing values
> tend towards the left. When R hits 1, it is fully
> panned to the left.
>
> The tan function tends to infinity at 90 degrees, or
> PI/2 radians. Thus is makes no sense for pos to
> take a value > 6. In fact, 6 itself is special. You can
> interpret it as meaning R = 1.
>
> The spec. indicates that pos == 7 disables the panning
> effect.
>
> So I contend that encoders will only generate values
> from 0 - 7, even though the field can be 4 bits, only
> 3 are used for intensity coding. Thus my fix will
> trash one row of the pow[] table which is generated
> in the same loop, which does use 4 bits.
>
> If you look at mad (libmad/layer3.c), you will see that
> it confirms this. The implementor of mad clearly
> thought about what this part of 11172-3 is supposed
> to achieve.
>
> A better fix is therefore:
>
>
>   for(i=0;i<16;i++) {
>     double r;
>
>     if (i < 6) {
>     double t = tan( (double) i * M_PI / 12.0 );
>     r = t / (1.0 + t);
>     }
>     else
>     r = 1.0;
>
>     tan1_1[i] = r;
>     tan2_1[i] = 1.0 - r;
>     tan1_2[i] = M_SQRT2 * r;
>     tan2_2[i] = M_SQRT2 * (1.0 - r);
>
>
>
>

Re: [9fans] mpg123

[nigel]

> thanks Nigel
> Heard my first plan9 mp3 late last night!
> I ended up subtracting 0.001 from that eqation and let
> it run the 1 -> 16 loop! doesnt seem to have warped it 2
> much!

That's almost bound to be a bad decision. It will warp
all the values, rather than just the one which I suspect
is not used.

You forced me to get my copy of 11172-3 off the shelf.

These tables are used for intensity coding, e.g. the left
channel has the intensity of the frequency bucket,
the right channel the pan ratio.

Given that the equation is

ratio = tan(pos * pi / 12)
L' = L * ratio / (1 + ratio)
R' = R * 1 / (1 + ratio)

which can be rewritten as

ratio = tan(pos * pi / 12)
R = ratio/(1 + ratio)
L' = L * R
R' = L * (1 - R)

R = 0 pans fully to the right, and increasing values
tend towards the left. When R hits 1, it is fully
panned to the left.

The tan function tends to infinity at 90 degrees, or
PI/2 radians. Thus is makes no sense for pos to
take a value > 6. In fact, 6 itself is special. You can
interpret it as meaning R = 1.

The spec. indicates that pos == 7 disables the panning
effect.

So I contend that encoders will only generate values
from 0 - 7, even though the field can be 4 bits, only
3 are used for intensity coding. Thus my fix will
trash one row of the pow[] table which is generated
in the same loop, which does use 4 bits.

If you look at mad (libmad/layer3.c), you will see that
it confirms this. The implementor of mad clearly
thought about what this part of 11172-3 is supposed
to achieve.

A better fix is therefore:


  for(i=0;i<16;i++) {
    double r;

    if (i < 6) {
	double t = tan( (double) i * M_PI / 12.0 );
	r = t / (1.0 + t);
    }
    else
	r = 1.0;

    tan1_1[i] = r;
    tan2_1[i] = 1.0 - r;
    tan1_2[i] = M_SQRT2 * r;
    tan2_2[i] = M_SQRT2 * (1.0 - r);

Re: [9fans] mpg123

[Conor Williams]

thanks Nigel
Heard my first plan9 mp3 late last night!
I ended up subtracting 0.001 from that eqation and let
it run the 1 -> 16 loop! doesnt seem to have warped it 2
much!
Russ & co - maybe set the default irq for a sb16 card to
5 as thats what it is out of the box!
l8r
will551

----- Original Message -----
From: "Nigel Roles" <nigel@9fs.org>
To: <9fans@cse.psu.edu>
Sent: Monday, February 10, 2003 2:14 PM
Subject: RE: [9fans] mpg123


> Because it computes (around ine 170 of layer3.c)
>
> 1 - tan(3 * Pi / 4)
>
> which is 0
>
> and then a series of values which involve dividing
> by it.
>
> In my experience, most mp3 code available on
> lunix is full of this kind of stuff.
>
> It's no wonder that if you port an encoder or decoder
> from one platform to another you get a different answer.
>
> It's only because the x86 FPU gives an answer to
> tan(3 * PI / 4)
>
> "that is almost, but not quite, entirely unlike"
>
> -1 that this works at all.
>
> Write out 100 times, "floating point numbers
> are approximate numbers".
>
> I would guess that row 9 of the table is not used,
> so that putting
>
> if (i == 9) continue;
>
> in the loop will fix it.
>
> -----Original Message-----
> From: 9fans-admin@cse.psu.edu [mailto:9fans-admin@cse.psu.edu]On Behalf
> Of Conor Williams
> Sent: 10 February 2003 09:01
> To: 9fans@cse.psu.edu
> Subject: [9fans] mpg123
>
>
> ne one use mpg123?
> I got it compiled but now im getting:
> mpg123 107: suicide: sys: fp: division by zero: fppc=0x6389 status=0xb9b4
>     pc=0x00006380
>
> when I try and play anything using the command
> mpg123 -s pc.mp3 > /dev/audio
>
> any one got Tad Hunts email address - thats the guy who
> ported over mpg123 to plan 9?
> tx
> will551
>
>

Re: [9fans] mpg123

[Tad Hunt]

I seem to remember something along these lines.  Try
running it like this:

	mpg123 < pc.mp3 > /dev/audio

I think there should be a README with the patch that
describes some crash like this.  Unfortunately I haven't
had time to touch any plan9 stuff for about two years.

-Tad

In message <017201c2d0e2$e7273d50$4d4a800a@will551>, you said:
;ne one use mpg123?
;I got it compiled but now im getting:
;mpg123 107: suicide: sys: fp: division by zero: fppc=0x6389 status=0xb9b4
;    pc=0x00006380
;
;when I try and play anything using the command
;mpg123 -s pc.mp3 > /dev/audio
;
;any one got Tad Hunts email address - thats the guy who
;ported over mpg123 to plan 9?
;tx
;will551

Re: [9fans] mpg123

[Fco.J.Ballesteros]

[-- Attachment #1: Type: text/plain, Size: 46 bytes --]

http://plan9.escet.urjc.es/ or just google :-)

[-- Attachment #2: Type: message/rfc822, Size: 2173 bytes --]

From: "Conor Williams" <connorwilliamsie@aol.com>
To: <9fans@cse.psu.edu>
Subject: Re: [9fans] mpg123
Date: Mon, 10 Feb 2003 15:53:16 -0000
Message-ID: <011001c2d11c$8685f290$4d4a800a@will551>

it doesnt seem to be on the plan9.bell-labs site?
have u a url?
tx
will551

----- Original Message -----
From: "Fco.J.Ballesteros" <nemo@plan9.escet.urjc.es>
To: <9fans@cse.psu.edu>
Sent: Monday, February 10, 2003 2:34 PM
Subject: RE: [9fans] mpg123


> The mad player uses fixed point and is quite optimized.
> I think the ported version is in our web page.
> It's what I use in the bitsy and works properly there
> (needless to say on a pc).
>

Re: [9fans] mpg123

[Conor Williams]

it doesnt seem to be on the plan9.bell-labs site?
have u a url?
tx
will551

----- Original Message -----
From: "Fco.J.Ballesteros" <nemo@plan9.escet.urjc.es>
To: <9fans@cse.psu.edu>
Sent: Monday, February 10, 2003 2:34 PM
Subject: RE: [9fans] mpg123


> The mad player uses fixed point and is quite optimized.
> I think the ported version is in our web page.
> It's what I use in the bitsy and works properly there
> (needless to say on a pc).
>

Re: [9fans] mpg123

[Fco.J.Ballesteros]

[-- Attachment #1: Type: text/plain, Size: 91 bytes --]

You have to compile madplay and not minimad.
Minimad does almost no error recovery at all.

[-- Attachment #2: Type: message/rfc822, Size: 3447 bytes --]

From: Axel Belinfante <Axel.Belinfante@cs.utwente.nl>
To: 9fans@cse.psu.edu
Subject: Re: [9fans] mpg123
Date: Mon, 10 Feb 2003 15:53:00 +0100
Message-ID: <200302101453.h1AEr0Z20581@zamenhof.cs.utwente.nl>

> The mad player uses fixed point and is quite optimized.
> I think the ported version is in our web page.
> It's what I use in the bitsy and works properly there
> (needless to say on a pc).

I used it, took it from your web page, but I found that
there are some mp3's which it doesn't play but which
mpg123 does play.  (something to do with mp3 headers?)

> Because it computes (around line 170 of layer3.c)
> 1 - tan(3 * Pi / 4)     which is 0
> and then a series of values which involve dividing
> by it.

I hacked around this in mpg123  by checking for a
divisor of 0.0 and replacing it by something small
but unequal to zero.  I got away with that but
Nigels solution (which I did not try) looks nicer.

> I would guess that row 9 of the table is not used,
> so that putting
>	 if (i == 9) continue;
> in the loop will fix it.

Axel.

Re: [9fans] mpg123

[Axel Belinfante]

> The mad player uses fixed point and is quite optimized.
> I think the ported version is in our web page.
> It's what I use in the bitsy and works properly there
> (needless to say on a pc).

I used it, took it from your web page, but I found that
there are some mp3's which it doesn't play but which
mpg123 does play.  (something to do with mp3 headers?)

> Because it computes (around line 170 of layer3.c)
> 1 - tan(3 * Pi / 4)     which is 0
> and then a series of values which involve dividing
> by it.

I hacked around this in mpg123  by checking for a
divisor of 0.0 and replacing it by something small
but unequal to zero.  I got away with that but
Nigels solution (which I did not try) looks nicer.

> I would guess that row 9 of the table is not used,
> so that putting
>	 if (i == 9) continue;
> in the loop will fix it.

Axel.

RE: [9fans] mpg123

[Fco.J.Ballesteros]

[-- Attachment #1: Type: text/plain, Size: 186 bytes --]

The mad player uses fixed point and is quite optimized.
I think the ported version is in our web page.
It's what I use in the bitsy and works properly there
(needless to say on a pc).

[-- Attachment #2: Type: message/rfc822, Size: 2753 bytes --]

From: "Nigel Roles" <nigel@9fs.org>
To: <9fans@cse.psu.edu>
Subject: RE: [9fans] mpg123
Date: Mon, 10 Feb 2003 14:14:56 -0000
Message-ID: <HOEHIDJJJINMLFPOLFJCIEKPCKAA.nigel@9fs.org>

Because it computes (around ine 170 of layer3.c)

1 - tan(3 * Pi / 4)

which is 0

and then a series of values which involve dividing
by it.

In my experience, most mp3 code available on
lunix is full of this kind of stuff.

It's no wonder that if you port an encoder or decoder
from one platform to another you get a different answer.

It's only because the x86 FPU gives an answer to
tan(3 * PI / 4)

"that is almost, but not quite, entirely unlike"

-1 that this works at all.

Write out 100 times, "floating point numbers
are approximate numbers".

I would guess that row 9 of the table is not used,
so that putting

if (i == 9) continue;

in the loop will fix it.

-----Original Message-----
From: 9fans-admin@cse.psu.edu [mailto:9fans-admin@cse.psu.edu]On Behalf
Of Conor Williams
Sent: 10 February 2003 09:01
To: 9fans@cse.psu.edu
Subject: [9fans] mpg123


ne one use mpg123?
I got it compiled but now im getting:
mpg123 107: suicide: sys: fp: division by zero: fppc=0x6389 status=0xb9b4
    pc=0x00006380

when I try and play anything using the command
mpg123 -s pc.mp3 > /dev/audio

any one got Tad Hunts email address - thats the guy who
ported over mpg123 to plan 9?
tx
will551

RE: [9fans] mpg123

[Nigel Roles]

Because it computes (around ine 170 of layer3.c)

1 - tan(3 * Pi / 4)

which is 0

and then a series of values which involve dividing
by it.

In my experience, most mp3 code available on
lunix is full of this kind of stuff.

It's no wonder that if you port an encoder or decoder
from one platform to another you get a different answer.

It's only because the x86 FPU gives an answer to
tan(3 * PI / 4)

"that is almost, but not quite, entirely unlike"

-1 that this works at all.

Write out 100 times, "floating point numbers
are approximate numbers".

I would guess that row 9 of the table is not used,
so that putting

if (i == 9) continue;

in the loop will fix it.

-----Original Message-----
From: 9fans-admin@cse.psu.edu [mailto:9fans-admin@cse.psu.edu]On Behalf
Of Conor Williams
Sent: 10 February 2003 09:01
To: 9fans@cse.psu.edu
Subject: [9fans] mpg123


ne one use mpg123?
I got it compiled but now im getting:
mpg123 107: suicide: sys: fp: division by zero: fppc=0x6389 status=0xb9b4
    pc=0x00006380

when I try and play anything using the command
mpg123 -s pc.mp3 > /dev/audio

any one got Tad Hunts email address - thats the guy who
ported over mpg123 to plan 9?
tx
will551

Re: [9fans] mpg123

[Russ Cox]

try http://pdos.lcs.mit.edu/~rsc/mp3dec.tgz.
it's a (possibly different) port of mpg123
that works well for me.  i don't remember who
did it originally.

[9fans] mpg123

[Conor Williams]

ne one use mpg123?
I got it compiled but now im getting:
mpg123 107: suicide: sys: fp: division by zero: fppc=0x6389 status=0xb9b4
    pc=0x00006380

when I try and play anything using the command
mpg123 -s pc.mp3 > /dev/audio

any one got Tad Hunts email address - thats the guy who
ported over mpg123 to plan 9?
tx
will551