[MPlayer-dev-eng] [PATCH] Do not close feed in the middle of decoding.

[Thomas Orgis]

Am Wed, 17 Jul 2013 20:33:55 +0200
schrieb Reimar Döffinger <Reimar.Doeffinger at gmx.de>: 

> Since there is no logic to try and open it again,
> its only effect will be that MPlayer is permanently
> stuck in an error state.
> With some libmpg123 versions it even seems to cause a
> crash, see bugzilla #2149.

... and I am not sure how guaranteed is nice behaviour when trying to
continue decoding after the error. I am not sure right now in what
state libmpg123 is left after failing to switch output format. It
_should_ be robust against further calls, but I'd have to check my code
to ensure that.

With the time I don't have, I at least managed to understand why the
error in bug 2149 is about inability to change format and stand-alone
mpg123 does not show anything relating in-stream format (here: sampling
rate) change: It's MPlayer's demuxer (or something around that, I'm no
MPlayer expert) pre-filtering the stream and removing obvious junk. It
feeds the next thing looking like an MPEG frame to libmpg123. This
frame happens to indicate a change from 44100 Hz to 24000 Hz. Two
issues:

1. We disallowed resampling in libmpg123 (to ensure fixed output size
per frame) and fixed the rate at 44100 Hz
-> failure to set up decoding of 24000 Hz frame, error

2. The internal resync of libmpg123 (as used by mpg123 command),
faced with the original data, gets started on the junk before the 24000
Hz header and actually throws that header away because, being in resync
mode, this new header is checked for validity by peeking ahead for
another header after this frame. This extra check fixes a lot of false
positives in the damaged test files I have (including fuzzed ones).

So, for one, this is caused by too much (half) smarts in either MPlayer
or libmpg123. Removing the extra filtering on MPlayer's side should
have prevented that particular case. But still, you can construct
streams without junk that switch sampling rates. They will pass the
demuxer, they will pass libmpg123's parser. They will fail to decode
because of fixed per-stream sampling rate in ad_mpg123. So I guess my
task would be to add a flag to libmpg123 to silently discard such
changing frames as invalid and hoping that the stream returns to
something manageable later..

The mpg123 console app can change output format on-the-fly; I guess
MPlayer doesn't want to enter that territory? Especially considering
that in reality, we are talking about damaged streams as I don't see
folks putting mixed sampling rate material into one (video) stream on
purpose, except just to mess things up.

Do you have some comments on my interpretation of the issue? Is it
possible to feed libmpg123 the original packets so that the two resync
routines don't collide? You want more dumb libmpg123 that just decodes
the handed frames (== packets) without additional parsing and ignore
the errors it encounters?


Alrighty then,

Thomas (mpg123 maintainer, if someone forgot;-)

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20130723/ae806bd9/attachment.asc>