Kamel-Media / Kamel

Kotlin asynchronous media loading and caching library for Compose.
Apache License 2.0
656 stars 25 forks source link

Android minSdk version too high #115

Open Skaldebane opened 2 months ago

Skaldebane commented 2 months ago

Hi, I just updated the library to 1.0.0-beta.7 (from 1.0.0-beta.4), and now I'm getting this error while building the Android target:

.../composeApp/src/androidMain/AndroidManifest.xml Error:
    uses-sdk:minSdkVersion 24 cannot be smaller than version 28 declared in library [media.kamel:kamel-decoder-animated-image-android:1.0.0-beta.7] ~/.gradle/caches/transforms-4/...../transformed/kamel-decoder-animated-image-release/AndroidManifest.xml as the library might be using APIs not available in 24
    Suggestion: use a compatible library with a minSdk of at most 24,
        or increase this project's minSdk version to at least 28,
        or use tools:overrideLibrary="io.kamel.image" to force usage (may lead to runtime failures)

I understand that the minSdk for Kamel (in particular the kamel-decoder-animated-image-android artifact) has been bumped to 28 for some reason, but it feels a bit restrictive. It's the only library thus far that doesn't support all the way down to SDK 21 (and at the very least, the default for new multiplatform projects, SDK 24).

Note that I'm using the media.kamel:kamel-image-default artifact, and this is pulled in automatically with it. If this particular artifact requires use of SDK 28, maybe it's a good idea to remove it from the kamel-image-default dependency.

Another approach is building some backwards-compatible backport of this functionality for older Android versions, but I'm not sure how doable is that (I don't use the functionality of this dependency myself, so I don't know much), or maybe setting minSdk to 21 and annotating the relevant functions with @TargetApi to show warnings in the IDE.

Just throwing some ideas, would love to know what you think about this.

And thanks for the great library!

luca992 commented 2 months ago

Hmm yeah good point. I think I'll try lowering the minsdk and adding @TargetApi annotations. Having gif support in the defaults is something people might want (up for debate). I'm sure an implementation could be made for lower versions, but I do not have time at the moment unfortunately.

Skaldebane commented 2 months ago

Thanks for the response! Yeah, that seems like a good approach for a start.

As for implementing it for all API versions, Coil seems to have implemented a backwards-compatible (but slower) decoder, as the note at the bottom of this page indicates: https://coil-kt.github.io/coil/gifs/. Maybe you could take inspiration from them (if I find some time, I may give it a try and submit a PR if I have any success with it).

In Coil they simply expose both decoders, and it's up to the developer to choose which one to use (usually, the developer should check the SDK version, if larger than 28 use the optimized one, otherwise the compat one), with code like this in a custom Application subclass:

class MyApp : Application(), ImageLoaderFactory {
    override fun newImageLoader(): ImageLoader {
        return ImageLoader(this)
            .newBuilder()
            .components(
                // native decoder (fast)
                if (Build.VERSION.SDK_INT >= 28)  add(ImageDecoderDecoder.Factory())
                // backwards compatible decoder (slow)
                else add(GifDecoder.Factory())
            )
            .build()
    }
}

Now that's Coil, so this might have to be done differently in Kamel. The main thing in question is whether such a check should happen automatically, or whether it's the library consumer's responsibility to do this.

My personal preference would be for this to implicitly be done for me (and have the library choose the most efficient path based on the SDK version), but there may be some benefits to making this explicit that I'm missing.

luca992 commented 2 months ago

I could always make two android decoders. And have the android version depend on both and pick according to api version. So both would be included in kamel-image-default. But if a person didn't want to include both. They can always choose which dependencies they want... and/or I could just make different versions of kamel-image-default like kamel-image-default-android-28