[MPlayer-dev-eng] [PATCH] update ad_mpg123 in preparation to replace mp3lib

[Diego Biurrun]

On Wed, Feb 29, 2012 at 06:35:22PM +0100, Thomas Orgis wrote:
> Am Wed, 29 Feb 2012 16:37:48 +0100
> schrieb Diego Biurrun <diego at biurrun.de>: 
> 
> > What's the minimum library version you are requiring now?
> 
> 1.2.0, for safety with damaged streams (the MPG123_RESYNC_LIMIT
> setting). I added a check for that to configure, see updated patch
> (also with the formatting stuff).

I'm under the impression you could assume a much newer version as a
reasonable baseline.  Debian stable has 1.12.

> --- libmpcodecs/ad_mpg123.c	(Revision 34780)
> +++ libmpcodecs/ad_mpg123.c	(Arbeitskopie)
> @@ -291,7 +205,19 @@
>  
> +/* return MPG123_OK if decoder is ready to produce output
> +   the other usual return code would be MPG123_NEED_MORE. */
> +static int have_initial_frame(mpg123_handle *mh)
> +{
> +    long rate;
> +    int chan, enc;
> +    /* Abusing the format query function.
> +       It returns MPG123_OK if an intial frame has been parsed.
> +       It returns MPG123_NEED_MORE if input data is needed. */

You usually add stars in front of multiline comments.

> +     /* Not handing NULL pointers for compatibility with old libmpg123. */
> +    return mpg123_getformat(mh, &rate, &chan, &enc);

handLing?

Also, what version would be required here?

> @@ -318,16 +244,11 @@
>  
> +        /* Feed the decoder. This will only fire from second round on. */

the second

> @@ -353,16 +274,45 @@
>  
> +#ifdef AD_MPG123_FRAMEWISE
> +            /* Have to use mpg123_feed() to avoid decoding here. */
> +            ret = mpg123_feed(con->handle, inbuf, incount);
> +#else
>              /* Do not use mpg123_feed(), added in later libmpg123 versions. */
>              ret = mpg123_decode(con->handle, inbuf, incount, NULL, 0, NULL);

Again, what version are we talking about?

>          /* Older mpg123 versions might indicate MPG123_DONE, so be prepared. */
>          else if (ret == MPG123_ERR || ret == MPG123_DONE)
>              break;

same

> --- configure	(Revision 34780)
> +++ configure	(Arbeitskopie)
> @@ -6298,12 +6298,15 @@
>  
> -# Any version of libmpg123 shall be fine.
> +# Any version of libmpg123 that knows MPG123_RESYNC_LIMIT shall be fine.
> +# That is, 1.2.0 onwards. Recommened is 1.14 onwards, though.
>  echocheck "mpg123 support"
>  def_mpg123='#undef CONFIG_MPG123'
>  if test "$_mpg123" = auto; then
>    _mpg123=no
> -  statement_check mpg123.h 'mpg123_init()' -lmpg123 && _mpg123=yes && extra_ldflags="$extra_ldflags -lmpg123"
> +  statement_check mpg123.h 'mpg123_init()' -lmpg123 &&
> +  statement_check mpg123.h 'mpg123_param(NULL, MPG123_RESYNC_LIMIT, -1, 0.)' -lmpg123 &&
> +  _mpg123=yes && extra_ldflags="$extra_ldflags -lmpg123"

Why do you need to check both functions and not just the second?

Diego