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

change imageId in AnimatedImageResult from the class hash to decodeHa… #2612

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

WhileCrow
Copy link

@WhileCrow WhileCrow commented May 27, 2021

I create an issue a few days ago, #2605, I'm kind of dying to know if this is a bug? And if this is, is my way of fixing it appropriate?

Motivation (required)

Fix issue where fresco fails to cache accurately the gif frames.
What existing problem does the pull request solve?
First,I want to talk briefly about the Fresco gif cache:

  1. First, disk cache; It's right;
  2. Second, encode cache (EncodeImage.class); It's right;
  3. Third, decode memory cache (Key is FrameKey and Value is every frame of gif); Error here;

When the encode cache is decoded to memory cache, the encodeImage will be decoded and packed into AnimatedImage, and finally will be decoded into CloseableReference.
Like this:
1,AnimatedImage( GifImage implement AnimatedImage)contain the info of gif like loopCount, frameCount and encode frames bytes .
2,CloseableReference,a closeable bitmap, expression each frame memory cache and decoded by encode frames bytes .

But, AnimatedImage was further encapsulated as a new AnimatedImageResult object. At that Time, AnimatedImageResult hashCode changed every time.So encodeImage was fixed, but AnimatedImageResult that decoded by encodeImage changes. That's why decoded memory is disabled while encode memory is enabled.

private AnimatedFrameCache createAnimatedFrameCache(
      final AnimatedImageResult animatedImageResult) {
    return new AnimatedFrameCache(
        new AnimationFrameCacheKey(animatedImageResult.hashCode()), mBackingCache);
}

Test Plan

Load the same GIF multiple times, and :

  1. Look at the Android profiler to see if the memory keeps increasingl
  2. Debug if the count of entries in the cache pool keeps increasing.
    Like the image in the Gif cache doesn't seem to be working. #2605 shows.

Next Steps

Could I know when will you fix this bug and which way will be select? Maybe merge this PR or you have other ideas.

I look forward to your reply.😁

Copy link
Author

@WhileCrow WhileCrow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no review change

@@ -90,7 +90,7 @@ public CloseableImage decodeGif(
sGifAnimatedImageDecoder.decodeFromNativeMemory(
input.getNativePtr(), input.size(), options);
}
return getCloseableImage(options, gifImage, bitmapConfig);
return getCloseableImage(bytesRef.getValueHash(), options, gifImage, bitmapConfig);
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe I should replace it with cache key

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

Successfully merging this pull request may close these issues.

2 participants