Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 138002 - toolame-0.2l-{r1,r2) incorrectly parses RIFF header
Summary: toolame-0.2l-{r1,r2) incorrectly parses RIFF header
Status: RESOLVED TEST-REQUEST
Alias: None
Product: Gentoo Linux
Classification: Unclassified
Component: New packages (show other bugs)
Hardware: AMD64 Linux
: High normal (vote)
Assignee: Gentoo Sound Team
URL: http://sourceforge.net/tracker/?func=...
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-06-25 20:27 UTC by Russell Mora
Modified: 2009-05-30 12:55 UTC (History)
1 user (show)

See Also:
Package list:
Runtime testing required: ---


Attachments
toolame-02l-uint32_t.patch (toolame-02l-uint32_t.patch,624 bytes, patch)
2009-05-30 12:27 UTC, Samuli Suominen (RETIRED)
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Russell Mora 2006-06-25 20:27:12 UTC
When a RIFF WAVE file is used for input toolame (both 02l-r1 and 02l-r2) incorrectly parses the header and spits out msgs about "Unknown samp freq":

# file input.wav
input.wav: RIFF (little-endian) data, WAVE audio, Microsoft PCM, 16 bit, stereo 48000 Hz
# toolame -s 48 -p 2 -b 384 input.wav output.mp2
Parsing Wave File Header
>>> Unknown samp freq 824633720880000 Hz in Wave Header
>>> Default 44.1 kHz samp freq selected
>>> Input Wave File is Stereo
--------------------------------------------
<...>

(NOTE: it also ignores the -s option telling it what the sample rate is...)

This is due to a simple type size error - this fixes the bug:

--- audio_read.c_orig   2006-06-25 22:57:24.000000000 -0400
+++ audio_read.c        2006-06-25 22:57:58.000000000 -0400
@@ -329,7 +329,7 @@
       }
     }
     if (NativeByteOrder == order_littleEndian) {
-      samplerate = *(unsigned long *) (&wave_header_buffer[24]);
+      samplerate = *(unsigned int *) (&wave_header_buffer[24]);
     } else {
       samplerate = wave_header_buffer[27] +
        (wave_header_buffer[26] << 8) +


Now it parses the header correctly:

# toolame -p 2 -b 384 input.wav output.mp2
Parsing Wave File Header
>>> 48000 Hz sampling freq selected
>>> Input Wave File is Stereo
--------------------------------------------
Input File : 'input.wav'   48.0 kHz
Output File: 'output.mp2'
384 kbps MPEG-1 Layer II j-stereo Psy model 2
[De-emph:Off    Copyright:No    Original:No     CRC:Off]
[Padding:Normal Byte-swap:Off   Chanswap:Off    DAB:Off]
ATH adjustment 0.000000
--------------------------------------------
encode_init: using tablenum 0 with sblimit 27
absthr[][] sampling frequency index: 2
Hit end of audio data
Avg slots/frame = 1152.000; b/smp = 8.00; bitrate = 384.000 kbps

Done
#

HTH
Comment 1 Alessio Cassibba (X-Drum) 2006-11-10 11:55:10 UTC
hi can't reproduce this problem
all is working without the patch:

x-drum@Storm Desktop $ file testfile.wav
testfile.wav: RIFF (little-endian) data, WAVE audio, Microsoft PCM, 16 bit, stereo 48000 Hz

x-drum@Storm Desktop $ toolame -s 48 -p 2 -b 384 testfile.wav output.mp2
Parsing Wave File Header
>>> 48000 Hz sampling freq selected
>>> Input Wave File is Stereo
--------------------------------------------
[..]

x-drum@Storm Desktop $ toolame -p 2 -b 384 testfile.wav output.mp2
Parsing Wave File Header
>>> 48000 Hz sampling freq selected
>>> Input Wave File is Stereo
--------------------------------------------
[..]

Comment 2 Russell Mora 2006-11-11 13:29:56 UTC
I don't want to ask a stupid question, but you did try this is on a x86-64 system, right?  The problem is only seen on systems where long is not 4 bytes, which is the size of the sample-rate field in the PCM header....

Otherwise let me know what version you are using so I can try it out too :-)
Comment 3 Steve Dibb (RETIRED) gentoo-dev 2007-03-18 16:49:42 UTC
Eh, I can't reproduce the error either on amd64
Comment 4 Russell Mora 2007-04-21 18:48:37 UTC
Hmm, this could happen if the bps is zero.  This is how I generate the wav file in the first place:

host ~ # mplayer -really-quiet -frames 100 -vo null -ao pcm:file=test.wav /media/videos/01.\ wake-up\ call.avi
mplayer: could not open config files /root/.lircrc and /etc/lircrc
mplayer: No such file or directory
host ~ #
host ~ # file test.wav
test.wav: RIFF (little-endian) data, WAVE audio, Microsoft PCM, 16 bit, stereo 48000 Hz
host ~ #

Other encoders seem to handle it OK:

host ~ # twolame test.wav output.mp2
---------------------------------------------------------
Input File: test.wav
Input Format: WAV (Microsoft)
Output File: output.mp2
---------------------------------------------------------
LibTwoLame 0.3.8 (http://www.twolame.org)
Input : 48000 Hz, 2 channels
Output: 48000 Hz, Stereo
192 kbps CBR MPEG-1 Layer II psycho model=3
[De-emph:Off    Copyright:No    Original:Yes]
[Padding:Off    CRC:Off         DAB:Off     ]
---------------------------------------------------------
[0139]
Encoding Finished.
host ~ #

Just toolame has problems (due to the bad cast outlined earlier):

host ~ # toolame test.wav output.mp2
Parsing Wave File Header
>>> Unknown samp freq 824633720880000 Hz in Wave Header
>>> Default 44.1 kHz samp freq selected
>>> Input Wave File is Stereo
--------------------------------------------
Input File : 'test.wav'   44.1 kHz
Output File: 'output.mp2'
192 kbps MPEG-1 Layer II j-stereo Psy model 1
[De-emph:Off    Copyright:No    Original:No     CRC:Off]
[Padding:Normal Byte-swap:Off   Chanswap:Off    DAB:Off]
ATH adjustment 0.000000
--------------------------------------------
encode_init: using tablenum 1 with sblimit 30
Hit end of audio data
Avg slots/frame = 626.935; b/smp = 4.35; bitrate = 191.999 kbps

Done
host ~ # 

Note that 824633720880000 == 0x2EE000000BB80.  Splitting this up we get 0xBB80 == 48000 (the sample rate) and 0x2EE00 == 192000 (the bps), which is why I suggested that you would not see this bug if the bps was zero (or the arch is not LP64).

I don't know what else could cause the non-reproducibility of this bug - my install is nothing unusual:

host ~ # grep FLAGS /etc/make.conf
CFLAGS="-march=athlon64 -O2 -pipe"
CXXFLAGS="${CFLAGS}"
host ~ #

Let me know if I can provide any other information.
Comment 5 Stian Skjelstad 2007-11-28 11:54:49 UTC
+      samplerate = *(unsigned int *) (&wave_header_buffer[24]);

should perhaps have been

+      samplerate = *(uint32_t *)(&wave_header_buffer[24]);

Comment 6 Russell Mora 2007-11-28 22:54:34 UTC
Yes, uint32_t is more correct (guaranteed to be correct)
Comment 7 Samuli Suominen (RETIRED) gentoo-dev 2009-05-29 05:44:14 UTC
Can we please have a unified diff against the original sources?

Not a fan of copy'n'pasting lines from bugzilla like this.
Comment 8 Russell Mora 2009-05-29 14:40:50 UTC
It is already there in the original description - just substitute in the uint32_t type, i.e.

--- audio_read.c_orig   2006-06-25 22:57:24.000000000 -0400
+++ audio_read.c        2006-06-25 22:57:58.000000000 -0400
@@ -329,7 +329,7 @@
       }
     }
     if (NativeByteOrder == order_littleEndian) {
-      samplerate = *(unsigned long *) (&wave_header_buffer[24]);
+      samplerate = *(uint32_t *) (&wave_header_buffer[24]);
     } else {
       samplerate = wave_header_buffer[27] +
        (wave_header_buffer[26] << 8) +

Comment 9 Steve Dibb (RETIRED) gentoo-dev 2009-05-29 14:53:16 UTC
x86_64-pc-linux-gnu-gcc -O2 -pipe -march=athlon64 -DNDEBUG -DINLINE=inline -Wall -DNEWENCODE -DNEWATAN -c psycho_4.c -o psycho_4.o
x86_64-pc-linux-gnu-gcc -O2 -pipe -march=athlon64 -DNDEBUG -DINLINE=inline -Wall -DNEWENCODE -DNEWATAN -c fft.c -o fft.o
x86_64-pc-linux-gnu-gcc -O2 -pipe -march=athlon64 -DNDEBUG -DINLINE=inline -Wall -DNEWENCODE -DNEWATAN -c subband.c -o subband.o
x86_64-pc-linux-gnu-gcc -O2 -pipe -march=athlon64 -DNDEBUG -DINLINE=inline -Wall -DNEWENCODE -DNEWATAN -c audio_read.c -o audio_read.o
audio_read.c: In function ‘parse_input_file’:
audio_read.c:332: error: ‘uint32_t’ undeclared (first use in this function)
audio_read.c:332: error: (Each undeclared identifier is reported only once
audio_read.c:332: error: for each function it appears in.)
audio_read.c:332: error: expected expression before ‘)’ token
make: *** [audio_read.o] Error 1
Comment 10 Russell Mora 2009-05-29 17:31:26 UTC
Fair enough - in that case use the original diff supplied.  It should work on LP64 and LLP64 system (on I32 system).
Comment 11 Stian Skjelstad 2009-05-29 22:44:25 UTC
> audio_read.c: In function ‘parse_input_file’:
> audio_read.c:332: error: ‘uint32_t’ undeclared (first use in this function)
> audio_read.c:332: error: (Each undeclared identifier is reported only once
> audio_read.c:332: error: for each function it appears in.)
> audio_read.c:332: error: expected expression before ‘)’ token

#include <stdint.h>

should solve that
Comment 12 Samuli Suominen (RETIRED) gentoo-dev 2009-05-30 12:27:49 UTC
Created attachment 192973 [details, diff]
toolame-02l-uint32_t.patch

This is what I meant by unified and attached patch. Sorry guys, if I wasn't clear before.

Has this been posted upstream?
Comment 13 Samuli Suominen (RETIRED) gentoo-dev 2009-05-30 12:38:26 UTC
So, yes, please test this. It's -r3 in Portage now, sync in a hour or so.

I'll report this to upstream bugtracker. I *really* don't know if this is the solution upstream wants, but we'll see.