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

Skipping the bitmap pool if the image to be decoded is a GIF #36

Merged
merged 3 commits into from
Nov 9, 2013

Conversation

savvasdalkitsis
Copy link
Contributor

Exposed a method to return the type of the image to be decoded. For now supports GIF, PNG_A, PNG, JPEG and UNKNOWN

We skip the bitmap pool if the image is a GIF because of :

https://groups.google.com/forum/#!msg/android-developers/Mp0MFVFi1Fo/e8ZQ9FGdWdEJ

@sjudd
Copy link
Collaborator

sjudd commented Nov 6, 2013

This looks good to me, one question: Have you tested with tiffs? I realized my original code doesn't consider them in hasAlpha. From the link you posted I'd expect them to cause the same exception.

Thanks again for submitting these, I'll merge tonight if I don't get time before then.

…add more types in the future and they will skip the pool automatically
@savvasdalkitsis
Copy link
Contributor Author

I am not entirely sure how to check for TIFFs in the general case to be honest. What I have done now is, instead of looking for GIFs in order to skip the pool, I skip it if the type is not JPG, PNG or PNG_A (which I changed from the previous APNG which means something else btw).

This way when new types of images are added, the pool skipping will happen automatically unless we add that type to the set of recognized types. Also, I am hoping since I haven't tested, a TIFF will not return JPG or PNG with the current code, so the skipping will happen.

Let me know if this is sufficient.

@sjudd
Copy link
Collaborator

sjudd commented Nov 7, 2013

LGTM.

Tiffs have a different four byte header so right now they will just be classified as unknown which is fine.

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

Successfully merging this pull request may close these issues.

3 participants