Reduced epsilons (#13)

* Derive dir from filename * Reduced epsilons * Simplified code * Do not shadow builtin * Test saving EXIF instance without orientation * More closely match wheels-dependencies --------- Co-authored-by: Andrew Murray <[email protected]>
fdintino · Dec 24, 2024 · 3a9a3ab · 3a9a3ab
1 parent 9b6e575
commit 3a9a3ab
Show file tree

Hide file tree

Showing 4 changed files with 68 additions and 95 deletions.
diff --git 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:
         """
@@ -329,34 +329,45 @@ 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()
         exif[274] = orientation
         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:
                 assert "exif" not in reloaded.info
             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")
             with pytest.raises(SyntaxError):
                 im.save(test_file, exif=b"invalid")
 
     @pytest.mark.parametrize(
-        "rot,mir,exif_orientation",
+        "rot, mir, exif_orientation",
         [
             (0, 0, 4),
             (0, 1, 2),
@@ -574,7 +585,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 +644,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 +660,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]

diff --git 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
diff --git 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",

diff --git 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",