Re: [patch v2] external MPEG audio decoding using libmpg123

Am Mon, 28 May 2018 15:40:15 +0200
schrieb Alexander Leidinger <[email protected]>: 

> Glancing at it (mostly high-level stuff, not really looking line by  
> line at C-code):
>   - why do you comment out "-I\$(srcdir)"?

I had trouble with lame's mpg123.h being found before libmpg123's
mpg123.h. I guess things got a bit inconsistent during testing
aproaches. I am trying to get rid of those changes …

>   - "--disable-decoder", what do you mean by "overridden by  
> libmpg123"? 

I moved that bit to the description of --disable-libmpg123 and changed
the wording:

  --disable-decoder          Exclude internal mpg123 decoder
  --disable-libmpg123        Disable use of external libmpg123 decoder
    (normally favoured over internal decoder)

> After reading the sh-code I think you implemented "if no  
> libmpg123, then internal decoder, if internal decoder disabled, then  
> no decoder".

Yeah, sounds about right … I was thinking that the idea is to replace
the internal decoder, so the --disable-decoder flag should be kept to
mean sort of the the same thing in future: Include decoding ability.
The separate --disable/enable-libmpg123 flag is for testing purposes as
long as both implementations exist. In a final change that drops the
internal decoder, I'd also drop the libmpg123 flag and just have
‘decoder’ mean use of libmpg123. I fixed the logic to really disable
all decoding with --disable-decoder now.

I'm not opposing simpler configure logic, it's just what I came up with
to be able to test both implementations.

>   - libmp3lame/Makefile.am: I rather prefer the math lib last

Fine.

Please see the attached updated patch.


Alrighty then,

Thomas
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Lame-dev mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/lame-dev