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

Rebuild Android AARCH64 FFmpeg libraries with --disable-asm #61

Closed
wants to merge 1 commit into from
Closed

Rebuild Android AARCH64 FFmpeg libraries with --disable-asm #61

wants to merge 1 commit into from

Conversation

0x647262
Copy link

@0x647262 0x647262 commented Mar 10, 2021

FFmpeg version: 3.0.12

Closes #59

I attempted to use a more recent release (tested with both 4.3.2 & 3.2.15), but a handful of titles exhibited the following regression during cutscenes / videos:

ScreenRecord-2021-03-10-00-02-38.mp4

For reference here's a working build using the libraries in this MR (FFmpeg v3.0.12):

ScreenRecord-2021-03-10-00-35-57.mp4

I used my ppsspp branch this project to generate these files: https://github.com/0x647262/ffmpeg-android-maker

@hrydgard
Copy link
Owner

--disable-asm will presumably slow down / increase battery consumption, but maybe not a big deal given the small resolution of PSP videos?

@0x647262
Copy link
Author

0x647262 commented Mar 11, 2021

--disable-asm will presumably slow down / increase battery consumption, but maybe not a big deal given the small resolution of PSP videos?

There doesn't seem to be any performance impact from my limited testing. I haven't had time to test battery life consumption, but I'm going to go out on a limb and say that it's likely an unnoticible impact in games that don't rely heavily on cutscenes.

On a related note, I was also able to build a package using dynamicly linked (without the --disable-asm option) libraries instead of the staticly linked ones... Is there any particular reason that static libraries were chosen for PPSSPP's usecase?

@hrydgard
Copy link
Owner

Static libraries were chosen for Android and general ease of deployment on platforms like iOS where signing becomes a mess as soon as you have more than one binary, etc.

@0x647262
Copy link
Author

@hrydgard any reservations against getting this merged?

@hrydgard
Copy link
Owner

I'd like to understand why --disable-asm fixes anything. Is there asm code in there that's not PIC-safe, or something?

@0x647262
Copy link
Author

Is there asm code in there that's not PIC-safe, or something?

That's correct.

More information about it can be gleaned from: https://stackoverflow.com/a/39965908 and the links contained within that answer. It seems the release I'm using (3.0.12) has the patch mentioned in the linked reply but the build still complains about text relocations when built without --disable-asm, so I'm at a bit of a loss...

@hrydgard
Copy link
Owner

hrydgard commented Mar 18, 2021

Have to admit I'm still a little confused here. This PR rebuilds the Android target. However, #59 is about some non Android linux platform. And I have no issues with the existing Android builds...

Also, like has been said, I think the path forward is integrating the parts of ffmpeg that we use into our build system instead of having these binaries around...

But if this really solves something for the time being, I might be OK with it.

@xsacha
Copy link
Collaborator

xsacha commented Mar 20, 2021

I always found it easier to place the binaries in here because FFMPEG rarely requires any updates. I'm not exactly sure why the update was needed here (core functionality of FFMPEG hasn't changed in a long time). The main reasons I update it in my own projects are related to format support (AV1 codec), newer NVDEC implementations (nvidia decoder) or embedded hardware support.
Basically, I wouldn't update FFMPEG unless it's absolutely required. In my own projects (facial recognition, video analytics), I still use a very ancient version for Android and it works great even on the latest phones.
Yeah static is definitely the best idea, especially for mobile, as @hrydgard mentioned.

@0x647262
Copy link
Author

However, #59 is about some non Android linux platform.

Reviewing the issue it is (Linux?) ARM64 specific, so this doesn't actually close #59, it only handles the Android ARM64 usecase.

Also, like has been said, I think the path forward is integrating the parts of ffmpeg that we use into our build system instead of having these binaries around

How big of a task is this? I ask because if it's mostly busywork I wouldn't mind working on it.

And I have no issues with the existing Android builds...
...
But if this really solves something for the time being, I might be OK with it.

It's definitely a stopgap for Android ARM64 builds. I think it's still working with your builds because they may be using an older toolchain? I'm able to reproduce the error from #59 on both my local machine (SDK 26.1.1 / NDK 21.4.7075529 & 22.0.7026061) and on F-Droid's build server.

@unknownbrackets
Copy link
Collaborator

I think building FFmpeg on Linux especially makes a lot of sense, because of the variance. Other platforms, especially Windows and Android could be trickier.

Building FFmpeg requires yasm, at least, and on Windows msys2 (well, only the configure script requires this.) For Windows and Android builds, this would make PPSSPP harder to compile. We've tried to keep the Windows build very easy for users to build; it's basically three steps:

  1. Install Visual Studio (which can actually install Git for you.)
  2. Clone the repo and its submodules.
  3. Compile.

There's no "install msys2" or "learn how to install required dev packages using msys2" or "put yasm on your path" in that list.

Similarly, the process for building Android, even on Windows, is basically:

  1. Install Android Studio.
  2. Install the Android SDK and NDK using Android Studio.
  3. Clone the repo and its submodules.
  4. Compile.

More steps, but still pretty much the bare minimum.

It is harder to compile on Linux and macOS / iOS (but only just.) There you have to install packages mostly unavoidably, so having to install yasm as well is almost not adding anything.

With that said, maybe we could just translate the relevant asm to C inline asm and put all the ffmpeg files into a vcxproj file / CMakeLists.txt for Android. I think it's only the configure script that requires msys2 tools / versions of gawk and make.

Obviously if we didn't use the configure script, this would make it harder to update, which is only relevant to us really for recording videos of gameplay - the parts of FFmpeg we use for video and audio decoding have not changed in any useful way for us in years (basically since they fixed the last bugs in ATRAC3+ support.) Either way, this is probably the path to a simplified build on all platforms without shipping prebuilt libs...

-[Unknown]

@0x647262
Copy link
Author

0x647262 commented May 31, 2021

Thanks for the overview @unknownbrackets!

I attempted to integrate building FFmpeg for Android builds, and it's definitely something outside of my current capabilities.

With that said, maybe we could just translate the relevant asm to C inline asm and put all the ffmpeg files into a vcxproj file / CMakeLists.txt for Android. I think it's only the configure script that requires msys2 tools / versions of gawk and make.

AFAIK, this is what their --disable-asm and --disable-yasm build options do, assuming I'm understanding those build flags correctly... Though that is where the aforementioned performance penalties are incurred, since the {Y,}ASM bits that are replaced by C were written in {Y,}ASM specifically for perfomance.

I should have some time to dedicate to this MR this upcoming week, so if there is anything holding this MR back that needs to be addressed just let me know. Thanks again for your time!

@0x647262 0x647262 closed this by deleting the head repository Apr 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Linker fails on ARM64
4 participants