From 70b415980b7c85745e32ccf3ffbf0c1c9f4a1333 Mon Sep 17 00:00:00 2001 From: Andrew Murray Date: Wed, 18 Dec 2024 11:34:12 +1100 Subject: [PATCH 1/6] Derive dir from filename --- winbuild/build_prepare.py | 1 - 1 file changed, 1 deletion(-) diff --git a/winbuild/build_prepare.py b/winbuild/build_prepare.py index c84f1000b39..32dcdb3c86e 100644 --- a/winbuild/build_prepare.py +++ b/winbuild/build_prepare.py @@ -391,7 +391,6 @@ def cmd_msbuild( "libavif": { "url": f"https://github.com/AOMediaCodec/libavif/archive/v{V['LIBAVIF']}.zip", "filename": f"libavif-{V['LIBAVIF']}.zip", - "dir": f"libavif-{V['LIBAVIF']}", "license": "LICENSE", "build": [ f"{sys.executable} -m pip install meson", From a53840829fd1711e96260c4cfdf241ad3b366e17 Mon Sep 17 00:00:00 2001 From: Andrew Murray Date: Sun, 22 Dec 2024 04:37:10 +1100 Subject: [PATCH 2/6] Reduced epsilons --- Tests/test_file_avif.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/Tests/test_file_avif.py b/Tests/test_file_avif.py index 1bc7299b62e..aa523a8c3bb 100644 --- a/Tests/test_file_avif.py +++ b/Tests/test_file_avif.py @@ -147,7 +147,7 @@ def test_read(self) -> None: # generated with: # avifdec hopper.avif hopper_avif_write.png assert_image_similar_tofile( - image, "Tests/images/avif/hopper_avif_write.png", 12.0 + image, "Tests/images/avif/hopper_avif_write.png", 11.5 ) def _roundtrip(self, tmp_path: Path, mode: str, epsilon: float) -> None: @@ -163,7 +163,7 @@ def _roundtrip(self, tmp_path: Path, mode: str, epsilon: float) -> None: if mode == "RGB": # avifdec hopper.avif avif/hopper_avif_write.png assert_image_similar_tofile( - image, "Tests/images/avif/hopper_avif_write.png", 12.0 + image, "Tests/images/avif/hopper_avif_write.png", 6.02 ) # This test asserts that the images are similar. If the average pixel @@ -181,7 +181,7 @@ def test_write_rgb(self, tmp_path: Path) -> None: Does it have the bits we expect? """ - self._roundtrip(tmp_path, "RGB", 12.5) + self._roundtrip(tmp_path, "RGB", 8.62) def test_AvifEncoder_with_invalid_args(self) -> None: """ @@ -574,7 +574,7 @@ def test_p_mode_transparency(self) -> None: im_png.save(buf_out, format="AVIF", quality=100) with Image.open(buf_out) as expected: - assert_image_similar(im_png.convert("RGBA"), expected, 1) + assert_image_similar(im_png.convert("RGBA"), expected, 0.17) def test_decoder_strict_flags(self) -> None: # This would fail if full avif strictFlags were enabled @@ -633,10 +633,10 @@ def test_write_animation_L(self, tmp_path: Path) -> None: assert im.n_frames == orig.n_frames # Compare first and second-to-last frames to the original animated GIF - assert_image_similar(im.convert("RGB"), orig.convert("RGB"), 25.0) + assert_image_similar(im.convert("RGB"), orig.convert("RGB"), 2.25) orig.seek(orig.n_frames - 2) im.seek(im.n_frames - 2) - assert_image_similar(im.convert("RGB"), orig.convert("RGB"), 25.0) + assert_image_similar(im.convert("RGB"), orig.convert("RGB"), 2.54) def test_write_animation_RGB(self, tmp_path: Path) -> None: """ @@ -649,11 +649,11 @@ def check(temp_file: str) -> None: assert im.n_frames == 4 # Compare first frame to original - assert_image_similar(im, frame1.convert("RGBA"), 25.0) + assert_image_similar(im, frame1.convert("RGBA"), 2.7) # Compare second frame to original im.seek(1) - assert_image_similar(im, frame2.convert("RGBA"), 25.0) + assert_image_similar(im, frame2.convert("RGBA"), 4.1) with self.star_frames() as frames: frame1 = frames[0] From 5e794a4955317ed812b229e4dc3720d8b974e0cc Mon Sep 17 00:00:00 2001 From: Andrew Murray Date: Tue, 24 Dec 2024 11:24:34 +1100 Subject: [PATCH 3/6] Simplified code --- src/_avif.c | 112 +++++++++++++++++----------------------------------- 1 file changed, 37 insertions(+), 75 deletions(-) diff --git a/src/_avif.c b/src/_avif.c index 9d83ec241cd..d86ab340a42 100644 --- a/src/_avif.c +++ b/src/_avif.c @@ -78,34 +78,39 @@ exc_type_for_avif_result(avifResult result) { static uint8_t irot_imir_to_exif_orientation(const avifImage *image) { + uint8_t axis; #if AVIF_VERSION_MAJOR >= 1 - uint8_t axis = image->imir.axis; + axis = image->imir.axis; #else - uint8_t axis = image->imir.mode; + axis = image->imir.mode; #endif uint8_t angle = image->irot.angle; - int irot = !!(image->transformFlags & AVIF_TRANSFORM_IROT); - int imir = !!(image->transformFlags & AVIF_TRANSFORM_IMIR); - if (irot && angle == 1) { - if (imir) { - return axis ? 7 // 90 degrees anti-clockwise then swap left and right. - : 5; // 90 degrees anti-clockwise then swap top and bottom. + int imir = image->transformFlags & AVIF_TRANSFORM_IMIR; + int irot = image->transformFlags & AVIF_TRANSFORM_IROT; + if (irot) { + if (angle == 1) { + if (imir) { + return axis ? 7 // 90 degrees anti-clockwise then swap left and right. + : 5; // 90 degrees anti-clockwise then swap top and bottom. + } + return 6; // 90 degrees anti-clockwise. } - return 6; // 90 degrees anti-clockwise. - } - if (irot && angle == 2) { - if (imir) { - return axis ? 4 // 180 degrees anti-clockwise then swap left and right. - : 2; // 180 degrees anti-clockwise then swap top and bottom. + if (angle == 2) { + if (imir) { + return axis + ? 4 // 180 degrees anti-clockwise then swap left and right. + : 2; // 180 degrees anti-clockwise then swap top and bottom. + } + return 3; // 180 degrees anti-clockwise. } - return 3; // 180 degrees anti-clockwise. - } - if (irot && angle == 3) { - if (imir) { - return axis ? 5 // 270 degrees anti-clockwise then swap left and right. - : 7; // 270 degrees anti-clockwise then swap top and bottom. + if (angle == 3) { + if (imir) { + return axis + ? 5 // 270 degrees anti-clockwise then swap left and right. + : 7; // 270 degrees anti-clockwise then swap top and bottom. + } + return 8; // 270 degrees anti-clockwise. } - return 8; // 270 degrees anti-clockwise. } if (imir) { return axis ? 2 // Swap left and right. @@ -116,18 +121,13 @@ irot_imir_to_exif_orientation(const avifImage *image) { 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 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 + image->transformFlags |= AVIF_TRANSFORM_IMIR; #if AVIF_VERSION_MAJOR >= 1 image->imir.axis = 1; #else @@ -136,67 +136,34 @@ exif_orientation_to_irot_imir(avifImage *image, int orientation) { break; 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->transformFlags |= AVIF_TRANSFORM_IROT; image->irot.angle = 2; -#if AVIF_VERSION_MAJOR >= 1 - image->imir.axis = 0; // ignored -#else - image->imir.mode = 0; // ignored -#endif break; 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 + image->transformFlags |= AVIF_TRANSFORM_IMIR; break; 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->transformFlags |= 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 break; 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->transformFlags |= AVIF_TRANSFORM_IROT; image->irot.angle = 3; -#if AVIF_VERSION_MAJOR >= 1 - image->imir.axis = 0; // ignored -#else - image->imir.mode = 0; // ignored -#endif break; 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->transformFlags |= 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 break; 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->transformFlags |= AVIF_TRANSFORM_IROT; image->irot.angle = 1; -#if AVIF_VERSION_MAJOR >= 1 - image->imir.axis = 0; // ignored -#else - image->imir.mode = 0; // ignored -#endif break; } } @@ -381,13 +348,8 @@ AvifEncoderNew(PyObject *self_, PyObject *args) { enc_options.tile_rows_log2 = normalize_tiles_log2(tile_rows_log2); enc_options.tile_cols_log2 = normalize_tiles_log2(tile_cols_log2); - - if (alpha_premultiplied == Py_True) { - enc_options.alpha_premultiplied = AVIF_TRUE; - } else { - enc_options.alpha_premultiplied = AVIF_FALSE; - } - + enc_options.alpha_premultiplied = + (alpha_premultiplied == Py_True) ? AVIF_TRUE : AVIF_FALSE; enc_options.autotiling = (autotiling == Py_True) ? AVIF_TRUE : AVIF_FALSE; // Create a new animation encoder and picture frame @@ -573,9 +535,9 @@ _encoder_add(AvifEncoderObject *self, PyObject *args) { return NULL; } - is_first_frame = (self->frame_index == -1); + is_first_frame = self->frame_index == -1; - if ((image->width != width) || (image->height != height)) { + if (image->width != width || image->height != height) { PyErr_Format( PyExc_ValueError, "Image sequence dimensions mismatch, %ux%u != %ux%u", From 5b5d344a08b71f7db7be77d3bd91cd6f75b40080 Mon Sep 17 00:00:00 2001 From: Andrew Murray Date: Tue, 24 Dec 2024 11:58:34 +1100 Subject: [PATCH 4/6] Do not shadow builtin --- Tests/test_file_avif.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Tests/test_file_avif.py b/Tests/test_file_avif.py index aa523a8c3bb..530305a978f 100644 --- a/Tests/test_file_avif.py +++ b/Tests/test_file_avif.py @@ -329,11 +329,11 @@ def test_exif(self) -> None: exif = im.getexif() assert exif[274] == 3 - @pytest.mark.parametrize("bytes,orientation", [(True, 1), (False, 2)]) + @pytest.mark.parametrize("use_bytes, orientation", [(True, 1), (False, 2)]) def test_exif_save( self, tmp_path: Path, - bytes: bool, + use_bytes: bool, orientation: int, ) -> None: exif = Image.Exif() @@ -341,7 +341,7 @@ def test_exif_save( exif_data = exif.tobytes() with Image.open(TEST_AVIF_FILE) as im: test_file = str(tmp_path / "temp.avif") - im.save(test_file, exif=exif_data if bytes else exif) + im.save(test_file, exif=exif_data if use_bytes else exif) with Image.open(test_file) as reloaded: if orientation == 1: @@ -356,7 +356,7 @@ def test_exif_invalid(self, tmp_path: Path) -> None: im.save(test_file, exif=b"invalid") @pytest.mark.parametrize( - "rot,mir,exif_orientation", + "rot, mir, exif_orientation", [ (0, 0, 4), (0, 1, 2), From aa5b7d5fac3251d8fc99840bf9522532a9a2620b Mon Sep 17 00:00:00 2001 From: Andrew Murray Date: Tue, 24 Dec 2024 12:05:02 +1100 Subject: [PATCH 5/6] Test saving EXIF instance without orientation --- Tests/test_file_avif.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/Tests/test_file_avif.py b/Tests/test_file_avif.py index 530305a978f..7a92f781a9c 100644 --- a/Tests/test_file_avif.py +++ b/Tests/test_file_avif.py @@ -349,6 +349,17 @@ def test_exif_save( else: assert reloaded.info["exif"] == exif_data + def test_exif_without_orientation(self, tmp_path: Path): + exif = Image.Exif() + exif[272] = b"test" + exif_data = exif.tobytes() + with Image.open(TEST_AVIF_FILE) as im: + test_file = str(tmp_path / "temp.avif") + im.save(test_file, exif=exif) + + with Image.open(test_file) as reloaded: + assert reloaded.info["exif"] == exif_data + def test_exif_invalid(self, tmp_path: Path) -> None: with Image.open(TEST_AVIF_FILE) as im: test_file = str(tmp_path / "temp.avif") From 51db808dc8832727f7c5e738c3516859a056cb91 Mon Sep 17 00:00:00 2001 From: Andrew Murray Date: Tue, 24 Dec 2024 12:23:49 +1100 Subject: [PATCH 6/6] More closely match wheels-dependencies --- depends/install_libavif.sh | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/depends/install_libavif.sh b/depends/install_libavif.sh index 29796a74dcc..ef4d30cbb39 100755 --- a/depends/install_libavif.sh +++ b/depends/install_libavif.sh @@ -7,7 +7,7 @@ version=1.1.1 pushd libavif-$version -if uname -s | grep -q Darwin; then +if [ $(uname) == "Darwin" ]; then PREFIX=$(brew --prefix) else PREFIX=/usr @@ -49,15 +49,16 @@ if [ "$HAS_ENCODER" != 1 ] || [ "$HAS_DECODER" != 1 ]; then LIBAVIF_CMAKE_FLAGS+=(-DAVIF_CODEC_AOM=LOCAL) fi -cmake -G Ninja -S . -B build \ +cmake \ -DCMAKE_INSTALL_PREFIX=$PREFIX \ - -DAVIF_LIBSHARPYUV=LOCAL \ - -DAVIF_LIBYUV=LOCAL \ - -DCMAKE_BUILD_TYPE=Release \ -DCMAKE_INSTALL_NAME_DIR=$PREFIX/lib \ + -DCMAKE_BUILD_TYPE=Release \ -DCMAKE_MACOSX_RPATH=OFF \ - "${LIBAVIF_CMAKE_FLAGS[@]}" + -DAVIF_LIBSHARPYUV=LOCAL \ + -DAVIF_LIBYUV=LOCAL \ + "${LIBAVIF_CMAKE_FLAGS[@]}" \ + . -sudo ninja -C build install +sudo make install popd