Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FFmpeg build: Update to 7.0 #6255

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

FreezyLemon
Copy link
Contributor

@FreezyLemon FreezyLemon commented Apr 19, 2024

Build artifacts on my fork. For comparison: Last FFmpeg build on main repo

Binary sizes increase a bit on all platforms, can't do much to avoid that in this case.

Changes:

  • Update FFMPEG_VERSION to "7.0"
  • Remove old workarounds (fixed upstream)
  • Add Android and iOS builds
  • Address a few misc. things

PR including the .NET-side migration: #6256

Files that should be manually removed when updating binaries:

  • Old library versions (e.g. libswscale.so.5 is replaced by libswscale.so.8)
  • Unneeded Android files:
    • libavdevice
    • libavfilter
    • libcpufeatures
    • libc++_shared
    • libmobileffmpeg*
    • libswresample
  • Unneeded iOS files:
    • libavdevice
    • libswresample

Note for later: There are some new hwaccels (vulkan, d3d12, metal?)

@smoogipoo
Copy link
Contributor

smoogipoo commented Apr 22, 2024

We will need to somehow build Android/iOS before this is merged. We include those from an external source right now (which we shouldn't be doing anyway) directly into the osu.Framework.iOS and osu.Framework.Android projects.

Source (iOS): https://github.com/arthenica/ffmpeg-kit as of this PR
Source (Android): https://github.com/tanersener/mobile-ffmpeg as of this PR

@FreezyLemon
Copy link
Contributor Author

Yes, I already noticed this when I realized (in the other PR) that I completely forgot about mobile platforms. Even if ffmpeg-kit did have 7.0 releases out now, I still think it would make sense to do the build in here. Partly for control over the binaries and partly for size optimizations

@FreezyLemon FreezyLemon marked this pull request as draft April 22, 2024 21:35
- Use Android NDK preinstalled in GHA
- Target ARMv7-A, ARMv8-A and (mostly for emulators) x86, x86_64
- Target API level 21 (Android 5+)
- Always enable NEON (requirement for API level 21)
- Collect binaries in osu.Framework.Android so they're bundled
  into the AAR/APK
@FreezyLemon
Copy link
Contributor Author

Added Android build. Looks good so far, but still needs testing (x86, x86_64 are probably not that important as they're most likely just used for emulators).

I enabled NEON in all cases since the minimum supported API level (21) requires it:

All ARMv8-based ("arm64") Android devices support Neon. Almost all ARMv7-based ("32-bit") Android devices support Neon, including all devices that shipped with API level 21 or later.

Raising the API level to 21 means only Android 5+ is supported. A build for API levels < 21 would need to be built without the NDK (or with a very old version) and would probably require more work to get running. But if this is a goal, I can try to do that.

@smoogipoo
Copy link
Contributor

smoogipoo commented Apr 23, 2024

21 is fine. It's what we specify in the props file anyway https://github.com/ppy/osu/blob/1f010c4bfd8236e817e1a13d9616827c97966db1/osu.Android.props#L3

<SupportedOSPlatformVersion>21.0</SupportedOSPlatformVersion>

@bdach
Copy link
Collaborator

bdach commented Apr 23, 2024

Binaries from https://github.com/FreezyLemon/osu-framework/actions/runs/8791966661 seem to work fine in visual tests on both of my devices (one is armeabi-v7a, the other is arm64-v8a, don't care about x86 as it's irrelevant). So good to go from that angle.

@FreezyLemon
Copy link
Contributor Author

FreezyLemon commented Apr 23, 2024

Added iOS build (iOS-xcframework is the final artifact after merging). Like the rest, needs testing. I tried to recreate the current xcframework files as closely as possible.

I looked at veldrid for the CFBundleIdentifier and chose "sh.ppy.osu.Framework.iOS.$lib_name". So e.g. sh.ppy.osu.Framework.iOS.libavcodec.

@FreezyLemon FreezyLemon marked this pull request as ready for review April 23, 2024 15:46
Copy link
Member

@frenzibyte frenzibyte left a comment

Choose a reason for hiding this comment

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

iOS build process needs to be revised, see discord.

prep and build are only separated in order to
facilitate patching, and patching is easier before
./configure is run
@FreezyLemon
Copy link
Contributor Author

After some coordination with @frenzibyte, I found this line in ffmpeg-kit's build scripts. It is pretty non-obvious, but this modifies the config.mak file so that the install_name embedded in the binaries is compatible with framework bundles. The new patch file shows the exact change done by that sed invocation.

The binaries now seem to load, but the proper testing hasn't been done yet.

@frenzibyte
Copy link
Member

Have tested and reported in #6256 (comment), this PR is good to go with regards to the iOS build scripts 👍

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.

None yet

4 participants