`test_embedart.py`: `EmbedartCliTest.test_accept_similar_art` fails

[doronbehar]

Problem

Here on NixOS, we continuously rebuild all packages that their dependencies have been changed. Recently, beets have been failing to build because it's tests have failed:

=================================== FAILURES ===================================
___________________ EmbedartCliTest.test_accept_similar_art ____________________

args = (<test.test_embedart.EmbedartCliTest testMethod=test_accept_similar_art>,)
kwargs = {}

    def wrapper(*args, **kwargs):
        if not ArtResizer.shared.can_compare:
            raise unittest.SkipTest("compare not available")
        else:
>           return test(*args, **kwargs)

test/test_embedart.py:38: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
test/test_embedart.py:170: in test_accept_similar_art
    self.assertEqual(mediafile.images[0].data, self.image_data,
E   AssertionError: b'\xf[35 chars]01\x00\x00\x01\x00\x01\x00\x00\xff\xdb\x00C\x0[34652 chars]\xd9' != b'\xf[35 chars]01\x01\x00H\x00H\x00\x00\xff\xdb\x00C\x00\x03\[240374 chars]\xd9' : Image written is not /build/source/test/rsrc/abbey-similar.jpg

Full build log is available here:

https://hydra.nixos.org/build/226088427/nixlog/1

Setup

• OS: NixOS
• Python version: 3.10.11
• beets version: 1.6.0
• Turning off plugins made problem go away (yes/no): Irrelevant

[Scrumplex]

This was introduced by an imagemagick update (NixOS/nixpkgs#240476 https://github.com/NixOS/nixpkgs/commit/e0334495f78fd862cbb6985b25b41dd197bb462c).

git bisect log

# bad: [4bc72cae107788bf3f24f30db2e2f685c9298dc9] Merge pull request #240491 from r-ryantm/auto-update/gcompris
# good: [50cd94e6230b7b7e2b8e7ba2be17f2931a6c82e7] Merge pull request #240428 from r-ryantm/auto-update/hcloud
git bisect start 'nixos-unstable' 'nixos-unstable~100'
# good: [13ab4547dc02e63589607c5cf1d3e2b18b0f5b64] Merge pull request #240384 from hraban/sbcl/2.3.6
git bisect good 13ab4547dc02e63589607c5cf1d3e2b18b0f5b64
# good: [3dadeec9a92e005ee4d0e5bbea725c250819ccaa] Merge pull request #240487 from chvp/upd/hookshot
git bisect good 3dadeec9a92e005ee4d0e5bbea725c250819ccaa
# bad: [aecd4e5de22dedd36c7d1544affe1e4d92d9610c] Merge pull request #240461 from aaronjheng/credential-detector
git bisect bad aecd4e5de22dedd36c7d1544affe1e4d92d9610c
# bad: [2d355cacaeb36af1258554496fec354f225c15a7] Merge pull request #240218 from NixOS/pr/mplhep_init
git bisect bad 2d355cacaeb36af1258554496fec354f225c15a7
# good: [3791e8908eca998d9ccb805f9b473e740e4ab14e] Merge pull request #240482 from natsukium/fzf-fish/update
git bisect good 3791e8908eca998d9ccb805f9b473e740e4ab14e
# good: [81bdbd2525a52200af0540eb3ca01d0dd73ef120] Merge pull request #240225 from leona-ya/paperless-1-16-5
git bisect good 81bdbd2525a52200af0540eb3ca01d0dd73ef120
# good: [560a07f5070b390103fbb3eb8ae12ed1b2955bf8] python310Packages.mplhep: init at 0.3.28
git bisect good 560a07f5070b390103fbb3eb8ae12ed1b2955bf8
# bad: [0431f99716658d09864cf1b6dac7ba23a6ddfc27] Merge pull request #240476 from dotlambda/imagemagick-7.1.1-12
git bisect bad 0431f99716658d09864cf1b6dac7ba23a6ddfc27
# bad: [e0334495f78fd862cbb6985b25b41dd197bb462c] imagemagick: 7.1.1-11 -> 7.1.1-12
git bisect bad e0334495f78fd862cbb6985b25b41dd197bb462c
# first bad commit: [e0334495f78fd862cbb6985b25b41dd197bb462c] imagemagick: 7.1.1-11 -> 7.1.1-12

[sampsyo]

Thanks for the detailed report! At a glance, it seems like the problem is that ImageMagick (i.e., magick compare) used to say "yes" to our similarity query in this test and now it says "no." That's too bad, and it's a little tricky to fix without a local way to reproduce it… has anybody reading this (perhaps using the above-indicated version of ImageMagick) been able to see this happening locally?

If so, doing some debugging in the compare method in artresizer.py could confirm this hypothesis. If that's what's going on, maybe we just need to change the test file so the similarity comparison triggers correctly again.

[Scrumplex]

Yep. It looks like this is a problem with ImageMagick. I ran the comparison done by beets (convert <beets-src>/test/rsrc/abbey-similar.jpg /tmp/nix-shell.kbVep7/tmpl0ecssu_.jpg -colorspace gray MIFF:- | compare -metric PHASH - null:) with the two ImageMagick versions in question:

ImageMagick 7.1.1-12 Q16-HDRI x86_64 b3f8ed7a7:20230625 https://imagemagick.org produces 136.198

ImageMagick 7.1.1-11 Q16-HDRI x86_64 f04a7eb33:20230528 https://imagemagick.org produces 11.7978

[Scrumplex]

I have bisected this behavior in ImageMagick:

# bad: [a09d8dd5e3a92362cf70c184670b23163587e6f8] release
# good: [11ffa6eb4548644a718158daa286295ed3174054] release
git bisect start '7.1.1-12' '7.1.1-11'
# bad: [195a19168f8dfbfedc21b20a1ca3515bac96f524] latest CSS
git bisect bad 195a19168f8dfbfedc21b20a1ca3515bac96f524
# bad: [cd6b43771b82392decefecadc86a9ba6fd30cad3] reject invalid BMP image @ https://github.com/ImageMagick/ImageMagick/issues/6393
git bisect bad cd6b43771b82392decefecadc86a9ba6fd30cad3
# bad: [3f9df4fd698ca93b304dee4691d7f98e1a99ffc4] Changed options for heif build.
git bisect bad 3f9df4fd698ca93b304dee4691d7f98e1a99ffc4
# bad: [f45cb56383bda833708f08d6f8a580c833ffd1c9] default colorspaces are xyY and HSB
git bisect bad f45cb56383bda833708f08d6f8a580c833ffd1c9
# good: [3c43475bb5dc7eec9af3babc789bf8bf65677e90] beta release
git bisect good 3c43475bb5dc7eec9af3babc789bf8bf65677e90
# first bad commit: [f45cb56383bda833708f08d6f8a580c833ffd1c9] default colorspaces are xyY and HSB

The offending commit:

commit f45cb56383bda833708f08d6f8a580c833ffd1c9
Author: Cristy <urban-warrior@imagemagick.org>
Date:   Mon May 29 17:32:01 2023 -0400

    default colorspaces are xyY and HSB

diff --git a/MagickCore/statistic.c b/MagickCore/statistic.c
index 29f5e99b5..13b1592dc 100644
--- a/MagickCore/statistic.c
+++ b/MagickCore/statistic.c
@@ -1752,7 +1752,7 @@ MagickExport ChannelPerceptualHash *GetImagePerceptualHash(const Image *image,
   if (artifact != NULL)
     colorspaces=AcquireString(artifact);
   else
-    colorspaces=AcquireString("sRGB,HCLp");
+    colorspaces=AcquireString("xyY,HSB");
   perceptual_hash[0].number_colorspaces=0;
   perceptual_hash[0].number_channels=0;
   q=colorspaces;

Not sure if this is a bug or intended behavior.

[Scrumplex]

A potential workaround is adding -define phash:colorspaces=sRGB,HCLp to the compare command. This restores the previous behavior of returning 11.7978

[sampsyo]

Wow; thank you for diagnosing this! I propose we just add that flag, since I confess I don't understand the nuances that made the ImageMagick folks prefer that default, so I have no grounds to believe it will switch back.

[Scrumplex]

Great! I will prepare a PR in a moment