From c40bcbfc874242a2d7f5463cf9a13684a68e3f9e Mon Sep 17 00:00:00 2001 From: Andrew Murray Date: Tue, 3 Dec 2024 10:31:57 +1100 Subject: [PATCH] Improved error handling --- .ci/install.sh | 3 +- .github/workflows/wheels-dependencies.sh | 15 +-- Tests/check_wheel.py | 2 +- Tests/test_file_avif.py | 2 - docs/handbook/image-file-formats.rst | 9 +- docs/installation/building-from-source.rst | 9 +- src/PIL/AvifImagePlugin.py | 38 +++--- src/_avif.c | 147 +++++++++++++-------- winbuild/build_prepare.py | 46 ++++--- 9 files changed, 155 insertions(+), 116 deletions(-) diff --git a/.ci/install.sh b/.ci/install.sh index f24768d78..b24744049 100755 --- a/.ci/install.sh +++ b/.ci/install.sh @@ -23,8 +23,7 @@ if [[ $(uname) != CYGWIN* ]]; then sudo apt-get -qq install libfreetype6-dev liblcms2-dev python3-tk\ ghostscript libjpeg-turbo-progs libopenjp2-7-dev\ cmake meson imagemagick libharfbuzz-dev libfribidi-dev\ - sway wl-clipboard libopenblas-dev\ - ninja-build build-essential nasm + sway wl-clipboard libopenblas-dev nasm fi python3 -m pip install --upgrade pip diff --git a/.github/workflows/wheels-dependencies.sh b/.github/workflows/wheels-dependencies.sh index 956793b58..69a2e3c34 100755 --- a/.github/workflows/wheels-dependencies.sh +++ b/.github/workflows/wheels-dependencies.sh @@ -114,7 +114,7 @@ function install_rav1e { curl -sLo - \ https://github.com/xiph/rav1e/releases/download/v$RAV1E_VERSION/librav1e-$RAV1E_VERSION-$suffix.tar.gz \ - | tar -C $BUILD_PREFIX --exclude LICENSE --exclude LICENSE --exclude '*.so' --exclude '*.dylib' -zxf - + | tar -C $BUILD_PREFIX --exclude LICENSE --exclude '*.so' --exclude '*.dylib' -zxf - if [ -z "$IS_MACOS" ]; then sed -i 's/-lgcc_s/-lgcc_eh/g' "${BUILD_PREFIX}/lib/pkgconfig/rav1e.pc" @@ -133,6 +133,7 @@ EOF } function build_libavif { + if [ -e libavif-stamp ]; then return; fi install_rav1e python3 -m pip install meson ninja @@ -140,13 +141,11 @@ function build_libavif { build_simple nasm 2.16.03 https://www.nasm.us/pub/nasm/releasebuilds/2.16.03/ fi - local cmake=$(get_modern_cmake) local out_dir=$(fetch_unpack https://github.com/AOMediaCodec/libavif/archive/refs/tags/v$LIBAVIF_VERSION.tar.gz libavif-$LIBAVIF_VERSION.tar.gz) - (cd $out_dir \ - && $cmake \ + && cmake \ -DCMAKE_INSTALL_PREFIX=$BUILD_PREFIX \ - -DCMAKE_INSTALL_NAME_DIR=$BUILD_PREFIX/lib \ + -DCMAKE_INSTALL_LIBDIR=$BUILD_PREFIX/lib \ -DCMAKE_BUILD_TYPE=Release \ -DBUILD_SHARED_LIBS=OFF \ -DAVIF_LIBSHARPYUV=LOCAL \ @@ -159,11 +158,7 @@ function build_libavif { -DCMAKE_MODULE_PATH=/tmp/cmake/Modules \ . \ && make install) - - if [[ "$MB_ML_LIBC" == "manylinux" ]]; then - 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 { diff --git a/Tests/check_wheel.py b/Tests/check_wheel.py index 002dccde6..4c3f634a6 100644 --- a/Tests/check_wheel.py +++ b/Tests/check_wheel.py @@ -18,7 +18,7 @@ def test_wheel_modules() -> None: except ImportError: expected_modules.remove("tkinter") - # libavif is not available on windows for x86 and ARM64 architectures + # libavif is not available on Windows for x86 and ARM64 architectures if sys.platform == "win32": if platform.machine() == "ARM64" or struct.calcsize("P") == 4: expected_modules.remove("avif") diff --git a/Tests/test_file_avif.py b/Tests/test_file_avif.py index 32dd694f5..9a3bb6b73 100644 --- a/Tests/test_file_avif.py +++ b/Tests/test_file_avif.py @@ -127,8 +127,6 @@ class TestUnsupportedAvif: @skip_unless_feature("avif") class TestFileAvif: def test_version(self) -> None: - _avif.AvifCodecVersions() - version = features.version_module("avif") assert version is not None assert re.search(r"\d+\.\d+\.\d+$", version) diff --git a/docs/handbook/image-file-formats.rst b/docs/handbook/image-file-formats.rst index 0660c8e82..689990840 100644 --- a/docs/handbook/image-file-formats.rst +++ b/docs/handbook/image-file-formats.rst @@ -1370,17 +1370,16 @@ The :py:meth:`~PIL.Image.Image.save` method supports the following options: YUV range, either "full" or "limited". Defaults to "full" **codec** - AV1 codec to use for encoding. Possible values are "aom", "rav1e", and - "svt", depending on what codecs were compiled with libavif. Defaults to - "auto", which will choose the first available codec in the order of the - preceding list. + AV1 codec to use for encoding. Specific values are "aom", "rav1e", and + "svt", presuming the chosen codec is available. Defaults to "auto", which + will choose the first available codec in the order of the preceding list. **tile_rows** / **tile_cols** For tile encoding, the (log 2) number of tile rows and columns to use. Valid values are 0-6, default 0. **alpha_premultiplied** - Encode the image with premultiplied alpha, defaults ``False`` + Encode the image with premultiplied alpha. Defaults to ``False`` **icc_profile** The ICC Profile to include in the saved file. diff --git a/docs/installation/building-from-source.rst b/docs/installation/building-from-source.rst index 7020f3457..1447b049e 100644 --- a/docs/installation/building-from-source.rst +++ b/docs/installation/building-from-source.rst @@ -164,9 +164,11 @@ Many of Pillow's features require external libraries: The easiest way to install external libraries is via `Homebrew `_. After you install Homebrew, run:: - brew install libjpeg libraqm libtiff little-cms2 openjpeg webp + brew install libavif libjpeg libraqm libtiff little-cms2 openjpeg webp - To install libavif on macOS use Homebrew to install its build dependencies:: + If you would like to use libavif with more codecs than just aom, then + instead of installing libavif through Homebrew directly, you can use + Homebrew to install libavif's build dependencies:: brew install aom dav1d rav1e @@ -224,8 +226,7 @@ Many of Pillow's features require external libraries: sudo pkg install jpeg-turbo tiff webp lcms2 freetype2 openjpeg harfbuzz fribidi libxcb libavif - See ``depends/install_raqm_cmake.sh`` to install libraqm and - ``depends/install_libavif.sh`` to install libavif. + See ``depends/install_raqm_cmake.sh`` to install libraqm. .. tab:: Android diff --git a/src/PIL/AvifImagePlugin.py b/src/PIL/AvifImagePlugin.py index 1c90f428a..c92e2534e 100644 --- a/src/PIL/AvifImagePlugin.py +++ b/src/PIL/AvifImagePlugin.py @@ -24,17 +24,11 @@ _VALID_AVIF_MODES = {"RGB", "RGBA"} 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 @@ -72,6 +74,11 @@ class AvifImageFile(ImageFile.ImageFile): ) raise SyntaxError(msg) + if DECODE_CODEC_CHOICE != "auto" and not _avif.decoder_codec_available( + DECODE_CODEC_CHOICE + ): + msg = "Invalid opening codec" + raise ValueError(msg) self._decoder = _avif.AvifDecoder( self.fp.read(), DECODE_CODEC_CHOICE, @@ -104,10 +111,8 @@ class AvifImageFile(ImageFile.ImageFile): 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 @@ -153,6 +158,9 @@ def _save( speed = info.get("speed", 6) max_threads = info.get("max_threads", _get_default_max_threads()) codec = info.get("codec", "auto") + if codec != "auto" and not _avif.encoder_codec_available(codec): + msg = "Invalid saving codec" + raise ValueError(msg) range_ = info.get("range", "full") tile_rows_log2 = info.get("tile_rows", 0) tile_cols_log2 = info.get("tile_cols", 0) @@ -199,7 +207,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 diff --git a/src/_avif.c b/src/_avif.c index d0bb81f46..1d4bb7401 100644 --- a/src/_avif.c +++ b/src/_avif.c @@ -218,30 +218,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 @@ -336,17 +349,6 @@ AvifEncoderNew(PyObject *self_, PyObject *args) { enc_options.codec = AVIF_CODEC_CHOICE_AUTO; } else { enc_options.codec = avifCodecChoiceFromName(codec); - if (enc_options.codec == AVIF_CODEC_CHOICE_AUTO) { - PyErr_Format(PyExc_ValueError, "Invalid codec: %s", codec); - return NULL; - } else { - const char *codec_name = - avifCodecName(enc_options.codec, AVIF_CODEC_FLAG_CAN_ENCODE); - if (codec_name == NULL) { - PyErr_Format(PyExc_ValueError, "AV1 Codec cannot encode: %s", codec); - return NULL; - } - } } if (strcmp(range, "full") == 0) { @@ -410,9 +412,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; @@ -430,14 +441,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; @@ -446,20 +467,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; + } } exif_orientation_to_irot_imir(image, exif_orientation); @@ -498,7 +537,6 @@ _encoder_add(AvifEncoderObject *self, PyObject *args) { PyObject *ret = Py_None; int is_first_frame; - int channels; avifRGBImage rgb; avifResult result; @@ -561,13 +599,19 @@ _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); + 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( @@ -679,18 +723,6 @@ AvifDecoderNew(PyObject *self_, PyObject *args) { codec = AVIF_CODEC_CHOICE_AUTO; } else { codec = avifCodecChoiceFromName(codec_str); - if (codec == AVIF_CODEC_CHOICE_AUTO) { - PyErr_Format(PyExc_ValueError, "Invalid codec: %s", codec_str); - return NULL; - } else { - const char *codec_name = avifCodecName(codec, AVIF_CODEC_FLAG_CAN_DECODE); - if (codec_name == NULL) { - PyErr_Format( - PyExc_ValueError, "AV1 Codec cannot decode: %s", codec_str - ); - return NULL; - } - } } self = PyObject_New(AvifDecoderObject, &AvifDecoder_Type); @@ -717,14 +749,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), @@ -815,7 +857,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), @@ -847,7 +888,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 @@ -893,10 +942,7 @@ static struct PyMethodDef _encoder_methods[] = { // AvifDecoder type definition static PyTypeObject AvifEncoder_Type = { - // clang-format off - PyVarObject_HEAD_INIT(NULL, 0) - .tp_name = "AvifEncoder", - // clang-format on + PyVarObject_HEAD_INIT(NULL, 0).tp_name = "AvifEncoder", .tp_basicsize = sizeof(AvifEncoderObject), .tp_dealloc = (destructor)_encoder_dealloc, .tp_flags = Py_TPFLAGS_DEFAULT, @@ -912,10 +958,7 @@ static struct PyMethodDef _decoder_methods[] = { // AvifDecoder type definition static PyTypeObject AvifDecoder_Type = { - // clang-format off - PyVarObject_HEAD_INIT(NULL, 0) - .tp_name = "AvifDecoder", - // clang-format on + PyVarObject_HEAD_INIT(NULL, 0).tp_name = "AvifDecoder", .tp_basicsize = sizeof(AvifDecoderObject), .tp_itemsize = 0, .tp_dealloc = (destructor)_decoder_dealloc, @@ -923,13 +966,6 @@ static PyTypeObject AvifDecoder_Type = { .tp_methods = _decoder_methods, }; -PyObject * -AvifCodecVersions() { - char codecVersions[256]; - avifCodecVersions(codecVersions); - return PyUnicode_FromString(codecVersions); -} - /* -------------------------------------------------------------------- */ /* Module Setup */ /* -------------------------------------------------------------------- */ @@ -937,7 +973,6 @@ AvifCodecVersions() { static PyMethodDef avifMethods[] = { {"AvifDecoder", AvifDecoderNew, METH_VARARGS}, {"AvifEncoder", AvifEncoderNew, METH_VARARGS}, - {"AvifCodecVersions", AvifCodecVersions, METH_NOARGS}, {"decoder_codec_available", _decoder_codec_available, METH_VARARGS}, {"encoder_codec_available", _encoder_codec_available, METH_VARARGS}, {NULL, NULL} diff --git a/winbuild/build_prepare.py b/winbuild/build_prepare.py index 8ee9cd90e..4ebf1f042 100644 --- a/winbuild/build_prepare.py +++ b/winbuild/build_prepare.py @@ -122,7 +122,7 @@ V = { "TIFF": "4.6.0", "XZ": "5.6.3", "ZLIB": "1.3.1", - "MESON": "1.5.2", + "MESON": "1.6.0", "LIBAVIF": "1.1.1", "RAV1E": "0.7.1", } @@ -673,21 +673,24 @@ def build_dep(name: str, prefs: dict[str, str], verbose: bool) -> str: def build_dep_all(disabled: list[str], prefs: dict[str, str], verbose: bool) -> None: lines = [r'call "{build_dir}\build_env.cmd"'] gha_groups = "GITHUB_ACTIONS" in os.environ - scripts = ["install_meson.cmd"] for dep_name in DEPS: print() if dep_name in disabled: print(f"Skipping disabled dependency {dep_name}") continue + + scripts = [] + if dep_name == "libavif": + scripts.append("install_meson.cmd") scripts.append(build_dep(dep_name, prefs, verbose)) - for script in scripts: - if gha_groups: - lines.append(f"@echo ::group::Running {script}") - lines.append(rf'cmd.exe /c "{{build_dir}}\{script}"') - lines.append("if errorlevel 1 echo Build failed! && exit /B 1") - if gha_groups: - lines.append("@echo ::endgroup::") + for script in scripts: + if gha_groups: + lines.append(f"@echo ::group::Running {script}") + lines.append(rf'cmd.exe /c "{{build_dir}}\{script}"') + lines.append("if errorlevel 1 echo Build failed! && exit /B 1") + if gha_groups: + lines.append("@echo ::endgroup::") print() lines.append("@echo All Pillow dependencies built successfully!") write_script("build_dep_all.cmd", lines, prefs, verbose) @@ -830,18 +833,19 @@ def main() -> None: print() write_script(".gitignore", ["*"], prefs, args.verbose) - write_script( - "install_meson.cmd", - [ - r'call "{build_dir}\build_env.cmd"', - "@echo " + ("=" * 70), - f"@echo ==== {'Building meson':<60} ====", - "@echo " + ("=" * 70), - f"{sys.executable} -m pip install meson=={V['MESON']}", - ], - prefs, - args.verbose, - ) + if "libavif" not in disabled: + write_script( + "install_meson.cmd", + [ + r'call "{build_dir}\build_env.cmd"', + "@echo " + ("=" * 70), + f"@echo ==== {'Building meson':<60} ====", + "@echo " + ("=" * 70), + f"{sys.executable} -m pip install meson=={V['MESON']}", + ], + prefs, + args.verbose, + ) build_env(prefs, args.verbose) build_dep_all(disabled, prefs, args.verbose)