configure: set _FILE_OFFSET_BITS to detect fseeko
On Android fseeko()
is not detected properly if if _FILE_OFFSET_BITS
is not set.
It's always set in https://code.videolan.org/robUx4/x264/-/blob/master/common/osdep.h
Merge request reports
Activity
mentioned in commit robUx4/vlc@754ef17d
mentioned in commit robUx4/vlc@e750fb57
mentioned in commit vikram-kangotra/vlc@e2629bc5
mentioned in commit robUx4/vlc@b56abfa2
mentioned in commit robUx4/vlc@fc747154
I think this looks reasonable - @BugMaster or @gramner?
After reading https://android.googlesource.com/platform/bionic/+/master/docs/32-bit-abi.md I don't quite understand what the final result will be. Are we now finding a function that is not actually available with
_FILE_OFFSET_BITS=64
because it is an old ABI (before 24), or are we not finding it now, although it is there.If I understand the commit message correctly, there is a function there, but it's only visible if we define
_FILE_OFFSET_BITS=64
. In the practical x264 code, we do define_FILE_OFFSET_BITS=64
, as referenced incommon/osdep.h
, but configure currently doesn't find it. By adding the same define at configure time, as we use in the real code, we would actually find and use it.Well, I'm not sure about this understanding, because https://developer.android.com/ndk/guides/common-problems says to disable
_FILE_OFFSET_BITS=64
or raise yourminSdkVersion
in case of problem.Setting that value enforces the use of the 64-bit variant of the API. For example that's how we do it in VLC.
However the VLC contribs differ in the usage of that define: https://code.videolan.org/search?search=_FILE_OFFSET_BITS&nav_source=navbar&project_id=435&group_id=9&search_code=true&repository_ref=master.
We turn off fseeko when targetting old Android on 32-bit systems in FLAC. Or we enforce using the 32-bit variant in Lua.
Given that last patch, the issue is clearer.
fseeko()
can be linked with, but on 32-bit variants of Android before 24 it's assumingoff_t
is the 32-bit variant. After that with the define and a recent NDK it will point to the right call.So the patch is not really correct for older NDKs. It would be more correct to check the NDK version, but I'm not sure you want that in x264.
For the record, VLC 4.0 is targeting Android v21. There is no fseeko64 there.
So the patch is not really correct for older NDKs.
Which version of NDK? I have tested with android-ndk-r27b and APIs 21 and 24. I also have android-ndk-r15c. And what would be incorrect? It would detect fseeko because it always ignored _FILE_OFFSET_BITS? We can swap fseeko and fseeko64 checks but I am not sure old versions of NDK supported fseeko64 anyway.
For the record, VLC 4.0 is targeting Android v21. There is no fseeko64.
And it wouldn't be detected with API 21 at least with android-ndk-r27b
I think:
- when targetting v21 without
_FILE_OFFSET_BITS
: fseeko() calls the 32-bit fseeko() - when targetting v24 without
_FILE_OFFSET_BITS
: fseeko() calls the 32-bit fseeko() - when targetting v21 with
_FILE_OFFSET_BITS
: fseeko() calls the 32-bit fseeko() - when targetting v24 with
_FILE_OFFSET_BITS
: fseeko() calls the 64-bit fseeko64()
In the current state of the code, it should detect fseeko is there in all cases. But it should map to fseeko64 on Android v24 (it's not possible to link with it before that). Not only that by
off_t
should match which version of fseeko is used.So maybe the trick is just to fail the fseeko detection for Android older than 24.
- when targetting v21 without
So maybe the trick is just to fail the fseeko detection for Android older than 24.
And it will fail with !161 (merged) here config.log
Not only that by
off_t
should match which version of fseeko is used.We don't use
off_t
type anywhere.And it will fail with !161 (merged) here config.log
Then I think !161 (merged) is OK.
By the way, if you want some Android tests in your CI you can use the Docker images from VLC 4 (which has NDK 26 or 27).
mentioned in commit robUx4/vlc@97b885cc
mentioned in merge request !161 (merged)
Invalid syntax for cc_check (second parameter is CFLAGS). Replaced by !161 (merged).