From: Tony Cook Date: Thu, 31 Aug 2006 13:16:45 +0000 (+0000) Subject: Changes to GIF support: X-Git-Tag: Imager-0.54~7 X-Git-Url: http://git.imager.perl.org/imager.git/commitdiff_plain/1660561cb575afc5b2249a5c67a6f4172705beef?ds=sidebyside Changes to GIF support: - the Nescape loop extension block is written if gif_loop is set. This requires libungif/libgif 4.1.3 or later. Resolves: http://rt.cpan.org/Ticket/Display.html?id=21185 - we now set the GIF header depending on the features enabled in the GIF. Since we don't do configure type probes for bugs we have to enable this based on the gif_lib.h version number, it's enabled for 4.1.0 and later, but requires 4.1.3 and later (maybe 4.1.3) to run without crashing. You can avoid this crash by passing --nogifsetversion to Makefile.PL, or by installing a non-buggy libungif/libgif, if there is such a thing. --- diff --git a/Makefile.PL b/Makefile.PL index b1338860..28e2fb7f 100644 --- a/Makefile.PL +++ b/Makefile.PL @@ -38,6 +38,7 @@ my @incpaths; # places to look for headers my @libpaths; # places to look for libraries my $noprobe; # non-zero to disable newer probes my $noexif; # non-zero to disable EXIF parsing of JPEGs +my $no_gif_set_version; # disable calling EGifSetGifVersion GetOptions("help" => \$help, "enable=s" => \@enable, "disable=s" => \@disable, @@ -46,7 +47,8 @@ GetOptions("help" => \$help, "noprobe" => \$noprobe, "verbose|v" => \$VERBOSE, "nolog" => \$NOLOG, - "noexif" => \$noexif); + "noexif" => \$noexif, + "nogifsetversion" => \$no_gif_set_version); if ($VERBOSE) { print "Verbose mode\n"; @@ -304,6 +306,8 @@ sub gifcheck { push @defines, [ IM_GIFMAJOR => $major, "Parsed giflib version" ]; push @defines, [ IM_GIFMINOR => $minor ]; + push @defines, [ IM_NO_SET_GIF_VERSION => 1, "Disable EGifSetGifVersion" ] + if $no_gif_set_version; } diff --git a/gif.c b/gif.c index 84b51bf1..792b8407 100644 --- a/gif.c +++ b/gif.c @@ -1369,9 +1369,11 @@ static int do_comments(GifFileType *gf, i_img *img) { Internal. Add the Netscape2.0 loop extension block, if requested. -The code for this function is currently "#if 0"ed out since the giflib -extension writing code currently doesn't seem to support writing -application extension blocks. +Giflib/libungif prior to 4.1.1 didn't support writing application +extension blocks, so we don't attempt to write them for older versions. + +Giflib/libungif prior to 4.1.3 used the wrong write mechanism when +writing extension blocks so that they could only be written to files. =cut */ @@ -1387,13 +1389,13 @@ static int do_ns_loop(GifFileType *gf, i_img *img) If giflib's callback interface wasn't broken by default, I'd force file writes to use callbacks, but it is broken by default. */ -#if 0 /* yes this was another attempt at supporting the loop extension */ +#if IM_GIFMAJOR == 4 && IM_GIFMINOR >= 1 int loop_count; if (i_tags_get_int(&img->tags, "gif_loop", 0, &loop_count)) { unsigned char nsle[12] = "NETSCAPE2.0"; unsigned char subblock[3]; - if (EGifPutExtension(gf, 0xFF, 11, nsle) == GIF_ERROR) { + if (EGifPutExtensionFirst(gf, APPLICATION_EXT_FUNC_CODE, 11, nsle) == GIF_ERROR) { gif_push_error(); i_push_error(0, "writing loop extension"); return 0; @@ -1401,18 +1403,14 @@ static int do_ns_loop(GifFileType *gf, i_img *img) subblock[0] = 1; subblock[1] = loop_count % 256; subblock[2] = loop_count / 256; - if (EGifPutExtension(gf, 0, 3, subblock) == GIF_ERROR) { + if (EGifPutExtensionLast(gf, APPLICATION_EXT_FUNC_CODE, 3, subblock) == GIF_ERROR) { gif_push_error(); i_push_error(0, "writing loop extention sub-block"); return 0; } - if (EGifPutExtension(gf, 0, 0, subblock) == GIF_ERROR) { - gif_push_error(); - i_push_error(0, "writing loop extension terminator"); - return 0; - } } #endif + return 1; } @@ -1487,24 +1485,53 @@ 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. + =cut */ static void gif_set_version(i_quantize *quant, i_img **imgs, int count) { - /* the following crashed giflib - the EGifSetGifVersion() is seriously borked in giflib - it's less borked in the ungiflib beta, but we don't have a mechanism - to distinguish them - Needs to be updated to support tags. - if (opts->delay_count - || opts->user_input_count - || opts->disposal_count - || opts->loop_count - || quant->transp != tr_none) +#if (IM_GIFMAJOR >= 4 || IM_GIFMAJOR == 4 && IM_GIFMINOR >= 1) \ + && !defined(IM_NO_SET_GIF_VERSION) + int need_89a = 0; + int temp; + int i; + + if (quant->transp != tr_none) + need_89a = 1; + else { + for (i = 0; i < count; ++i) { + if (i_tags_get_int(&imgs[i]->tags, "gif_delay", 0, &temp)) { + need_89a = 1; + break; + } + if (i_tags_get_int(&imgs[i]->tags, "gif_user_input", 0, &temp) && temp) { + need_89a = 1; + break; + } + if (i_tags_get_int(&imgs[i]->tags, "gif_disposal", 0, &temp)) { + need_89a = 1; + break; + } + if (i_tags_get_int(&imgs[i]->tags, "gif_loop", 0, &temp)) { + need_89a = 1; + break; + } + } + } + if (need_89a) EGifSetGifVersion("89a"); - else + else EGifSetGifVersion("87a"); - */ +#endif } static int diff --git a/t/t105gif.t b/t/t105gif.t index 2c8175fe..b6f7366f 100644 --- a/t/t105gif.t +++ b/t/t105gif.t @@ -1,8 +1,18 @@ #!perl -w +=pod + +IF THIS TEST CRASHES + +Giflib/libungif have a long history of bugs, so if this script crashes +and you aren't running version 4.1.4 of giflib or libungif then +UPGRADE. + +=cut + use strict; $|=1; use lib 't'; -use Test::More tests => 107; +use Test::More tests => 113; use Imager qw(:all); BEGIN { require "t/testtools.pl"; } use Carp 'confess'; @@ -636,6 +646,26 @@ EOS cmp_ok($res->errstr, "=~", 'page 3 not found', "check error message"); } + { + # testing writing the loop extension + my $im1 = Imager->new(xsize => 100, ysize => 100); + $im1->box(filled => 1, color => '#FF0000'); + my $im2 = Imager->new(xsize => 100, ysize => 100); + $im2->box(filled => 1, color => '#00FF00'); + ok(Imager->write_multi({ + gif_loop => 5, + gif_delay => 50, + file => 'testout/t105loop.gif' + }, $im1, $im2), + "write with loop extension"); + + my @im = Imager->read_multi(file => 'testout/t105loop.gif'); + is(@im, 2, "read loop images back"); + is($im[0]->tags(name => 'gif_loop'), 5, "first loop read back"); + is($im[1]->tags(name => 'gif_loop'), 5, "second loop read back"); + is($im[0]->tags(name => 'gif_delay'), 50, "first delay read back"); + is($im[1]->tags(name => 'gif_delay'), 50, "second delay read back"); + } } sub test_readgif_cb {