From 80deca83d0ef20c6fe59a298c77752b16ccce70b Mon Sep 17 00:00:00 2001 From: Andrew Murray Date: Wed, 13 Nov 2024 21:35:09 +1100 Subject: [PATCH 1/5] Do not apply EXIF orientation when saving, since EXIF is already saved --- src/PIL/AvifImagePlugin.py | 16 +---- src/_avif.c | 117 +------------------------------------ 2 files changed, 2 insertions(+), 131 deletions(-) diff --git a/src/PIL/AvifImagePlugin.py b/src/PIL/AvifImagePlugin.py index 1c90f428a09..de770cfd6f0 100644 --- a/src/PIL/AvifImagePlugin.py +++ b/src/PIL/AvifImagePlugin.py @@ -4,7 +4,7 @@ from io import BytesIO from typing import IO -from . import ExifTags, Image, ImageFile +from . import Image, ImageFile try: from . import _avif @@ -164,19 +164,6 @@ def _save( if isinstance(exif, Image.Exif): exif = exif.tobytes() - exif_orientation = 0 - if exif: - exif_data = Image.Exif() - try: - exif_data.load(exif) - except SyntaxError: - pass - else: - orientation_tag = next( - k for k, v in ExifTags.TAGS.items() if v == "Orientation" - ) - exif_orientation = exif_data.get(orientation_tag) or 0 - xmp = info.get("xmp", im.info.get("xmp") or im.info.get("XML:com.adobe.xmp")) if isinstance(xmp, str): @@ -220,7 +207,6 @@ def _save( autotiling, icc_profile or b"", exif or b"", - exif_orientation, xmp or b"", advanced, ) diff --git a/src/_avif.c b/src/_avif.c index d0bb81f4619..21adf673662 100644 --- a/src/_avif.c +++ b/src/_avif.c @@ -76,118 +76,6 @@ exc_type_for_avif_result(avifResult result) { } } -static void -exif_orientation_to_irot_imir(avifImage *image, int orientation) { - const avifTransformFlags otherFlags = - image->transformFlags & ~(AVIF_TRANSFORM_IROT | AVIF_TRANSFORM_IMIR); - - // - // Mapping from Exif orientation as defined in JEITA CP-3451C section 4.6.4.A - // Orientation to irot and imir boxes as defined in HEIF ISO/IEC 28002-12:2021 - // sections 6.5.10 and 6.5.12. - switch (orientation) { - case 1: // The 0th row is at the visual top of the image, and the 0th column is - // the visual left-hand side. - image->transformFlags = otherFlags; - image->irot.angle = 0; // ignored -#if AVIF_VERSION_MAJOR >= 1 - image->imir.axis = 0; // ignored -#else - image->imir.mode = 0; // ignored -#endif - return; - case 2: // The 0th row is at the visual top of the image, and the 0th column is - // the visual right-hand side. - image->transformFlags = otherFlags | AVIF_TRANSFORM_IMIR; - image->irot.angle = 0; // ignored -#if AVIF_VERSION_MAJOR >= 1 - image->imir.axis = 1; -#else - image->imir.mode = 1; -#endif - return; - case 3: // The 0th row is at the visual bottom of the image, and the 0th column - // is the visual right-hand side. - image->transformFlags = otherFlags | AVIF_TRANSFORM_IROT; - image->irot.angle = 2; -#if AVIF_VERSION_MAJOR >= 1 - image->imir.axis = 0; // ignored -#else - image->imir.mode = 0; // ignored -#endif - return; - case 4: // The 0th row is at the visual bottom of the image, and the 0th column - // is the visual left-hand side. - image->transformFlags = otherFlags | AVIF_TRANSFORM_IMIR; - image->irot.angle = 0; // ignored -#if AVIF_VERSION_MAJOR >= 1 - image->imir.axis = 0; -#else - image->imir.mode = 0; -#endif - return; - case 5: // The 0th row is the visual left-hand side of the image, and the 0th - // column is the visual top. - image->transformFlags = - otherFlags | AVIF_TRANSFORM_IROT | AVIF_TRANSFORM_IMIR; - image->irot.angle = 1; // applied before imir according to MIAF spec - // ISO/IEC 28002-12:2021 - section 7.3.6.7 -#if AVIF_VERSION_MAJOR >= 1 - image->imir.axis = 0; -#else - image->imir.mode = 0; -#endif - return; - case 6: // The 0th row is the visual right-hand side of the image, and the 0th - // column is the visual top. - image->transformFlags = otherFlags | AVIF_TRANSFORM_IROT; - image->irot.angle = 3; -#if AVIF_VERSION_MAJOR >= 1 - image->imir.axis = 0; // ignored -#else - image->imir.mode = 0; // ignored -#endif - return; - case 7: // The 0th row is the visual right-hand side of the image, and the 0th - // column is the visual bottom. - image->transformFlags = - otherFlags | AVIF_TRANSFORM_IROT | AVIF_TRANSFORM_IMIR; - image->irot.angle = 3; // applied before imir according to MIAF spec - // ISO/IEC 28002-12:2021 - section 7.3.6.7 -#if AVIF_VERSION_MAJOR >= 1 - image->imir.axis = 0; -#else - image->imir.mode = 0; -#endif - return; - case 8: // The 0th row is the visual left-hand side of the image, and the 0th - // column is the visual bottom. - image->transformFlags = otherFlags | AVIF_TRANSFORM_IROT; - image->irot.angle = 1; -#if AVIF_VERSION_MAJOR >= 1 - image->imir.axis = 0; // ignored -#else - image->imir.mode = 0; // ignored -#endif - return; - default: // reserved - break; - } - - // The orientation tag is not mandatory (only recommended) according to JEITA - // CP-3451C section 4.6.8.A. The default value is 1 if the orientation tag is - // missing, meaning: - // The 0th row is at the visual top of the image, and the 0th column is the visual - // left-hand side. - image->transformFlags = otherFlags; - image->irot.angle = 0; // ignored -#if AVIF_VERSION_MAJOR >= 1 - image->imir.axis = 0; // ignored -#else - image->imir.mode = 0; // ignored -#endif -} - static int _codec_available(const char *name, uint32_t flags) { avifCodecChoice codec = avifCodecChoiceFromName(name); @@ -257,7 +145,6 @@ AvifEncoderNew(PyObject *self_, PyObject *args) { int qmax; int quality; int speed; - int exif_orientation; int max_threads; PyObject *icc_bytes; PyObject *exif_bytes; @@ -274,7 +161,7 @@ AvifEncoderNew(PyObject *self_, PyObject *args) { if (!PyArg_ParseTuple( args, - "IIsiiiiissiiOOSSiSO", + "IIsiiiiissiiOOSSSO", &width, &height, &subsampling, @@ -291,7 +178,6 @@ AvifEncoderNew(PyObject *self_, PyObject *args) { &autotiling, &icc_bytes, &exif_bytes, - &exif_orientation, &xmp_bytes, &advanced )) { @@ -461,7 +347,6 @@ AvifEncoderNew(PyObject *self_, PyObject *args) { PyBytes_GET_SIZE(xmp_bytes) ); } - exif_orientation_to_irot_imir(image, exif_orientation); self->image = image; self->frame_index = -1; From 7d591339a16904630196a0134f2c04af34208aa0 Mon Sep 17 00:00:00 2001 From: Andrew Murray Date: Wed, 13 Nov 2024 22:20:42 +1100 Subject: [PATCH 2/5] Allow unsupported warning to be returned when identifying container brands --- src/PIL/AvifImagePlugin.py | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/PIL/AvifImagePlugin.py b/src/PIL/AvifImagePlugin.py index de770cfd6f0..41172e27ad7 100644 --- a/src/PIL/AvifImagePlugin.py +++ b/src/PIL/AvifImagePlugin.py @@ -24,17 +24,11 @@ def _accept(prefix: bytes) -> bool | str: if prefix[4:8] != b"ftyp": return False - coding_brands = (b"avif", b"avis") - container_brands = (b"mif1", b"msf1") major_brand = prefix[8:12] - if major_brand in coding_brands: - if not SUPPORTED: - return ( - "image file could not be identified because AVIF " - "support not installed" - ) - return True - if major_brand in container_brands: + if major_brand in ( + # coding brands + b"avif", + b"avis", # We accept files with AVIF container brands; we can't yet know if # the ftyp box has the correct compatible brands, but if it doesn't # then the plugin will raise a SyntaxError which Pillow will catch @@ -42,6 +36,14 @@ def _accept(prefix: bytes) -> bool | str: # # Also, because this file might not actually be an AVIF file, we # don't raise an error if AVIF support isn't properly compiled. + b"mif1", + b"msf1", + ): + if not SUPPORTED: + return ( + "image file could not be identified because AVIF " + "support not installed" + ) return True return False @@ -104,10 +106,8 @@ def load(self) -> Image.core.PixelAccess | None: data, timescale, tsp_in_ts, dur_in_ts = self._decoder.get_frame( self.__frame ) - timestamp = round(1000 * (tsp_in_ts / timescale)) - duration = round(1000 * (dur_in_ts / timescale)) - self.info["timestamp"] = timestamp - self.info["duration"] = duration + self.info["timestamp"] = round(1000 * (tsp_in_ts / timescale)) + self.info["duration"] = round(1000 * (dur_in_ts / timescale)) self.__loaded = self.__frame # Set tile @@ -186,7 +186,7 @@ def _save( ) raise ValueError(msg) advanced = tuple( - [(str(k).encode("utf-8"), str(v).encode("utf-8")) for k, v in advanced] + (str(k).encode("utf-8"), str(v).encode("utf-8")) for k, v in advanced ) # Setup the AVIF encoder From 04020eec1be1995a9faaac9e39ce4061ff36a1f9 Mon Sep 17 00:00:00 2001 From: Andrew Murray Date: Wed, 13 Nov 2024 22:30:21 +1100 Subject: [PATCH 3/5] Removed unused variable --- src/_avif.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/_avif.c b/src/_avif.c index 21adf673662..82f5b954e88 100644 --- a/src/_avif.c +++ b/src/_avif.c @@ -383,7 +383,6 @@ _encoder_add(AvifEncoderObject *self, PyObject *args) { PyObject *ret = Py_None; int is_first_frame; - int channels; avifRGBImage rgb; avifResult result; @@ -446,10 +445,8 @@ _encoder_add(AvifEncoderObject *self, PyObject *args) { if (strcmp(mode, "RGBA") == 0) { rgb.format = AVIF_RGB_FORMAT_RGBA; - channels = 4; } else { rgb.format = AVIF_RGB_FORMAT_RGB; - channels = 3; } avifRGBImageAllocatePixels(&rgb); From 78dcac15aecdd697fbd7577c88c2bf615095d474 Mon Sep 17 00:00:00 2001 From: Andrew Murray Date: Mon, 18 Nov 2024 21:25:33 +1100 Subject: [PATCH 4/5] Improved error handling --- src/_avif.c | 103 +++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 89 insertions(+), 14 deletions(-) diff --git a/src/_avif.c b/src/_avif.c index 82f5b954e88..c50068b860e 100644 --- a/src/_avif.c +++ b/src/_avif.c @@ -106,30 +106,43 @@ _encoder_codec_available(PyObject *self, PyObject *args) { return PyBool_FromLong(is_available); } -static void +static int _add_codec_specific_options(avifEncoder *encoder, PyObject *opts) { Py_ssize_t i, size; PyObject *keyval, *py_key, *py_val; char *key, *val; if (!PyTuple_Check(opts)) { - return; + PyErr_SetString(PyExc_ValueError, "Invalid advanced codec options"); + return 1; } size = PyTuple_GET_SIZE(opts); for (i = 0; i < size; i++) { keyval = PyTuple_GetItem(opts, i); if (!PyTuple_Check(keyval) || PyTuple_GET_SIZE(keyval) != 2) { - return; + PyErr_SetString(PyExc_ValueError, "Invalid advanced codec options"); + return 1; } py_key = PyTuple_GetItem(keyval, 0); py_val = PyTuple_GetItem(keyval, 1); if (!PyBytes_Check(py_key) || !PyBytes_Check(py_val)) { - return; + PyErr_SetString(PyExc_ValueError, "Invalid advanced codec options"); + return 1; } key = PyBytes_AsString(py_key); val = PyBytes_AsString(py_val); - avifEncoderSetCodecSpecificOption(encoder, key, val); + + avifResult result = avifEncoderSetCodecSpecificOption(encoder, key, val); + if (result != AVIF_RESULT_OK) { + PyErr_Format( + exc_type_for_avif_result(result), + "Setting advanced codec options failed: %s", + avifResultToString(result) + ); + return 1; + } } + return 0; } // Encoder functions @@ -296,9 +309,18 @@ AvifEncoderNew(PyObject *self_, PyObject *args) { encoder->autoTiling = enc_options.autotiling; #endif + if (advanced != Py_None) { #if AVIF_VERSION >= 80200 - _add_codec_specific_options(encoder, advanced); + if (_add_codec_specific_options(encoder, advanced)) { + return NULL; + } +#else + PyErr_SetString( + PyExc_ValueError, "Advanced codec options require libavif >= 0.8.2" + ); + return NULL; #endif + } self->encoder = encoder; @@ -316,14 +338,24 @@ AvifEncoderNew(PyObject *self_, PyObject *args) { image->alphaPremultiplied = enc_options.alpha_premultiplied; #endif + avifResult result; if (PyBytes_GET_SIZE(icc_bytes)) { self->icc_bytes = icc_bytes; Py_INCREF(icc_bytes); - avifImageSetProfileICC( + + result = avifImageSetProfileICC( image, (uint8_t *)PyBytes_AS_STRING(icc_bytes), PyBytes_GET_SIZE(icc_bytes) ); + if (result != AVIF_RESULT_OK) { + PyErr_Format( + exc_type_for_avif_result(result), + "Setting ICC profile failed: %s", + avifResultToString(result) + ); + return NULL; + } } else { image->colorPrimaries = AVIF_COLOR_PRIMARIES_BT709; image->transferCharacteristics = AVIF_TRANSFER_CHARACTERISTICS_SRGB; @@ -332,20 +364,38 @@ AvifEncoderNew(PyObject *self_, PyObject *args) { if (PyBytes_GET_SIZE(exif_bytes)) { self->exif_bytes = exif_bytes; Py_INCREF(exif_bytes); - avifImageSetMetadataExif( + + result = avifImageSetMetadataExif( image, (uint8_t *)PyBytes_AS_STRING(exif_bytes), PyBytes_GET_SIZE(exif_bytes) ); + if (result != AVIF_RESULT_OK) { + PyErr_Format( + exc_type_for_avif_result(result), + "Setting EXIF data failed: %s", + avifResultToString(result) + ); + return NULL; + } } if (PyBytes_GET_SIZE(xmp_bytes)) { self->xmp_bytes = xmp_bytes; Py_INCREF(xmp_bytes); - avifImageSetMetadataXMP( + + result = avifImageSetMetadataXMP( image, (uint8_t *)PyBytes_AS_STRING(xmp_bytes), PyBytes_GET_SIZE(xmp_bytes) ); + if (result != AVIF_RESULT_OK) { + PyErr_Format( + exc_type_for_avif_result(result), + "Setting XMP data failed: %s", + avifResultToString(result) + ); + return NULL; + } } self->image = image; @@ -449,7 +499,15 @@ _encoder_add(AvifEncoderObject *self, PyObject *args) { rgb.format = AVIF_RGB_FORMAT_RGB; } - avifRGBImageAllocatePixels(&rgb); + result = avifRGBImageAllocatePixels(&rgb); + if (result != AVIF_RESULT_OK) { + PyErr_Format( + exc_type_for_avif_result(result), + "Pixel allocation failed: %s", + avifResultToString(result) + ); + return NULL; + } if (rgb.rowBytes * rgb.height != size) { PyErr_Format( @@ -599,14 +657,24 @@ AvifDecoderNew(PyObject *self_, PyObject *args) { #endif self->decoder->codecChoice = codec; - avifDecoderSetIOMemory( + result = avifDecoderSetIOMemory( self->decoder, (uint8_t *)PyBytes_AS_STRING(self->data), PyBytes_GET_SIZE(self->data) ); + if (result != AVIF_RESULT_OK) { + PyErr_Format( + exc_type_for_avif_result(result), + "Setting IO memory failed: %s", + avifResultToString(result) + ); + avifDecoderDestroy(self->decoder); + self->decoder = NULL; + Py_DECREF(self); + return NULL; + } result = avifDecoderParse(self->decoder); - if (result != AVIF_RESULT_OK) { PyErr_Format( exc_type_for_avif_result(result), @@ -697,7 +765,6 @@ _decoder_get_frame(AvifDecoderObject *self, PyObject *args) { } result = avifDecoderNthImage(decoder, frame_index); - if (result != AVIF_RESULT_OK) { PyErr_Format( exc_type_for_avif_result(result), @@ -729,7 +796,15 @@ _decoder_get_frame(AvifDecoderObject *self, PyObject *args) { return NULL; } - avifRGBImageAllocatePixels(&rgb); + result = avifRGBImageAllocatePixels(&rgb); + if (result != AVIF_RESULT_OK) { + PyErr_Format( + exc_type_for_avif_result(result), + "Pixel allocation failed: %s", + avifResultToString(result) + ); + return NULL; + } Py_BEGIN_ALLOW_THREADS result = avifImageYUVToRGB(image, &rgb); Py_END_ALLOW_THREADS From e11d8bdb5ec118b35d38653af12dde5e7bd01e35 Mon Sep 17 00:00:00 2001 From: Andrew Murray Date: Tue, 19 Nov 2024 12:03:35 +1100 Subject: [PATCH 5/5] Added stamp --- .github/workflows/wheels-dependencies.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/wheels-dependencies.sh b/.github/workflows/wheels-dependencies.sh index b1154f23bb1..021a7bac65b 100755 --- a/.github/workflows/wheels-dependencies.sh +++ b/.github/workflows/wheels-dependencies.sh @@ -134,6 +134,7 @@ EOF } function build_libavif { + if [ -e libavif-stamp ]; then return; fi install_rav1e python3 -m pip install meson ninja @@ -165,6 +166,7 @@ function build_libavif { cp /usr/local/lib64/libavif.a /usr/local/lib cp /usr/local/lib64/pkgconfig/libavif.pc /usr/local/lib/pkgconfig fi + touch libavif-stamp } function build {