fix: swap tone mapping logic for icc profile set (#281)

Author: salarkhan

avif.go

@@ -155,9 +155,17 @@ func (d *avifDecoder) Close() {
 // Encoder Implementation
 // ----------------------------------------
 
-func newAvifEncoder(decodedBy Decoder, dstBuf []byte) (*avifEncoder, error) {
+func newAvifEncoder(decodedBy Decoder, dstBuf []byte, config *EncodeConfig) (*avifEncoder, error) {
 	dstBuf = dstBuf[:1]
-	icc := decodedBy.ICC()
+
+	// Use ICC override from config if provided, otherwise use decoder's ICC
+	var icc []byte
+	if config != nil && len(config.ICCOverride) > 0 {
+		icc = config.ICCOverride
+	} else {
+		icc = decodedBy.ICC()
+	}
+
 	loopCount := decodedBy.LoopCount()
 	bgColor := decodedBy.BackgroundColor()
 

avif_test.go

@@ -164,7 +164,7 @@ func testNewAvifEncoder(t *testing.T) {
 			defer decoder.Close()
 
 			dstBuf := make([]byte, destinationBufferSize)
-			encoder, err := newAvifEncoder(decoder, dstBuf)
+			encoder, err := newAvifEncoder(decoder, dstBuf, nil)
 			if err != nil {
 				t.Fatalf("Unexpected error: %v", err)
 			}
@@ -252,7 +252,7 @@ func testAvifEncoderEncode(t *testing.T) {
 			}
 
 			dstBuf := make([]byte, destinationBufferSize)
-			encoder, err := newAvifEncoder(decoder, dstBuf)
+			encoder, err := newAvifEncoder(decoder, dstBuf, nil)
 			if err != nil {
 				t.Fatalf("Failed to create a new AVIF encoder: %v", err)
 			}

color_info.cpp

@@ -0,0 +1,27 @@
+#include "color_info.hpp"
+#include <lcms2.h>
+
+// Maximum ICC profile size we're willing to parse (1MB)
+static const size_t MAX_ICC_PROFILE_SIZE = 1024 * 1024;
+
+// Check if ICC profile indicates HDR (PQ or HLG transfer function)
+bool is_hdr_transfer_function(const uint8_t* icc_data, size_t icc_len)
+{
+    if (!icc_data || icc_len == 0 || icc_len > MAX_ICC_PROFILE_SIZE) {
+        return false;
+    }
+
+    cmsHPROFILE profile = cmsOpenProfileFromMem(icc_data, icc_len);
+    if (!profile) {
+        return false;
+    }
+
+    uint8_t transfer = CICP_TRANSFER_UNSPECIFIED;
+    cmsVideoSignalType* cicp = (cmsVideoSignalType*)cmsReadTag(profile, cmsSigcicpTag);
+    if (cicp && cicp->TransferCharacteristics != 0) {
+        transfer = static_cast<uint8_t>(cicp->TransferCharacteristics);
+    }
+
+    cmsCloseProfile(profile);
+    return (transfer == CICP_TRANSFER_PQ) || (transfer == CICP_TRANSFER_HLG);
+}

color_info.hpp

@@ -0,0 +1,26 @@
+#pragma once
+
+#include <stddef.h>
+#include <stdint.h>
+
+// CICP Transfer Characteristics (ITU-T H.273)
+#define CICP_TRANSFER_UNSPECIFIED   0
+#define CICP_TRANSFER_PQ            16  // SMPTE ST 2084 (HDR10)
+#define CICP_TRANSFER_HLG           18  // ARIB STD-B67 (HLG)
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/**
+ * Check if an ICC profile indicates HDR content (PQ or HLG transfer function).
+ * Returns true if the profile's CICP tag indicates PQ or HLG transfer characteristics.
+ */
+bool is_hdr_transfer_function(
+    const uint8_t* icc_data,
+    size_t icc_len
+);
+
+#ifdef __cplusplus
+}
+#endif

giflib.go

@@ -235,7 +235,8 @@ func (d *gifDecoder) SkipFrame() error {
 
 // newGifEncoder creates a new GIF encoder that will write to the provided buffer.
 // Requires the original decoder that was used to decode the source GIF.
-func newGifEncoder(decodedBy Decoder, buf []byte) (*gifEncoder, error) {
+// config is accepted for API uniformity but not used by GIF encoder.
+func newGifEncoder(decodedBy Decoder, buf []byte, config *EncodeConfig) (*gifEncoder, error) {
 	// we must have a decoder since we can't build our own palettes
 	// so if we don't get a gif decoder, bail out
 	if decodedBy == nil {

lilliput.go

@@ -4,6 +4,7 @@ package lilliput
 
 import (
 	"bytes"
+	_ "embed"
 	"errors"
 	"strings"
 	"time"
@@ -14,6 +15,12 @@ const (
 	ICCProfileBufferSize = 32768
 )
 
+// SRGBICCProfile is the sRGB ICC profile (v4) - used when force_sdr overrides HDR ICC profiles.
+// Source: https://github.com/saucecontrol/Compact-ICC-Profiles
+//
+//go:embed icc_profiles/srgb_profile.icc
+var SRGBICCProfile []byte
+
 var (
 	ErrInvalidImage     = errors.New("unrecognized image format")
 	ErrDecodingFailed   = errors.New("failed to decode image")
@@ -156,30 +163,40 @@ func NewDecoderWithOptionalToneMapping(buf []byte, toneMappingEnabled bool) (Dec
 	return newAVCodecDecoder(buf)
 }
 
-// NewEncoder returns an Encode which can be used to encode Framebuffer
+// EncodeConfig provides configuration options for encoders.
+// This struct provides a clean extension point for future encoding config
+// without changing function signatures.
+type EncodeConfig struct {
+	// ICCOverride overrides the decoder's ICC profile when set.
+	// Used for HDR→SDR conversion to force sRGB output.
+	ICCOverride []byte
+}
+
+// NewEncoder returns an Encoder which can be used to encode Framebuffer
 // into compressed image data. ext should be a string like ".jpeg" or
 // ".png". decodedBy is optional and can be the Decoder used to make
 // the Framebuffer. dst is where an encoded image will be written.
-func NewEncoder(ext string, decodedBy Decoder, dst []byte) (Encoder, error) {
+// config can be nil to use default settings.
+func NewEncoder(ext string, decodedBy Decoder, dst []byte, config *EncodeConfig) (Encoder, error) {
 	if strings.ToLower(ext) == ".gif" {
-		return newGifEncoder(decodedBy, dst)
+		return newGifEncoder(decodedBy, dst, config)
 	}
 
 	if strings.ToLower(ext) == ".webp" {
-		return newWebpEncoder(decodedBy, dst)
+		return newWebpEncoder(decodedBy, dst, config)
 	}
 
 	if strings.ToLower(ext) == ".avif" {
-		return newAvifEncoder(decodedBy, dst)
+		return newAvifEncoder(decodedBy, dst, config)
 	}
 
 	if strings.ToLower(ext) == ".mp4" || strings.ToLower(ext) == ".webm" {
 		return nil, errors.New("Encoder cannot encode into video types")
 	}
 
 	if strings.ToLower(ext) == ".thumbhash" {
-		return newThumbhashEncoder(decodedBy, dst)
+		return newThumbhashEncoder(decodedBy, dst, config)
 	}
 
-	return newOpenCVEncoder(ext, decodedBy, dst)
+	return newOpenCVEncoder(ext, decodedBy, dst, config)
 }

opencv.go

@@ -3,7 +3,7 @@ package lilliput
 // #include "opencv.hpp"
 // #include "avif.hpp"
 // #include "webp.hpp"
-// #include "tone_mapping.hpp"
+// #include "color_info.hpp"
 import "C"
 
 import (
@@ -268,38 +268,13 @@ func (f *Framebuffer) OrientationTransform(orientation ImageOrientation) {
 	f.height = int(C.opencv_mat_get_height(f.mat))
 }
 
-// ApplyToneMapping applies HDR to SDR tone mapping if the ICC profile indicates HDR content.
-// This is an in-place operation that replaces the framebuffer's contents with tone-mapped data.
-// If the image is not HDR or tone mapping is not needed, the framebuffer is unchanged (copied in-place).
-// Returns an error if tone mapping fails.
-func (f *Framebuffer) ApplyToneMapping(icc []byte) error {
-	if f.mat == nil {
-		return ErrInvalidImage
-	}
-
-	var iccPtr unsafe.Pointer
-	if len(icc) > 0 {
-		iccPtr = unsafe.Pointer(&icc[0])
-	}
-
-	toneMappedMat := C.apply_tone_mapping(
-		f.mat,
-		(*C.uint8_t)(iccPtr),
-		C.size_t(len(icc)))
-
-	if toneMappedMat == nil {
-		return ErrInvalidImage
+// IsHDRICCProfile checks if an ICC profile indicates HDR content (PQ or HLG transfer function).
+// Returns true if the profile's CICP tag indicates PQ or HLG transfer characteristics.
+func IsHDRICCProfile(icc []byte) bool {
+	if len(icc) == 0 {
+		return false
 	}
-
-	// Replace the current mat with the tone-mapped one
-	C.opencv_mat_release(f.mat)
-	f.mat = toneMappedMat
-
-	// Update dimensions in case they changed (they shouldn't, but be safe)
-	f.width = int(C.opencv_mat_get_width(f.mat))
-	f.height = int(C.opencv_mat_get_height(f.mat))
-
-	return nil
+	return bool(C.is_hdr_transfer_function((*C.uint8_t)(unsafe.Pointer(&icc[0])), C.size_t(len(icc))))
 }
 
 // ResizeTo performs a resizing transform on the Framebuffer and puts the result
@@ -764,7 +739,9 @@ func (d *openCVDecoder) SkipFrame() error {
 	return ErrSkipNotSupported
 }
 
-func newOpenCVEncoder(ext string, decodedBy Decoder, dstBuf []byte) (*openCVEncoder, error) {
+// newOpenCVEncoder creates an OpenCV-based encoder.
+// config is accepted for API uniformity but not used by OpenCV encoder.
+func newOpenCVEncoder(ext string, decodedBy Decoder, dstBuf []byte, config *EncodeConfig) (*openCVEncoder, error) {
 	dstBuf = dstBuf[:1]
 	dst := C.opencv_mat_create_empty_from_data(C.int(cap(dstBuf)), unsafe.Pointer(&dstBuf[0]))
 

opencv_test.go

@@ -269,7 +269,7 @@ func TestICC(t *testing.T) {
 
 			// try encoding a WebP image, including ICC profile data when available
 			dstBuf := make([]byte, destinationBufferSize)
-			encoder, err := newWebpEncoder(decoder, dstBuf)
+			encoder, err := newWebpEncoder(decoder, dstBuf, nil)
 			if err != nil {
 				t.Fatalf("Failed to create a new webp encoder: %v", err)
 			}

ops.go

@@ -331,16 +331,6 @@ func (o *ImageOps) Transform(d Decoder, opt *ImageOptions, dst []byte) ([]byte,
 			}
 		}
 
-		// Apply tone mapping if requested (before encoding)
-		if !emptyFrame && opt.ForceSdr {
-			icc := d.ICC()
-			if len(icc) > 0 {
-				if err := o.active().ApplyToneMapping(icc); err != nil {
-					return nil, err
-				}
-			}
-		}
-
 		// encode the frame to the output buffer
 		var content []byte
 		if emptyFrame {
@@ -415,7 +405,18 @@ func (o *ImageOps) initializeTransform(d Decoder, opt *ImageOptions, dst []byte)
 		return nil, nil, err
 	}
 
-	enc, err := NewEncoder(opt.FileType, d, dst)
+	// Build encode config, including ICC override for HDR→SDR conversion
+	var encodeConfig *EncodeConfig
+	if opt.ForceSdr {
+		icc := d.ICC()
+		if len(icc) > 0 && IsHDRICCProfile(icc) {
+			encodeConfig = &EncodeConfig{
+				ICCOverride: SRGBICCProfile,
+			}
+		}
+	}
+
+	enc, err := NewEncoder(opt.FileType, d, dst, encodeConfig)
 	if err != nil {
 		return nil, nil, err
 	}

thumbhash.go

@@ -18,7 +18,8 @@ type thumbhashEncoder struct {
 // newThumbhashEncoder creates a new ThumbHash encoder instance.
 // It takes a decoder and a buffer as input, initializing the C-based encoder.
 // Returns an error if the provided buffer is too small.
-func newThumbhashEncoder(decodedBy Decoder, buf []byte) (*thumbhashEncoder, error) {
+// config is accepted for API uniformity but not used by ThumbHash encoder.
+func newThumbhashEncoder(decodedBy Decoder, buf []byte, config *EncodeConfig) (*thumbhashEncoder, error) {
 	buf = buf[:1]
 	enc := C.thumbhash_encoder_create(unsafe.Pointer(&buf[0]), C.size_t(cap(buf)))
 	if enc == nil {