diff --git a/picard/const/defaults.py b/picard/const/defaults.py index e42f8d6cb..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,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/__init__.py b/picard/coverart/__init__.py index 04843ec02..88f3eea1f 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: @@ -72,10 +73,10 @@ class CoverArt: else: log.debug("Cover art disabled by user options.") - def _set_metadata(self, coverartimage, data): + def _set_metadata(self, coverartimage, data, image_info): try: if coverartimage.can_be_processed: - run_image_processors(data, coverartimage) + run_image_processors(coverartimage, data, image_info) else: coverartimage.set_tags_data(data) if coverartimage.can_be_saved_to_metadata: @@ -117,16 +118,17 @@ class CoverArt: }, echo=None ) - filters_result = True - if coverartimage.can_be_filtered: - filters_result = run_image_filters(data) - if filters_result: - try: - 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 + try: + image_info = imageinfo.identify(data) + filters_result = True + if coverartimage.can_be_filtered: + filters_result = run_image_filters(data, image_info, self.album, coverartimage) + if filters_result: + 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. + return self.next_in_queue() diff --git a/picard/coverart/processing/__init__.py b/picard/coverart/processing/__init__.py index 5085c7684..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): +def run_image_filters(data, image_info, album, coverartimage): for f in ext_point_cover_art_filters: - if not f(data): + 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(data, coverartimage): +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) + 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 8961c126a..2ea74a8c9 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, image_info, album, coverartimage): + return _check_threshold_size(image_info.width, image_info.height) def size_metadata_filter(metadata): @@ -58,5 +55,38 @@ def size_metadata_filter(metadata): return _check_threshold_size(metadata['width'], metadata['height']) +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 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, 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()) + 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 + + 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/options.py b/picard/options.py index fe9b3a079..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,6 +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 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 6a5742895..ce191a53d 100644 --- a/picard/ui/forms/ui_options_cover.py +++ b/picard/ui/forms/ui_options_cover.py @@ -31,6 +31,23 @@ 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.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) @@ -100,6 +117,9 @@ 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.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 8009bfe8c..6951fce0b 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,11 +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']) @@ -77,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): @@ -92,6 +105,11 @@ 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.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']) @@ -109,6 +127,10 @@ 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['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() @@ -121,5 +143,23 @@ 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): + 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 + self.dont_replace_excluded_types = excluded_types + register_options_page(CoverOptionsPage) diff --git a/picard/util/imagelist.py b/picard/util/imagelist.py index 0ed70c40d..2a8ec7317 100644 --- a/picard/util/imagelist.py +++ b/picard/util/imagelist.py @@ -101,3 +101,14 @@ 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: + 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_types] = image + return types_dict diff --git a/test/test_coverart_processing.py b/test/test_coverart_processing.py index 0efd50fd1..36c0c5d78 100644 --- a/test/test_coverart_processing.py +++ b/test/test_coverart_processing.py @@ -26,10 +26,13 @@ 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, + image_types_filter, size_filter, size_metadata_filter, ) @@ -43,6 +46,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): @@ -50,7 +54,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): @@ -59,17 +68,22 @@ 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, + '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) 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, 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} @@ -79,6 +93,38 @@ class ImageFiltersTest(PicardTestCase): self.assertTrue(size_metadata_filter(image_metadata2)) self.assertTrue(size_metadata_filter(image_metadata3)) + 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) + 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)) + + 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): @@ -105,8 +151,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 +200,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 +283,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 +301,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) diff --git a/ui/options_cover.ui b/ui/options_cover.ui index 1af325c11..67f9ca876 100644 --- a/ui/options_cover.ui +++ b/ui/options_cover.ui @@ -45,6 +45,37 @@ + + + + Never replace cover images with smaller ones + + + + + + + + + Never replace cover images matching selected types + + + + + + + + 0 + 0 + + + + Select Types... + + + + + @@ -130,8 +161,7 @@ - - .. + Qt::ToolButtonIconOnly @@ -150,8 +180,7 @@ - - .. + Qt::ToolButtonIconOnly