Bug#697144: dir2ogg: Broken sound speed of converted files

[Thomas Orgis]

TLDR: Please check with current mpg123 snapshot and think about either
prescribing format and letting mpg123 write really raw data or make
proper use of the improper WAV header.

Hello,

mpg123 upstream here. I really did not intend to break dir2ogg in
various ways. I totally regret touching WAV writing code at all. While
I think that it is a bit bold to pipe something like WAV to stdout and
expect it to work (many programs really are not happy with the
inconsistent headers that this produces, take audacity or even just
libsndfile), there are some that could make use of this. Among those
are oggenc and lame, for that matter.

I do wonder why you have

            opts=['-r', '-R', str(mutagen.File(self.song).info.sample_rate)]

for mpg123 output in dir2ogg. Is this already an attempt to fix broken
WAV output by mpg123? If not, I really have to wonder why you do not just use

	mpg123 -s input.mp3 | ogggenc -r -R $rate -C $channels -B 16 -o output.ogg

or rather

	mpg123 -e s24  -s input.mp3 | ogggenc -r -R $rate -C $channels -B 24 -o output.ogg

for avoiding extra quality impact from rounding to 16 bit in between
(sadly, oggenc does not seem to support float, which would avoid
integer intermediate altogether).

Note $channels ... current dir2ogg will screw up if the input is a mono
file. It assumes stereo. 

Thing is, you tell mpg123 to write WAV headers and then tell oggenc to
ignore them via -r. That confuses me. If you want raw data, tell mpg123
to deliver raw data to stdout via the -s switch.

But, once we got through this mess with the repeatedly messed up
WAV-to-stdout (which I always recommend to use with caution only ...),
for example using current http://mpg123.org/snapshot, which shall
become mpg123 1.16.0, you can just do that:

	src/mpg123 -w - -e s24 input.mp3 | oggenc  -o output.ogg -

and have it magically work. Note that the '-e s24' might not be
available on certain builds of mpg123. If it is supported, it appears in

	mpg123 --longhelp

in this line:

 -e <c> --encoding <c>     force a specific encoding (s16 u16 u8 s8 ulaw alaw f32 s32 u32 s24 u24)

I might add a specific switch to ask about that (mpg123
--list-encodings or such) to 1.16.0, if it's desired.

To support all versions of mpg123, you can extend your hack that
inserts the sample rate via mutagen and have it provide the full format
(including channel count) and just pipe the raw data from mpg123 via
-s. The option for s24 format would just reduce the quality loss a
bit ... if you care about that when converting between lossy formats;-)

Using the WAV output of a minimally required mpg123 >= 1.16.0 would
avoid the question of checking which output format is available. People
can make different builds of mpg123 that have different (default)
sample format (floating point, for example).

It's interesting that oggenc and lame are the only tools I found with
some quick tests that are happy with the kind of broken WAV you get via
pipes. But then, if you intend to store such files, you'd better write
them properly.


Alrighty then,

Thomas
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/pkg-multimedia-maintainers/attachments/20130317/3280f96e/attachment.pgp>