fix build on arm64-Linux by pedrolcl · Pull Request #30 · madebr/mpg123

pedrolcl · 2026-08-21T16:14:01Z

I have been requested in this PR to submit upstream the two patches already applied to the vcpkg port.

This PR includes two patches to the CMake build system:

Define HAVE_CPU when the architecture is arm64 (not only for APPLE), and check CMAKE_CROSSCOMPILING if not.
Add one library to pkgconfig

The solved issue was a build failure on arm64 (that you can see in the workflow CMake-Linux.yml added in PR #29 )

[ 12%] Building C object src/libmpg123/CMakeFiles/libmpg123.dir/home/runner/work/mpg123/mpg123/src/libmpg123/parse.c.o
In file included from /home/runner/work/mpg123/mpg123/src/libmpg123/mpg123lib_intern.h:307,
                 from /home/runner/work/mpg123/mpg123/src/libmpg123/parse.c:9:
/home/runner/work/mpg123/mpg123/src/libmpg123/optimize.h:198:2: error: #error "Bad decoder choice together with fixed point math!"
  198 | #error "Bad decoder choice together with fixed point math!"
      |  ^~~~~
gmake[2]: *** [src/libmpg123/CMakeFiles/libmpg123.dir/build.make:79: src/libmpg123/CMakeFiles/libmpg123.dir/home/runner/work/mpg123/mpg123/src/libmpg123/parse.c.o] Error 1
gmake[1]: *** [CMakeFiles/Makefile2:498: src/libmpg123/CMakeFiles/libmpg123.dir/all] Error 2
gmake: *** [Makefile:136: all] Error 2
Error: Process completed with exit code 2.

github-actions · 2026-08-21T16:20:00Z

The mpg123-devel mailing list has been notified of the existence of this pr.

sobukus · 2026-08-22T19:46:35Z

+if(WANT_WIN32_UNICODE)
+    string(APPEND LIBSYN123_LIBS " -lshlwapi")
+endif()


Why that? What does libsyn123 use on windows-specific API? Maybe I've forgotten, but it should be just numbers in there, no I/O …

@sobukus: Looking to the history of this patch it was originated in PR microsoft/vcpkg#42601 to fix an issue microsoft/vcpkg#42319 that you already discussed (microsoft/vcpkg#42319 (comment)). Then the large patch was modified and reduced to the minimal current implementation in microsoft/vcpkg#49645.

I don't quite have the processing power to revisit all of that discussion.

Can you verify that the state without the shlwapi added to libsyn123 does work? Seems like this is a tanget to the symbol visibility issue discussed there.

Can you verify that the state without the shlwapi added to libsyn123 does work? Seems like this is a tanget to the symbol visibility issue discussed there.

This is consequence of your commit b3cbfe6 where you wrote:

build system: fix up library dependencies, add Libs.private to .pc
This separates the dependencies of our libraries from the dependencies
of the end-user programs. For this, libcompat got split again, to
separate the libdl-using part for libout123 only. Messy, but seems to
work.

The pkg-config files now contain Libs.private to enable proper usage
of our static libs. This is mainly interesting for Windows, but also
covers libm usage on Unix.

This patch only adds shlwapi to the list of Libs.private on libsyn123.pc, because libsyn123 also needs shlwapi thru the 'compat' module, exactly like libmpg123 and libout123. In my opinion, this library should also link shlwapi:

target_link_libraries(${TARGET} PRIVATE $<$<BOOL:${HAVE_M}>:m> $<$<BOOL:${WANT_WIN32_UNICODE}>:shlwapi>)

This is only useful for Windows, when building static mpg123 libraries, and using pkg-config in a downstream project, which is a not very common scenario.

When I added L31, there was also in target_link_libraries(... $<$<BOOL:${WANT_WIN32_UNICODE}>:shlwapi>...).
Later revisions of the patch might have ignored upstream changes. Maybe it became obsolete.

Also, this is completely unrelated to the topic "arm64-linux".

This is only useful for Windows, when building static mpg123 libraries, and using pkg-config in a downstream project, which is a not very common scenario.

Common or not, vcpkg CI builds downstream packages, and it builds static configurations for Linux and for Windows. Having a portable configuration information system like pkg-config also on Windows facilitates porting other packages.

sobukus · 2026-08-22T19:56:50Z

Applied the FPU part.

sobukus · 2026-08-24T05:25:26Z

Am 23. August 2026 23:28:37 MESZ schrieb "Pedro López-Cabanillas" ***@***.***>:

@pedrolcl commented on this pull request. > +if(WANT_WIN32_UNICODE) + string(APPEND LIBSYN123_LIBS " -lshlwapi") +endif() > Can you verify that the state without the shlwapi added to libsyn123 does work? Seems like this is a tanget to the symbol visibility issue discussed there. This is consequence of your commit b3cbfe6 where you wrote: > build system: fix up library dependencies, add Libs.private to .pc This separates the dependencies of our libraries from the dependencies of the end-user programs. For this, libcompat got split again, to separate the libdl-using part for libout123 only. Messy, but seems to work.

Yes, for libmpg123 and libout123, there is the shlwapi use via libcompat. But in this very commit, libsyn123 got changed from the compat lib to only compat_str, without the path handling that needs win32-specifics. Only the other two libs need that, in my view.

This patch only adds shlwapi to the list of Libs.private on libsyn123.pc, because libsyn123 also needs shlwapi thru the 'compat' module, exactly like libmpg123 and libout123.

Libsyn123 doesn't access the filesystem. It only does memory management. It does no wide string processing. Hence it does not need COMPAT_LIBS. Or am I missing something, again?

…

-- sent from mobile device, trustworthy or not

pedrolcl · 2026-08-24T11:30:19Z

Sorry, but I wasn't involved in the creation / development of the pkconfig.diff patch, and I don't care too much either. I'm only submitting here what is already in the vcpkg port of mpg123.

what I can see in compat_str.c are some Windows Unicode functions, that maybe have something to do with the shell string handling API?

mpg123/src/compat/compat_str.c

Line 89 in 771e737

/* Windows Unicode stuff */

I'm summoning @dg0yt and @toge here just in case they want to explain if this patch is only required in vcpkg context or why it would be useful upstream.

sobukus · 2026-08-24T14:24:48Z

Yes, there is the win32_wide API, but it is not used by libsyn123. One could further separate out compat lib code ... but it is clear (to me;-) that libsyn123 is not supposed to use any system-specific string API. I hope a review of the vcpkg patch shows that this shlwapi dep is unneccessary. -- sent from mobile device, trustworthy or not

dg0yt · 2026-08-24T17:17:21Z

+if(WANT_WIN32_UNICODE)
+    string(APPEND LIBSYN123_LIBS " -lshlwapi")
+endif()


When I added L31, there was also in target_link_libraries(... $<$<BOOL:${WANT_WIN32_UNICODE}>:shlwapi>...).
Later revisions of the patch might have ignored upstream changes. Maybe it became obsolete.

Also, this is completely unrelated to the topic "arm64-linux".

dg0yt · 2026-08-24T17:23:37Z

+if(WIN32 OR ARCH_IS_ARM64)
    set(HAVE_FPU 1)
+elseif(CMAKE_CROSSCOMPILING OR DEFINED HAVE_FPU)
+    set(HAVE_FPU 1 CACHE STRING "Override in triplet if needed")


This comment makes no sense outside vcpkg. ("Triplet" is a vcpkg term.)

BTW introducing CACHE{HAVE_FPU} as "1" only because HAVE_FPU is defined (regardless of value) may not be what you want in general.

You wrote those two lines, two years ago:
https://github.com/microsoft/vcpkg/blame/master/ports/mpg123/have-fpu.diff

I've only submitted what is currently on the vcpkg port, and was not yet submitted upstream. Can you please propose an alternative?

I wrote it for vcpkg two years ago. In vcpkg, it is always a standalone, fresh build. But it may be different in general (repeated config; subproject). I don't want to dive too much into it now.

Basically HAVE_FPU should always be a cache variable. Then the value does not to be determined in repeated configuration runs, and the user might set it in advance or change it later in order to skip or override determination logic.

dg0yt · 2026-08-24T17:28:29Z

+if(WANT_WIN32_UNICODE)
+    string(APPEND LIBSYN123_LIBS " -lshlwapi")
+endif()


This is only useful for Windows, when building static mpg123 libraries, and using pkg-config in a downstream project, which is a not very common scenario.

Common or not, vcpkg CI builds downstream packages, and it builds static configurations for Linux and for Windows. Having a portable configuration information system like pkg-config also on Windows facilitates porting other packages.

fix build on arm64-Linux

5389672

pedrolcl mentioned this pull request Aug 21, 2026

[mpg123] build fix on Linux-arm64 microsoft/vcpkg#53496

Merged

7 tasks

github-actions Bot added the mailing-list-notified label Aug 21, 2026

sobukus reviewed Aug 22, 2026

View reviewed changes

dg0yt reviewed Aug 24, 2026

View reviewed changes

Conversation

pedrolcl commented Aug 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Aug 21, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pedrolcl Aug 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sobukus commented Aug 22, 2026

Uh oh!

sobukus commented Aug 24, 2026 via email

Uh oh!

pedrolcl commented Aug 24, 2026

Uh oh!

sobukus commented Aug 24, 2026 via email

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dg0yt Aug 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

pedrolcl commented Aug 21, 2026 •

edited

Loading

pedrolcl Aug 22, 2026 •

edited

Loading

dg0yt Aug 24, 2026 •

edited

Loading