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

Make check for __ANDROID_API__ run time instead of compile time #1262

Closed
Consti10 opened this issue May 19, 2020 · 6 comments
Closed

Make check for __ANDROID_API__ run time instead of compile time #1262

Consti10 opened this issue May 19, 2020 · 6 comments

Comments

@Consti10
Copy link

I'm always frustrated when I cannot use newly introduced NDK features because I am limited by my minSdkVersion . I understand that it is not possible to support them on older devices, but I don't understand why I cannot use NDK features introduced after my minSdkVersion on Devices where the OS is higher than my minSdkVersion.

E.g. I want to do somtehing like this:

#if __ANDROID_API__> __ANDROID_API_P__ //28
    // Use a cool NDK feature
#else
    // Fall back to using some workaround that works on all devices
#endif

But NOT evaluated at compile time but rather at run time.

if(my application is running on Android P or higher){
     // Use a cool NDK feature
}else{
      // Fall back to using some workaround that works on all devices
}

Is that possible ? You could compile your native code multiple times for each minSdkVersion but that seems like unneccesary hustle to me.

@DanAlbert
Copy link
Member

You can do this with dlopen/dlsym, but it's not the most ergonomic thing in the world.

We've occasionally talked about making this a bit easier by making weak symbols for everything, but that doesn't help when the whole library is unavailable in your minSdkVersion. Plus, since there are no strong checks for this, it would be really, really easy to forget the condition (or more likely not even know it's required).

My preferred answer to this problem is to hide all the ugliness in jetpack. It can do all the dlsym crap on your behalf and provide a fallback implementation when it's not available, or have some easier conditional access method when no fallback is possible.

@DanAlbert
Copy link
Member

(had closed because I'm fairly certain we already have something open for this, but I don't want to dig it up on my phone so I'll try to find it tomorrow)

@DanAlbert DanAlbert reopened this May 19, 2020
@Consti10
Copy link
Author

I always tought the Android NDK Native APIs .so files come with the Android OS and only the header files are distributed via gradle & NDK ?

If you are worried about accidental use what about replacing

#if __ANDROID_API__ >= 28
/**
 * Release the reference to the native ASurfaceTexture acquired with
 * ASurfaceTexture_fromSurfaceTexture().
 * Failing to do so will result in leaked memory and graphic resources.
 * \param st A ASurfaceTexture reference acquired with ASurfaceTexture_fromSurfaceTexture()
 */
void ASurfaceTexture_release(ASurfaceTexture* st) __INTRODUCED_IN(28);
#endif /* __ANDROID_API__ >= 28 */

With something like that:

#if __ANDROID_API__ >= 28 || CHECK_AVAILABILITY_AT_RUN_TIME_USE_WITH_CAUTION
/**
 * Release the reference to the native ASurfaceTexture acquired with
 * ASurfaceTexture_fromSurfaceTexture().
 * Failing to do so will result in leaked memory and graphic resources.
 * \param st A ASurfaceTexture reference acquired with ASurfaceTexture_fromSurfaceTexture()
 */
void ASurfaceTexture_release(ASurfaceTexture* st) __INTRODUCED_IN(28);
#endif /* __ANDROID_API__ >= 28  || CHECK_AVAILABILITY_AT_RUN_TIME_USE_WITH_CAUTION */

Then every app developer can use it like this:

#define CHECK_AVAILABILITY_AT_RUN_TIME_USE_WITH_CAUTION
#include <surface_texture.h>

@Consti10
Copy link
Author

Consti10 commented May 19, 2020

I did some experiments. First, I tried to recreate the above idea by re-defining macros. This did not work. Snippet:

#undef __ANDROID_API__
#define __ANDROID_API__ 28
#undef __INTRODUCED_IN
#define __INTRODUCED_IN(api_level) __attribute__((annotate("introduced_in=21")))

#include <android/surface_texture_jni.h>
#include <android/surface_texture.h>

#undef __ANDROID_API__
#undef __INTRODUCED_IN
#define __ANDROID_API__ 21
#define __INTRODUCED_IN(api_level) __attribute__((annotate("introduced_in=" #api_level)))

However, manually passing "-DANDROID_PLATFORM=28" to cmake even tough I am using minSdkVersion=21 resultet in my code compiling and running on both api 21 and api 28

Is there any disadvantage to manually specifying ANDROID_PLATFORM
except the missing compile time check ? Do you have any ideas for alternative solution(s) ?

@DanAlbert
Copy link
Member

DanAlbert commented May 19, 2020

A scary looking macro only solves part of the problem (you still have no way to weakly load the library that didn't exist in your minSdkVersion), and there's no way to ensure that some third party dependency won't infect the user's build. If you depend on libfoo and foo.h does #define AVAILABILITY_AT_RUNTIME, every Android header you include after that header will be affected. The opposite is also true. If you include those headers without that defined, then include foo.h, it won't reinclude those headers because #pragma once and so it won't see the definitions it needs.

@DanAlbert
Copy link
Member

Here's the original bug: #837. Going to move this discussion over there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants