From 09e542517a05d031c8308405a63490067c4b3908 Mon Sep 17 00:00:00 2001 From: DarthSim Date: Thu, 18 Apr 2024 19:51:40 +0300 Subject: [PATCH] Extract finalizing to a separate pipeline; Fix watermark colors on non-sRGB images --- processing/export_color_profile.go | 2 +- processing/import_color_profile.go | 3 +- processing/pipeline.go | 2 - processing/processing.go | 8 ++ processing/watermark.go | 22 ++-- vips/vips.c | 173 +++++++++++++++++++---------- vips/vips.go | 29 ++++- vips/vips.h | 4 +- 8 files changed, 165 insertions(+), 78 deletions(-) diff --git a/processing/export_color_profile.go b/processing/export_color_profile.go index d15d869f..30218536 100644 --- a/processing/export_color_profile.go +++ b/processing/export_color_profile.go @@ -15,7 +15,7 @@ func exportColorProfile(pctx *pipelineContext, img *vips.Image, po *options.Proc } } - if pctx.iccImported { + if img.ColourProfileImported() { if keepProfile { // We imported ICC profile and want to keep it, // so we need to export it diff --git a/processing/import_color_profile.go b/processing/import_color_profile.go index 9641466f..b56ea7a5 100644 --- a/processing/import_color_profile.go +++ b/processing/import_color_profile.go @@ -8,7 +8,7 @@ import ( ) func importColorProfile(pctx *pipelineContext, img *vips.Image, po *options.ProcessingOptions, imgdata *imagedata.ImageData) error { - if pctx.iccImported { + if img.ColourProfileImported() { return nil } @@ -26,7 +26,6 @@ func importColorProfile(pctx *pipelineContext, img *vips.Image, po *options.Proc if err := img.ImportColourProfile(); err != nil { return err } - pctx.iccImported = true } if convertToLinear { diff --git a/processing/pipeline.go b/processing/pipeline.go index 0644262a..64612413 100644 --- a/processing/pipeline.go +++ b/processing/pipeline.go @@ -30,8 +30,6 @@ type pipelineContext struct { hscale float64 dprScale float64 - - iccImported bool } type pipelineStep func(pctx *pipelineContext, img *vips.Image, po *options.ProcessingOptions, imgdata *imagedata.ImageData) error diff --git a/processing/processing.go b/processing/processing.go index c7e12c11..35c52129 100644 --- a/processing/processing.go +++ b/processing/processing.go @@ -35,6 +35,9 @@ var mainPipeline = pipeline{ fixSize, flatten, watermark, +} + +var finalizePipeline = pipeline{ exportColorProfile, stripMetadata, } @@ -196,6 +199,7 @@ func transformAnimated(ctx context.Context, img *vips.Image, po *options.Process delay = delay[:framesCount] } + img.SetInt("imgproxy-is-animated", 1) img.SetInt("page-height", frames[0].Height()) img.SetIntSlice("delay", delay) img.SetInt("loop", loop) @@ -314,6 +318,10 @@ func ProcessImage(ctx context.Context, imgdata *imagedata.ImageData, po *options } } + if err := finalizePipeline.Run(ctx, img, po, imgdata); err != nil { + return nil, err + } + if po.Format == imagetype.AVIF && (img.Width() < 16 || img.Height() < 16) { if img.HasAlpha() { po.Format = imagetype.PNG diff --git a/processing/watermark.go b/processing/watermark.go index 32b8aa41..fababa91 100644 --- a/processing/watermark.go +++ b/processing/watermark.go @@ -18,7 +18,6 @@ var watermarkPipeline = pipeline{ scale, rotateAndFlip, padding, - stripMetadata, } func prepareWatermark(wm *vips.Image, wmData *imagedata.ImageData, opts *options.WatermarkOptions, imgWidth, imgHeight int, offsetScale float64, framesCount int) error { @@ -31,8 +30,6 @@ func prepareWatermark(wm *vips.Image, wmData *imagedata.ImageData, opts *options po.Dpr = 1 po.Enlarge = true po.Format = wmData.Type - po.StripMetadata = true - po.KeepCopyright = false if opts.Scale > 0 { po.Width = imath.Max(imath.ScaleToEven(imgWidth, opts.Scale), 1) @@ -80,16 +77,15 @@ func prepareWatermark(wm *vips.Image, wmData *imagedata.ImageData, opts *options } } - wm.RemovePaletteBitDepth() + // We don't want any headers to be copied from the watermark to the image + if err := wm.StripAll(); err != nil { + return err + } return nil } func applyWatermark(img *vips.Image, wmData *imagedata.ImageData, opts *options.WatermarkOptions, offsetScale float64, framesCount int) error { - if err := img.RgbColourspace(); err != nil { - return err - } - wm := new(vips.Image) defer wm.Clear() @@ -101,6 +97,16 @@ func applyWatermark(img *vips.Image, wmData *imagedata.ImageData, opts *options. return err } + if !img.ColourProfileImported() { + if err := img.ImportColourProfile(); err != nil { + return err + } + } + + if err := img.RgbColourspace(); err != nil { + return err + } + opacity := opts.Opacity * config.WatermarkOpacity // If we replicated the watermark and need to apply it to an animated image, diff --git a/vips/vips.c b/vips/vips.c index d6ac7e0f..0a0a2e78 100644 --- a/vips/vips.c +++ b/vips/vips.c @@ -285,15 +285,6 @@ vips_get_palette_bit_depth(VipsImage *image) return 0; } -void -vips_remove_palette_bit_depth(VipsImage *image) -{ - vips_image_remove(image, VIPS_META_PALETTE_BITS_DEPTH); -#ifdef VIPS_META_PALETTE - vips_image_remove(image, VIPS_META_PALETTE); -#endif -} - VipsBandFormat vips_band_format(VipsImage *in) { @@ -372,23 +363,37 @@ vips_resize_go(VipsImage *in, VipsImage **out, double wscale, double hscale) return res; } +/* We don't really need to return the size since we check if the buffer is at least + * the size of ICC header, and all we need is a header + */ +static const void * +vips_icc_get_header(VipsImage *in) +{ + const void *data = NULL; + size_t data_len = 0; + + if (!vips_image_get_typeof(in, VIPS_META_ICC_NAME) || + vips_image_get_blob(in, VIPS_META_ICC_NAME, &data, &data_len)) + return NULL; + + /* Less than header size + */ + if (!data || data_len < 128) + return NULL; + + return data; +} + int vips_icc_is_srgb_iec61966(VipsImage *in) { - const void *data; - size_t data_len; - // 1998-12-01 static char date[] = { 7, 206, 0, 2, 0, 9 }; // 2.1 static char version[] = { 2, 16, 0, 0 }; - if (vips_image_get_blob(in, VIPS_META_ICC_NAME, &data, &data_len)) - return FALSE; - - /* Less than header size - */ - if (data_len < 128) + const void *data = vips_icc_get_header(in); + if (!data) return FALSE; /* Predict it is sRGB IEC61966 2.1 by checking some header fields @@ -400,6 +405,19 @@ vips_icc_is_srgb_iec61966(VipsImage *in) (memcmp(data + 8, version, 4) == 0)); // Version } +static VipsPCS +vips_icc_get_pcs(VipsImage *in) +{ + const void *data = vips_icc_get_header(in); + if (!data) + return VIPS_PCS_LAB; + + if (memcmp(data + 20, "XYZ ", 4) == 0) + return VIPS_PCS_XYZ; + + return VIPS_PCS_LAB; +} + int vips_has_embedded_icc(VipsImage *in) { @@ -409,25 +427,28 @@ vips_has_embedded_icc(VipsImage *in) int vips_icc_import_go(VipsImage *in, VipsImage **out) { - return vips_icc_import(in, out, "embedded", TRUE, "pcs", VIPS_PCS_LAB, NULL); + const int res = vips_icc_import(in, out, "embedded", TRUE, "pcs", vips_icc_get_pcs(in), NULL); + if (!res && *out) + vips_image_set_int(*out, "imgproxy-icc-imported", 1); + return res; } int vips_icc_export_go(VipsImage *in, VipsImage **out) { - return vips_icc_export(in, out, "pcs", VIPS_PCS_LAB, NULL); + return vips_icc_export(in, out, "pcs", vips_icc_get_pcs(in), NULL); } int vips_icc_export_srgb(VipsImage *in, VipsImage **out) { - return vips_icc_export(in, out, "output_profile", "sRGB", "pcs", VIPS_PCS_LAB, NULL); + return vips_icc_export(in, out, "output_profile", "sRGB", "pcs", vips_icc_get_pcs(in), NULL); } int vips_icc_transform_go(VipsImage *in, VipsImage **out) { - return vips_icc_transform(in, out, "sRGB", "embedded", TRUE, "pcs", VIPS_PCS_LAB, NULL); + return vips_icc_transform(in, out, "sRGB", "embedded", TRUE, "pcs", vips_icc_get_pcs(in), NULL); } int @@ -773,11 +794,68 @@ vips_arrayjoin_go(VipsImage **in, VipsImage **out, int n) return vips_arrayjoin(in, out, n, "across", 1, NULL); } +typedef struct { + int strip_all; + int keep_exif_copyright; + int keep_animation; +} VipsStripOptions; + +void * +vips_strip_fn(VipsImage *in, const char *name, GValue *value, void *a) +{ + VipsStripOptions *opts = (VipsStripOptions *) a; + + if (strcmp(name, "vips-sequential") == 0) + return NULL; + + if (!opts->strip_all) { + if ((strcmp(name, VIPS_META_ICC_NAME) == 0) || +#ifdef VIPS_META_BITS_PER_SAMPLE + (strcmp(name, VIPS_META_BITS_PER_SAMPLE) == 0) || +#endif +#ifdef VIPS_META_PALETTE + (strcmp(name, VIPS_META_PALETTE) == 0) || +#endif + (strcmp(name, VIPS_META_PALETTE_BITS_DEPTH) == 0) || + (strcmp(name, "background") == 0) || + (strcmp(name, "vips-loader") == 0) || + (vips_isprefix("imgproxy-", name))) + return NULL; + + if (opts->keep_exif_copyright) + if ((strcmp(name, VIPS_META_EXIF_NAME) == 0) || + (strcmp(name, "exif-ifd0-Copyright") == 0) || + (strcmp(name, "exif-ifd0-Artist") == 0)) + return NULL; + + if (opts->keep_animation) + if ((strcmp(name, "page-height") == 0) || + (strcmp(name, "delay") == 0) || + (strcmp(name, "loop") == 0) || + (strcmp(name, "n-pages") == 0)) + return NULL; + } + + vips_image_remove(in, name); + + return NULL; +} + int vips_strip(VipsImage *in, VipsImage **out, int keep_exif_copyright) { static double default_resolution = 72.0 / 25.4; + VipsStripOptions opts = { + .strip_all = 0, + .keep_exif_copyright = FALSE, + .keep_animation = FALSE, + }; + + if (vips_image_get_typeof(in, "imgproxy-is-animated") && + vips_image_get_int(in, "imgproxy-is-animated", &opts.keep_animation)) + opts.keep_animation = FALSE; + if (vips_copy( in, out, "xres", default_resolution, @@ -785,45 +863,28 @@ vips_strip(VipsImage *in, VipsImage **out, int keep_exif_copyright) NULL)) return 1; - gchar **fields = vips_image_get_fields(in); + vips_image_map(*out, vips_strip_fn, &opts); - for (int i = 0; fields[i] != NULL; i++) { - gchar *name = fields[i]; + return 0; +} - if ( - (strcmp(name, VIPS_META_ICC_NAME) == 0) || -#ifdef VIPS_META_BITS_PER_SAMPLE - (strcmp(name, VIPS_META_BITS_PER_SAMPLE) == 0) || -#endif - (strcmp(name, VIPS_META_PALETTE_BITS_DEPTH) == 0) || - (strcmp(name, "width") == 0) || - (strcmp(name, "height") == 0) || - (strcmp(name, "bands") == 0) || - (strcmp(name, "format") == 0) || - (strcmp(name, "coding") == 0) || - (strcmp(name, "interpretation") == 0) || - (strcmp(name, "xoffset") == 0) || - (strcmp(name, "yoffset") == 0) || - (strcmp(name, "xres") == 0) || - (strcmp(name, "yres") == 0) || - (strcmp(name, "vips-loader") == 0) || - (strcmp(name, "background") == 0) || - (strcmp(name, "vips-sequential") == 0) || - (vips_isprefix("imgproxy-", name))) - continue; +int +vips_strip_all(VipsImage *in, VipsImage **out) +{ + VipsStripOptions opts = { + .strip_all = TRUE, + .keep_exif_copyright = FALSE, + .keep_animation = FALSE, + }; - if (keep_exif_copyright) { - if ( - (strcmp(name, VIPS_META_EXIF_NAME) == 0) || - (strcmp(name, "exif-ifd0-Copyright") == 0) || - (strcmp(name, "exif-ifd0-Artist") == 0)) - continue; - } + if (vips_copy(in, out, NULL)) + return 1; - vips_image_remove(*out, name); - } + vips_image_map(*out, vips_strip_fn, &opts); - g_strfreev(fields); + /* vips doesn't include "palette-bit-depth" to the map of fields + */ + vips_image_remove(*out, VIPS_META_PALETTE_BITS_DEPTH); return 0; } diff --git a/vips/vips.go b/vips/vips.go index b1d19eda..41bc028e 100644 --- a/vips/vips.go +++ b/vips/vips.go @@ -573,10 +573,6 @@ func (img *Image) SetBlob(name string, value []byte) { C.vips_image_set_blob_copy(img.VipsImage, cachedCString(name), unsafe.Pointer(&value[0]), C.size_t(len(value))) } -func (img *Image) RemovePaletteBitDepth() { - C.vips_remove_palette_bit_depth(img.VipsImage) -} - func (img *Image) CastUchar() error { var tmp *C.VipsImage @@ -740,7 +736,10 @@ func (img *Image) ImportColourProfile() error { return nil } - if C.vips_has_embedded_icc(img.VipsImage) == 0 { + // Don't import is there's no embedded profile or embedded profile is sRGB + if C.vips_has_embedded_icc(img.VipsImage) == 0 || + (C.vips_image_guess_interpretation(img.VipsImage) == C.VIPS_INTERPRETATION_sRGB && + C.vips_icc_is_srgb_iec61966(img.VipsImage) == 1) { return nil } @@ -753,6 +752,11 @@ func (img *Image) ImportColourProfile() error { return nil } +func (img *Image) ColourProfileImported() bool { + imported, err := img.GetIntDefault("imgproxy-icc-imported", 0) + return imported > 0 && err == nil +} + func (img *Image) ExportColourProfile() error { var tmp *C.VipsImage @@ -791,7 +795,9 @@ func (img *Image) TransformColourProfile() error { var tmp *C.VipsImage // Don't transform is there's no embedded profile or embedded profile is sRGB - if C.vips_has_embedded_icc(img.VipsImage) == 0 || C.vips_icc_is_srgb_iec61966(img.VipsImage) == 1 { + if C.vips_has_embedded_icc(img.VipsImage) == 0 || + (C.vips_image_guess_interpretation(img.VipsImage) == C.VIPS_INTERPRETATION_sRGB && + C.vips_icc_is_srgb_iec61966(img.VipsImage) == 1) { return nil } @@ -889,3 +895,14 @@ func (img *Image) Strip(keepExifCopyright bool) error { return nil } + +func (img *Image) StripAll() error { + var tmp *C.VipsImage + + if C.vips_strip_all(img.VipsImage, &tmp) != 0 { + return Error() + } + C.swap_and_clear(&img.VipsImage, tmp) + + return nil +} diff --git a/vips/vips.h b/vips/vips.h index e055e47e..5a972383 100644 --- a/vips/vips.h +++ b/vips/vips.h @@ -28,12 +28,9 @@ int vips_black_go(VipsImage **out, int width, int height, int bands); int vips_fix_float_tiff(VipsImage *in, VipsImage **out); int vips_get_orientation(VipsImage *image); -void vips_strip_meta(VipsImage *image); VipsBandFormat vips_band_format(VipsImage *in); -void vips_remove_palette_bit_depth(VipsImage *image); - gboolean vips_is_animated(VipsImage *in); int vips_image_get_array_int_go(VipsImage *image, const char *name, int **out, int *n); @@ -81,6 +78,7 @@ int vips_linecache_seq(VipsImage *in, VipsImage **out, int tile_height); int vips_arrayjoin_go(VipsImage **in, VipsImage **out, int n); int vips_strip(VipsImage *in, VipsImage **out, int keep_exif_copyright); +int vips_strip_all(VipsImage *in, VipsImage **out); int vips_jpegsave_go(VipsImage *in, void **buf, size_t *len, int quality, int interlace); int vips_pngsave_go(VipsImage *in, void **buf, size_t *len, int interlace, int quantize,