From ac4c2d2b3df53f7218859b8ee4fb1123f210cb70 Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Mon, 5 Dec 2022 15:41:54 +0000 Subject: [PATCH] gifsave: deprecate reoptimise, add reuse (#3213) see https://github.com/libvips/libvips/discussions/3211 --- ChangeLog | 1 + libvips/foreign/cgifsave.c | 59 +++++++++++++++++++++++++------------- 2 files changed, 40 insertions(+), 20 deletions(-) diff --git a/ChangeLog b/ChangeLog index 910d97ab..4e36d52b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -25,6 +25,7 @@ master - add @wrap to vips_text() - GIF load supports truncated frames [tlsa] - EXIF support for PNG load and save +- deprecate gifsave reoptimise, add reuse 9/11/22 started 8.13.4 - missing include in mosaic_fuzzer [ServOKio] diff --git a/libvips/foreign/cgifsave.c b/libvips/foreign/cgifsave.c index 32ece4e5..88cffd3f 100644 --- a/libvips/foreign/cgifsave.c +++ b/libvips/foreign/cgifsave.c @@ -3,6 +3,8 @@ * 22/8/21 lovell * 18/1/22 TheEssem * - fix change detector + * 3/12/22 + * - deprecate reoptimise, add reuse */ /* @@ -55,11 +57,6 @@ #include /* The modes we work in. - * - * VIPS_FOREIGN_SAVE_CGIF_MODE_GLOBAL: - * - * Each frame is dithered to single global colour table taken from the - * input image "gif-palette" metadata item. * * VIPS_FOREIGN_SAVE_CGIF_MODE_LOCAL: * @@ -67,8 +64,13 @@ * frames with a local palette if they start to drift too far from the * first frame. * - * We pick GLOBAL if "gif-palette" is set. We pick LOCAL if there is - * no "gif-palette", or if @reoptimise is set. + * VIPS_FOREIGN_SAVE_CGIF_MODE_GLOBAL: + * + * Each frame is dithered to single global colour table taken from the + * input image "gif-palette" metadata item. + * + * We use LOCAL by default. We use GLOBAL if @reuse is set and there's + * a palette attached to the image to be saved. */ typedef enum _VipsForeignSaveCgifMode { VIPS_FOREIGN_SAVE_CGIF_MODE_GLOBAL, @@ -82,7 +84,7 @@ typedef struct _VipsForeignSaveCgif { int effort; int bitdepth; double interframe_maxerror; - gboolean reoptimise; + gboolean reuse; gboolean interlace; double interpalette_maxerror; VipsTarget *target; @@ -137,6 +139,10 @@ typedef struct _VipsForeignSaveCgif { CGIF_Config cgif_config; int n_palettes_generated; + + /* Deprecated. + */ + gboolean reoptimise; } VipsForeignSaveCgif; typedef VipsForeignSaveClass VipsForeignSaveCgifClass; @@ -707,7 +713,7 @@ vips_foreign_save_cgif_build( VipsObject *object ) /* Read the palette on the input if we've not been asked to * reoptimise. */ - if( !cgif->reoptimise && + if( cgif->reuse && vips_image_get_typeof( cgif->in, "gif-palette" ) ) { if( vips_image_get_array_int( cgif->in, "gif-palette", &cgif->palette, &cgif->n_colours ) ) @@ -827,11 +833,11 @@ vips_foreign_save_cgif_class_init( VipsForeignSaveCgifClass *class ) G_STRUCT_OFFSET( VipsForeignSaveCgif, interframe_maxerror ), 0, 32, 0.0 ); - VIPS_ARG_BOOL( class, "reoptimise", 14, - _( "Reoptimise palettes" ), - _( "Reoptimise colour palettes" ), + VIPS_ARG_BOOL( class, "reuse", 14, + _( "Reuse palette" ), + _( "Reuse palette from input" ), VIPS_ARGUMENT_OPTIONAL_INPUT, - G_STRUCT_OFFSET( VipsForeignSaveCgif, reoptimise ), + G_STRUCT_OFFSET( VipsForeignSaveCgif, reuse ), FALSE ); VIPS_ARG_DOUBLE( class, "interpalette_maxerror", 15, @@ -848,6 +854,16 @@ vips_foreign_save_cgif_class_init( VipsForeignSaveCgifClass *class ) G_STRUCT_OFFSET( VipsForeignSaveCgif, interlace ), FALSE ); + /* Not a good thing to have enabled by default since it can cause very + * mysterious behaviour that varies with the input image. + */ + VIPS_ARG_BOOL( class, "reoptimise", 17, + _( "Reoptimise palettes" ), + _( "Reoptimise colour palettes" ), + VIPS_ARGUMENT_OPTIONAL_INPUT | VIPS_ARGUMENT_DEPRECATED, + G_STRUCT_OFFSET( VipsForeignSaveCgif, reoptimise ), + FALSE ); + } static void @@ -857,7 +873,7 @@ vips_foreign_save_cgif_init( VipsForeignSaveCgif *gif ) gif->effort = 7; gif->bitdepth = 8; gif->interframe_maxerror = 0.0; - gif->reoptimise = FALSE; + gif->reuse = FALSE; gif->interlace = FALSE; gif->interpalette_maxerror = 3.0; gif->mode = VIPS_FOREIGN_SAVE_CGIF_MODE_GLOBAL; @@ -1042,7 +1058,7 @@ vips_foreign_save_cgif_buffer_init( VipsForeignSaveCgifBuffer *buffer ) * * @effort: %gint, quantisation CPU effort * * @bitdepth: %gint, number of bits per pixel * * @interframe_maxerror: %gdouble, maximum inter-frame error for transparency - * * @reoptimise: %gboolean, reoptimise colour palettes + * * @reuse: %gboolean, reuse palette from input * * @interlace: %gboolean, write an interlaced (progressive) GIF * * @interpalette_maxerror: %gdouble, maximum inter-palette error for palette * reusage @@ -1063,9 +1079,12 @@ vips_foreign_save_cgif_buffer_init( VipsForeignSaveCgifBuffer *buffer ) * Pixels which don't change from frame to frame can be made transparent, * improving the compression rate. Default 0. * - * If @reoptimise is TRUE, new palettes will be generated. Use - * @interpalette_maxerror to set the threshold below which one of the previously - * generated palettes will be reused. + * Use @interpalette_maxerror to set the threshold below which the + * previously generated palette will be reused. + * + * If @reuse is TRUE, the GIF will be saved with a single global + * palette taken from the metadata in @in, and no new palette optimisation + * will be done. * * If @interlace is TRUE, the GIF file will be interlaced (progressive GIF). * These files may be better for display over a slow network @@ -1101,7 +1120,7 @@ vips_gifsave( VipsImage *in, const char *filename, ... ) * * @effort: %gint, quantisation CPU effort * * @bitdepth: %gint, number of bits per pixel * * @interframe_maxerror: %gdouble, maximum inter-frame error for transparency - * * @reoptimise: %gboolean, reoptimise colour palettes + * * @reuse: %gboolean, reuse palette from input * * @interlace: %gboolean, write an interlaced (progressive) GIF * * @interpalette_maxerror: %gdouble, maximum inter-palette error for palette * reusage @@ -1156,7 +1175,7 @@ vips_gifsave_buffer( VipsImage *in, void **buf, size_t *len, ... ) * * @effort: %gint, quantisation CPU effort * * @bitdepth: %gint, number of bits per pixel * * @interframe_maxerror: %gdouble, maximum inter-frame error for transparency - * * @reoptimise: %gboolean, reoptimise colour palettes + * * @reuse: %gboolean, reuse palette from input * * @interlace: %gboolean, write an interlaced (progressive) GIF * * @interpalette_maxerror: %gdouble, maximum inter-palette error for palette * reusage