From 4093acc0f76fe77c014822de8979285f9383b5a4 Mon Sep 17 00:00:00 2001 From: twodoorcoupe Date: Thu, 4 Jul 2024 12:09:49 +0200 Subject: [PATCH 1/6] Add option to never replace an image with a smaller one --- picard/coverart/__init__.py | 35 +++++++++++++++++++++-------- picard/options.py | 1 + picard/ui/forms/ui_options_cover.py | 4 ++++ picard/ui/options/cover.py | 3 +++ picard/util/imagelist.py | 6 +++++ ui/options_cover.ui | 13 +++++++---- 6 files changed, 49 insertions(+), 13 deletions(-) diff --git a/picard/coverart/__init__.py b/picard/coverart/__init__.py index 04843ec02..2cf5f009b 100644 --- a/picard/coverart/__init__.py +++ b/picard/coverart/__init__.py @@ -49,6 +49,7 @@ from picard.extension_points.cover_art_processors import ( ) from picard.extension_points.metadata import register_album_metadata_processor from picard.i18n import N_ +from picard.util import imageinfo class CoverArt: @@ -59,6 +60,7 @@ class CoverArt: self.metadata = metadata self.release = release # not used in this class, but used by providers self.front_image_found = False + self.previous_images = album.orig_metadata.images.get_types_dict() def __repr__(self): return "%s for %r" % (self.__class__.__name__, self.album) @@ -99,6 +101,23 @@ class CoverArt: except (CoverArtImageIdentificationError, CoverArtProcessingError) as e: self.album.error_append(e) + def _should_save_image(self, coverartimage, data, info): + log.warning("image save check") + config = get_config() + if config.setting['dont_replace_with_smaller_cover']: + downloaded_types = coverartimage.normalized_types() + log.warning(downloaded_types) + log.warning(self.previous_images.keys()) + if downloaded_types in self.previous_images: + previous_image = self.previous_images[downloaded_types] + log.warning(previous_image) + if info.width < previous_image.width or info.height < previous_image.height: + log.debug("Discarding cover art. A bigger image with the same types is already embedded.") + return False + if coverartimage.can_be_filtered: + return run_image_filters(data) + return True + def _coverart_downloaded(self, coverartimage, data, http, error): """Handle finished download, save it to metadata""" self.album._requests -= 1 @@ -117,16 +136,14 @@ class CoverArt: }, echo=None ) - filters_result = True - if coverartimage.can_be_filtered: - filters_result = run_image_filters(data) - if filters_result: - try: + try: + info = imageinfo.identify(data) + if self._should_save_image(coverartimage, data, info): self._set_metadata(coverartimage, data) - except CoverArtImageIOError: - # It doesn't make sense to store/download more images if we can't - # save them in the temporary folder, abort. - return + except (CoverArtImageIOError, imageinfo.IdentificationError): + # It doesn't make sense to store/download more images if we can't + # save them in the temporary folder, abort. + return self.next_in_queue() diff --git a/picard/options.py b/picard/options.py index fe9b3a079..b10dcd438 100644 --- a/picard/options.py +++ b/picard/options.py @@ -166,6 +166,7 @@ TextOption('setting', 'cd_lookup_device', ','.join(DEFAULT_DRIVES)) ListOption('setting', 'ca_providers', DEFAULT_CA_PROVIDERS, title=N_("Cover art providers")) TextOption('setting', 'cover_image_filename', DEFAULT_COVER_IMAGE_FILENAME, title=N_("File name for images")) BoolOption('setting', 'embed_only_one_front_image', True, title=N_("Embed only a single front image")) +BoolOption('setting', 'dont_replace_with_smaller_cover', False, title=N_("Never replace front images with smaller ones")) BoolOption('setting', 'image_type_as_filename', False, title=N_("Always use the primary image type as the file name for non-front images")) BoolOption('setting', 'save_images_overwrite', False, title=N_("Overwrite existing image files")) BoolOption('setting', 'save_images_to_files', False, title=N_("Save cover images as separate files")) diff --git a/picard/ui/forms/ui_options_cover.py b/picard/ui/forms/ui_options_cover.py index 6a5742895..9eccc49a7 100644 --- a/picard/ui/forms/ui_options_cover.py +++ b/picard/ui/forms/ui_options_cover.py @@ -31,6 +31,9 @@ class Ui_CoverOptionsPage(object): self.cb_embed_front_only = QtWidgets.QCheckBox(parent=self.save_images_to_tags) self.cb_embed_front_only.setObjectName("cb_embed_front_only") self.vboxlayout.addWidget(self.cb_embed_front_only) + self.cb_dont_replace_with_smaller = QtWidgets.QCheckBox(parent=self.save_images_to_tags) + self.cb_dont_replace_with_smaller.setObjectName("cb_dont_replace_with_smaller") + self.vboxlayout.addWidget(self.cb_dont_replace_with_smaller) self.verticalLayout.addWidget(self.save_images_to_tags) self.save_images_to_files = QtWidgets.QGroupBox(parent=CoverOptionsPage) self.save_images_to_files.setCheckable(True) @@ -100,6 +103,7 @@ class Ui_CoverOptionsPage(object): def retranslateUi(self, CoverOptionsPage): self.save_images_to_tags.setTitle(_("Embed cover images into tags")) self.cb_embed_front_only.setText(_("Embed only a single front image")) + self.cb_dont_replace_with_smaller.setText(_("Never replace cover images with smaller ones")) self.save_images_to_files.setTitle(_("Save cover images as separate files")) self.label_use_filename.setText(_("Use the following file name for images:")) self.save_images_overwrite.setText(_("Overwrite the file if it already exists")) diff --git a/picard/ui/options/cover.py b/picard/ui/options/cover.py index 8009bfe8c..374368bf5 100644 --- a/picard/ui/options/cover.py +++ b/picard/ui/options/cover.py @@ -67,6 +67,7 @@ class CoverOptionsPage(OptionsPage): self.register_setting('save_images_to_tags', ['save_images_to_tags']) self.register_setting('embed_only_one_front_image', ['cb_embed_front_only']) + self.register_setting('dont_replace_with_smaller_cover', ['dont_replace_with_smaller_cover']) self.register_setting('save_images_to_files', ['save_images_to_files']) self.register_setting('cover_image_filename', ['cover_image_filename']) self.register_setting('save_images_overwrite', ['save_images_overwrite']) @@ -92,6 +93,7 @@ class CoverOptionsPage(OptionsPage): config = get_config() self.ui.save_images_to_tags.setChecked(config.setting['save_images_to_tags']) self.ui.cb_embed_front_only.setChecked(config.setting['embed_only_one_front_image']) + self.ui.cb_dont_replace_with_smaller.setChecked(config.setting['dont_replace_with_smaller_cover']) self.ui.save_images_to_files.setChecked(config.setting['save_images_to_files']) self.ui.cover_image_filename.setText(config.setting['cover_image_filename']) self.ui.save_images_overwrite.setChecked(config.setting['save_images_overwrite']) @@ -109,6 +111,7 @@ class CoverOptionsPage(OptionsPage): config = get_config() config.setting['save_images_to_tags'] = self.ui.save_images_to_tags.isChecked() config.setting['embed_only_one_front_image'] = self.ui.cb_embed_front_only.isChecked() + config.setting['dont_replace_with_smaller_cover'] = self.ui.cb_dont_replace_with_smaller.isChecked() config.setting['save_images_to_files'] = self.ui.save_images_to_files.isChecked() config.setting['cover_image_filename'] = self.ui.cover_image_filename.text() config.setting['save_images_overwrite'] = self.ui.save_images_overwrite.isChecked() diff --git a/picard/util/imagelist.py b/picard/util/imagelist.py index 0ed70c40d..dd186312b 100644 --- a/picard/util/imagelist.py +++ b/picard/util/imagelist.py @@ -101,3 +101,9 @@ class ImageList(MutableSequence): self._hash_dict = {img.datahash.hash(): img for img in self._images} self._dirty = False return self._hash_dict + + def get_types_dict(self): + types_dict = dict() + for image in self._images: + types_dict[image.normalized_types()] = image + return types_dict diff --git a/ui/options_cover.ui b/ui/options_cover.ui index 1af325c11..a61ea60b4 100644 --- a/ui/options_cover.ui +++ b/ui/options_cover.ui @@ -45,6 +45,13 @@ + + + + Never replace cover images with smaller ones + + + @@ -130,8 +137,7 @@ - - .. + Qt::ToolButtonIconOnly @@ -150,8 +156,7 @@ - - .. + Qt::ToolButtonIconOnly From e8e3bc7188a2b4b12e3cabe512ea79ddbe223371 Mon Sep 17 00:00:00 2001 From: twodoorcoupe Date: Thu, 4 Jul 2024 12:31:15 +0200 Subject: [PATCH 2/6] Make filters use ImageInfo as well --- picard/coverart/__init__.py | 12 ++++------ picard/coverart/processing/__init__.py | 8 +++---- picard/coverart/processing/filters.py | 7 ++---- test/test_coverart_processing.py | 31 +++++++++++++++----------- 4 files changed, 28 insertions(+), 30 deletions(-) diff --git a/picard/coverart/__init__.py b/picard/coverart/__init__.py index 2cf5f009b..e3b4efe6a 100644 --- a/picard/coverart/__init__.py +++ b/picard/coverart/__init__.py @@ -74,10 +74,10 @@ class CoverArt: else: log.debug("Cover art disabled by user options.") - def _set_metadata(self, coverartimage, data): + def _set_metadata(self, coverartimage, data, info): try: if coverartimage.can_be_processed: - run_image_processors(data, coverartimage) + run_image_processors(coverartimage, data, info) else: coverartimage.set_tags_data(data) if coverartimage.can_be_saved_to_metadata: @@ -102,20 +102,16 @@ class CoverArt: self.album.error_append(e) def _should_save_image(self, coverartimage, data, info): - log.warning("image save check") config = get_config() if config.setting['dont_replace_with_smaller_cover']: downloaded_types = coverartimage.normalized_types() - log.warning(downloaded_types) - log.warning(self.previous_images.keys()) if downloaded_types in self.previous_images: previous_image = self.previous_images[downloaded_types] - log.warning(previous_image) if info.width < previous_image.width or info.height < previous_image.height: log.debug("Discarding cover art. A bigger image with the same types is already embedded.") return False if coverartimage.can_be_filtered: - return run_image_filters(data) + return run_image_filters(data, info) return True def _coverart_downloaded(self, coverartimage, data, http, error): @@ -139,7 +135,7 @@ class CoverArt: try: info = imageinfo.identify(data) if self._should_save_image(coverartimage, data, info): - self._set_metadata(coverartimage, data) + self._set_metadata(coverartimage, data, info) except (CoverArtImageIOError, imageinfo.IdentificationError): # It doesn't make sense to store/download more images if we can't # save them in the temporary folder, abort. diff --git a/picard/coverart/processing/__init__.py b/picard/coverart/processing/__init__.py index 5085c7684..85f15f451 100644 --- a/picard/coverart/processing/__init__.py +++ b/picard/coverart/processing/__init__.py @@ -39,9 +39,9 @@ from picard.extension_points.cover_art_processors import ( from picard.util.imageinfo import IdentificationError -def run_image_filters(data): +def run_image_filters(data, info): for f in ext_point_cover_art_filters: - if not f(data): + if not f(data, info): return False return True @@ -53,13 +53,13 @@ def run_image_metadata_filters(metadata): return True -def run_image_processors(data, coverartimage): +def run_image_processors(coverartimage, data, info): config = get_config() tags_data = data file_data = data try: start_time = time.time() - image = ProcessingImage(data) + image = ProcessingImage(data, info) both_queue, tags_queue, file_queue = get_cover_art_processors() for processor in both_queue: processor.run(image, ProcessingTarget.BOTH) diff --git a/picard/coverart/processing/filters.py b/picard/coverart/processing/filters.py index 8961c126a..f6df3c056 100644 --- a/picard/coverart/processing/filters.py +++ b/picard/coverart/processing/filters.py @@ -18,8 +18,6 @@ # along with this program; if not, write to the Free Software # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. -from PyQt6.QtGui import QImage - from picard import log from picard.config import get_config from picard.extension_points.cover_art_filters import ( @@ -47,9 +45,8 @@ def _check_threshold_size(width, height): return True -def size_filter(data): - image = QImage.fromData(data) - return _check_threshold_size(image.width(), image.height()) +def size_filter(data, info): + return _check_threshold_size(info.width, info.height) def size_metadata_filter(metadata): diff --git a/test/test_coverart_processing.py b/test/test_coverart_processing.py index 0efd50fd1..7dac01bad 100644 --- a/test/test_coverart_processing.py +++ b/test/test_coverart_processing.py @@ -50,7 +50,12 @@ def create_fake_image(width, height, image_format): image = QImage(width, height, QImage.Format.Format_ARGB32) image.save(buffer, image_format) buffer.close() - return buffer.data() + data = buffer.data() + try: + info = imageinfo.identify(data) + except imageinfo.IdentificationError: + info = None + return data, info class ImageFiltersTest(PicardTestCase): @@ -64,12 +69,12 @@ class ImageFiltersTest(PicardTestCase): self.set_config_values(settings) def test_filter_by_size(self): - image1 = create_fake_image(400, 600, 'png') - image2 = create_fake_image(500, 500, 'jpeg') - image3 = create_fake_image(600, 600, 'bmp') - self.assertFalse(size_filter(image1)) - self.assertTrue(size_filter(image2)) - self.assertTrue(size_filter(image3)) + image1, info1 = create_fake_image(400, 600, 'png') + image2, info2 = create_fake_image(500, 500, 'jpeg') + image3, info3 = create_fake_image(600, 600, 'tiff') + self.assertFalse(size_filter(image1, info1)) + self.assertTrue(size_filter(image2, info2)) + self.assertTrue(size_filter(image3, info3)) def test_filter_by_size_metadata(self): image_metadata1 = {'width': 400, 'height': 600} @@ -105,8 +110,8 @@ class ImageProcessorsTest(PicardTestCase): def _check_image_processors(self, size, expected_tags_size, expected_file_size=None): coverartimage = CoverArtImage() - image = create_fake_image(size[0], size[1], 'jpg') - run_image_processors(image, coverartimage) + image, info = create_fake_image(size[0], size[1], 'jpg') + run_image_processors(coverartimage, image, info) tags_size = (coverartimage.width, coverartimage.height) if config.setting['save_images_to_tags']: self.assertEqual(tags_size, expected_tags_size) @@ -154,7 +159,7 @@ class ImageProcessorsTest(PicardTestCase): self.set_config_values(self.settings) def _check_resize_image(self, size, expected_size): - image = ProcessingImage(create_fake_image(size[0], size[1], 'jpg')) + image = ProcessingImage(*create_fake_image(size[0], size[1], 'jpg')) processor = ResizeImage() processor.run(image, ProcessingTarget.TAGS) new_size = (image.get_qimage().width(), image.get_qimage().height()) @@ -237,7 +242,7 @@ class ImageProcessorsTest(PicardTestCase): self.set_config_values(self.settings) def _check_convert_image(self, format, expected_format): - image = ProcessingImage(create_fake_image(100, 100, format)) + image = ProcessingImage(*create_fake_image(100, 100, format)) processor = ConvertImage() processor.run(image, ProcessingTarget.TAGS) new_image = image.get_result() @@ -255,7 +260,7 @@ class ImageProcessorsTest(PicardTestCase): self.set_config_values(self.settings) def test_identification_error(self): - image = create_fake_image(0, 0, 'jpg') + image, info = create_fake_image(0, 0, 'jpg') coverartimage = CoverArtImage() with self.assertRaises(CoverArtProcessingError): - run_image_processors(image, coverartimage) + run_image_processors(coverartimage, image, info) From c96f0b07e66740b8b2930f24e823201357a43c67 Mon Sep 17 00:00:00 2001 From: twodoorcoupe Date: Mon, 8 Jul 2024 09:56:24 +0200 Subject: [PATCH 3/6] Move new option to image filters --- picard/coverart/__init__.py | 19 ++++-------------- picard/coverart/processing/__init__.py | 4 ++-- picard/coverart/processing/filters.py | 21 +++++++++++++++++++- picard/util/imagelist.py | 5 +++++ test/test_coverart_processing.py | 27 ++++++++++++++++++++++---- 5 files changed, 54 insertions(+), 22 deletions(-) diff --git a/picard/coverart/__init__.py b/picard/coverart/__init__.py index e3b4efe6a..adc4e0d71 100644 --- a/picard/coverart/__init__.py +++ b/picard/coverart/__init__.py @@ -60,7 +60,6 @@ class CoverArt: self.metadata = metadata self.release = release # not used in this class, but used by providers self.front_image_found = False - self.previous_images = album.orig_metadata.images.get_types_dict() def __repr__(self): return "%s for %r" % (self.__class__.__name__, self.album) @@ -101,19 +100,6 @@ class CoverArt: except (CoverArtImageIdentificationError, CoverArtProcessingError) as e: self.album.error_append(e) - def _should_save_image(self, coverartimage, data, info): - config = get_config() - if config.setting['dont_replace_with_smaller_cover']: - downloaded_types = coverartimage.normalized_types() - if downloaded_types in self.previous_images: - previous_image = self.previous_images[downloaded_types] - if info.width < previous_image.width or info.height < previous_image.height: - log.debug("Discarding cover art. A bigger image with the same types is already embedded.") - return False - if coverartimage.can_be_filtered: - return run_image_filters(data, info) - return True - def _coverart_downloaded(self, coverartimage, data, http, error): """Handle finished download, save it to metadata""" self.album._requests -= 1 @@ -134,7 +120,10 @@ class CoverArt: ) try: info = imageinfo.identify(data) - if self._should_save_image(coverartimage, data, info): + filters_result = True + if coverartimage.can_be_filtered: + filters_result = run_image_filters(data, info, self.album, coverartimage) + if filters_result: self._set_metadata(coverartimage, data, info) except (CoverArtImageIOError, imageinfo.IdentificationError): # It doesn't make sense to store/download more images if we can't diff --git a/picard/coverart/processing/__init__.py b/picard/coverart/processing/__init__.py index 85f15f451..4847a03c9 100644 --- a/picard/coverart/processing/__init__.py +++ b/picard/coverart/processing/__init__.py @@ -39,9 +39,9 @@ from picard.extension_points.cover_art_processors import ( from picard.util.imageinfo import IdentificationError -def run_image_filters(data, info): +def run_image_filters(data, info, album, coverartimage): for f in ext_point_cover_art_filters: - if not f(data, info): + if not f(data, info, album, coverartimage): return False return True diff --git a/picard/coverart/processing/filters.py b/picard/coverart/processing/filters.py index f6df3c056..c1a8f238d 100644 --- a/picard/coverart/processing/filters.py +++ b/picard/coverart/processing/filters.py @@ -45,7 +45,7 @@ def _check_threshold_size(width, height): return True -def size_filter(data, info): +def size_filter(data, info, album, coverartimage): return _check_threshold_size(info.width, info.height) @@ -55,5 +55,24 @@ def size_metadata_filter(metadata): return _check_threshold_size(metadata['width'], metadata['height']) +def bigger_previous_image_filter(data, info, album, coverartimage): + config = get_config() + if config.setting['dont_replace_with_smaller_cover'] and config.setting['save_images_to_tags']: + downloaded_types = coverartimage.normalized_types() + previous_images = album.orig_metadata.images.get_types_dict() + if downloaded_types in previous_images: + previous_image = previous_images[downloaded_types] + if info.width < previous_image.width or info.height < previous_image.height: + log.debug("Discarding cover art. A bigger image with the same types is already embedded.") + return False + return True + + +def image_types_filter(data, info, album, coverartimage): + return True + + register_cover_art_filter(size_filter) register_cover_art_metadata_filter(size_metadata_filter) +register_cover_art_filter(bigger_previous_image_filter) +register_cover_art_filter(image_types_filter) diff --git a/picard/util/imagelist.py b/picard/util/imagelist.py index dd186312b..d0b3df7cb 100644 --- a/picard/util/imagelist.py +++ b/picard/util/imagelist.py @@ -105,5 +105,10 @@ class ImageList(MutableSequence): def get_types_dict(self): types_dict = dict() for image in self._images: + image_types = image.normalized_types() + if image_types in types_dict: + previous_image = types_dict[image_types] + if image.width > previous_image.width or image.height > previous_image.height: + continue types_dict[image.normalized_types()] = image return types_dict diff --git a/test/test_coverart_processing.py b/test/test_coverart_processing.py index 7dac01bad..a8cdfd87c 100644 --- a/test/test_coverart_processing.py +++ b/test/test_coverart_processing.py @@ -26,10 +26,12 @@ from PyQt6.QtGui import QImage from test.picardtestcase import PicardTestCase from picard import config +from picard.album import Album from picard.const.cover_processing import ResizeModes from picard.coverart.image import CoverArtImage from picard.coverart.processing import run_image_processors from picard.coverart.processing.filters import ( + bigger_previous_image_filter, size_filter, size_metadata_filter, ) @@ -43,6 +45,7 @@ from picard.extension_points.cover_art_processors import ( ProcessingTarget, ) from picard.util import imageinfo +from picard.util.imagelist import ImageList def create_fake_image(width, height, image_format): @@ -64,7 +67,9 @@ class ImageFiltersTest(PicardTestCase): settings = { 'filter_cover_by_size': True, 'cover_minimum_width': 500, - 'cover_minimum_height': 500 + 'cover_minimum_height': 500, + 'dont_replace_with_smaller_cover': True, + 'save_images_to_tags': True, } self.set_config_values(settings) @@ -72,9 +77,9 @@ class ImageFiltersTest(PicardTestCase): image1, info1 = create_fake_image(400, 600, 'png') image2, info2 = create_fake_image(500, 500, 'jpeg') image3, info3 = create_fake_image(600, 600, 'tiff') - self.assertFalse(size_filter(image1, info1)) - self.assertTrue(size_filter(image2, info2)) - self.assertTrue(size_filter(image3, info3)) + self.assertFalse(size_filter(image1, info1, None, None)) + self.assertTrue(size_filter(image2, info2, None, None)) + self.assertTrue(size_filter(image3, info3, None, None)) def test_filter_by_size_metadata(self): image_metadata1 = {'width': 400, 'height': 600} @@ -84,6 +89,20 @@ class ImageFiltersTest(PicardTestCase): self.assertTrue(size_metadata_filter(image_metadata2)) self.assertTrue(size_metadata_filter(image_metadata3)) + def test_filter_by_previous_image_size(self): + previous_coverartimage = CoverArtImage(types=['front'], support_types=True) + previous_coverartimage.width = 1000 + previous_coverartimage.height = 1000 + album = Album(None) + album.orig_metadata.images = ImageList([previous_coverartimage]) + image1, info1 = create_fake_image(500, 500, 'jpg') + image2, info2 = create_fake_image(2000, 2000, 'jpg') + coverartimage = CoverArtImage(types=['front'], support_types=True) + self.assertFalse(bigger_previous_image_filter(image1, info1, album, coverartimage)) + self.assertTrue(bigger_previous_image_filter(image2, info2, album, coverartimage)) + coverartimage = CoverArtImage(types=['back'], support_types=True) + self.assertTrue(bigger_previous_image_filter(image1, info1, album, coverartimage)) + class ImageProcessorsTest(PicardTestCase): def setUp(self): From 17f5c61ae1502a42b0d607b8933ce91655114861 Mon Sep 17 00:00:00 2001 From: twodoorcoupe Date: Mon, 8 Jul 2024 12:18:42 +0200 Subject: [PATCH 4/6] Add option to filter images by type --- picard/const/defaults.py | 2 ++ picard/coverart/processing/filters.py | 14 +++++++++++++ picard/options.py | 7 ++++++- picard/ui/forms/ui_options_cover.py | 16 +++++++++++++++ picard/ui/options/cover.py | 29 +++++++++++++++++++++++++++ test/test_coverart_processing.py | 24 +++++++++++++++++++++- ui/options_cover.ui | 24 ++++++++++++++++++++++ 7 files changed, 114 insertions(+), 2 deletions(-) diff --git a/picard/const/defaults.py b/picard/const/defaults.py index e42f8d6cb..7ee807307 100644 --- a/picard/const/defaults.py +++ b/picard/const/defaults.py @@ -85,6 +85,8 @@ DEFAULT_QUERY_LIMIT = 50 DEFAULT_DRIVES = get_default_cdrom_drives() +DEFAULT_CA_NEVER_REPLACE_TYPE_INCLUDE = ['front'] +DEFAULT_CA_NEVER_REPLACE_TYPE_EXCLUDE = ['matrix/runout', 'raw/unedited', 'watermark'] DEFAULT_CA_PROVIDERS = [ ('Cover Art Archive', True), ('UrlRelationships', True), diff --git a/picard/coverart/processing/filters.py b/picard/coverart/processing/filters.py index c1a8f238d..ebba55a5a 100644 --- a/picard/coverart/processing/filters.py +++ b/picard/coverart/processing/filters.py @@ -69,6 +69,20 @@ def bigger_previous_image_filter(data, info, album, coverartimage): def image_types_filter(data, info, album, coverartimage): + config = get_config() + if config.setting['dont_replace_cover_of_types'] and config.setting['save_images_to_tags']: + downloaded_types = set(coverartimage.normalized_types()) + never_replace_types = config.setting['dont_replace_included_types'] + always_replace_types = config.setting['dont_replace_excluded_types'] + previous_image_types = album.orig_metadata.images.get_types_dict() + if downloaded_types.intersection(always_replace_types): + return True + for previous_image_type in previous_image_types: + type_already_embedded = downloaded_types.intersection(previous_image_type) + should_not_replace = downloaded_types.intersection(never_replace_types) + if type_already_embedded and should_not_replace: + log.debug("Discarding cover art. An image with the same type is already embedded.") + return False return True diff --git a/picard/options.py b/picard/options.py index b10dcd438..710e95ca0 100644 --- a/picard/options.py +++ b/picard/options.py @@ -34,6 +34,8 @@ from picard.config import ( from picard.const import MUSICBRAINZ_SERVERS from picard.const.defaults import ( DEFAULT_AUTOBACKUP_DIRECTORY, + DEFAULT_CA_NEVER_REPLACE_TYPE_EXCLUDE, + DEFAULT_CA_NEVER_REPLACE_TYPE_INCLUDE, DEFAULT_CA_PROVIDERS, DEFAULT_CAA_IMAGE_SIZE, DEFAULT_CAA_IMAGE_TYPE_EXCLUDE, @@ -166,7 +168,10 @@ TextOption('setting', 'cd_lookup_device', ','.join(DEFAULT_DRIVES)) ListOption('setting', 'ca_providers', DEFAULT_CA_PROVIDERS, title=N_("Cover art providers")) TextOption('setting', 'cover_image_filename', DEFAULT_COVER_IMAGE_FILENAME, title=N_("File name for images")) BoolOption('setting', 'embed_only_one_front_image', True, title=N_("Embed only a single front image")) -BoolOption('setting', 'dont_replace_with_smaller_cover', False, title=N_("Never replace front images with smaller ones")) +BoolOption('setting', 'dont_replace_with_smaller_cover', False, title=N_("Never replace cover images with smaller ones")) +BoolOption('setting', 'dont_replace_cover_of_types', False, title=N_("Never replace cover images of the given types")) +ListOption('setting', 'dont_replace_included_types', DEFAULT_CA_NEVER_REPLACE_TYPE_INCLUDE, title=N_("Never replace cover images of these types")) +ListOption('setting', 'dont_replace_excluded_types', DEFAULT_CA_NEVER_REPLACE_TYPE_EXCLUDE, title=N_("Always replace cover images of these types")) BoolOption('setting', 'image_type_as_filename', False, title=N_("Always use the primary image type as the file name for non-front images")) BoolOption('setting', 'save_images_overwrite', False, title=N_("Overwrite existing image files")) BoolOption('setting', 'save_images_to_files', False, title=N_("Save cover images as separate files")) diff --git a/picard/ui/forms/ui_options_cover.py b/picard/ui/forms/ui_options_cover.py index 9eccc49a7..ce191a53d 100644 --- a/picard/ui/forms/ui_options_cover.py +++ b/picard/ui/forms/ui_options_cover.py @@ -34,6 +34,20 @@ class Ui_CoverOptionsPage(object): self.cb_dont_replace_with_smaller = QtWidgets.QCheckBox(parent=self.save_images_to_tags) self.cb_dont_replace_with_smaller.setObjectName("cb_dont_replace_with_smaller") self.vboxlayout.addWidget(self.cb_dont_replace_with_smaller) + self.never_replace_types_layout = QtWidgets.QHBoxLayout() + self.never_replace_types_layout.setObjectName("never_replace_types_layout") + self.cb_never_replace_types = QtWidgets.QCheckBox(parent=self.save_images_to_tags) + self.cb_never_replace_types.setObjectName("cb_never_replace_types") + self.never_replace_types_layout.addWidget(self.cb_never_replace_types) + self.select_types_button = QtWidgets.QPushButton(parent=self.save_images_to_tags) + sizePolicy = QtWidgets.QSizePolicy(QtWidgets.QSizePolicy.Policy.Fixed, QtWidgets.QSizePolicy.Policy.Fixed) + sizePolicy.setHorizontalStretch(0) + sizePolicy.setVerticalStretch(0) + sizePolicy.setHeightForWidth(self.select_types_button.sizePolicy().hasHeightForWidth()) + self.select_types_button.setSizePolicy(sizePolicy) + self.select_types_button.setObjectName("select_types_button") + self.never_replace_types_layout.addWidget(self.select_types_button) + self.vboxlayout.addLayout(self.never_replace_types_layout) self.verticalLayout.addWidget(self.save_images_to_tags) self.save_images_to_files = QtWidgets.QGroupBox(parent=CoverOptionsPage) self.save_images_to_files.setCheckable(True) @@ -104,6 +118,8 @@ class Ui_CoverOptionsPage(object): self.save_images_to_tags.setTitle(_("Embed cover images into tags")) self.cb_embed_front_only.setText(_("Embed only a single front image")) self.cb_dont_replace_with_smaller.setText(_("Never replace cover images with smaller ones")) + self.cb_never_replace_types.setText(_("Never replace cover images matching selected types")) + self.select_types_button.setText(_("Select Types...")) self.save_images_to_files.setTitle(_("Save cover images as separate files")) self.label_use_filename.setText(_("Use the following file name for images:")) self.save_images_overwrite.setText(_("Overwrite the file if it already exists")) diff --git a/picard/ui/options/cover.py b/picard/ui/options/cover.py index 374368bf5..2e630283c 100644 --- a/picard/ui/options/cover.py +++ b/picard/ui/options/cover.py @@ -31,6 +31,10 @@ from picard.config import ( Option, get_config, ) +from picard.const.defaults import ( + DEFAULT_CA_NEVER_REPLACE_TYPE_EXCLUDE, + DEFAULT_CA_NEVER_REPLACE_TYPE_INCLUDE, +) from picard.coverart.providers import cover_art_providers from picard.extension_points.options_pages import register_options_page from picard.i18n import ( @@ -38,6 +42,7 @@ from picard.i18n import ( gettext as _, ) +from picard.ui.caa_types_selector import CAATypesSelectorDialog from picard.ui.forms.ui_options_cover import Ui_CoverOptionsPage from picard.ui.moveable_list_view import MoveableListView from picard.ui.options import OptionsPage @@ -62,12 +67,17 @@ class CoverOptionsPage(OptionsPage): self.ui.save_images_to_files.clicked.connect(self.update_ca_providers_groupbox_state) self.ui.save_images_to_tags.clicked.connect(self.update_ca_providers_groupbox_state) self.ui.save_only_one_front_image.toggled.connect(self.ui.image_type_as_filename.setDisabled) + self.ui.cb_never_replace_types.toggled.connect(self.ui.select_types_button.setEnabled) + self.ui.select_types_button.clicked.connect(self.select_never_replace_image_types) self.move_view = MoveableListView(self.ui.ca_providers_list, self.ui.up_button, self.ui.down_button) self.register_setting('save_images_to_tags', ['save_images_to_tags']) self.register_setting('embed_only_one_front_image', ['cb_embed_front_only']) self.register_setting('dont_replace_with_smaller_cover', ['dont_replace_with_smaller_cover']) + self.register_setting('dont_replace_cover_of_types', ['dont_replace_cover_of_types']) + self.register_setting('dont_replace_included_types', ['dont_replace_included_types']) + self.register_setting('dont_replace_excluded_types', ['dont_replace_excluded_types']) self.register_setting('save_images_to_files', ['save_images_to_files']) self.register_setting('cover_image_filename', ['cover_image_filename']) self.register_setting('save_images_overwrite', ['save_images_overwrite']) @@ -78,6 +88,8 @@ class CoverOptionsPage(OptionsPage): def restore_defaults(self): # Remove previous entries self.ui.ca_providers_list.clear() + self.dont_replace_included_types = DEFAULT_CA_NEVER_REPLACE_TYPE_INCLUDE + self.dont_replace_excluded_types = DEFAULT_CA_NEVER_REPLACE_TYPE_EXCLUDE super().restore_defaults() def _load_cover_art_providers(self): @@ -94,6 +106,10 @@ class CoverOptionsPage(OptionsPage): self.ui.save_images_to_tags.setChecked(config.setting['save_images_to_tags']) self.ui.cb_embed_front_only.setChecked(config.setting['embed_only_one_front_image']) self.ui.cb_dont_replace_with_smaller.setChecked(config.setting['dont_replace_with_smaller_cover']) + self.ui.cb_never_replace_types.setChecked(config.setting['dont_replace_cover_of_types']) + self.ui.select_types_button.setEnabled(config.setting['dont_replace_cover_of_types']) + self.dont_replace_included_types = config.setting['dont_replace_included_types'] + self.dont_replace_excluded_types = config.setting['dont_replace_excluded_types'] self.ui.save_images_to_files.setChecked(config.setting['save_images_to_files']) self.ui.cover_image_filename.setText(config.setting['cover_image_filename']) self.ui.save_images_overwrite.setChecked(config.setting['save_images_overwrite']) @@ -112,6 +128,9 @@ class CoverOptionsPage(OptionsPage): config.setting['save_images_to_tags'] = self.ui.save_images_to_tags.isChecked() config.setting['embed_only_one_front_image'] = self.ui.cb_embed_front_only.isChecked() config.setting['dont_replace_with_smaller_cover'] = self.ui.cb_dont_replace_with_smaller.isChecked() + config.setting['dont_replace_cover_of_types'] = self.ui.cb_never_replace_types.isChecked() + config.setting['dont_replace_included_types'] = self.dont_replace_included_types + config.setting['dont_replace_excluded_types'] = self.dont_replace_excluded_types config.setting['save_images_to_files'] = self.ui.save_images_to_files.isChecked() config.setting['cover_image_filename'] = self.ui.cover_image_filename.text() config.setting['save_images_overwrite'] = self.ui.save_images_overwrite.isChecked() @@ -124,5 +143,15 @@ class CoverOptionsPage(OptionsPage): tags_enabled = self.ui.save_images_to_tags.isChecked() self.ui.ca_providers_groupbox.setEnabled(files_enabled or tags_enabled) + def select_never_replace_image_types(self): + (included_types, excluded_types, ok) = CAATypesSelectorDialog.display( + types_include=self.dont_replace_included_types, + types_exclude=self.dont_replace_excluded_types, + parent=self, + ) + if ok: + self.dont_replace_included_types = included_types + self.dont_replace_excluded_types = excluded_types + register_options_page(CoverOptionsPage) diff --git a/test/test_coverart_processing.py b/test/test_coverart_processing.py index a8cdfd87c..36c0c5d78 100644 --- a/test/test_coverart_processing.py +++ b/test/test_coverart_processing.py @@ -32,6 +32,7 @@ from picard.coverart.image import CoverArtImage from picard.coverart.processing import run_image_processors from picard.coverart.processing.filters import ( bigger_previous_image_filter, + image_types_filter, size_filter, size_metadata_filter, ) @@ -69,6 +70,9 @@ class ImageFiltersTest(PicardTestCase): 'cover_minimum_width': 500, 'cover_minimum_height': 500, 'dont_replace_with_smaller_cover': True, + 'dont_replace_cover_of_types': True, + 'dont_replace_included_types': ['front', 'booklet'], + 'dont_replace_excluded_types': ['back'], 'save_images_to_tags': True, } self.set_config_values(settings) @@ -89,12 +93,16 @@ class ImageFiltersTest(PicardTestCase): self.assertTrue(size_metadata_filter(image_metadata2)) self.assertTrue(size_metadata_filter(image_metadata3)) - def test_filter_by_previous_image_size(self): + def _create_fake_album(self): previous_coverartimage = CoverArtImage(types=['front'], support_types=True) previous_coverartimage.width = 1000 previous_coverartimage.height = 1000 album = Album(None) album.orig_metadata.images = ImageList([previous_coverartimage]) + return album + + def test_filter_by_previous_image_size(self): + album = self._create_fake_album() image1, info1 = create_fake_image(500, 500, 'jpg') image2, info2 = create_fake_image(2000, 2000, 'jpg') coverartimage = CoverArtImage(types=['front'], support_types=True) @@ -103,6 +111,20 @@ class ImageFiltersTest(PicardTestCase): coverartimage = CoverArtImage(types=['back'], support_types=True) self.assertTrue(bigger_previous_image_filter(image1, info1, album, coverartimage)) + def test_filter_by_image_type(self): + album = self._create_fake_album() + image, info = create_fake_image(1000, 1000, 'jpg') + coverartimage1 = CoverArtImage(types=['front'], support_types=True) + coverartimage2 = CoverArtImage(types=['back'], support_types=True) + coverartimage3 = CoverArtImage(types=['front', 'back'], support_types=True) + coverartimage4 = CoverArtImage(types=['spine'], support_types=True) + coverartimage5 = CoverArtImage(types=['booklet', 'spine'], support_types=True) + self.assertFalse(image_types_filter(image, info, album, coverartimage1)) + self.assertTrue(image_types_filter(image, info, album, coverartimage2)) + self.assertTrue(image_types_filter(image, info, album, coverartimage3)) + self.assertTrue(image_types_filter(image, info, album, coverartimage4)) + self.assertTrue(image_types_filter(image, info, album, coverartimage5)) + class ImageProcessorsTest(PicardTestCase): def setUp(self): diff --git a/ui/options_cover.ui b/ui/options_cover.ui index a61ea60b4..67f9ca876 100644 --- a/ui/options_cover.ui +++ b/ui/options_cover.ui @@ -52,6 +52,30 @@ + + + + + + Never replace cover images matching selected types + + + + + + + + 0 + 0 + + + + Select Types... + + + + + From fba6441fc9acac9c92d618cb00658a9ae0908b40 Mon Sep 17 00:00:00 2001 From: twodoorcoupe Date: Wed, 10 Jul 2024 13:45:59 +0200 Subject: [PATCH 5/6] Modify explanation text in the image type selector --- picard/coverart/__init__.py | 10 +++++----- picard/ui/options/cover.py | 8 ++++++++ picard/util/imagelist.py | 2 +- 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/picard/coverart/__init__.py b/picard/coverart/__init__.py index adc4e0d71..88f3eea1f 100644 --- a/picard/coverart/__init__.py +++ b/picard/coverart/__init__.py @@ -73,10 +73,10 @@ class CoverArt: else: log.debug("Cover art disabled by user options.") - def _set_metadata(self, coverartimage, data, info): + def _set_metadata(self, coverartimage, data, image_info): try: if coverartimage.can_be_processed: - run_image_processors(coverartimage, data, info) + run_image_processors(coverartimage, data, image_info) else: coverartimage.set_tags_data(data) if coverartimage.can_be_saved_to_metadata: @@ -119,12 +119,12 @@ class CoverArt: echo=None ) try: - info = imageinfo.identify(data) + image_info = imageinfo.identify(data) filters_result = True if coverartimage.can_be_filtered: - filters_result = run_image_filters(data, info, self.album, coverartimage) + filters_result = run_image_filters(data, image_info, self.album, coverartimage) if filters_result: - self._set_metadata(coverartimage, data, info) + self._set_metadata(coverartimage, data, image_info) except (CoverArtImageIOError, imageinfo.IdentificationError): # It doesn't make sense to store/download more images if we can't # save them in the temporary folder, abort. diff --git a/picard/ui/options/cover.py b/picard/ui/options/cover.py index 2e630283c..6951fce0b 100644 --- a/picard/ui/options/cover.py +++ b/picard/ui/options/cover.py @@ -144,10 +144,18 @@ class CoverOptionsPage(OptionsPage): self.ui.ca_providers_groupbox.setEnabled(files_enabled or tags_enabled) def select_never_replace_image_types(self): + instructions_bottom = N_( + "Embedded cover art images with a type found in the 'Include' list will never be replaced " + "by a newly downloaded image UNLESS they also have an image type in the 'Exclude' list. " + "Images with types found in the 'Exclude' list will always be replaced by downloaded images " + "of the same type. Images types not appearing in the 'Include' or 'Exclude' list will " + "not be considered when determining whether or not to replace an embedded cover art image.\n" + ) (included_types, excluded_types, ok) = CAATypesSelectorDialog.display( types_include=self.dont_replace_included_types, types_exclude=self.dont_replace_excluded_types, parent=self, + instructions_bottom=instructions_bottom, ) if ok: self.dont_replace_included_types = included_types diff --git a/picard/util/imagelist.py b/picard/util/imagelist.py index d0b3df7cb..2a8ec7317 100644 --- a/picard/util/imagelist.py +++ b/picard/util/imagelist.py @@ -110,5 +110,5 @@ class ImageList(MutableSequence): previous_image = types_dict[image_types] if image.width > previous_image.width or image.height > previous_image.height: continue - types_dict[image.normalized_types()] = image + types_dict[image_types] = image return types_dict From d1c8d1662679b0c6f6d59c9146cb22c171d80f1a Mon Sep 17 00:00:00 2001 From: twodoorcoupe Date: Wed, 10 Jul 2024 18:40:16 +0200 Subject: [PATCH 6/6] Fix names consistency for image_info --- picard/const/defaults.py | 8 ++++---- picard/coverart/processing/__init__.py | 8 ++++---- picard/coverart/processing/filters.py | 10 +++++----- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/picard/const/defaults.py b/picard/const/defaults.py index 7ee807307..f01792df5 100644 --- a/picard/const/defaults.py +++ b/picard/const/defaults.py @@ -72,8 +72,8 @@ DEFAULT_RELEASE_TYPE_SCORES = [(g, DEFAULT_RELEASE_SCORE) for g in list(RELEASE_ DEFAULT_CAA_IMAGE_SIZE = 500 -DEFAULT_CAA_IMAGE_TYPE_INCLUDE = ['front'] -DEFAULT_CAA_IMAGE_TYPE_EXCLUDE = ['matrix/runout', 'raw/unedited', 'watermark'] +DEFAULT_CAA_IMAGE_TYPE_INCLUDE = ('front',) +DEFAULT_CAA_IMAGE_TYPE_EXCLUDE = ('matrix/runout', 'raw/unedited', 'watermark') DEFAULT_LOCAL_COVER_ART_REGEX = r'^(?:cover|folder|albumart)(.*)\.(?:jpe?g|png|gif|tiff?|webp)$' @@ -85,8 +85,8 @@ DEFAULT_QUERY_LIMIT = 50 DEFAULT_DRIVES = get_default_cdrom_drives() -DEFAULT_CA_NEVER_REPLACE_TYPE_INCLUDE = ['front'] -DEFAULT_CA_NEVER_REPLACE_TYPE_EXCLUDE = ['matrix/runout', 'raw/unedited', 'watermark'] +DEFAULT_CA_NEVER_REPLACE_TYPE_INCLUDE = ('front',) +DEFAULT_CA_NEVER_REPLACE_TYPE_EXCLUDE = ('matrix/runout', 'raw/unedited', 'watermark') DEFAULT_CA_PROVIDERS = [ ('Cover Art Archive', True), ('UrlRelationships', True), diff --git a/picard/coverart/processing/__init__.py b/picard/coverart/processing/__init__.py index 4847a03c9..c663dcc24 100644 --- a/picard/coverart/processing/__init__.py +++ b/picard/coverart/processing/__init__.py @@ -39,9 +39,9 @@ from picard.extension_points.cover_art_processors import ( from picard.util.imageinfo import IdentificationError -def run_image_filters(data, info, album, coverartimage): +def run_image_filters(data, image_info, album, coverartimage): for f in ext_point_cover_art_filters: - if not f(data, info, album, coverartimage): + if not f(data, image_info, album, coverartimage): return False return True @@ -53,13 +53,13 @@ def run_image_metadata_filters(metadata): return True -def run_image_processors(coverartimage, data, info): +def run_image_processors(coverartimage, data, image_info): config = get_config() tags_data = data file_data = data try: start_time = time.time() - image = ProcessingImage(data, info) + image = ProcessingImage(data, image_info) both_queue, tags_queue, file_queue = get_cover_art_processors() for processor in both_queue: processor.run(image, ProcessingTarget.BOTH) diff --git a/picard/coverart/processing/filters.py b/picard/coverart/processing/filters.py index ebba55a5a..2ea74a8c9 100644 --- a/picard/coverart/processing/filters.py +++ b/picard/coverart/processing/filters.py @@ -45,8 +45,8 @@ def _check_threshold_size(width, height): return True -def size_filter(data, info, album, coverartimage): - return _check_threshold_size(info.width, info.height) +def size_filter(data, image_info, album, coverartimage): + return _check_threshold_size(image_info.width, image_info.height) def size_metadata_filter(metadata): @@ -55,20 +55,20 @@ def size_metadata_filter(metadata): return _check_threshold_size(metadata['width'], metadata['height']) -def bigger_previous_image_filter(data, info, album, coverartimage): +def bigger_previous_image_filter(data, image_info, album, coverartimage): config = get_config() if config.setting['dont_replace_with_smaller_cover'] and config.setting['save_images_to_tags']: downloaded_types = coverartimage.normalized_types() previous_images = album.orig_metadata.images.get_types_dict() if downloaded_types in previous_images: previous_image = previous_images[downloaded_types] - if info.width < previous_image.width or info.height < previous_image.height: + if image_info.width < previous_image.width or image_info.height < previous_image.height: log.debug("Discarding cover art. A bigger image with the same types is already embedded.") return False return True -def image_types_filter(data, info, album, coverartimage): +def image_types_filter(data, image_info, album, coverartimage): config = get_config() if config.setting['dont_replace_cover_of_types'] and config.setting['save_images_to_tags']: downloaded_types = set(coverartimage.normalized_types())