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

Use frames count instead of delay to check animated GIFs #119

Merged
merged 1 commit into from Nov 3, 2020

Conversation

nbianca
Copy link
Contributor

@nbianca nbianca commented Oct 28, 2020

There are GIFs with multiple frames and zero delay, but also GIFs with
a single frame and positive delay. The old algorithm failed for this
kind of images.

@Nakilon
Copy link
Contributor

Nakilon commented Oct 31, 2020

That's what I said: #114 (comment)
No need to do assumptions about whether it's animated or not -- just give library user the number of frames.

lib/fastimage.rb Outdated Show resolved Hide resolved
There are GIFs with multiple frames and zero delay, but also GIFs with
a single frame and positive delay. The old algorithm failed for this
kind of images.
@nbianca
Copy link
Contributor Author

nbianca commented Nov 3, 2020

@Nakilon, I could do that as well, but it would require parsing the whole GIF image. My understanding is that this library is meant to provide meta information by "fetching as little as needed".

@sdsykes, can you please review this again? I amended my commit and forced pushed.

@sdsykes
Copy link
Owner

sdsykes commented Nov 3, 2020

This looks ok!

@sdsykes sdsykes merged commit c93903e into sdsykes:master Nov 3, 2020
@nbianca
Copy link
Contributor Author

nbianca commented Dec 2, 2020

Hello @sdsykes! Did you set a date when you will cut a new version of this gem?

@SamSaffron
Copy link
Collaborator

@sdsykes so sorry for being a nag here, any chance you can cut a new version for the festive season?

@sdsykes
Copy link
Owner

sdsykes commented Dec 27, 2020

Well, a late festive version. I just pushed 2.2.1.

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.

None yet

4 participants