From c22fe4cd6434bcc1cc4904748f1f4889eb52343e Mon Sep 17 00:00:00 2001 From: twodoorcoupe Date: Sat, 15 Jun 2024 11:37:21 +0200 Subject: [PATCH 1/3] Add new external image to artwork infodialog --- picard/coverart/image.py | 2 +- picard/coverart/processing/__init__.py | 25 +++++++++------- picard/ui/infodialog.py | 41 +++++++++++++++++++++++++- test/test_coverart_processing.py | 4 ++- 4 files changed, 59 insertions(+), 13 deletions(-) diff --git a/picard/coverart/image.py b/picard/coverart/image.py index ab74bdb3b..c9a1b8812 100644 --- a/picard/coverart/image.py +++ b/picard/coverart/image.py @@ -322,7 +322,7 @@ class CoverArtImage: raise CoverArtImageIOError(e) def set_external_file_data(self, data): - self.external_file_coverart = CoverArtImage(data=data) + self.external_file_coverart = CoverArtImage(data=data, url=self.url) @property def maintype(self): diff --git a/picard/coverart/processing/__init__.py b/picard/coverart/processing/__init__.py index dca8fc27e..4e9ecee21 100644 --- a/picard/coverart/processing/__init__.py +++ b/picard/coverart/processing/__init__.py @@ -21,6 +21,7 @@ import time from picard import log +from picard.config import get_config from picard.coverart.processing import ( # noqa: F401 # pylint: disable=unused-import filters, processors, @@ -53,6 +54,7 @@ def run_image_metadata_filters(metadata): def run_image_processors(data, coverartimage): + config = get_config() tags_data = data file_data = data try: @@ -61,16 +63,18 @@ def run_image_processors(data, coverartimage): both_queue, tags_queue, file_queue = get_cover_art_processors() for processor in both_queue: processor.run(image, ProcessingTarget.BOTH) - tags_image = image.copy() - for processor in tags_queue: - processor.run(tags_image, ProcessingTarget.TAGS) - tags_data = tags_image.get_result(default_format=True) + if config.setting['save_images_to_tags']: + tags_image = image.copy() + for processor in tags_queue: + processor.run(tags_image, ProcessingTarget.TAGS) + tags_data = tags_image.get_result(default_format=True) coverartimage.set_tags_data(tags_data) - file_image = image.copy() - for processor in file_queue: - processor.run(file_image, ProcessingTarget.FILE) - file_data = file_image.get_result(default_format=True) - coverartimage.set_external_file_data(file_data) + if config.setting['save_images_to_files']: + file_image = image.copy() + for processor in file_queue: + processor.run(file_image, ProcessingTarget.FILE) + file_data = file_image.get_result(default_format=True) + coverartimage.set_external_file_data(file_data) log.debug( "Image processing for %s finished in %d ms", coverartimage, @@ -80,5 +84,6 @@ def run_image_processors(data, coverartimage): raise CoverArtProcessingError(e) except CoverArtProcessingError as e: coverartimage.set_tags_data(tags_data) - coverartimage.set_external_file_data(file_data) + if config.setting['save_images_to_files']: + coverartimage.set_external_file_data(file_data) raise e diff --git a/picard/ui/infodialog.py b/picard/ui/infodialog.py index a1a831802..ebd36c1ec 100644 --- a/picard/ui/infodialog.py +++ b/picard/ui/infodialog.py @@ -137,6 +137,19 @@ class ArtworkTableSimple(ArtworkTable): self.setColumnWidth(self.get_column_index('type'), self.TYPE_COLUMN_SIZE) +class ArtworkTableSimpleExternal(ArtworkTableSimple): + NUM_COLS = 3 + + _columns = { + 'type': 0, + 'new': 1, + 'external': 2, + } + + _labels = (_("Type"), _("Cover"), _("External Cover"),) + artwork_columns = ('new', 'external',) + + class ArtworkTableExisting(ArtworkTable): NUM_COLS = 3 @@ -150,11 +163,28 @@ class ArtworkTableExisting(ArtworkTable): artwork_columns = ('orig', 'new',) +class ArtworkTableExistingExternal(ArtworkTable): + NUM_COLS = 4 + + _columns = { + 'orig': 0, + 'type': 1, + 'new': 2, + 'external': 3, + } + + _labels = (_("Existing Cover"), _("Type"), _("New Cover"), _("New External Cover"),) + artwork_columns = ('orig', 'new', 'external',) + + class ArtworkRow: def __init__(self, orig_image=None, new_image=None, types=None): self.orig_image = orig_image self.new_image = new_image self.types = types + self.new_external_image = None + if self.new_image: + self.new_external_image = self.new_image.external_file_coverart class InfoDialog(PicardDialog): @@ -171,6 +201,9 @@ class InfoDialog(PicardDialog): self.new_images = sorted(obj.metadata.images) or [] self.orig_images = [] artworktable_class = ArtworkTableSimple + has_external_images = any(image.external_file_coverart for image in self.new_images) + if has_external_images: + artworktable_class = ArtworkTableSimpleExternal has_orig_images = hasattr(obj, 'orig_metadata') and obj.orig_metadata.images if has_orig_images and obj.orig_metadata.images != obj.metadata.images: @@ -180,6 +213,8 @@ class InfoDialog(PicardDialog): if is_track or is_linked_file or is_album_with_files: self.orig_images = sorted(obj.orig_metadata.images) artworktable_class = ArtworkTableExisting + if has_external_images: + artworktable_class = ArtworkTableExistingExternal self.ui.setupUi(self) self.ui.buttonBox.addButton( @@ -242,7 +277,11 @@ class InfoDialog(PicardDialog): col_index = self.artwork_table.get_column_index(colname) pixmap = None infos = None - source = 'new_image' if colname == 'new' else 'orig_image' + source = 'orig_image' + if colname == 'new': + source = 'new_image' + elif colname == 'external': + source = 'new_external_image' image = getattr(self.artwork_rows[row_index], source) item = QtWidgets.QTableWidgetItem() diff --git a/test/test_coverart_processing.py b/test/test_coverart_processing.py index 3c2fc7b6a..e16a15acb 100644 --- a/test/test_coverart_processing.py +++ b/test/test_coverart_processing.py @@ -115,7 +115,9 @@ class ImageProcessorsTest(PicardTestCase): 'cover_tags_maximum_height': 500, 'resize_images_saved_to_file': True, 'cover_file_maximum_width': 750, - 'cover_file_maximum_height': 750 + 'cover_file_maximum_height': 750, + 'save_images_to_tags': True, + 'save_images_to_files': True, } self.set_config_values(settings) sizes = [ From a127c6098d0d8e35d413629b85515b3f1326ad38 Mon Sep 17 00:00:00 2001 From: twodoorcoupe Date: Mon, 17 Jun 2024 12:28:30 +0200 Subject: [PATCH 2/3] Hide unneccessary columns in artwork infodialog and update unit tests --- picard/ui/infodialog.py | 63 ++++++++++++--------------- test/test_coverart_processing.py | 75 +++++++++++++++++--------------- 2 files changed, 68 insertions(+), 70 deletions(-) diff --git a/picard/ui/infodialog.py b/picard/ui/infodialog.py index ebd36c1ec..91ba6bdaa 100644 --- a/picard/ui/infodialog.py +++ b/picard/ui/infodialog.py @@ -45,6 +45,7 @@ from PyQt6 import ( from picard import log from picard.album import Album +from picard.config import get_config from picard.coverart.image import CoverArtImageIOError from picard.coverart.utils import translated_types_as_string from picard.file import File @@ -99,10 +100,11 @@ class ArtworkTable(QtWidgets.QTableWidget): V_SIZE = 230 NUM_ROWS = 0 - NUM_COLS = 2 + NUM_COLS = 3 _columns = {} _labels = () + _tooltips = {} artwork_columns = () def __init__(self, parent=None): @@ -116,6 +118,8 @@ class ArtworkTable(QtWidgets.QTableWidget): v_header.setDefaultSectionSize(self.V_SIZE) self.setHorizontalHeaderLabels(self._labels) + for colname, index in self._columns.items(): + self.horizontalHeaderItem(index).setToolTip(self._tooltips.get(colname, None)) def get_column_index(self, name): return self._columns[name] @@ -127,43 +131,22 @@ class ArtworkTableSimple(ArtworkTable): _columns = { 'type': 0, 'new': 1, + 'external': 2, } - _labels = (_("Type"), _("Cover"),) - artwork_columns = ('new',) + _labels = (_("Type"), _("New Tags Cover"), _("New External Cover"),) + artwork_columns = ('new', 'external',) + _tooltips = { + 'new': _("New cover art embedded into tags"), + 'external': _("New cover art saved as a separate file"), + } def __init__(self, parent=None): super().__init__(parent=parent) self.setColumnWidth(self.get_column_index('type'), self.TYPE_COLUMN_SIZE) -class ArtworkTableSimpleExternal(ArtworkTableSimple): - NUM_COLS = 3 - - _columns = { - 'type': 0, - 'new': 1, - 'external': 2, - } - - _labels = (_("Type"), _("Cover"), _("External Cover"),) - artwork_columns = ('new', 'external',) - - class ArtworkTableExisting(ArtworkTable): - NUM_COLS = 3 - - _columns = { - 'orig': 0, - 'type': 1, - 'new': 2, - } - - _labels = (_("Existing Cover"), _("Type"), _("New Cover"),) - artwork_columns = ('orig', 'new',) - - -class ArtworkTableExistingExternal(ArtworkTable): NUM_COLS = 4 _columns = { @@ -173,8 +156,12 @@ class ArtworkTableExistingExternal(ArtworkTable): 'external': 3, } - _labels = (_("Existing Cover"), _("Type"), _("New Cover"), _("New External Cover"),) + _labels = (_("Existing Cover"), _("Type"), _("New Tags Cover"), _("New External Cover"),) artwork_columns = ('orig', 'new', 'external',) + _tooltips = { + 'new': _("New cover art embedded into tags"), + 'external': _("New cover art saved as a separate file"), + } class ArtworkRow: @@ -201,20 +188,17 @@ class InfoDialog(PicardDialog): self.new_images = sorted(obj.metadata.images) or [] self.orig_images = [] artworktable_class = ArtworkTableSimple - has_external_images = any(image.external_file_coverart for image in self.new_images) - if has_external_images: - artworktable_class = ArtworkTableSimpleExternal + has_new_external_images = any(image.external_file_coverart for image in self.new_images) + has_new_different_images = obj.orig_metadata.images != obj.metadata.images has_orig_images = hasattr(obj, 'orig_metadata') and obj.orig_metadata.images - if has_orig_images and obj.orig_metadata.images != obj.metadata.images: + if has_orig_images and (has_new_different_images or has_new_external_images): is_track = isinstance(obj, Track) is_linked_file = isinstance(obj, File) and isinstance(obj.parent_item, Track) is_album_with_files = isinstance(obj, Album) and obj.get_num_total_files() > 0 if is_track or is_linked_file or is_album_with_files: self.orig_images = sorted(obj.orig_metadata.images) artworktable_class = ArtworkTableExisting - if has_external_images: - artworktable_class = ArtworkTableExistingExternal self.ui.setupUi(self) self.ui.buttonBox.addButton( @@ -365,6 +349,13 @@ class InfoDialog(PicardDialog): self._display_artwork_rows() self.artwork_table.itemDoubleClicked.connect(self.show_item) self.artwork_table.verticalHeader().resizeSections(QtWidgets.QHeaderView.ResizeMode.ResizeToContents) + config = get_config() + for colname in self.artwork_table.artwork_columns: + tags_image_not_used = colname == 'new' and not config.setting['save_images_to_tags'] + file_image_not_used = colname == 'external' and not config.setting['save_images_to_files'] + if tags_image_not_used or file_image_not_used: + col_index = self.artwork_table.get_column_index(colname) + self.artwork_table.setColumnHidden(col_index, True) def tab_hide(self, widget): tab = self.ui.tabWidget diff --git a/test/test_coverart_processing.py b/test/test_coverart_processing.py index e16a15acb..117853039 100644 --- a/test/test_coverart_processing.py +++ b/test/test_coverart_processing.py @@ -18,6 +18,9 @@ # along with this program; if not, write to the Free Software # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +from copy import copy +import itertools + from PyQt6.QtCore import QBuffer from PyQt6.QtGui import QImage @@ -46,13 +49,15 @@ def create_fake_image(width, height, image_format): class ImageFiltersTest(PicardTestCase): - def test_filter_by_size(self): + def setUp(self): settings = { 'filter_cover_by_size': True, 'cover_minimum_width': 500, 'cover_minimum_height': 500 } 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") @@ -61,12 +66,6 @@ class ImageFiltersTest(PicardTestCase): self.assertTrue(size_filter(image3)) def test_filter_by_size_metadata(self): - settings = { - 'filter_cover_by_size': True, - 'cover_minimum_width': 500, - 'cover_minimum_height': 500 - } - self.set_config_values(settings) image_metadata1 = {'width': 400, 'height': 600} image_metadata2 = {'width': 500, 'height': 500} image_metadata3 = {'width': 600, 'height': 600} @@ -76,14 +75,21 @@ class ImageFiltersTest(PicardTestCase): class ImageProcessorsTest(PicardTestCase): - def test_resize(self): - processor = ResizeImage() - settings = { + def setUp(self): + self.settings = { + 'enabled_plugins': [], 'resize_images_saved_to_tags': True, 'cover_tags_maximum_width': 500, - 'cover_tags_maximum_height': 500 + 'cover_tags_maximum_height': 500, + 'resize_images_saved_to_file': True, + 'cover_file_maximum_width': 750, + 'cover_file_maximum_height': 750, + 'save_images_to_tags': True, + 'save_images_to_files': True, } - self.set_config_values(settings) + self.set_config_values(self.settings) + + def test_resize(self): sizes = [ (500, 500), (1000, 500), @@ -98,6 +104,7 @@ class ImageProcessorsTest(PicardTestCase): (500, 500), (400, 400) ] + processor = ResizeImage() for size, expected_size in zip(sizes, expected_sizes): image = ProcessingImage(create_fake_image(size[0], size[1], "jpg")) processor.run(image, ProcessingTarget.TAGS) @@ -108,18 +115,6 @@ class ImageProcessorsTest(PicardTestCase): self.assertEqual(new_size, (image.info.width, image.info.height)) def test_image_processors(self): - settings = { - 'enabled_plugins': [], - 'resize_images_saved_to_tags': True, - 'cover_tags_maximum_width': 500, - 'cover_tags_maximum_height': 500, - 'resize_images_saved_to_file': True, - 'cover_file_maximum_width': 750, - 'cover_file_maximum_height': 750, - 'save_images_to_tags': True, - 'save_images_to_files': True, - } - self.set_config_values(settings) sizes = [ (1000, 1000), (1000, 500), @@ -130,16 +125,28 @@ class ImageProcessorsTest(PicardTestCase): ((500, 250), (750, 375)), ((500, 500), (600, 600)), ] - for size, expected_size in zip(sizes, expected_sizes): - coverartimage = CoverArtImage() - image = create_fake_image(size[0], size[1], "jpg") - run_image_processors(image, coverartimage) - tags_size = (coverartimage.width, coverartimage.height) - file_size = (coverartimage.external_file_coverart.width, coverartimage.external_file_coverart.height) - extension = coverartimage.extension[1:] - self.assertEqual(tags_size, expected_size[0]) - self.assertEqual(file_size, expected_size[1]) - self.assertEqual(extension, "jpg") + settings = copy(self.settings) + self.target_combinations = itertools.product([True, False], repeat=2) + for save_to_tags, save_to_file in self.target_combinations: + settings['save_images_to_tags'] = save_to_tags + settings['save_images_to_files'] = save_to_file + self.set_config_values(settings) + for size, expected_size in zip(sizes, expected_sizes): + coverartimage = CoverArtImage() + image = create_fake_image(size[0], size[1], "jpg") + run_image_processors(image, coverartimage) + tags_size = (coverartimage.width, coverartimage.height) + expected_size_tags = expected_size[0] if save_to_tags else size + self.assertEqual(tags_size, expected_size_tags) + if save_to_file: + external_cover = coverartimage.external_file_coverart + file_size = (external_cover.width, external_cover.height) + self.assertEqual(file_size, expected_size[1]) + else: + self.assertIsNone(coverartimage.external_file_coverart) + extension = coverartimage.extension[1:] + self.assertEqual(extension, "jpg") + self.set_config_values(self.settings) def test_identification_error(self): image = create_fake_image(0, 0, "jpg") From d5350386014f239b46ebb3c836e31a8170fe02f8 Mon Sep 17 00:00:00 2001 From: twodoorcoupe Date: Tue, 18 Jun 2024 11:45:14 +0200 Subject: [PATCH 3/3] Improve artwork infodialog --- picard/ui/infodialog.py | 54 +++++++++++++++++++++++++++++------------ 1 file changed, 38 insertions(+), 16 deletions(-) diff --git a/picard/ui/infodialog.py b/picard/ui/infodialog.py index 91ba6bdaa..1ed872606 100644 --- a/picard/ui/infodialog.py +++ b/picard/ui/infodialog.py @@ -128,22 +128,39 @@ class ArtworkTable(QtWidgets.QTableWidget): class ArtworkTableSimple(ArtworkTable): TYPE_COLUMN_SIZE = 140 + def __init__(self, parent=None): + super().__init__(parent=parent) + self.setColumnWidth(self.get_column_index('type'), self.TYPE_COLUMN_SIZE) + + +class ArtworkTableNew(ArtworkTableSimple): _columns = { 'type': 0, 'new': 1, 'external': 2, } - _labels = (_("Type"), _("New Tags Cover"), _("New External Cover"),) artwork_columns = ('new', 'external',) + _labels = (_("Type"), _("New Embedded"), _("New Exported"),) _tooltips = { 'new': _("New cover art embedded into tags"), 'external': _("New cover art saved as a separate file"), } - def __init__(self, parent=None): - super().__init__(parent=parent) - self.setColumnWidth(self.get_column_index('type'), self.TYPE_COLUMN_SIZE) + +class ArtworkTableOriginal(ArtworkTableSimple): + NUM_COLS = 2 + + _columns = { + 'type': 0, + 'new': 1, + } + + artwork_columns = ('new',) + _labels = (_("Type"), _("Existing Cover")) + _tooltips = { + 'new': _("Existing cover art already embedded into tags"), + } class ArtworkTableExisting(ArtworkTable): @@ -156,9 +173,10 @@ class ArtworkTableExisting(ArtworkTable): 'external': 3, } - _labels = (_("Existing Cover"), _("Type"), _("New Tags Cover"), _("New External Cover"),) artwork_columns = ('orig', 'new', 'external',) + _labels = (_("Existing Cover"), _("Type"), _("New Embedded"), _("New Exported"),) _tooltips = { + 'orig': _("Existing cover art already embedded into tags"), 'new': _("New cover art embedded into tags"), 'external': _("New cover art saved as a separate file"), } @@ -187,18 +205,20 @@ class InfoDialog(PicardDialog): self.new_images = sorted(obj.metadata.images) or [] self.orig_images = [] - artworktable_class = ArtworkTableSimple + artworktable_class = ArtworkTableNew - has_new_external_images = any(image.external_file_coverart for image in self.new_images) - has_new_different_images = obj.orig_metadata.images != obj.metadata.images + self.has_new_external_images = any(image.external_file_coverart for image in self.new_images) has_orig_images = hasattr(obj, 'orig_metadata') and obj.orig_metadata.images - if has_orig_images and (has_new_different_images or has_new_external_images): - is_track = isinstance(obj, Track) - is_linked_file = isinstance(obj, File) and isinstance(obj.parent_item, Track) - is_album_with_files = isinstance(obj, Album) and obj.get_num_total_files() > 0 - if is_track or is_linked_file or is_album_with_files: - self.orig_images = sorted(obj.orig_metadata.images) - artworktable_class = ArtworkTableExisting + if has_orig_images: + artworktable_class = ArtworkTableOriginal + has_new_different_images = obj.orig_metadata.images != obj.metadata.images + if has_new_different_images or self.has_new_external_images: + is_track = isinstance(obj, Track) + is_linked_file = isinstance(obj, File) and isinstance(obj.parent_item, Track) + is_album_with_files = isinstance(obj, Album) and obj.get_num_total_files() > 0 + if is_track or is_linked_file or is_album_with_files: + self.orig_images = sorted(obj.orig_metadata.images) + artworktable_class = ArtworkTableExisting self.ui.setupUi(self) self.ui.buttonBox.addButton( @@ -349,10 +369,12 @@ class InfoDialog(PicardDialog): self._display_artwork_rows() self.artwork_table.itemDoubleClicked.connect(self.show_item) self.artwork_table.verticalHeader().resizeSections(QtWidgets.QHeaderView.ResizeMode.ResizeToContents) + if isinstance(self.artwork_table, ArtworkTableOriginal): + return config = get_config() for colname in self.artwork_table.artwork_columns: tags_image_not_used = colname == 'new' and not config.setting['save_images_to_tags'] - file_image_not_used = colname == 'external' and not config.setting['save_images_to_files'] + file_image_not_used = colname == 'external' and not self.has_new_external_images if tags_image_not_used or file_image_not_used: col_index = self.artwork_table.get_column_index(colname) self.artwork_table.setColumnHidden(col_index, True)