Skip to content

Conversation

@theogf
Copy link
Contributor

@theogf theogf commented Feb 4, 2024

When using (the absolutely amazing) run_wizard the following failed to work:

 - Linux armv7l {call_abi=eabihf, libc=musl}
 - Linux i686 {libc=musl}
 - Linux armv6l {call_abi=eabihf, libc=musl}
 - Linux x86_64 {libc=musl}
 - Linux aarch64 {libc=musl}
 - macOS x86_64
 - macOS aarch64

I guess it can be updated later to support at least macOS?

It's my first use of BinaryBuilder so I might have messed up some part?

Copy link
Member

@giordano giordano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Error on Darwin is

[22:45:08] skelform.c:214:5: error: incompatible function pointer types initializing 'sox_format_handler_seek' (aka 'int (*)(struct sox_format_t *, unsigned long)') with an expression of type 'int (sox_format_t *, uint64_t)' (aka 'int (struct sox_format_t *, unsigned long long)') [-Wincompatible-function-pointer-types]
[22:45:08]     seek, encodings, NULL, sizeof(priv_t)
[22:45:08]     ^~~~

I wonder if we need something like chirlu/sox#2 (replacing uint64_t -> sox_uint64_t) somewhere.

@giordano
Copy link
Member

giordano commented Feb 4, 2024

I guess it can be updated later to support at least macOS?

It's up to you to decide whether to resolve the issue now (above I gave a possible hint), or postpone it to a follow-up PR, but at least now there's a record of what's wrong.

@theogf
Copy link
Contributor Author

theogf commented Feb 5, 2024

I guess it can be updated later to support at least macOS?

It's up to you to decide whether to resolve the issue now (above I gave a possible hint), or postpone it to a follow-up PR, but at least now there's a record of what's wrong.

I pinged the PR there, but it sounds unlikely it will be merged. Do you think one could add the patch in the MacOS installation script? E.g. using git patch

@theogf
Copy link
Contributor Author

theogf commented Feb 5, 2024

I got confused why the patch would not go through, but that's because it looks like the source code already contains this change...

Copy link
Member

@fingolfin fingolfin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@theogf It seems this is another PR that "fell of the stack", sorry about that :-(. If you are still interested in getting this merged, I left a few suggestions, but I guess they are not strictly necessary, we could probably also merge "as is" -- but you'd first have to rebase it on latest master so the CI tests can run through.


# Collection of sources required to complete build
sources = [
GitSource("https://github.com/theogf/sox.git", "6c0b78e6f6735085d7379ac2b9499f47bfda94ea")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be an unofficial mirror? Wouldn't it be better to use the official archive https://sourceforge.net/projects/sox/files/sox/14.4.2/sox-14.4.2.tar.bz2 ?

Suggested change
GitSource("https://github.com/theogf/sox.git", "6c0b78e6f6735085d7379ac2b9499f47bfda94ea")
ArchiveSource("https://sourceforge.net/projects/sox/files/sox/$(version)/sox-$(version).tar.bz2",
"81a6956d4330e75b5827316e44ae381e6f1e8928003c6aa45896da9041ea149c"),

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I just realized this is your own repository it points out, with a patch applied! OK, but I still think it's better to use the official archive with a .patch file. This can be easily obtained: From the webpage representing your patch commit, i.e., theogf/sox@6c0b78e, you can get it by adding .patch to the URL, that is: https://github.com/theogf/sox/commit/6c0b78e6f6735085d7379ac2b9499f47bfda94ea.patch -- you can download that file, and add it here (but with a better name, e.g. sox_uint64.patch -- typically we add subdirectories bundled/patches/, there are plenty examples in this repository.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the patch is still missing which I think explains the build errors on macOS.

On RISC-V it dies with

[23:00:46] Invalid configuration `riscv64-linux-gnu': machine `riscv64' not recognized
2025-11-25 00:00:46 CEST
[23:00:46] configure: error: /bin/sh ./config.sub riscv64-linux-gnu failed

which might suggest you need that autoreconf -i call after all, to get a newer config.sub and config.guess

@theogf
Copy link
Contributor Author

theogf commented Nov 24, 2025

@theogf It seems this is another PR that "fell of the stack", sorry about that :-(. If you are still interested in getting this merged, I left a few suggestions, but I guess they are not strictly necessary, we could probably also merge "as is" -- but you'd first have to rebase it on latest master so the CI tests can run through.

I moved on to use ffmpeg in the end (with its own issues) but I will try to finish this anyway

@theogf theogf force-pushed the wizard/sox-v14.4.2_e5bd1933 branch from 49e4060 to 491ff7f Compare November 24, 2025 21:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants