Merge pull request #2514 from twodoorcoupe/display_external_file

Add new column for external images to artwork infodialog

picard/coverart/image.py

@@ -322,7 +322,7 @@ def set_tags_data(self, data):
             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):

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

picard/ui/infodialog.py

@@ -45,6 +45,7 @@
 
 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 @@ def __init__(self, parent=None):
         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]
@@ -124,37 +128,68 @@ def get_column_index(self, name):
 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"), _("Cover"),)
-    artwork_columns = ('new',)
+    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):
-    NUM_COLS = 3
+    NUM_COLS = 4
 
     _columns = {
         'orig': 0,
         'type': 1,
         'new': 2,
+        'external': 3,
     }
 
-    _labels = (_("Existing Cover"), _("Type"), _("New Cover"),)
-    artwork_columns = ('orig', 'new',)
+    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"),
+    }
 
 
 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):
@@ -170,16 +205,20 @@ def __init__(self, obj, parent=None):
 
         self.new_images = sorted(obj.metadata.images) or []
         self.orig_images = []
-        artworktable_class = ArtworkTableSimple
+        artworktable_class = ArtworkTableNew
 
+        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 obj.orig_metadata.images != obj.metadata.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(
@@ -242,7 +281,11 @@ def _display_artwork_image_cell(self, row_index, colname):
         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()
 
@@ -326,6 +369,15 @@ def _display_artwork_tab(self):
         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 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)
 
     def tab_hide(self, widget):
         tab = self.ui.tabWidget

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 @@ def test_filter_by_size(self):
         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 @@ def test_filter_by_size_metadata(self):
 
 
 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 @@ def test_resize(self):
             (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,16 +115,6 @@ def test_resize(self):
             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
-        }
-        self.set_config_values(settings)
         sizes = [
             (1000, 1000),
             (1000, 500),
@@ -128,16 +125,28 @@ def test_image_processors(self):
             ((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")