Commit 21356a04d for imagemagick.org
commit 21356a04d5aa0a5f5f70dbd4182a15048ec03394
Author: Greg B <64932474+gregbenz@users.noreply.github.com>
Date: Fri Sep 4 17:10:08 2026 -0500
Preserve Ultra HDR metadata precision (#8938)
diff --git a/Makefile.in b/Makefile.in
index 4ec85b7bc..142ed32ad 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -6269,6 +6269,7 @@ TESTS_TESTS = \
tests/cli-pcx.tap \
tests/cli-pipe.tap \
tests/cli-svg.tap \
+ tests/cli-uhdr.tap \
tests/validate-colorspace.tap \
tests/validate-compare.tap \
tests/validate-composite.tap \
diff --git a/coders/uhdr.c b/coders/uhdr.c
index 3829d9b95..4b1f8bd2c 100644
--- a/coders/uhdr.c
+++ b/coders/uhdr.c
@@ -146,7 +146,7 @@ static Image *ReadUHDRImage(const ImageInfo *image_info,
ExceptionInfo *exception)
{
#define SetHDRGMProperty(name,value) \
- (void) FormatLocaleString(buffer,sizeof(buffer),"%f",(value)); \
+ (void) FormatLocaleString(buffer,sizeof(buffer),"%.9g",(double) (value)); \
(void) SetImageProperty(image,"hdrgm:" name,buffer,exception)
#define SetHDRGMPropertyInt(name,value) \
(void) FormatLocaleString(buffer,sizeof(buffer),"%d",(value)); \
@@ -155,8 +155,8 @@ static Image *ReadUHDRImage(const ImageInfo *image_info,
(void) FormatLocaleString(buffer,sizeof(buffer),"%.17g",(double) (value)); \
(void) SetImageProperty(image,"hdrgm:" name,buffer,exception)
#define SetHDRGMProperty3(name,value,value1,value2) \
- (void) FormatLocaleString(buffer,sizeof(buffer),"%f,%f,%f", \
- (value),(value1),(value2)); \
+ (void) FormatLocaleString(buffer,sizeof(buffer),"%.9g,%.9g,%.9g", \
+ (double) (value),(double) (value1),(double) (value2)); \
(void) SetImageProperty(image,"hdrgm:" name,buffer,exception)
Image
@@ -1143,14 +1143,71 @@ static StringInfo *TransformGainMapProfile(const ImageInfo *image_info,
return(profile);
}
+static const char *SkipHDRGMWhitespace(const char *value)
+{
+ while ((*value != '\0') &&
+ (strchr(" \f\n\r\t\v",(int) ((unsigned char) *value)) !=
+ (char *) NULL))
+ value++;
+ return(value);
+}
+
+static MagickBooleanType ParseHDRGMProperty(const char *value,float *result)
+{
+ char
+ *q;
+
+ value=SkipHDRGMWhitespace(value);
+ *result=StringToFloat(value,&q);
+ if ((q == value) || (IsNaN((double) *result) != 0) ||
+ (*result > FLT_MAX) || (*result < -FLT_MAX))
+ return(MagickFalse);
+ q=(char *) SkipHDRGMWhitespace(q);
+ return(*q == '\0' ? MagickTrue : MagickFalse);
+}
+
+static MagickBooleanType ParseHDRGMProperty3(const char *value,
+ float values[3])
+{
+ char
+ *q;
+
+ ssize_t
+ i;
+
+ for (i=0; i < 3; i++)
+ {
+ value=SkipHDRGMWhitespace(value);
+ values[i]=StringToFloat(value,&q);
+ if ((q == value) || (IsNaN((double) values[i]) != 0) ||
+ (values[i] > FLT_MAX) || (values[i] < -FLT_MAX))
+ return(MagickFalse);
+ q=(char *) SkipHDRGMWhitespace(q);
+ if (i == 2)
+ return(*q == '\0' ? MagickTrue : MagickFalse);
+ if (*q != ',')
+ return(MagickFalse);
+ value=q+1;
+ }
+ return(MagickFalse);
+}
+
static MagickBooleanType WriteUHDRImage(const ImageInfo *image_info,
Image *images,ExceptionInfo *exception)
{
#define GetHDRGMProperty(name,field) \
do { \
const char *v = GetImageProperty(image,"hdrgm:" name,exception); \
- if (v != (const char *) NULL) \
- gainmap_info.field=(float) atof(v); \
+ float value; \
+ if ((v != (const char *) NULL) && \
+ (ParseHDRGMProperty(v,&value) == MagickFalse)) \
+ { \
+ (void) ThrowMagickException(exception,GetMagickModule(),OptionError, \
+ "InvalidArgument","`hdrgm:%s'",name); \
+ status=MagickFalse; \
+ } \
+ else if (v != (const char *) NULL) \
+ gainmap_info.field=value; \
} while (0)
#define GetHDRGMPropertyInt(name,field) \
do { \
@@ -1161,9 +1218,20 @@ static MagickBooleanType WriteUHDRImage(const ImageInfo *image_info,
#define GetHDRGMProperty3(name,field0,field1,field2) \
do { \
const char *v = GetImageProperty(image,"hdrgm:" name,exception); \
- if (v != (const char *) NULL) \
- (void) sscanf(v,"%f,%f,%f",&gainmap_info.field0,&gainmap_info.field1, \
- &gainmap_info.field2); \
+ float values[3]; \
+ if ((v != (const char *) NULL) && \
+ (ParseHDRGMProperty3(v,values) == MagickFalse)) \
+ { \
+ (void) ThrowMagickException(exception,GetMagickModule(),OptionError, \
+ "InvalidArgument","`hdrgm:%s'",name); \
+ status=MagickFalse; \
+ } \
+ else if (v != (const char *) NULL) \
+ { \
+ gainmap_info.field0=values[0]; \
+ gainmap_info.field1=values[1]; \
+ gainmap_info.field2=values[2]; \
+ } \
} while (0)
Image
@@ -1201,10 +1269,6 @@ static MagickBooleanType WriteUHDRImage(const ImageInfo *image_info,
assert(image->signature == MagickCoreSignature);
if (IsEventLogging() != MagickFalse)
(void) LogMagickEvent(TraceEvent, GetMagickModule(), "%s", image->filename);
- status=OpenBlob(image_info,image,WriteBinaryBlobMode,exception);
- if (status == MagickFalse)
- return (status);
-
const StringInfo *gainmap_profile = GetImageProfile(image,"hdrgm");
const size_t
@@ -1222,10 +1286,7 @@ static MagickBooleanType WriteUHDRImage(const ImageInfo *image_info,
resized_gainmap_profile=TransformGainMapProfile(image_info,image,
gainmap_profile,&gainmap_transform_status,exception);
if (gainmap_transform_status == MagickFalse)
- {
- (void) CloseBlob(image);
- return(MagickFalse);
- }
+ return(MagickFalse);
if (resized_gainmap_profile != (StringInfo *) NULL)
gainmap_profile=(const StringInfo *) resized_gainmap_profile;
base_image_profile=EncodeBaseImageProfile(image_info,image,exception);
@@ -1233,7 +1294,6 @@ static MagickBooleanType WriteUHDRImage(const ImageInfo *image_info,
{
if (resized_gainmap_profile != (StringInfo *) NULL)
resized_gainmap_profile=DestroyStringInfo(resized_gainmap_profile);
- (void) CloseBlob(image);
return(MagickFalse);
}
base_image.data=(void *) GetStringInfoDatum(base_image_profile);
@@ -1261,6 +1321,13 @@ static MagickBooleanType WriteUHDRImage(const ImageInfo *image_info,
GetHDRGMProperty("HDRCapacityMin",hdr_capacity_min);
GetHDRGMProperty("HDRCapacityMax",hdr_capacity_max);
GetHDRGMPropertyInt("UseBaseColorGrade",use_base_cg);
+ if (status == MagickFalse)
+ {
+ if (resized_gainmap_profile != (StringInfo *) NULL)
+ resized_gainmap_profile=DestroyStringInfo(resized_gainmap_profile);
+ base_image_profile=DestroyStringInfo(base_image_profile);
+ return(MagickFalse);
+ }
preserve_gainmap=MagickTrue;
}
@@ -1276,6 +1343,15 @@ static MagickBooleanType WriteUHDRImage(const ImageInfo *image_info,
"invalid hdr color transfer received, ","%s","exiting ... ");
return(MagickFalse);
}
+ status=OpenBlob(image_info,image,WriteBinaryBlobMode,exception);
+ if (status == MagickFalse)
+ {
+ if (resized_gainmap_profile != (StringInfo *) NULL)
+ resized_gainmap_profile=DestroyStringInfo(resized_gainmap_profile);
+ if (base_image_profile != (StringInfo *) NULL)
+ base_image_profile=DestroyStringInfo(base_image_profile);
+ return(status);
+ }
/*
HDR intent:
diff --git a/tests/Makefile.am b/tests/Makefile.am
index e0cf6c506..7a81b7d31 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -43,6 +43,7 @@ TESTS_TESTS = \
tests/cli-pcx.tap \
tests/cli-pipe.tap \
tests/cli-svg.tap \
+ tests/cli-uhdr.tap \
tests/validate-colorspace.tap \
tests/validate-compare.tap \
tests/validate-composite.tap \
diff --git a/tests/cli-uhdr.tap b/tests/cli-uhdr.tap
new file mode 100755
index 000000000..1299b4089
--- /dev/null
+++ b/tests/cli-uhdr.tap
@@ -0,0 +1,214 @@
+#!/bin/sh
+#
+# Copyright 1999 ImageMagick Studio LLC, a non-profit organization
+# dedicated to making software imaging solutions freely available.
+#
+# You may not use this file except in compliance with the License. You may
+# obtain a copy of the License at
+#
+# https://imagemagick.org/license/
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+# Regression tests for Ultra HDR metadata preservation.
+#
+. ./common.shi
+. ${srcdir}/tests/common.shi
+
+raw_source=cli-uhdr-offset-raw.jpg
+source=cli-uhdr-offset-source.jpg
+output=cli-uhdr-offset-output.jpg
+transformed_output=cli-uhdr-offset-transformed.jpg
+locale_output=cli-uhdr-offset-locale.jpg
+jpeg_probe=cli-uhdr-jpeg-probe.jpg
+whitespace_output=cli-uhdr-offset-whitespace.jpg
+invalid_output=cli-uhdr-offset-invalid.jpg
+capacity_min=1.00000012
+capacity_max=2.00000024
+expected_offsets='1.00000001e-07,1.00000001e-07,1.00000001e-07 1.00000001e-07,1.00000001e-07,1.00000001e-07'
+
+cleanup()
+{
+ rm -f "$raw_source" "$source" "$output" "$transformed_output" \
+ "$locale_output" "$jpeg_probe" "$whitespace_output" "$invalid_output"
+}
+
+check_value()
+{
+ description=$1
+ actual=$2
+ expected=$3
+
+ if [ "X$actual" = "X$expected" ]; then
+ echo "ok - $description"
+ else
+ echo "not ok - $description"
+ echo "# expected '$expected', got '$actual'"
+ fi
+}
+
+cleanup
+if ! ${MAGICK} -list configure 2>/dev/null | awk '
+ $1 == "DELEGATES" {
+ for (i=2; i <= NF; i++)
+ if ($i == "uhdr")
+ found=1
+ }
+ END { exit(found ? 0 : 1) }
+ '; then
+ echo "1..0 # SKIP UHDR coder unavailable"
+ cleanup
+ exit 0
+fi
+jpeg_format=
+if ${MAGICK} "${SRCDIR}/rose.pnm" "$jpeg_probe" >/dev/null 2>&1; then
+ jpeg_format=`${IDENTIFY} -quiet -format '%m' "$jpeg_probe" 2>/dev/null`
+fi
+if [ "X$jpeg_format" != "XJPEG" ]; then
+ echo "1..0 # SKIP JPEG coder unavailable"
+ cleanup
+ exit 0
+fi
+
+echo "1..11"
+if ! ${MAGICK} \( "${SRCDIR}/rose.pnm" -depth 8 \) \
+ \( "${SRCDIR}/rose.pnm" -depth 16 \) \
+ -define uhdr:hdr-color-transfer=linear "UHDR:$raw_source" \
+ >/dev/null 2>&1; then
+ echo "Bail out! unable to create UHDR test image"
+ cleanup
+ exit 0
+fi
+if ! ${MAGICK} "UHDR:$raw_source" \
+ -set hdrgm:OffsetSDR '0.0000001,0.0000001,0.0000001' \
+ -set hdrgm:OffsetHDR '0.0000001,0.0000001,0.0000001' \
+ "UHDR:$source" >/dev/null 2>&1; then
+ echo "Bail out! unable to create UHDR metadata fixture"
+ cleanup
+ exit 0
+fi
+
+if ${MAGICK} "UHDR:$source" \
+ -set hdrgm:OffsetSDR ' 0.0000001 , 0.0000001 , 0.0000001 ' \
+ -set hdrgm:OffsetHDR ' 0.0000001 , 0.0000001 , 0.0000001 ' \
+ -set hdrgm:HDRCapacityMin " $capacity_min " \
+ -set hdrgm:HDRCapacityMax " $capacity_max " \
+ "UHDR:$whitespace_output" >/dev/null 2>&1; then
+ whitespace_metadata=`${IDENTIFY} -quiet -format \
+ '%[hdrgm:OffsetSDR] %[hdrgm:OffsetHDR] %[hdrgm:HDRCapacityMin] %[hdrgm:HDRCapacityMax]' \
+ "UHDR:$whitespace_output" 2>/dev/null`
+else
+ whitespace_metadata=
+fi
+check_value "metadata permits surrounding whitespace" "$whitespace_metadata" \
+ "$expected_offsets $capacity_min $capacity_max"
+
+printf 'sentinel' > "$invalid_output"
+if ${MAGICK} "UHDR:$source" -set hdrgm:OffsetSDR '0.1,invalid,0.3' \
+ "UHDR:$invalid_output" >/dev/null 2>&1; then
+ echo "not ok - malformed vector metadata is rejected"
+else
+ content=`cat "$invalid_output" 2>/dev/null`
+ check_value "malformed vector metadata is rejected without truncating output" \
+ "$content" sentinel
+fi
+printf 'sentinel' > "$invalid_output"
+if ${MAGICK} "UHDR:$source" -set hdrgm:HDRCapacityMax '2junk' \
+ "UHDR:$invalid_output" >/dev/null 2>&1; then
+ echo "not ok - malformed scalar metadata is rejected"
+else
+ content=`cat "$invalid_output" 2>/dev/null`
+ check_value "malformed scalar metadata is rejected without truncating output" \
+ "$content" sentinel
+fi
+
+source_offsets=`${IDENTIFY} -quiet -format \
+ '%[hdrgm:OffsetSDR] %[hdrgm:OffsetHDR]' "UHDR:$source" 2>/dev/null`
+if ${MAGICK} "UHDR:$source" \
+ -set hdrgm:HDRCapacityMin "$capacity_min" \
+ -set hdrgm:HDRCapacityMax "$capacity_max" \
+ "UHDR:$output" >/dev/null 2>&1; then
+ output_offsets=`${IDENTIFY} -quiet -format \
+ '%[hdrgm:OffsetSDR] %[hdrgm:OffsetHDR]' "UHDR:$output" 2>/dev/null`
+ output_capacity=`${IDENTIFY} -quiet -format \
+ '%[hdrgm:HDRCapacityMin] %[hdrgm:HDRCapacityMax]' \
+ "UHDR:$output" 2>/dev/null`
+else
+ output_offsets=
+ output_capacity=
+fi
+
+if ${MAGICK} "UHDR:$source" \
+ -set hdrgm:HDRCapacityMin "$capacity_min" \
+ -set hdrgm:HDRCapacityMax "$capacity_max" -resize 50% \
+ "UHDR:$transformed_output" >/dev/null 2>&1; then
+ transformed_offsets=`${IDENTIFY} -quiet -format \
+ '%[hdrgm:OffsetSDR] %[hdrgm:OffsetHDR]' \
+ "UHDR:$transformed_output" 2>/dev/null`
+ transformed_capacity=`${IDENTIFY} -quiet -format \
+ '%[hdrgm:HDRCapacityMin] %[hdrgm:HDRCapacityMax]' \
+ "UHDR:$transformed_output" 2>/dev/null`
+else
+ transformed_offsets=
+ transformed_capacity=
+fi
+
+offsets_nonzero=`echo "$source_offsets" | awk -F '[, ]+' \
+ '{ result=(NF == 6); for (i=1; i <= NF; i++) if (($i+0) == 0 || \
+ ($i+0) >= 0.000001 || ($i+0) <= -0.000001) result=0; print result }'`
+if [ "X$offsets_nonzero" = "X1" ]; then
+ echo "ok - small nonzero offsets are retained as properties"
+else
+ echo "not ok - small nonzero offsets are retained as properties"
+ echo "# got '$source_offsets'"
+fi
+check_value "source uses controlled offset values" "$source_offsets" \
+ "$expected_offsets"
+offsets_nonzero=`echo "$output_offsets" | awk -F '[, ]+' \
+ '{ result=(NF == 6); for (i=1; i <= NF; i++) if (($i+0) == 0 || \
+ ($i+0) >= 0.000001 || ($i+0) <= -0.000001) result=0; print result }'`
+if [ "X$offsets_nonzero" = "X1" ]; then
+ echo "ok - small nonzero offsets survive a read/write cycle"
+else
+ echo "not ok - small nonzero offsets survive a read/write cycle"
+ echo "# got '$output_offsets'"
+fi
+check_value "offset values are unchanged" "$output_offsets" \
+ "$source_offsets"
+check_value "capacity values retain float precision" "$output_capacity" \
+ "$capacity_min $capacity_max"
+check_value "offset values survive a geometry transform" \
+ "$transformed_offsets" "$source_offsets"
+check_value "capacity values survive a geometry transform" \
+ "$transformed_capacity" "$capacity_min $capacity_max"
+
+comma_locale=
+for candidate in de_DE.UTF-8 fr_FR.UTF-8 nl_NL.UTF-8; do
+ decimal_point=`LC_ALL=$candidate locale decimal_point 2>/dev/null`
+ if [ "X$decimal_point" = "X," ]; then
+ comma_locale=$candidate
+ break
+ fi
+done
+if [ -n "$comma_locale" ]; then
+ if LC_ALL=$comma_locale ${MAGICK} "UHDR:$source" \
+ -set hdrgm:HDRCapacityMin "$capacity_min" \
+ -set hdrgm:HDRCapacityMax "$capacity_max" \
+ "UHDR:$locale_output" >/dev/null 2>&1; then
+ locale_metadata=`${IDENTIFY} -quiet -format \
+ '%[hdrgm:OffsetSDR] %[hdrgm:OffsetHDR] %[hdrgm:HDRCapacityMin] %[hdrgm:HDRCapacityMax]' \
+ "UHDR:$locale_output" 2>/dev/null`
+ else
+ locale_metadata=
+ fi
+ check_value "metadata parsing is locale independent" "$locale_metadata" \
+ "$source_offsets $capacity_min $capacity_max"
+else
+ echo "ok - metadata parsing is locale independent # SKIP comma locale unavailable"
+fi
+cleanup
+: