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

disable fseeko in android before API 24 #691

Merged
merged 2 commits into from
Apr 22, 2024
Merged

Conversation

robUx4
Copy link
Contributor

@robUx4 robUx4 commented Apr 22, 2024

It can be linked in the NDK so check_function_exists() detects it. But it's only supporting _FILE_OFFSET_BITS=64
since Android API 24.

With NDK 26 it's no longer possible to build assuming the API is always available.

Fallback to 32-bit fseek/ftell whenever HAVE_FSEEKO is not set.

robUx4 added 2 commits April 22, 2024 12:04
It can be linked in the NDK so check_function_exists detects it. But it's only supporting _FILE_OFFSET_BITS=64
since Android API 24 [1].

With NDK 26 it's no longer possible to build assuming the API is always available.

[1] https://android.googlesource.com/platform/bionic/+/main/docs/32-bit-abi.md
@ktmf01
Copy link
Collaborator

ktmf01 commented Apr 22, 2024

Hi @robUx4,

Thanks for the PR. However, I don't know how to check this. There is another PR, #659, that tries to do the same, but simpler. Could you perhaps review that one, as it does not seem to rely on CMake?

@robUx4
Copy link
Contributor Author

robUx4 commented Apr 22, 2024

It does not "rely" on CMake because the fix in the header will work for every autotools build that also detects fseeko() is not available.

The CMake fix is to handle the fseeko detection for Android too.

@ktmf01
Copy link
Collaborator

ktmf01 commented Apr 22, 2024

Yes, but I guess that means the autotools build system would need a fix too? (I'm not saying it needs a fix in this PR though, I'm just wondering whether this means this PR only fixes it for CMake building) I have absolutely zero knowledge on building for Android.

@robUx4
Copy link
Contributor Author

robUx4 commented Apr 22, 2024

autotools uses AC_FUNC_FSEEKO which is a common autoconf function: https://www.gnu.org/software/autoconf/manual/autoconf-2.66/html_node/Particular-Functions.html. It will define HAVE_FSEEKO accordingly.

I assume it's working with Android too, but we (VLC) do not have any contrib that use it (or rather we build with CMake for those who do).

@ktmf01
Copy link
Collaborator

ktmf01 commented Apr 22, 2024

Thanks! I guess there's no patching for autotools necessary there indeed.

@ktmf01 ktmf01 merged commit 49ab34d into xiph:master Apr 22, 2024
14 checks passed
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.

2 participants