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

[Thomas Orgis]

Am Thu, 13 May 2010 04:52:15 +0300
schrieb Uoti Urpala <uoti.urpala at pp1.inet.fi>: 

> You're talking about a potential different implementation that handles
> things at a packet level, not the current patch?

Yes. I'm guessing ... Reimar mentioned ds_read_packet() ... that would
give input in form of packets as they are in the a/v stream, I presume.
But those packets are not mpeg frames. 

> The current patch seems
> to feed the decoder 1024 bytes at the time, which should make your "4."
> the largest contribution and result in more buffering than you're
> talking about here.

That's an arbitrary number. we could feed somethin based on the size of
the last decoded frame ... 

> > For the proper feeding it would be crucial here to have proper MPEG
> > stream parsing in advance, though.
> 
> Adding parsing is possible, but does add some complexity so a separate
> parsing step is better avoided if reasonably easy to do.

I wasn't sure about that yet... so there isn't any MPEG parsing going
on in MPlayer, yes? So if a stream, be it embedded in a video or as
standalone audio file, contains funny things like LAME header or ID3
tags, those will still be present. All data that does not correlate to
actual audio output. But then, a sane packaging of an MPEG audio stream
to a video would strip such things as they clearly would mess up a/v
sync, alone due to the fact that different MPEG audio decoders will
handle such stuff differently. The LAME header, for example, is
designed to result in a silent frame of audio for decoders that are not
aware of it, while decoders that _are_ aware are supposed to skip it.

The more I think about that part, the more I hope that nobody puts such
"enriched" MPEG audio into videos... but then, I might not grasp what
magic MPlayer can do to handle such things. What I do know is how badly
a/v sync can get with DVB-T streams that have bad reception and cannot
stop wondering how that is actually possible (a lot of buffering
running wild...). But that's not the point here.

> (It's almost certain that some mp3 input streams will not be correctly
> packetized, so relying on things working without parsing is not an
> option.)

Since we settled that there is no parsing at the MPEG level going on...
then the only thing MPlayer does is scan the data a bit to determine
what to decoder to run on it. So... you could point me to the code that
prevents free format MP3 streams being sent to mpg123. We could decode
them without fuss. They're not massively relevant, but possible.

Example: http://mpg123.org/test/drum-free133.mp3 

Mpg123 can decode that one just fine (recent versions, at least, 1.4.x
should be too old, though).

> > But then, if we'd use the API that is large file sensitive (a read
> > callback), one can make mpg123 just read exactly the frame data needed
> > for decoding instead of feeding a fixed input block.
> 
> I think current ad_mp3lib does basically this, though with the second
> timing method I mentioned. I haven't looked at its sync behavior in
> detail, but my impression is that while status line behavior shows it's
> not completely accurate (and it cannot be because it uses the
> bitrate-dependent timing method with VBR) the buffering amounts stay low
> enough that the inaccuracies don't cause problems in most practical
> cases.

So that is always an option...


> Counting MPEG frames wouldn't really work I think. If you assume
> properly packetized input then frames will correspond to packets, but if
> you handle other kinds of input directly in the decoder then frame
> counts don't help much. You need to know what _packet_ given output
> corresponds to, even if the packet contained multiple MPEG frames or
> only part of one. And even if you assume packetized input you at least
> have to deal correctly with the case where a packet is corrupted and
> doesn't really contain a frame (total counts since resync feel quite
> fragile for tracking things).

OK...

Actually, when we decide that counting MPEG frames doesn't yield
much... I think that we don't actually need to leave the current api of
feeding blocks of data to mpg123_decode(). There is a way to get
packets from MPlayer, with a timestamp, right (be it real or made up)? 
If we feed one packet and read decoded data from mpg123 until it
signals that it needs more data, we get the data corresponding to that
packet. There is still the question of MPEG frames being split among
packets, but this sounds like much less of an issue than blindly
pushing 1K blocks (or 0.5K blocks... or whatnot).

Can you show me an example of the API to use in ad_mpg123.c to get
packetized input with pts? We might get it done properly and still keep
the compatibility to about any libmpg123 out there.

> Well it's hard to comment on the dual-mode idea since I'm not familiar
> with the API and don't know how just how simple or complex it would be.

I attached my first version of ad_mpg123.c for illustration. It uses
the current verison of callback API as available in mpg123 1.12 .
The input reading and decoding happens all in mpg123_read(), similar to
the call to mp3lib in the old code.
Note that, when switching to large file aware API, we have the
issue of the MPlayer build always defining _FILE_OFFSET_BITS, even on x86-64 ...
which would break linking on that platform. If we stay with the
functions in use now, this is no problem.


Alrighty then,

Thomas.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ad_mpg123_handle.c
Type: text/x-c++src
Size: 11688 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20100513/59154c78/attachment.cc>
-------------- 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/20100513/59154c78/attachment.pgp>