[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

toolame: remove from unstable, update in stable



Dear -release,
[CC'ing pkg-multimedia-maintainers as the current toolame maintainers,
pkg-mythtv-maintainers as the current twolame maintainers and
debian-devel for general discussion ;) ]

I am going to request the removal of the toolame package from unstable
and testing in the short term. toolame hasn't seen a release in six
years, it is dead upstream, it has bugs and is superseded by its
successor twolame (yes, there's a t_o_olame and a t_w_olame). People
should really be using twolame instead, as it has an active upstream,
has its bugs fixed and provides a shared library that is already used
by projects such as gstreamer and audacity.

However, I think the toolame package in stable may stay both for the
convenience of our users and because it's not *that* bad after all. I
have prepared a  package targeted at Lenny that fixes two portability
bugs and some minor lintian warnings. It can be found at mentors.d.n
[1], the debdiff is attached to this mail. Please tell me if or under
which restrictions it is OK to upload it to stable or s-p-u.

My general question regarding the removal of toolame is: Should we
provide a transition and if yes... how? I consider the following methods to deal with this issue:
1. Remove the toolame package from unstable and testing. End of story.
2. Provide an empty toolame package that depends on twolame.
3a. Provide a wrapper in the toolame package that calls the twolame
binary with the same command line options (or some translations respectively). toolame and twolame are command line compatible except for some very exotic flags.
3b. The twolame source creates a toolame package that contains the
above mentioned wrapper script.
4. Make the twolame package provide toolame, so that at least the few recommends on the toolame package can be satisfied.
5. What else can you imagine/recommend...?

Thank you very much for your suggestions.

Cheers,
Fabian


[1] http://mentors.debian.net/debian/pool/main/t/toolame/toolame_02l-7.dsc

--
Dipl.-Phys. Fabian Greffrath

Ruhr-Universität Bochum
Lehrstuhl für Energieanlagen und Energieprozesstechnik (LEAT)
Universitätsstr. 150, IB 3/134
D-44780 Bochum

Telefon: +49 (0)234 / 32-26334
Fax:     +49 (0)234 / 32-14227
E-Mail:  greffrath@leat.ruhr-uni-bochum.de

diff -u toolame-02l/debian/rules toolame-02l/debian/rules
--- toolame-02l/debian/rules
+++ toolame-02l/debian/rules
@@ -3,6 +3,8 @@
 # Uncomment this to turn on verbose mode.
 #export DH_VERBOSE=1
 
+include /usr/share/quilt/quilt.make
+
 CFLAGS = -Wall -g
 
 ifneq (,$(findstring noopt,$(DEB_BUILD_OPTIONS)))
@@ -11,7 +13,7 @@
 	CFLAGS += -O2
 endif
 
-build: build-stamp
+build: $(QUILT_STAMPFN) build-stamp
 build-stamp:
 	dh_testdir
 
@@ -20,14 +22,14 @@
 
 	touch build-stamp
 
-clean:
+clean: unpatch
 	dh_testdir
 	dh_testroot
 	rm -f build-stamp
 
 	# Cleaning package
-	-$(MAKE) clean
-	rm -rf toolame
+	[ ! -f Makefile ] || $(MAKE) clean
+	rm -f toolame
 
 	dh_clean
 
diff -u toolame-02l/debian/changelog toolame-02l/debian/changelog
--- toolame-02l/debian/changelog
+++ toolame-02l/debian/changelog
@@ -1,3 +1,23 @@
+toolame (02l-7) unstable; urgency=low
+
+  * debian/control:
+    + Added Build-Depends on quilt.
+    + Bumped Standards-Version to 3.8.0.
+    + Removed the "XS-" prefix from the Vcs-Svn and Vcs-Browser fields.
+    + Added Homepage field and removed it from the package description.
+    + Fixed "binary-control-field-duplicates-source field". 
+  * debian/rules:
+    + Included quilt for patch management.
+    + Fixed "debian-rules-ignores-make-clean-error".
+  * debian/patches/01-WAV-header-read-incorrectly-on-64-bit-platforms.patch:
+    + New patch to fix usage of "unsigned long" on 64-bit little-endian
+      platforms (Closes: #504308). Thanks Christian Grigis <glove@grigri.org>!
+  * debian/patches/02-initialize-header-padding-field-before-parsing-arguments.patch:
+    + New patch to initialize the 1-bit 'header->padding' field before parsing
+      the arguments. Thanks Christian Grigis <glove@grigri.org>!
+
+ -- Fabian Greffrath <fabian@debian-unofficial.org>  Wed, 11 Mar 2009 12:53:11 +0100
+
 toolame (02l-6) unstable; urgency=low
 
   [ Sam Hocevar ]
diff -u toolame-02l/debian/control toolame-02l/debian/control
--- toolame-02l/debian/control
+++ toolame-02l/debian/control
@@ -3,13 +3,13 @@
 Priority: optional
 Maintainer: Debian multimedia packages maintainers <pkg-multimedia-maintainers@lists.alioth.debian.org>
 Uploaders: Fabian Greffrath <fabian@debian-unofficial.org>, Sam Hocevar (Debian packages) <sam+deb@zoy.org>
-Build-Depends: debhelper (>= 5)
-Standards-Version: 3.7.2
-XS-Vcs-Svn: svn://svn.debian.org/pkg-multimedia/unstable/toolame
-XS-Vcs-Browser: http://svn.debian.org/wsvn/pkg-multimedia/unstable/toolame/
+Build-Depends: debhelper (>= 5), quilt
+Standards-Version: 3.8.0
+Vcs-Svn: svn://svn.debian.org/pkg-multimedia/unstable/toolame
+Vcs-Browser: http://svn.debian.org/wsvn/pkg-multimedia/unstable/toolame/
+Homepage: http://toolame.sourceforge.net/
 
 Package: toolame
-Section: sound
 Architecture: any
 Depends: ${shlibs:Depends}, ${misc:Depends}
 Description: MPEG-1 layer 2 audio encoder
@@ -22,2 +21,0 @@
- .
-  Homepage: <http://toolame.sourceforge.net/>
only in patch2:
unchanged:
--- toolame-02l.orig/debian/patches/01-WAV-header-read-incorrectly-on-64-bit-platforms.patch
+++ toolame-02l/debian/patches/01-WAV-header-read-incorrectly-on-64-bit-platforms.patch
@@ -0,0 +1,73 @@
+From: Christian Grigis <glove@grigri.org>
+To: Debian Bug Tracking System <submit@bugs.debian.org>
+Subject: toolame: WAV header read incorrectly on 64-bit platforms
+Date: Sun, 02 Nov 2008 19:28:25 +0100
+
+Package: toolame
+Version: 02l-6
+Severity: important
+Tags: patch
+
+When reading the sample frequency of the WAV header (a 32-bit
+value), a pointer to an "unsigned long" is used on little-endian
+platforms. Unfortunately, "unsigned long" is 64-bit on 64-bit platforms:
+
+% file audio.wav
+audio.wav: RIFF (little-endian) data, WAVE audio, Microsoft PCM, 16 bit, stereo 48000 Hz
+
+% toolame -s 48 audio.wav
+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 : 'audio.wav'   44.1 kHz
+Output File: 'audio.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
+--------------------------------------------
+(...)
+
+(note that 824633720880000 & 0xFFFFFFFF == 48000)
+
+I patched it using a uint32_t to be sure of the length:
+
+% ./toolame-02l/toolame -s 48 audio.wav 
+Parsing Wave File Header
+>>> 48000 Hz sampling freq selected
+>>> Input Wave File is Stereo
+--------------------------------------------
+Input File : 'audio.wav'   48.0 kHz
+Output File: 'audio.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
+--------------------------------------------
+(...)
+
+Best regards,
+
+-Christian
+
+--- toolame-02l.orig/audio_read.c
++++ toolame-02l/audio_read.c
+@@ -1,6 +1,7 @@
+ #include <stdio.h>
+ #include <stdlib.h>
+ #include <string.h>
++#include <stdint.h>
+ #include "common.h"
+ #include "encoder.h"
+ #include "options.h"
+@@ -329,7 +330,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) +
only in patch2:
unchanged:
--- toolame-02l.orig/debian/patches/series
+++ toolame-02l/debian/patches/series
@@ -0,0 +1,2 @@
+01-WAV-header-read-incorrectly-on-64-bit-platforms.patch
+02-initialize-header-padding-field-before-parsing-arguments.patch
only in patch2:
unchanged:
--- toolame-02l.orig/debian/patches/02-initialize-header-padding-field-before-parsing-arguments.patch
+++ toolame-02l/debian/patches/02-initialize-header-padding-field-before-parsing-arguments.patch
@@ -0,0 +1,104 @@
+From: Christian Grigis <glove@grigri.org>
+To: Debian Bug Tracking System <submit@bugs.debian.org>
+Subject: toolame: Encoded files are sometimes corrupted (bad 'padding' information)
+Date: Wed, 19 Nov 2008 09:41:08 +0100
+
+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
+
+--- toolame-02l.orig/toolame.c
++++ toolame-02l/toolame.c
+@@ -644,6 +644,7 @@
+   inPath[0] = '\0';
+   outPath[0] = '\0';
+   header->lay = DFLT_LAY;
++  header->padding = 0;
+   switch (DFLT_MOD) {
+   case 's':
+     header->mode = MPG_MD_STEREO;


Reply to: