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

Preserve gif animations #481

Merged
merged 1 commit into from
Jan 6, 2017
Merged

Conversation

trsteel88
Copy link

This is a combination of #348 and #437.

@dupuchba
Copy link

where are we with this one ? @romainneutron

@trsteel88
Copy link
Author

I'm actually a little worried about this one.

If someone uploads an image with 1,000 frames, that is 1000 images that need to be processed I think? A 10000 by 10000 pixel image could cause quite a bit of load in that instance.

@dupuchba
Copy link

@trsteel88 agree with you but as a a user of Imagine, it's upon to you to make the choice to allow GIF or not.
Personally, I use this special code at a client's (a big french application) and it works perfectly.

What's left to let it merged ?

@trsteel88
Copy link
Author

I think it's good to go.

Would probably be a good idea to add a frame limit. So if that was 10 and there was 1000 frames it would take every 100th frame.

@dupuchba
Copy link

@trsteel88 why not, as long as we manage to document it well! Maybe a configurable frame limit ?

@burzum burzum merged commit 4cb07ad into php-imagine:develop Jan 6, 2017
@burzum
Copy link
Contributor

burzum commented Jan 6, 2017

@trsteel88 can you add the frame limit option? :)

} catch (\ImagickException $e) {
throw new RuntimeException('Crop operation failed', $e->getCode(), $e);
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Wrong CS

@hjanuschka
Copy link

@burzum any chances on getting a new release?

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

Successfully merging this pull request may close these issues.

5 participants