mp3 file crashes mplayer

[lauri.kasanen@…]

The attached mp3 file crashes mplayer. Mplayer is SVN r36363.

The GDB output is useless despite --enable-debug, so also attaching valgrind output. I'm not sure if this is a bug in libmpg123, or if mplayer is passing invalid data there, please advise.

"MPlayer interrupted by signal 11 in module: decode_audio"

[lauri.kasanen@…]

A small mp3 that crashes mplayer

[lauri.kasanen@…]

GDB output

[lauri.kasanen@…]

valgrind output

[lauri.kasanen@…]

A previous version, SVN-r32769, does not crash. It clicks instead.

The previous version defaulted to mp3lib instead of mpg123, but forcing mpg123 on it via "-ac mpg123" still does not crash.

[reimar]

This is slightly questionable behaviour on MPlayer's side, so I have sent a patch.
However mostly this seems to be an outdated libmpg123 on your side, mine on Debian (1.15.3-1) handles this case fine.
As a workaround, compiling without libmpg123 or using -ac ffmp3 also avoids the issue.

[thomas-forum@…]

(mpg123 upstream here)

Indeed, this is a rather old libmpg123 (4.5 years, from mpg132 1.6.4). I am
promptly reminded of this piece I wrote in the mpg123 mplayer code, in
control():

/* MPlayer ignores this case! It just keeps on decoding.

• So we have to make sure resync never fails ... */

mp_msg(MSGT_DECAUDIO, MSGL_ERR,

"mpg123 cannot reopen stream for resync.\n");

return CONTROL_FALSE;

To handle this,

mpg123_param(con->handle, MPG123_RESYNC_LIMIT, -1, 0.0);

is set before decoding, ensuring maximum error resiliency. The actual failure
libmpg123 encounters is about inability to set the output format after it found
something it thinks it can decode. That one puzzles me, I cannot reproduce it
off-hand with stand-alone mpg123 1.6.4 . Do you employ a special build (with
certain features disabled)? Reimar: Does your build just not crash (by chance)
or does it really not get an error from libmpg123, as it should be? I need to
check the logic if there is indeed still a loophole via an attempted and
failing format switch in current libmpg123.

In any case, there is some irony in some folks bugging me to make libmpg123
more unforgiving to detect bad files and programs on the other side crashing
unless we continue no matter what. Reimar: Is it planned that MPlayer handles
the failure as indicated by CONTROL_FALSE in future? Otherwise, one might
clarify that for codec writers.

[lauri.kasanen@…]

Thomas:

As far as I remember, the only tweaks are with some output drivers disabled and possibly C{,XX}FLAGS.

Or do mean mplayer config? It was all default values for this binary.

[thomas-forum@…]

I meant disabling of features of libmpg123 that concern output format (automatic resampling, for example). General CFLAGS should not make a difference.

I take it you got a mpg123 binary matching the libmpg123 you used. Can you send me the screen output of mpg123 -vvv test.mp3? My build of mpg123 1.6.4 does a resync, but no fatal complaint, nothing about an attempt to change output format.

We may take this off the MPlayer bugzilla, though. For MPlayer, the bug was that the mpg123 input closed on the error while MPlayer ignored that. That there is an error to begin with, is some kind of mpg123 bug. Feel free to mail me at maintainer@…, or open a tracker item (​http://mpg123.org/bugs).

But the pragmatic resolution is to update mpg123 to something more recent; I cannot promise to find the time to really settle it with the old build.

[lauri.kasanen@…]

Sent an email.

Re this bug, will there be a notification when the patch is committed, so I know when to test?

[reimar]

Reimar: Is it planned that MPlayer handles the failure as indicated by CONTROL_FALSE in future?

MPlayer by policy always tries to get as much data as possible out of the file. Which in this specific case it does by "handling" it via ignoring it.
Though actually I'd say the resync function is just broken: there is only one good reason for resync to fail, and that is reaching the end of the stream. In all other cases there is still data that might be decodeable and thus no reason to stop.
Which also tells you how you can make MPlayer stop: by setting EOF on the stream. But as said, I'd consider that the wrong way to do, it is not the decoder's job to push policy on the application by deciding when to give up trying to decode, that should be handled at a higher layer (in the case of MPlayer, that layer being the user).

[thomas-forum@…]

preliminary libmpcodecs/ad_mpg123.c (tested with MPlayer-1.1.1)

[thomas-forum@…]

OK, I see that I can just make ad_mpg123 change formats (even if it is bogus in the specific case here). This eliminates the error from libmpg123, as all formats are allowed.

I also made another testfile with a valid sample rate change: ​http://mpg123.org/test/44and22.mp3 . This plays fine with plain mpg123 and my build of MPlayer. With -ac ffmp3, you get speedup in the second part, with -ac mpg123, the output rate is matched and you get a normal beat (some old techno beat) following a male voice saying "the will".

I simply attached the changed ad_mpg123.c to be put into libmpcodecs/ , so that it can be tested with release and svn (in case Reimar's patch would conflict).

Reimar: Now, the only occasions of error returns from libmpg123 can be considered exceptions: Either a bug in libmpg123 or a library build that has non-default functionality. Would it be proper to EOF the stream or rather rudely tell MPlayer to exit, as the library is not really compatible (or buggy)?
I like being defensive ... as the library could do _anyting_. But one could also simply ignore such exceptional error returns, as, well, an external lib can do _anything_, even claiming success while messing up.

[reimar]

This has actually been fixed a long time ago in r36461, closing.