Build ffmpeg for linux-arm, linux-arm64, linux-x86 #6288
Conversation
kvnp
changed the title
There was a problem hiding this comment.
Some notes:
- Please remove changes not related to the Linux build (macOS bump, merging the Windows build process). These are out of scope and maybe change the resulting binaries which would need testing
- The x86 build doesn't work. x86 support is not necessary so you don't need to include it at all, but if you do want to include it, please make sure it works.
I wonder if there's a way to achieve the same builds using cross-compilation similar to the Windows build scripts. That would make it a lot easier to compile these locally (installing QEMU and docker is fairly hefty for these simple builds). That said, it's not a high priority or blocking issue.
.github/workflows/build-ffmpeg.yml
Outdated
| strategy: | ||
| matrix: | ||
| include: | ||
| - { name: macOS-universal, path: osu.Framework.NativeLibs/runtimes/osx/native } | ||
| - { name: linux-arm64, path: osu.Framework.NativeLibs/runtimes/linux-arm64/native} | ||
| - { name: linux-arm, path: osu.Framework.NativeLibs/runtimes/linux-arm/native} | ||
| - { name: linux-x64, path: osu.Framework.NativeLibs/runtimes/linux-x64/native} | ||
| - { name: linux-x86, path: osu.Framework.NativeLibs/runtimes/linux-x86/native} | ||
| - { name: win-arm64, path: osu.Framework.NativeLibs/runtimes/win-arm64/native} | ||
| - { name: win-x64, path: osu.Framework.NativeLibs/runtimes/win-x64/native} | ||
| - { name: win-x86, path: osu.Framework.NativeLibs/runtimes/win-x86/native} |
There was a problem hiding this comment.
Not a fan of this. This will create 8 separate "Update FFmpeg binaries" PRs every time this workflow is run.
.github/workflows/build-ffmpeg.yml
Outdated
| options: -v ${{ github.workspace }}:/workspace -w /workspace -e DEBIAN_FRONTEND=noninteractive | ||
| run: | | ||
| apt-get update -y -qq | ||
| apt-get install -y build-essential curl nasm libva-dev libvdpau-dev |
There was a problem hiding this comment.
libva-dev and libvdpau-dev can be removed from this list. They are from an older version of the build process but were accidentally kept in the dependency list.
nasm is only required for x86 and x86_64 builds.
|
|
||
| if [ $(uname -m) == "x86_64" ]; then | ||
| ARCH="x64" | ||
| elif [ $(uname -m) == "i686" ]; then | ||
| ARCH="x86" | ||
| elif [ $(uname -m) == "aarch64" ]; then | ||
| ARCH="arm64" | ||
| elif [ $(uname -m) == "armv7l" ]; then | ||
| ARCH="arm" | ||
| else | ||
| echo "Unsupported architecture: $(uname -m)" | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
Please move this to the top (like the arch-related stuff in the other build scripts)
Co-authored-by: FreezyLemon <[email protected]>
|
This one can be updated now that #6255 has been merged. |
linux-x86 now builds after disabling asm. I'm not sure about the performance implications, but I'm glad to be one step ahead for now. According to this issue, GCC is having trouble building FFMPEG for x86 32-bit on a 64-bit system. I would open a new PR when this is fixed.
I find it much easier to set up Docker than to try to piece together a cross-compilation toolchain. If multiplatform images are needed, which is the case here, Docker can install the QEMU binaries by itself with a single command. I would leave it as is for now, but would like to revisit this in the future when I have more experience with cross compiling on Linux. |
kvnp
changed the title
There was a problem hiding this comment.
Thanks for addressing my review. I left some feedback, sadly the dpkg change breaks the scripts for me (I'm not on a Debian-based distro).
linux-x86 now builds after disabling asm. I'm not sure about the performance implications, ...
It's cool that you got it to work, but without someone confirming that the binaries are usable in terms of performance, I'm not entirely sure it's a great idea to include them..
I find it much easier to set up Docker than to try to piece together a cross-compilation toolchain.
FFmpeg really doesn't require much of a toolchain, basically gcc/clang plus the GNU assembler. It shouldn't be more complicated than the Windows script that already exists. To be fair, Docker is still simpler, but it's just not my preference.
| if [ "$(dpkg --print-architecture)" = "i386" ]; then | ||
| apt-get install -y nasm | ||
| fi |
There was a problem hiding this comment.
--disable-asm on x86 Linux should mean that this is not needed
| if [ "$(dpkg --print-architecture)" = "amd64" ]; then | ||
| arch="x64" | ||
| elif [ "$(dpkg --print-architecture)" = "i386" ]; then | ||
| arch="x86" | ||
| elif [ "$(dpkg --print-architecture)" = "arm64" ]; then | ||
| arch="arm64" | ||
| elif [ "$(dpkg --print-architecture)" = "armhf" ]; then | ||
| arch="arm" | ||
| else | ||
| echo "Unsupported architecture: $(dpkg --print-architecture)" | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
Please don't use dpkg for this. uname is available almost everywhere while dpkg is Debian-specific.
There was a problem hiding this comment.
to add to this, you can use uname -m for this specifically and use a case-switch so you don't have to nest it this way.
| if [ "$arch" = "x86" ] && [ "$(uname -s)" = "Linux" ]; then | ||
| FFMPEG_FLAGS+=( | ||
| --disable-asm | ||
| ) | ||
| fi |
There was a problem hiding this comment.
Could be moved to build-linux.sh (the other platforms also do arch-specific changes inside their own build scripts, see for example build-win.sh lines 23 - 38)
|
Looks like ARM64 runners will be available in the end of the year and is available now for GitHub Enterprise Cloud/Team orgs. I suggest holding off this one until this becomes generally available. |
This PR adds the targets linux-arm, linux-arm64 as part of ppy/osu-deploy#170. Will be updated once #6255 is merged.