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

Quality param ignored on GIFs #359

Open
wmichelin opened this issue May 22, 2023 · 5 comments
Open

Quality param ignored on GIFs #359

wmichelin opened this issue May 22, 2023 · 5 comments

Comments

@wmichelin
Copy link

I am working on GIF compression, and the resultant files, even when aggressively using the quality param, are virtually always larger than the source GIF, by a significant margin.

Looking at the VIPs library documentation, I don't see any quality param supported at all for GIFs, which leads me to believe that this parameter may be erroneously included in the govips bindings.

Please let me know your thoughts. Thank you!

@g-mero
Copy link

g-mero commented Jul 30, 2023

Try this.

img.ExportGIF(&vips.GifExportParams{
			StripMetadata: true,
			Quality:       75,
			Effort:   7,
			Bitdepth: 8,
		})

@n0vad3v
Copy link
Contributor

n0vad3v commented Mar 18, 2024

From govips code, I found the Quality or Q is not included in set_gifsave_options, so setting Quality here might be useless.

// https://libvips.github.io/libvips/API/current/VipsForeignSave.html#vips-gifsave-buffer
int set_gifsave_options(VipsOperation *operation, SaveParams *params) {
  int ret = 0;
  // See for argument values: https://www.libvips.org/API/current/VipsForeignSave.html#vips-gifsave
  if (params->gifDither > 0.0 && params->gifDither <= 1.0) {
    ret = vips_object_set(VIPS_OBJECT(operation), "dither", params->gifDither, NULL);
  }
  if (params->gifEffort >= 1 && params->gifEffort <= 10) {
    ret = vips_object_set(VIPS_OBJECT(operation), "effort", params->gifEffort, NULL);
  }
  if (params->gifBitdepth >= 1 && params->gifBitdepth <= 8) {
      ret = vips_object_set(VIPS_OBJECT(operation), "bitdepth", params->gifBitdepth, NULL);
  }
  return ret;
}

From libvips there is also no Quality option:

/**
 * vips_gifsave: (method)
 * @in: image to save
 * @filename: file to write to
 * @...: %NULL-terminated list of optional named arguments
 *
 * Optional arguments:
 *
 * * @dither: %gdouble, quantisation dithering level
 * * @effort: %gint, quantisation CPU effort
 * * @bitdepth: %gint, number of bits per pixel
 * * @interframe_maxerror: %gdouble, maximum inter-frame error for transparency
 * * @reuse: %gboolean, reuse palette from input
 * * @interlace: %gboolean, write an interlaced (progressive) GIF
 * * @interpalette_maxerror: %gdouble, maximum inter-palette error for palette
 *   reusage
 *
 * Write to a file in GIF format.
 *
 * Use @dither to set the degree of Floyd-Steinberg dithering
 * and @effort to control the CPU effort (1 is the fastest,
 * 10 is the slowest, 7 is the default).
 *
 * Use @bitdepth (from 1 to 8, default 8) to control the number
 * of colours in the palette. The first entry in the palette is
 * always reserved for transparency. For example, a bitdepth of
 * 4 will allow the output to contain up to 15 colours.
 *
 * Use @interframe_maxerror to set the threshold below which pixels are
 * considered equal.
 * Pixels which don't change from frame to frame can be made transparent,
 * improving the compression rate. Default 0.
 *
 * 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
 * connection, but need more memory to encode.
 *
 * See also: vips_image_new_from_file().
 *
 * Returns: 0 on success, -1 on error.
 */

Maybe we should remove this Quality parameter from GifExportParams to reduce confusion?

@n0vad3v
Copy link
Contributor

n0vad3v commented Mar 18, 2024

Update, govips will use magicksave_buffer if VIPS version is not >= 8.12

#if (VIPS_MAJOR_VERSION >= 8) && (VIPS_MINOR_VERSION >= 12)
      return save_buffer("gifsave_buffer", params, set_gifsave_options);
#else
      return save_buffer("magicksave_buffer", params, set_magicksave_options);
#endif

And Quality is supported in magicksave_buffer.

// https://libvips.github.io/libvips/API/current/VipsForeignSave.html#vips-magicksave-buffer
int set_magicksave_options(VipsOperation *operation, SaveParams *params) {
  int ret = vips_object_set(VIPS_OBJECT(operation), "format", "GIF", NULL);
  if (!ret && params->quality) {
    ret = vips_object_set(VIPS_OBJECT(operation), "quality", params->quality,
                          NULL);
  }
  return ret;
}
/**
 * vips_magicksave: (method)
 * @in: image to save
 * @filename: file to write to
 * @...: %NULL-terminated list of optional named arguments
 *
 * Optional arguments:
 *
 * * @quality: %gint, quality factor
 * * @format: %gchararray, format to save as
 * * @optimize_gif_frames: %gboolean, apply GIF frames optimization
 * * @optimize_gif_transparency: %gboolean, apply GIF transparency optimization
 * * @bitdepth: %gint, number of bits per pixel
 *
 * Write an image using libMagick.
 *
 * Use @quality to set the quality factor. Default 0.
 *
 * Use @format to explicitly set the save format, for example, "BMP". Otherwise
 * the format is guessed from the filename suffix.
 *
 * If @optimize_gif_frames is set, GIF frames are cropped to the smallest size
 * while preserving the results of the GIF animation. This takes some time for
 * computation but saves some time on encoding and produces smaller files in
 * some cases.
 *
 * If @optimize_gif_transparency is set, pixels that don't change the image
 * through animation are made transparent. This takes some time for computation
 * but saves some time on encoding and produces smaller files in some cases.
 *
 * @bitdepth specifies the number of bits per pixel. The image will be quantized
 * and dithered if the value is within the valid range (1 to 8).
 *
 * See also: vips_magicksave_buffer(), vips_magickload().
 *
 * Returns: 0 on success, -1 on error.
 */

@tonimelisma
Copy link
Collaborator

Hey @n0vad3v thank you very much for this. Yes, removing the parameter would make sense since the backend has changed but that would break backwards compatibility so I don't think it's possible. I suppose it would be best to just add this to the documentation.

@n0vad3v
Copy link
Contributor

n0vad3v commented Mar 18, 2024

@tonimelisma , yes, I'm trying to add more docs on this parameter and do some modifications on set_gifsave_options and set_magicksave_options, wait for my PR. 😇

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

No branches or pull requests

4 participants