[MPlayer-dev-eng] [PATCH] replacement for internal mpg123 fork (mp3lib), Final version?

[Thomas Orgis]

Am Sun, 16 May 2010 12:10:46 +0200
schrieb Diego Biurrun <diego at biurrun.de>: 
 
> Hmm, quick benchmarks on my K6-III show external mpg123 to be about
> 20-25% slower.  What gives?

First: It's a bit unfortunate that it's apparently not possible to
profile mplayer with gprof ... well, on x86 at least:

cc -DHAVE_AV_CONFIG_H -I.. -I..  -MD -MP -Wstrict-prototypes -Wmissing-prototypes -Wundef -Wdisabled-optimization -Wno-pointer-sign -Wdeclaration-after-statement -std=gnu99 -Wall -Wno-switch -Wpointer-arith -Wredundant-decls -O4 -march=native -mtune=native -pipe -ffast-math -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE64_SOURCE -Ilibdvdread4 -I.  -D_REENTRANT -I/usr/include/directfb -I/usr/X11/include   -I/usr/include/freetype2 -g -pg     -c -o h264_cabac.o h264_cabac.c
cabac.h: In function 'get_cabac_noinline':
cabac.h:527: error: can't find a register in class 'GENERAL_REGS' while reloading 'asm'
cabac.h:527: error: 'asm' operand has impossible constraints
make[1]: *** [h264_cabac.o] Error 1
make[1]: Leaving directory `/tmp/mplayer-svn/libavcodec'

I guess I could circumvent this, but resorting to x86-64, I see that I
fail to get profiling working together with the linked libmpg123.so ...
Well, digging out the runtime profile of Sun Studio yielded nothing
substantial, too -- apart from the actual decoding in libmpg123 really
needing less CPU.

But then it occured to me: This is an effect that one sees enforced
after dropping the fixed 1K input buffer for the packets, that at least
for a single MPEG audio file are smaller than that. I was aware of the
buffering code inside libmpg123 for the mode of operation we use here
being sub-optimal, but so far it worked fine.
Running mplayer with -ao pcm:file=/dev/null does show that this I/O
inefficency does have significant influence when comparing to mp3lib,
essentially the mpg123 engine itself.

Long story short: mpg123 calls malloc() too often. It works with an
internal linked list of buffers, allocating and freeing as new data
arrives and old data is discarded. I am hesitating to use a fixed
FIFO buffer there because it would need to be big enough to
accomodate any ID3v2 tags one might encounter, and one can only guess
how big those can grow. Of course, one can add more hacks to
special-case that... but that doesn't help the tidyness of the I/O code
of which the buffer is only one aspect.

But what I could do, and what I did in mpg123 SVN, is to manage buffers
with a minimal size of 4K instead allocating a new one for each bit of
data that arrives. This brings the decoding time with mplayer for a set
of reference files down to 12 seconds on my 1.2GHz Core2Duo compared to
12.7 seconds with mp3lib.

Would you be so kind to test with a libmpg123 built from
http://mpg123.org/snapshot ? No need to rebuild mplayer, just running
with LD_LIBRARY_PATH is enough, as, like I said before, I intend
libmpg123 to stay backwards compatible;-)
A question is now how relevant this buffer tuning is for realtime
decoding instead of timing with maximal throughput. Well, one could
circumvent the sub-optimal buffering in older libmpg123 versions (older
than the next version to be released;-) by resorting to the first patch
I did, using a callback for reading... but it would be tricky to keep
the PTS timing code in that case, one would need buffering in ad_mpg123
for partial packets.

In any case, you of course get the best performance of mpg123 with the
latest version... as it should be the case with mplayer, too. We have a
compromise to find between (theoretical?) raw throughput performance,
A/V accuracy and compatibility to existing installs of libmpg123.
Keeping it working correctly with old installs, but recommending the
latest libmpg123 for best performance doesn't sound so bad for me.
What do you think?

Attached is an updated patch with outdated comments/defines removed.


Alrighty then,

Thomas.

PS: I checked again with valgrind if my mpg123 changes cause some
memory mishap ... and accidentally ran mplayer with -ac mp3. Valgrind
didn't like that:

Forced audio codec: mp3
Opening audio decoder: [mp3lib] MPEG layer-2, layer-3
unhandled opc_aux = 0x 2
first_opcode == 0xDF
vex amd64->IR: unhandled instruction bytes: 0xDF 0x93 0x0 0x1 0x0 0x0
==27093== valgrind: Unrecognised instruction at address 0x63f6f3.

I didn't pursue that further now, as I am working on replacing that
code... but I want to have that mentioned at least. Chances are that
valgrind is to blame, as the code works...

PPS: On my K6-III+, I tried to test mplayer SVN, but got that one:

Program received signal SIGILL, Illegal instruction.
0x080875ae in mp_msg ()

Rebuilding with debugging symbols now... but that takes time on the old
machine, so I spare that for another post.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mplayer-libmpg123.diff
Type: text/x-patch
Size: 21334 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20100516/385701ea/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/20100516/385701ea/attachment.pgp>