[MPlayer-dev-eng] [PATCH] replacement for internal mpg123 fork (mp3lib)

[Thomas Orgis]

Hi MPlayer folks,

Diego prompted me about writing a replacement for the mpg123 code fork
you have in MPlayer, dubbed mp3lib. After some initial conversation
with him, I am now approaching the public with a patch that adds a new
decoder named mpg123, based on SVN revision 31163.
This one uses the external libmpg123 decoder library for
high-performance MPEG audio decoding.

Diego imagined that I would simply replace some guts of
libmpcodecs/ad_mp3lib.c, but the interface of what the MPlayer team
made of mpg123 code years ago has not much to do with the interface of
libmpg123 that I designed, well, years later;-)

The main idea was to get rid of mp3lib, which is a kind of internal
copy of a library -- it _is_ the same code base that libmpg123 is built
upon. In fact, libmpg123 incorporates improvements pioneered in
MPlayer's mp3lib (some assembly optimizations). So, I have to thank you
for that and offer the improved parsing code in return (with additional
improvements in the decoder, too). One could use some added
features/configurability of libmpg123, but I am focussing on basic
stream decoding for now. 

My initial patch featured less complicated code (well, that might be
personal opinion) at the expense of relying on a very recent mpg123
install (version 1.12). Diego didn't like that, so here is a version
that uses a very basic level of mpg123 API that works with version
1.0.0 of the library (I tested that on Linux/x86-64) and as a bonus
avoids issues caused by mismatch in large file setup (early
libmpg123 versions don't have large file support enabled).

I tried to come close to the coding standard using

    indent -kr -ts 4 --no-tabs libmpcodecs/ad_mpg123.c

I hope there really are only minor style issues left after this. If so,
one could perhaps work on a better parameter setup for indent...

Testing happened only on Linux/x86-64 so far... so someone should have
a try at least with i386, as that's still a rather common platform.


Open issues:
- I added code for setting input bitrate for free format streams, but
  observed that the stream parser of MPlayer does not like free format.
  Once it does, the mpg123 decoder should decode those properly (as
  does the standalone mpg123). If you never intend to add free format
  support, then one can strip a bit of code. Not much, though.

Open questions:
- Shall one keep the default of mpg123 to cut padding silence
  at beginning and end (unless there were seeks) if such info is in the
  file, i.e. "gapless decoding"? I wonder if that has implications on
  a/v sync (be it positive or negative).
- Would it be beneficial to decode to floating point or 32 bit integer
  instead of 16 bit? Might be better for quality if there's
  postprocessing (there's SSE-accelerated float output).
- What configurable libmpg123 features can/want one use in MPlayer?
  Generic decoder choice (different optimizations, dithered 16 bit
  output), zero-CPU-cost preamp (also via RVA2/ReplayGain info),
  zero-CPU-cost equalizer (scaling of MPEG bands)...
- Whatever I forgot now but mentioned in some comment in the source
  code.

Well, that's it so far tonight. I hope you like it... Good night.


Alrighty then,

Thomas.

PS: I tested mplayer with my added decoder with valgrind, checking if I
introduced something stupid, and this is the only thing that showed up
(bad wrapping caused by ... well, automatic eMail wrapping):

==27873== 21 bytes in 2 blocks are definitely lost in loss record 2 of
15 ==27873==    at 0x4C255FE: malloc (vg_replace_malloc.c:207)
==27873==    by 0xB9F33F1: strdup (in /lib/libc-2.11.1.so)
==27873==    by 0x5A1C7E: get_term_charset (getch2.c:301)
==27873==    by 0x497CAB: mp_msg_init (mp_msg.c:96)
==27873==    by 0x49AFFC: main (mplayer.c:2613)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mplayer-libmpg123.diff
Type: text/x-patch
Size: 20321 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20100512/e5c4e71f/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20100512/e5c4e71f/attachment.pgp>