Reuse lame flags in mpg123

[joetoddsonos]

The LAME_CFLAGS and LAME_LIBS variables don't seem to be used in the libsndfile configure script - https://github.com/libsndfile/libsndfile/blob/master/configure.ac#L437.

We could fix it there or just fudge it here by injecting them into the MPG123 flags?

[joetoddsonos]

I see you have already done this in #14

[bastibe]

I apparently did not do it correctly, though. If you find a working version, please do let me know!

[joetoddsonos]

Hmm with these changes, otool showed me that the system lame library was no longer a dependency of libsndfile.dylib, and I was able to use the generated sndfile-info binary to parse an MP3 file. Are there any other checks I should be doing?

[bastibe]

That sounds much more successful than my attempt was.

[joetoddsonos]

That is strange, the same build job worked on my fork - https://github.com/joetoddsonos/libsndfile-binaries/runs/7634991941?check_suite_focus=true

[bastibe]

This is very strange. It doesn't seem to find any of the libraries. There is something else going on, I fear.

The build job on your fork does not seem to mention "lame" or "mpg" anywhere. That's weird as well.

Regrettably, my wife's Mac is too old to run the build script, so I can't debug this locally.

[joetoddsonos]

Oh I built the wrong branch on my fork, can now reproduce so will debug and get back to you 👍

[joetoddsonos]

Ok I think that has now fixed the build issue. It seemed like the download link for mpg123 and lame was unreliable, no idea why this caused all libraries not to be found but 🤷‍♂️

Successful build on my fork: https://github.com/joetoddsonos/libsndfile-binaries/actions/runs/2789826082

➜  Downloads otool -L libsndfile.dylib
libsndfile.dylib:
	/usr/local/lib/libsndfile.1.dylib (compatibility version 2.0.0, current version 2.34.0)
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1311.0.0)

Attached a dump of the strings included in the lib, I think it includes at least something from each of the dependencies
libsndfile-dylib-strings.log

[bastibe]

Thank you so much for your continued work on this issue!

I think the line ld: warning: dylib (/usr/local/lib/libmp3lame.dylib) was built for newer macOS version (11.0) than being linked (10.9) indicates that it's still linking against the system liblame dylib instead of our hand-compiled static library.

[joetoddsonos]

No problem! My understanding was that that warning reflects the fact that we are building on macOS 11, but the MACOSX_DEPLOYMENT_TARGET is set to 10.9.

If you compare the output from otool between your bastibe/1.1.0-build branch and the latest action build, it shows the system libmp3lame.dylib is no longer a dependency.

➜  libsndfile-binaries git:(bastibe/1.1.0-build) ✗ otool -L libsndfile_x86_64.dylib
libsndfile_x86_64.dylib:
	/usr/local/lib/libsndfile.1.dylib (compatibility version 2.0.0, current version 2.34.0)
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1311.0.0)
	/usr/local/opt/lame/lib/libmp3lame.0.dylib (compatibility version 1.0.0, current version 1.0.0)
➜  libsndfile-binaries git:(bastibe/1.1.0-build) ✗ otool -L ~/Downloads/lib/libsndfile.dylib
/Users/joe.todd/Downloads/lib/libsndfile.dylib:
	/usr/local/lib/libsndfile.1.dylib (compatibility version 2.0.0, current version 2.34.0)
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1311.0.0)

[joetoddsonos]

You can prove this in a slightly hacky way by placing the libsndfile.dylib into your system directory and trying to run sndfile-info on an MP3 file. If you also temporarily remove libmp3lame from your system directory you will notice that the previous build shows an error when reading an MP3 file.

➜  libsndfile-binaries git:(bastibe/1.1.0-build) ✗ sndfile-info ~/Music/Tabs/Intervals-Circadian/3.\ Jam\ Tracks/03\ -\ LUNARTIC\ \(Jam\ Track\).mp3
dyld[90368]: Library not loaded: '/usr/local/opt/lame/lib/libmp3lame.0.dylib'
  Referenced from: '/usr/local/lib/libsndfile.1.dylib'
  Reason: tried: '/usr/local/opt/lame/lib/libmp3lame.0.dylib' (no such file), '/usr/local/lib/libmp3lame.0.dylib' (no such file), '/usr/lib/libmp3lame.0.dylib' (no such file), '/usr/local/Cellar/lame/3.100/lib/libmp3lame.0.dylib' (no such file), '/usr/local/lib/libmp3lame.0.dylib' (no such file), '/usr/lib/libmp3lame.0.dylib' (no such file)

whereas with the latest action build, it parses the MP3 file ok.

[bastibe]

That's wonderful news! Thank you so much! This might have finally resolved that last blocker for the next release of soundfile!

I'll build the next beta release tomorrow.