Bug#506193: toolame: Encoded files are sometimes corrupted (bad 'padding' information)

[Christian Grigis]

Package: toolame
Version: 02l-6
Severity: important
Tags: patch

Hello,

When encoding a mono/16-bit/48kHz WAV file, the resulting file is
sometimes corrupted in the 'padding' information of the headers, which
renders it unplayable by a decoder:

% file test.wav 
test.wav: RIFF (little-endian) data, WAVE audio, Microsoft PCM, 16 bit, mono 48000 Hz

% toolame test.wav 
Parsing Wave File Header
>>> 48000 Hz sampling freq selected
>>> Input Wave File is Mono
--------------------------------------------
Input File : 'test.wav'   48.0 kHz
Output File: 'test.mp2'
192 kbps MPEG-1 Layer II single-ch Psycho model=1  (Mode_Extension=0)
[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
Hit end of audio data
Avg slots/frame = 576.000; b/smp = 4.00; bitrate = 192.000 kbps

Done

% od -Ax -tx1 test.mp2 | head -1
000000 ff fd a6 c0 55 68 88 87 77 8d f6 b6 22 00 00 ee
             ^^
--> Incorrect (padding is ON)

(...)

% toolame test.wav 
Parsing Wave File Header
>>> 48000 Hz sampling freq selected
>>> Input Wave File is Mono
--------------------------------------------
Input File : 'test.wav'   48.0 kHz
Output File: 'test.mp2'
192 kbps MPEG-1 Layer II single-ch Psycho model=1  (Mode_Extension=0)
[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
Hit end of audio data
Avg slots/frame = 576.000; b/smp = 4.00; bitrate = 192.000 kbps

Done

% od -Ax -tx1 test.mp2 | head -1
000000 ff fd a4 c0 55 68 88 87 77 8d f6 b6 22 00 00 ee
             ^^
--> Correct (padding is OFF)

(See for example http://www.mpgedit.org/mpgedit/mpeg_format/mpeghdr.htm
for the header details.)

Tracking this problem down, it appears that the 1-bit 'header->padding'
field is not initialized before parsing the arguments, and if no
argument changes this field, it is left unitialized, resulting in a
possible corruption.

The attached patch initializes the field to 0 before parsing the
arguments.

It seems that the problem does not appear with the (more common) 44.1
kHz sampling rate. My guess, after browsing through the source, is that
in that case, sizes of packets are such that the encoder has to set the
padding field for each packet, whereas with 48 kHz, all packets are the
same size, and the encoder uses the unitialized value throughout the
whole output file.

By the way, I recently became aware of the 'twolame' package, which
is a fork of 'toolame'. A quick test with it seems to show that this bug
(as well as the previous one I reported, #504308) is not present in
it. Should 'toolame' then be considered obsoleted by 'twolame'?

Thanks and best regards,

-Christian

-- System Information:
Debian Release: lenny/sid
  APT prefers unstable
  APT policy: (500, 'unstable'), (500, 'testing'), (500, 'stable'), (1, 'experimental')
Architecture: amd64 (x86_64)

Kernel: Linux 2.6.27.6-mooch.1-amd64 (SMP w/2 CPU cores; PREEMPT)
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8) (ignored: LC_ALL set to en_US.UTF8)
Shell: /bin/sh linked to /bin/bash

Versions of packages toolame depends on:
ii  libc6                         2.7-16     GNU C Library: Shared libraries

toolame recommends no packages.

toolame suggests no packages.

-- no debconf information
-------------- next part --------------
A non-text attachment was scrubbed...
Name: patch.txt
Type: text/x-diff
Size: 445 bytes
Desc: not available
Url : http://lists.alioth.debian.org/pipermail/pkg-multimedia-maintainers/attachments/20081119/84c587b7/attachment.diff