Re: Update mpg123 during 2021Q1 freeze?

[Thomas Klausner <wiz%NetBSD.org@localhost>]

On Wed, Mar 24, 2021 at 01:15:40AM +0100, Dr. Thomas Orgis wrote:
> Hi,
> 
> as a first measure, it has been suggested to me to update mpg123 in
> pkgsrc to fix a nasty performance issue encountered on SPARC with the
> pkgsrc build of mpg123 1.26.x.
> 
> In short: This line in audio/mpg123/Makefile.common
> 
> CONFIGURE_ARGS+=        --with-optimization=0   # our CFLAGS are good enough
> 
> causes the default of -ffast-math being dropped from the mpg123 build.
> This causes floating point denormals to enter the picture when you use
> the proper resampler introduced in 1.26.0. These degrade performance
> horribly on SPARC (observed a factor of more than 1000, I kid you not).
> 
> This is fixed by letting mpg123 have its default flags, which is
> 
>     CFLAGS="-O2 -fomit-frame-pointer -funroll-all-loops -finline-functions -ffast-math $CFLAGS"
> .
> The environment-provided CFLAGS are appended, and this line is only the
> default when GCC is detected.
> 
> These flags are unlikely to appear as default pkgsrc CFLAGS, as they
> are rather aggressive for general use. But they are what is regularily
> used during mpg123 development and arguably for most builds out there.
> 
> At the same time, mpg123 1.26.5 avoids the big impact of the missing
> flags by adding (literally) a workaround to the filters to avoid
> denormals. So a simple update to this bugfix release would also help
> independently of the CFLAGS question.
> 
> The only other changes in the library fixes parsing of some ID3v2 tags
> that would have been mistreated before.
> 
> So, is it OK, to
> 
> a) drop the CFLAGS line from Makefile.common
> b) update audio/mpg123 from 1.26.4 to 1.26.5
> 
> during the freeze? At least one of those really should be done,
> preferrably both. But I might be biased as mpg123 upstream;-)

The update is definitely fine, mpg123 is a leaf package AFAIK.

We try to avoid having too specific CFLAGS in packages in pkgsrc, for
at least the following reasons:

* backwards compatibility with older CPUs on the same platform
* pkgsrc user preferences (setting CFLAGS themselves, e.g. using devel/cpuflags)
* compiler compatibility

That said, since I assume you'll be maintaining mpg123 in pkgsrc,
please go ahead with your change if you think it's for the best
considering these points.

Thanks,
 Thomas