Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

re-work GIF version setting to support the upcoming 5.0.1 mechanism

also test that it works, since it won't for 4.2.0 and 5.0.0
  • Loading branch information...
commit a93c43b4441dfc08c0abd1b9565236cd29aded9f 1 parent 9673d97
@tonycoz authored
Showing with 44 additions and 38 deletions.
  1. +2 −0  GIF/MANIFEST
  2. +16 −38 GIF/imgif.c
  3. +25 −0 GIF/t/t50header.t
  4. +1 −0  MANIFEST
View
2  GIF/MANIFEST
@@ -11,6 +11,8 @@ README
t/t10gif.t
t/t20new.t
t/t30fixed.t
+t/t40limit.t
+t/t50header.t
testimg/badindex.gif
testimg/bandw.gif
testimg/expected.gif
View
54 GIF/imgif.c
@@ -60,6 +60,7 @@ functionality with giflib3.
*/
#if defined(GIFLIB_MAJOR) && GIFLIB_MAJOR >= 5
+#define POST_SET_VERSION
#define myDGifOpen(userPtr, readFunc, Error) DGifOpen((userPtr), (readFunc), (Error))
#define myEGifOpen(userPtr, readFunc, Error) EGifOpen((userPtr), (readFunc), (Error))
#define myGifError(gif) ((gif)->Error)
@@ -67,6 +68,7 @@ functionality with giflib3.
#define FreeMapObject GifFreeMapObject
#else
+#define PRE_SET_VERSION
static GifFileType *
myDGifOpen(void *userPtr, InputFunc readFunc, int *error) {
GifFileType *result = DGifOpen(userPtr, readFunc);
@@ -1212,43 +1214,16 @@ make_gif_map(i_quantize *quant, i_img *img, int want_trans) {
}
/*
-=item gif_set_version(i_quantize *quant, i_img *imgs, int count)
+=item need_version_89a(i_quantize *quant, i_img *imgs, int count)
-We need to call EGifSetGifVersion() before opening the file - put that
-common code here.
-
-Unfortunately giflib 4.1.0 crashes when we use this. Internally
-giflib 4.1.0 has code:
-
- static char *GifVersionPrefix = GIF87_STAMP;
-
-and the code that sets the version internally does:
-
- strncpy(&GifVersionPrefix[3], Version, 3);
-
-which is very broken.
-
-Failing to set the correct GIF version doesn't seem to cause a problem
-with readers.
-
-Modern versions (4.1.4 anyway) of giflib/libungif handle
-EGifSetGifVersion correctly.
-
-If t/t105gif.t crashes here then run Makefile.PL with
---nogifsetversion, eg.:
-
- perl Makefile.PL --nogifsetversion
-
-or install a less buggy giflib.
-
-This code is completely unnecessary in giflib 5
+Return true if the file we're creating on these images needs a GIF89a
+header.
=cut
*/
-static void
-gif_set_version(i_quantize *quant, i_img **imgs, int count) {
-#if !defined(GIFLIB_MAJOR) || GIFLIB_MAJOR < 5
+static int
+need_version_89a(i_quantize *quant, i_img **imgs, int count) {
int need_89a = 0;
int temp;
int i;
@@ -1276,11 +1251,8 @@ gif_set_version(i_quantize *quant, i_img **imgs, int count) {
break;
}
}
- if (need_89a)
- EGifSetGifVersion("89a");
- else
- EGifSetGifVersion("87a");
-#endif
+
+ return need_89a;
}
static int
@@ -1857,7 +1829,9 @@ i_writegif_wiol(io_glue *ig, i_quantize *quant, i_img **imgs,
i_clear_error();
- gif_set_version(quant, imgs, count);
+#ifdef PRE_SET_VERSION
+ EGifSetGifVersion(need_version_89a(quant, imgs, count) ? "89a" : "87a");
+#endif
if ((GifFile = myEGifOpen((void *)ig, io_glue_write_cb, &gif_error )) == NULL) {
gif_push_error(gif_error);
@@ -1866,6 +1840,10 @@ i_writegif_wiol(io_glue *ig, i_quantize *quant, i_img **imgs,
return 0;
}
+#ifdef POST_SET_VERSION
+ EGifSetGifVersion(GifFile, need_version_89a(quant, imgs, count));
+#endif
+
result = i_writegif_low(quant, GifFile, imgs, count);
if (i_io_close(ig))
View
25 GIF/t/t50header.t
@@ -0,0 +1,25 @@
+#!perl -w
+use strict;
+use Test::More tests => 4;
+use Imager;
+use Imager::Test qw(test_image);
+
+# giflib 4.2.0 and 5.0.0 had a bug with producing the wrong
+# GIF87a/GIF89a header, test we get the right header
+# https://rt.cpan.org/Ticket/Display.html?id=79679
+# https://sourceforge.net/tracker/?func=detail&aid=3574283&group_id=102202&atid=631304
+my $im = test_image()->to_paletted();
+
+{
+ my $data;
+ ok($im->write(data => \$data, type => "gif"),
+ "write with no tags, should be GIF87a");
+ is(substr($data, 0, 6), "GIF87a", "check header is GIF87a");
+}
+
+{
+ my $data;
+ ok($im->write(data => \$data, type => "gif", gif_loop => 1),
+ "write with loop tags, should be GIF89a");
+ is(substr($data, 0, 6), "GIF89a", "check header is GIF89a");
+}
View
1  MANIFEST
@@ -80,6 +80,7 @@ GIF/t/t10gif.t
GIF/t/t20new.t
GIF/t/t30fixed.t
GIF/t/t40limit.t
+GIF/t/t50header.t
GIF/testimg/badindex.gif GIF with a bad color index
GIF/testimg/bandw.gif
GIF/testimg/expected.gif
Please sign in to comment.
Something went wrong with that request. Please try again.