Giphy / giphy-android-sdk

Home of the GIPHY SDK Android example app, along with Android SDK documentation, issue tracking, & release notes.
https://developers.giphy.com/
Mozilla Public License 2.0
93 stars 37 forks source link

[Android] Integrate GPHMediaView with recycler view give flickering effect while scrolling #228

Closed csidhiwala closed 4 days ago

csidhiwala commented 2 months ago

🐛 Bug Report

Hello, I have integrated GPHMediaView with recycler view. While scrolling I have observed flickering effect in GIF. Many times it loads with wrong GIF and than changed to correct.

To Reproduce

(Write your steps here:)

  1. Step 1...
  2. Step 2...
  3. Step 3...

Expected behavior

It should not give flickering effect while scrolling. Still it should reuse Gif ViewHolder.

(Write what you thought would happen.)

Actual Behavior

It's giving flickering effect while scrolling using recyclerview . It also loads wrong GIF initially and than got it correct after actual will get load

(Write what happened. Add screenshots, if applicable.)

Your Environment

Reproducible Demo

ALexanderLonsky commented 2 months ago

Hey @csidhiwala, Could you provide a reproducible demo? We have the grid-only solution based on RecyclerView, and it's not flickering. Alternatively, you can try using Glide to see if it works for your setup.

csidhiwala commented 2 months ago

Hey @csidhiwala, Could you provide a reproducible demo? We have the grid-only solution based on RecyclerView, and it's not flickering. Alternatively, you can try using Glide to see if it works for your setup.

Hey @ALexanderLonsky Basically I think here issue is due to caching. Does giphy-sdk supports gif caching? Here I have attached video of what exactly happening.

https://github.com/Giphy/giphy-android-sdk/assets/111134824/d1e1a04b-c120-44d0-ace8-d4be2e1f2854

ALexanderLonsky commented 2 months ago

In our current implementation, the setMediaWithId method asynchronously tries to fetch media assets from GIPHY. While we check if the returned media id matches the current mediaId before updating the media, this might still create a race condition. As an alternative, you could prefetch all the media using GPHCore.gifsByIds and then use the media objects instead of ids: use gifView.setMedia instead of gifView.setMediaWithId. We use Fresco cache, which is why setMedia works much better. In the meantime, I'll look into how we can improve setMediaWithId.

csidhiwala commented 2 months ago

In that case I need to store Media object against each id to my local cache and always check if media is available, call setMedia But in this case it will occupy memory in cache and may go out of memory if there will be lots of Gif loading on my stream / Feeds.

ALexanderLonsky commented 2 months ago

The Media object shouldn't consume much memory because it is a meta object. GIFs (content) are handled by Fresco's disk cache. You should implement an expirable(lru) cache that periodically checks and removes old media objects.

P.S. Don't forget to call gifView.recycle() in onViewRecycled()

ALexanderLonsky commented 2 months ago

@csidhiwala you can try this:

import android.content.Context
import com.giphy.sdk.core.network.response.MediaResponse
import com.google.gson.Gson
import com.google.gson.reflect.TypeToken
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.launch
import kotlinx.coroutines.withContext
import java.io.File
import java.io.FileReader
import java.io.FileWriter
import java.lang.reflect.Type
import com.giphy.sdk.core.GPHCore
import com.giphy.sdk.core.models.Media

interface GifDiskCache {
    fun get(gifId: String): MediaResponse?
    fun put(gifId: String, response: MediaResponse)
}

class DefaultGifDiskCache(private val cacheDir: File, private val maxSize: Int) : GifDiskCache {

    private val cache: MutableMap<String, File> = object : LinkedHashMap<String, File>(maxSize, 0.75f, true) {
        override fun removeEldestEntry(eldest: Map.Entry<String, File>): Boolean {
            if (size > maxSize) {
                eldest.value.delete()
                return true
            }
            return false
        }
    }

    private val gson = Gson()
    private val type: Type = object : TypeToken<MediaResponse>() {}.type

    init {
        if (!cacheDir.exists()) {
            cacheDir.mkdirs()
        }
    }

    override fun get(gifId: String): MediaResponse? {
        val file = cache[gifId] ?: return null
        if (!file.exists()) {
            cache.remove(gifId)
            return null
        }

        FileReader(file).use { reader ->
            return gson.fromJson(reader, type)
        }
    }

    override fun put(gifId: String, response: MediaResponse) {
        val file = File(cacheDir, gifId)
        FileWriter(file).use { writer ->
            gson.toJson(response, writer)
        }
        cache[gifId] = file
    }
}

object GifCacheManager {
    private lateinit var gifDiskCache: GifDiskCache

    fun initialize(context: Context) {
        val cacheDir = File(context.cacheDir, "gif_cache")
        if (!cacheDir.exists()) {
            cacheDir.mkdirs()
        }
        gifDiskCache = DefaultGifDiskCache(cacheDir, 100)
    }

    fun gifById(
        gifId: String,
        completionHandler: (result: MediaResponse?, e: Throwable?) -> Unit
    ) {
        CoroutineScope(Dispatchers.IO).launch {
            val cachedResponse = gifDiskCache.get(gifId)
            if (cachedResponse != null) {
                withContext(Dispatchers.Main) {
                    completionHandler.invoke(cachedResponse, null)
                }
                return@launch
            }

            GPHCore.gifById(gifId) { result, e ->
                if (result != null) {
                    CoroutineScope(Dispatchers.IO).launch {
                        gifDiskCache.put(gifId, result)
                    }
                }
                CoroutineScope(Dispatchers.Main).launch {
                    completionHandler.invoke(result, e)
                }
            }
        }
    }
}

Next, initialize the manager with a context:

GifCacheManager.initialize(itemView.context)

and use it in a view holder:

GifCacheManager.gifById(id) { result, e ->
                    if (result != null) {
                        gifView.setMedia(
                            result.data
                        )
                    }
                }

I tried this on my Android device, and it works well for me.

This way, you can adjust the cache mechanism to your needs.

Please let me know if this works for you.

csidhiwala commented 2 months ago

Thanks @ALexanderLonsky This will be super helpful. For GifCacheManager.initialize(itemView.context) I am assuming here it needs applicationContext. Can I initialise it in Application class?

ALexanderLonsky commented 2 months ago

Yes, it's required to provide a path for the cache directory.