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

[Thomas Orgis]

Am Wed, 12 May 2010 14:03:12 +0200
schrieb Diego Biurrun <diego at biurrun.de>: 

> On Wed, May 12, 2010 at 01:27:39PM +0200, Thomas Orgis wrote:
> > The next iteration, fixing stream input handling for large frames (I
> > got one bit wrong in the transition to the old-style API). CBR 320
> > works for me now.
> 
> You've extensively commented your code.  Converting those comments to
> Doxygen syntax would be even better.

Any hints on how it should be structured? Example files that fit into
the existing doxygen setup?
I mainly used doxygen for documenting the mpg123 API ... so, explaining
enums and function arguments, mainly. How should generic free-form text
look for MPlayer?

Hint: libmpcocecs/ad_sample.c could use a little love.


I incorporated the last mentioned alignments... and also added some
more explanation about the large file support thing, after realizing
that simply undefining _FILE_OFFSET_BITS doesn't help anything.

I quote myself, for the lazy folks who don't read attachments:

/* We avoid any usage of mpg123 API that is sensitive to the large file
 * support setting. This avoids conflicts between the large file setting
 * of MPlayer and mpg123. Beginning with mpg123 version 1.12 that would
 * be no issue anymore, but Diego muchly spoke for not demanding such
 * a new release.
 * Thusly, this code is intended to work with version 1.0.0 of libmpg123.
 *
 * Context: It mainly boils down to the mpg123 API containing functions
 * that take/return values of type off_t. Only later it has been realized
 * what issues that causes with the shape-shifting nature of that type
 * on some platforms (32 bit Linux and Solaris). Early versions of libmpg123
 * did not feature large file support in the build scripts and thus usually
 * use 32 bit off_t on 32 bit Linux/Solaris systems, later versions started
 * enabling large file support by default, switching to 64 bit off_t.
 * To avoid subtle breakage, affected function calls got renamed with a _64
 * suffix via macros in mpg123.h. Checks were added to prevent building
 * a program against a libmpg123 with a mismatch in large file setup.
 * This business ended with mpg123 version 1.12, which introduced a dual-mode
 * library, sporting both 32 bit and 64 bit off_t functions. 
 * 
 * At first, I attempted to forcedly undefine _FILE_OFFSET_BITS here to disable
 * the check for matching large file setup in mpg123 headers, but realized that
 * This does not work -- the checks were written in a way that requires correct
 * definition of _FILE_OFFSET_BITS, lack of it also leading to failure.
 * As MPlayer always defines _FILE_OFFSET_BITS, this means we are compatible
 * with basically all libmpg123 installs
 *   1. before version 1.6.0
 *   2. since 1.6.0 with default build (large files enabled)
 *   3. on platforms that are not affected by this mess (64bit, BSDs ...). */


Alrighty then,

Thomas.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mplayer-libmpg123.diff
Type: text/x-patch
Size: 22567 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20100512/2d7fd74b/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/2d7fd74b/attachment.pgp>