ImageList.get_types_dict() keeps the smallest image per type instead of the biggest, defeating "Never replace cover images with smaller ones"

XMLWordPrintable

      The "Never replace cover images with smaller ones" option (dont_replace_with_smaller_cover) is meant to stop a downloaded cover from replacing an already-embedded one when the download is smaller. It relies on ImageList.get_types_dict() to provide, per image type, the image already embedded. The size comparison in that method is inverted, so it keeps the smallest image of each type instead of the biggest.

      bigger_previous_image_filter() compares a download against this mapping and rejects it only when it is smaller than the stored image. Backed by the smallest embedded image, a download bigger than the smallest but smaller than the biggest passes the filter and replaces the bigger one — exactly what the option promises to prevent.

      The bug only surfaces when two or more images of the same type are already embedded, which is why it went unnoticed.

      Steps to reproduce

      1. Embed two front images of different sizes in a file, e.g. 1000x1000 and 200x200.
      2. Enable "Never replace cover images with smaller ones".
      3. Load a release whose front cover download is 500x500 (bigger than 200x200, smaller than 1000x1000).

      Expected result

      The 500x500 download is rejected; the embedded 1000x1000 front image is kept.

      Actual result

      The download is compared against the 200x200 image, passes the filter, and replaces the 1000x1000 image.

      Root cause

      In ImageList.get_types_dict() (picard/util/imagelist.py), when two images share a type the comparison is:

      if image.width > previous_image.width or image.height > previous_image.height:
          continue  # skips the bigger candidate -> keeps the smallest
      

      Suggested fix

      Invert the comparison so a smaller image never displaces the one already kept:

      if image.width < previous_image.width or image.height < previous_image.height:
          continue  # keeps the biggest
      

      History

      • 4093acc0f7 "Add option to never replace an image with a smaller one" (twodoorcoupe, 2024-07-04, PICARD-1477): get_types_dict() added as a plain "last image wins" map, no size logic.
      • c96f0b07e6 "Move new option to image filters" (twodoorcoupe, 2024-07-08): added the size tie-break, inverted. The commit subject is only about relocating code and says nothing about the comparison, so this looks like an accidental inversion rather than a deliberate choice.
      • b329b212d4 "PICARD-3294: Move 'never replace' cover art filters to file assignment time" restructured the filters but left the inverted comparison untouched.

      Intended semantics ("keep the biggest") are supported by the option label, the consumer bigger_previous_image_filter(), PICARD-1477, and the GSoC 2024 proposal (goal: "discard images if too small"). None of these, nor the forum thread, discuss the multi-image-per-type tie-break.

            Assignee:
            Zas
            Reporter:
            Zas
            Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

              Created:
              Updated:

                Version Package
                3.0.0rc1