Animated GIF support

[Elad-Laufer]

• Export native of GIF
• Support animated images resize
• Support animated images rotate
• Load from file with import options

[coveralls]

Coverage increased (+0.5%) to 76.054% when pulling 164901c on wix-incubator:animated-gif-support into 423eb69 on davidbyttow:master.

[davidbyttow]

Resolve conflicts?

[Elad-Laufer]

Resolve conflicts?

done

[AttilaTheFun]

I'm super excited for this to land! Right now I have some "if-gif" code to fall back on the builtin image package when dealing with gif images that I would love to delete.

[AttilaTheFun]

Awesome! @davidbyttow could we cut a new release including this commit?

[davidbyttow]

Done! https://github.com/davidbyttow/govips/releases/tag/v2.10.0 @AttilaTheFun

[AttilaTheFun]

Sweet I'll try it out later today. If I recall before, gifs would break when I tried to resize them.

As a result I had to store gifs in a separate S3 bucket with a separate CloudFront bucket which had my Lambda@Edge function disabled.

If gif resizing is working now, I'd be excited to unify gifs with the rest of the images on my server.

[AttilaTheFun]

@davidbyttow @Elad-Laufer So I just pulled down this version and tested it out but I'm still seeing the same issue I saw before. If I import a GIF from a file and then export it back to a file, it only seems to write the first frame of the GIF.

I'm not sure if this is still broken or if I'm "holding it wrong", but I've attached the images I used to test:

Original	VIPS

I'm importing the image from a file handle like so:

	img, err := vips.NewImageFromReader(reader)
	if err != nil {
		return nil, err
	}

And then I'm exporting it this way:

	ep := vips.NewGifExportParams()
	bytes, _, err := img.ExportGIF(ep)
	if err != nil {
		return nil, err
	}

And writing the bytes back to the new file. Is this supposed to be working? Or is there another way to import / export GIFs I should be using?

UPDATE: I found the test case that was added and saw that you must explicitly set the number of pages to -1 when importing a GIF. Once I did that it worked.

	importParams := vips.NewImportParams()
	importParams.NumPages.Set(-1)
	img, err := vips.LoadImageFromBuffer(buf, importParams)
	if err != nil {
		return nil, err
	}

That said, I believe defaulting to only loading the first frame of a GIF is a bad UX. I'd advocate for making the default value be -1 so that importing and exporting an image with the default parameters would round trip.

[Elad-Laufer]

That said, I believe defaulting to only loading the first frame of a GIF is a bad UX. I'd advocate for making the default value be -1 so that importing and exporting an image with the default parameters would round trip.

I agree, but that would change the current behavior and break backwards compatibility in an unexpected way as it will compile just fine and will behave differently. For example, when converting an animated image to a format that does not support animation such as JPEG, if more than one page is loaded the exported image will be the entire roll and not a single frame (which defaults to the first one).

This is probably another good point to consider in #248.

[davidbyttow]

There could be NewGifImportParams()?