Extract finalizing to a separate pipeline; Fix watermark colors on non-sRGB images

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

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 {

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

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

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,

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;
 }

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
+}

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,