Merge Image() and CoverArtImage()

- Info dialog Artwork tab now displays types and comment for cover art images
- a tooltip gives info about the source (url, caa url, local file, tag, ...)

picard/coverart.py

@@ -29,6 +29,7 @@ from functools import partial
 from picard import config, log
 from picard.util import mimetype, parse_amazon_url
 from picard.const import CAA_HOST, CAA_PORT
+from picard.coverartimage import CoverArtImage, CaaCoverArtImage
 from PyQt4.QtCore import QUrl, QObject
 
 # amazon image file names are unique on all servers and constructed like
@@ -90,63 +91,6 @@ _CAA_THUMBNAIL_SIZE_MAP = {
 }
 
 
-class CoverArtImage:
-
-    support_types = False
-    # consider all images as front if types aren't supported by provider
-    is_front = True
-
-    def __init__(self, url=None, types=[u'front'], comment=''):
-        if url is not None:
-            self.parse_url(url)
-        else:
-            self.url = None
-        self.types = types
-        self.comment = comment
-
-    def parse_url(self, url):
-        self.url = QUrl(url)
-        self.host = str(self.url.host())
-        self.port = self.url.port(80)
-        self.path = str(self.url.encodedPath())
-        if self.url.hasQuery():
-            self.path += '?' + str(self.url.encodedQuery())
-
-    def is_front_image(self):
-        # CAA has a flag for "front" image, use it in priority
-        if self.is_front:
-            return True
-        # no caa front flag, use type instead
-        return u'front' in self.types
-
-    def __repr__(self):
-        p = []
-        if self.url is not None:
-            p.append("url=%r" % self.url.toString())
-        p.append("types=%r" % self.types)
-        if self.comment:
-            p.append("comment=%r" % self.comment)
-        return "%s(%s)" % (self.__class__.__name__, ", ".join(p))
-
-    def __unicode__(self):
-        p = [u'Image']
-        if self.url is not None:
-            p.append(u"from %s" % self.url.toString())
-        p.append(u"of type %s" % u','.join(self.types))
-        if self.comment:
-            p.append(u"and comment '%s'" % self.comment)
-        return u' '.join(p)
-
-    def __str__(self):
-        return unicode(self).encode('utf-8')
-
-
-class CaaCoverArtImage(CoverArtImage):
-
-    is_front = False
-    support_types = True
-
-
 class CoverArt:
 
     def __init__(self, album, metadata, release):
@@ -259,21 +203,10 @@ class CoverArt:
             mime = mimetype.get_from_data(data, default="image/jpeg")
 
             try:
-                self.metadata.make_and_add_image(
-                    mime,
-                    data,
-                    types=coverartimage.types,
-                    comment=coverartimage.comment,
-                    is_front=coverartimage.is_front
-                )
+                coverartimage.set_data(data, mime)
+                self.metadata.append_image(coverartimage)
                 for track in self.album._new_tracks:
-                    track.metadata.make_and_add_image(
-                        mime,
-                        data,
-                        types=coverartimage.types,
-                        comment=coverartimage.comment,
-                        is_front=coverartimage.is_front
-                    )
+                    track.metadata.append_image(coverartimage)
                 # If the image already was a front image,
                 # there might still be some other non-CAA front
                 # images in the queue - ignore them.

picard/coverartimage.py

@@ -0,0 +1,271 @@
+# -*- coding: utf-8 -*-
+#
+# Picard, the next-generation MusicBrainz tagger
+# Copyright (C) 2007 Oliver Charles
+# Copyright (C) 2007-2011 Philipp Wolfer
+# Copyright (C) 2007, 2010, 2011 Lukáš Lalinský
+# Copyright (C) 2011 Michael Wiencek
+# Copyright (C) 2011-2012 Wieland Hoffmann
+# Copyright (C) 2013-2014 Laurent Monin
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+import os.path
+import shutil
+import sys
+import tempfile
+import traceback
+
+
+from hashlib import md5
+from os import fdopen, unlink
+from PyQt4.QtCore import QUrl, QObject
+from picard import config, log
+from picard.util import (
+    encode_filename,
+    mimetype as mime,
+    replace_win32_incompat,
+)
+from picard.util.textencoding import (
+    replace_non_ascii,
+    unaccent,
+)
+
+
+class CoverArtImage:
+
+    support_types = False
+    # consider all images as front if types aren't supported by provider
+    is_front = True
+    sourceprefix = "URL"
+
+    def __init__(self, url=None, types=[u'front'], comment='',
+                 data=None, mimetype="image/jpeg"):
+        if url is not None:
+            self.parse_url(url)
+        else:
+            self.url = None
+        self.types = types
+        self.comment = comment
+        self.datahash = None
+        self.tempfile_filename = None
+        if data is not None:
+            self.set_data(data, mimetype=mimetype)
+
+    def parse_url(self, url):
+        self.url = QUrl(url)
+        self.host = str(self.url.host())
+        self.port = self.url.port(80)
+        self.path = str(self.url.encodedPath())
+        if self.url.hasQuery():
+            self.path += '?' + str(self.url.encodedQuery())
+
+    @property
+    def source(self):
+        if self.url is not None:
+            return u"%s: %s" % (self.sourceprefix, self.url.toString())
+        else:
+            return u"%s" % self.sourceprefix
+
+    def is_front_image(self):
+        # CAA has a flag for "front" image, use it in priority
+        if self.is_front:
+            return True
+        # no caa front flag, use type instead
+        return u'front' in self.types
+
+    def __repr__(self):
+        p = []
+        if self.url is not None:
+            p.append("url=%r" % self.url.toString())
+        p.append("types=%r" % self.types)
+        if self.comment:
+            p.append("comment=%r" % self.comment)
+        return "%s(%s)" % (self.__class__.__name__, ", ".join(p))
+
+    def __unicode__(self):
+        p = [u'Image']
+        if self.url is not None:
+            p.append(u"from %s" % self.url.toString())
+        p.append(u"of type %s" % u','.join(self.types))
+        if self.comment:
+            p.append(u"and comment '%s'" % self.comment)
+        return u' '.join(p)
+
+    def __str__(self):
+        return unicode(self).encode('utf-8')
+
+    def set_data(self, data, mimetype="image/jpeg", filename=None):
+        """Store image data in a file, if data already exists in such file
+           it will be re-used and no file write occurs
+           A reference counter is handling case where more than one
+           cover art image are using the same data.
+        """
+        self.datalength = len(data)
+        self.extension = mime.get_extension(mime, ".jpg")
+        self.filename = filename
+        self.mimetype = mimetype
+
+        m = md5()
+        m.update(data)
+        datahash = m.hexdigest()
+        if self.datahash is not None and datahash != self.datahash:
+            # data is about to be replaced, eventually delete old attached file
+            self.delete_data()
+        self.datahash = datahash
+        QObject.tagger.images.lock()
+        self.tempfile_filename, refcount = QObject.tagger.images[self.datahash]
+        assert(refcount >= 0)
+        assert((self.tempfile_filename is None and not refcount)
+               or (self.tempfile_filename is not None and refcount))
+        if not refcount:
+            (fd, self.tempfile_filename) = tempfile.mkstemp(prefix="picard",
+                                                             suffix=self.extension)
+            with fdopen(fd, "wb") as imagefile:
+                imagefile.write(data)
+                log.debug("Saving image for %r (hash=%s) to %r" %
+                          (self, self.datahash, self.tempfile_filename))
+        # reference counter is always increased
+        refcount += 1
+        QObject.tagger.images[self.datahash] = (self.tempfile_filename, refcount)
+        QObject.tagger.images.unlock()
+
+    def delete_data(self):
+        """Delete file containing data if needed, or just decrease reference
+           counter.
+        """
+        if self.datahash is None:
+            assert(self.tempfile_filename is None)
+            return
+        QObject.tagger.images.lock()
+        _tempfile_filename, refcount = QObject.tagger.images[self.datahash]
+        QObject.tagger.images.unlock()
+        assert(_tempfile_filename is not None)
+        refcount -= 1
+        assert(refcount >= 0)
+        if refcount:
+            # file still used by another CoverArtImage
+            self.tempfile_filename = None
+            return
+        os.unlink(_tempfile_filename)
+        self.tempfile_filename = None
+        QObject.tagger.images.lock()
+        del QObject.tagger.images[self.datahash]
+        QObject.tagger.images.unlock()
+        self.datahash = None
+
+    def __del__(self):
+        try:
+            self.delete_data()
+        except:
+            pass
+
+    def maintype(self):
+        return self.types[0]
+
+    def _make_image_filename(self, filename, dirname, metadata):
+        if config.setting["ascii_filenames"]:
+            if isinstance(filename, unicode):
+                filename = unaccent(filename)
+            filename = replace_non_ascii(filename)
+        if not filename:
+            filename = "cover"
+        if not os.path.isabs(filename):
+            filename = os.path.join(dirname, filename)
+        # replace incompatible characters
+        if config.setting["windows_compatibility"] or sys.platform == "win32":
+            filename = replace_win32_incompat(filename)
+        # remove null characters
+        filename = filename.replace("\x00", "")
+        return encode_filename(filename)
+
+    def save(self, dirname, metadata, counters):
+        """Saves this image.
+
+        :dirname: The name of the directory that contains the audio file
+        :metadata: A metadata object
+        :counters: A dictionary mapping filenames to the amount of how many
+                    images with that filename were already saved in `dirname`.
+        """
+        if self.filename is not None:
+            log.debug("Using the custom file name %s", self.filename)
+            filename = self.filename
+        elif config.setting["caa_image_type_as_filename"]:
+            filename = self.maintype()
+            log.debug("Make filename from types: %r -> %r", self.types, filename)
+        else:
+            log.debug("Using default file name %s",
+                      config.setting["cover_image_filename"])
+            filename = config.setting["cover_image_filename"]
+        filename = self._make_image_filename(filename, dirname, metadata)
+
+        overwrite = config.setting["save_images_overwrite"]
+        ext = self.extension
+        image_filename = filename
+        if counters[filename] > 0:
+            image_filename = "%s (%d)" % (filename, counters[filename])
+        counters[filename] = counters[filename] + 1
+        while os.path.exists(image_filename + ext) and not overwrite:
+            if os.path.getsize(image_filename + ext) == self.datalength:
+                log.debug("Identical file size, not saving %r", image_filename)
+                break
+            image_filename = "%s (%d)" % (filename, counters[filename])
+            counters[filename] = counters[filename] + 1
+        else:
+            new_filename = image_filename + ext
+            # Even if overwrite is enabled we don't need to write the same
+            # image multiple times
+            if (os.path.exists(new_filename) and
+                    os.path.getsize(new_filename) == self.datalength):
+                    log.debug("Identical file size, not saving %r", image_filename)
+                    return
+            log.debug("Saving cover images to %r", image_filename)
+            new_dirname = os.path.dirname(image_filename)
+            if not os.path.isdir(new_dirname):
+                os.makedirs(new_dirname)
+            shutil.copyfile(self.tempfile_filename, new_filename)
+
+    @property
+    def data(self):
+        """Reads the data from the temporary file created for this image. May
+        raise IOErrors or OSErrors.
+        """
+        if not self.tempfile_filename:
+            return None
+        with open(self.tempfile_filename, "rb") as imagefile:
+            return imagefile.read()
+
+
+class CaaCoverArtImage(CoverArtImage):
+
+    is_front = False
+    support_types = True
+    sourceprefix = u"CAA"
+
+
+class TagCoverArtImage(CoverArtImage):
+
+    def __init__(self, file, tag=None, types=[u'front'], is_front=True,
+                 support_types=False, comment='', data=None,
+                 mimetype='image/jpeg'):
+        CoverArtImage.__init__(self, url=None, types=types, comment=comment,
+                               data=data, mimetype=mimetype)
+        self.sourcefile = file
+        self.tag = tag
+        self.is_front = is_front
+        self.support_types = support_types
+
+    @property
+    def source(self):
+       return u'Tag %s from %s' % (self.tag if self.tag else '', self.sourcefile)

picard/formats/apev2.py

@@ -24,6 +24,7 @@ import mutagen.wavpack
 import mutagen.optimfrog
 import mutagenext.tak
 from picard import config, log
+from picard.coverartimage import TagCoverArtImage
 from picard.file import File
 from picard.metadata import Metadata, save_this_image_to_tags
 from picard.util import encode_filename, sanitize_date, mimetype
@@ -63,7 +64,16 @@ class APEv2File(File):
                     if '\0' in values.value:
                         descr, data = values.value.split('\0', 1)
                         mime = mimetype.get_from_data(data, descr, 'image/jpeg')
-                        metadata.make_and_add_image(mime, data)
+                        metadata.append_image(
+                            TagCoverArtImage(
+                                file=filename,
+                                tag=origname,
+                                support_types=False,
+                                data=data,
+                                mimetype=mime
+                            )
+                        )
+
                 # skip EXTERNAL and BINARY values
                 if values.kind != mutagen.apev2.TEXT:
                     continue

picard/formats/asf.py

@@ -18,6 +18,7 @@
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
 
 from picard import config, log
+from picard.coverartimage import TagCoverArtImage
 from picard.file import File
 from picard.formats.id3 import types_and_front, image_type_as_id3_num
 from picard.util import encode_filename
@@ -142,8 +143,18 @@ class ASFFile(File):
                 for image in values:
                     (mime, data, type, description) = unpack_image(image.value)
                     types, is_front = types_and_front(type)
-                    metadata.make_and_add_image(mime, data, comment=description,
-                                                types=types, is_front=is_front)
+                    metadata.append_image(
+                        TagCoverArtImage(
+                            file=filename,
+                            tag=name,
+                            types=types,
+                            is_front=is_front,
+                            comment=description,
+                            support_types=True,
+                            data=data,
+                            mimetype=mime
+                        )
+                    )
                 continue
             elif name not in self.__RTRANS:
                 continue
@@ -170,7 +181,7 @@ class ASFFile(File):
                     continue
                 tag_data = pack_image(image.mimetype, image.data,
                                       image_type_as_id3_num(image.maintype()),
-                                      image.description)
+                                      image.comment)
                 cover.append(ASFByteArrayAttribute(tag_data))
             if cover:
                 file.tags['WM/Picture'] = cover

picard/formats/id3.py

@@ -24,6 +24,7 @@ import re
 from collections import defaultdict
 from mutagen import id3
 from picard import config, log
+from picard.coverartimage import TagCoverArtImage
 from picard.metadata import Metadata, save_this_image_to_tags, MULTI_VALUED_JOINER
 from picard.file import File
 from picard.formats.mutagenext import compatid3
@@ -262,8 +263,18 @@ class ID3File(File):
                     log.error("Invalid %s value '%s' dropped in %r", frameid, frame.text[0], filename)
             elif frameid == 'APIC':
                 types, is_front = types_and_front(frame.type)
-                metadata.make_and_add_image(frame.mime, frame.data, comment=frame.desc,
-                                            types=types, is_front=is_front)
+                metadata.append_image(
+                    TagCoverArtImage(
+                        file=filename,
+                        tag=frameid,
+                        types=types,
+                        is_front=is_front,
+                        comment=frame.desc,
+                        support_types=True,
+                        data=frame.data,
+                        mimetype=frame.mime
+                    )
+                )
             elif frameid == 'POPM':
                 # Rating in ID3 ranges from 0 to 255, normalize this to the range 0 to 5
                 if frame.email == config.setting['rating_user_email']:
@@ -318,7 +329,7 @@ class ID3File(File):
             # any description.
             counters = defaultdict(lambda: 0)
             for image in metadata.images:
-                desc = desctag = image.description
+                desc = desctag = image.comment
                 if not save_this_image_to_tags(image):
                     continue
                 if counters[desc] > 0:

picard/formats/mp4.py

@@ -19,6 +19,7 @@
 
 from mutagen.mp4 import MP4, MP4Cover
 from picard import config, log
+from picard.coverartimage import TagCoverArtImage
 from picard.file import File
 from picard.metadata import Metadata, save_this_image_to_tags
 from picard.util import encode_filename
@@ -141,9 +142,20 @@ class MP4File(File):
             elif name == "covr":
                 for value in values:
                     if value.imageformat == value.FORMAT_JPEG:
-                        metadata.make_and_add_image("image/jpeg", value)
+                        mime = "image/jpeg"
                     elif value.imageformat == value.FORMAT_PNG:
-                        metadata.make_and_add_image("image/png", value)
+                        mime = "image/png"
+                    else:
+                        continue
+                    metadata.append_image(
+                        TagCoverArtImage(
+                            file=filename,
+                            tag=name,
+                            support_types=False,
+                            data=value,
+                            mimetype=mime
+                        )
+                    )
 
         self._info(metadata, file)
         return metadata

picard/formats/vorbis.py

@@ -31,6 +31,7 @@ except ImportError:
     OggOpus = None
     with_opus = False
 from picard import config, log
+from picard.coverartimage import TagCoverArtImage
 from picard.file import File
 from picard.formats.id3 import types_and_front, image_type_as_id3_num
 from picard.metadata import Metadata, save_this_image_to_tags
@@ -97,10 +98,18 @@ class VCommentFile(File):
                 elif name == "metadata_block_picture":
                     image = mutagen.flac.Picture(base64.standard_b64decode(value))
                     types, is_front = types_and_front(image.type)
-                    metadata.make_and_add_image(image.mime, image.data,
-                                                comment=image.desc,
-                                                types=types,
-                                                is_front=is_front)
+                    metadata.append_image(
+                        TagCoverArtImage(
+                            file=filename,
+                            tag=name,
+                            types=types,
+                            is_front=is_front,
+                            comment=image.desc,
+                            support_types=True,
+                            data=image.data,
+                            mimetype=image.mime
+                        )
+                    )
                     continue
                 elif name in self.__translate:
                     name = self.__translate[name]
@@ -108,15 +117,31 @@ class VCommentFile(File):
         if self._File == mutagen.flac.FLAC:
             for image in file.pictures:
                 types, is_front = types_and_front(image.type)
-                metadata.make_and_add_image(image.mime, image.data, comment=image.desc,
-                                            types=types, is_front=is_front)
+                coverartimage = TagCoverArtImage(
+                    file=filename,
+                    tag='FLAC/PICTURE',
+                    types=types,
+                    is_front=is_front,
+                    comment=image.desc,
+                    support_types=True,
+                    data=image.data,
+                    mimetype=image.mime
+                )
+                metadata.append_image(coverartimage)
         # Read the unofficial COVERART tags, for backward compatibillity only
         if not "metadata_block_picture" in file.tags:
             try:
                 for index, data in enumerate(file["COVERART"]):
-                    metadata.make_and_add_image(file["COVERARTMIME"][index],
-                                                base64.standard_b64decode(data)
-                                                )
+                    mime = file["COVERARTMIME"][index]
+                    data = base64.standard_b64decode(data)
+                    coverartimage = TagCoverArtImage(
+                        file=filename,
+                        tag='COVERART',
+                        support_types=False,
+                        data=data,
+                        mimetype=mime
+                    )
+                    metadata.append_image(coverartimage)
             except KeyError:
                 pass
         self._info(metadata, file)
@@ -175,7 +200,7 @@ class VCommentFile(File):
                 picture = mutagen.flac.Picture()
                 picture.data = image.data
                 picture.mime = image.mimetype
-                picture.desc = image.description
+                picture.desc = image.comment
                 picture.type = image_type_as_id3_num(image.maintype())
                 if self._File == mutagen.flac.FLAC:
                     file.add_picture(picture)

picard/metadata.py

@@ -51,108 +51,6 @@ def save_this_image_to_tags(image):
     return image.is_front
 
 
-class Image(object):
-
-    """Wrapper around images. Instantiating an object of this class can raise
-    an IOError or OSError due to the usage of tempfiles underneath.
-    """
-
-    def __init__(self, data, mimetype="image/jpeg", types=[u"front"],
-                 comment="", filename=None, datahash="", is_front=True):
-        self.description = comment
-        (fd, self._tempfile_filename) = tempfile.mkstemp(prefix="picard")
-        with fdopen(fd, "wb") as imagefile:
-            imagefile.write(data)
-            log.debug("Saving image (hash=%s) to %r" % (datahash,
-                                                        self._tempfile_filename))
-        self.datalength = len(data)
-        self.extension = mime.get_extension(mime, ".jpg")
-        self.filename = filename
-        self.types = types
-        self.is_front = is_front
-        self.mimetype = mimetype
-
-    def maintype(self):
-        return self.types[0]
-
-    def _make_image_filename(self, filename, dirname, metadata):
-        if config.setting["ascii_filenames"]:
-            if isinstance(filename, unicode):
-                filename = unaccent(filename)
-            filename = replace_non_ascii(filename)
-        if not filename:
-            filename = "cover"
-        if not os.path.isabs(filename):
-            filename = os.path.join(dirname, filename)
-        # replace incompatible characters
-        if config.setting["windows_compatibility"] or sys.platform == "win32":
-            filename = replace_win32_incompat(filename)
-        # remove null characters
-        filename = filename.replace("\x00", "")
-        return encode_filename(filename)
-
-    def save(self, dirname, metadata, counters):
-        """Saves this image.
-
-        :dirname: The name of the directory that contains the audio file
-        :metadata: A metadata object
-        :counters: A dictionary mapping filenames to the amount of how many
-                    images with that filename were already saved in `dirname`.
-        """
-        if self.filename is not None:
-            log.debug("Using the custom file name %s", self.filename)
-            filename = self.filename
-        elif config.setting["caa_image_type_as_filename"]:
-            filename = self.maintype()
-            log.debug("Make filename from types: %r -> %r", self.types, filename)
-        else:
-            log.debug("Using default file name %s",
-                      config.setting["cover_image_filename"])
-            filename = config.setting["cover_image_filename"]
-        filename = self._make_image_filename(filename, dirname, metadata)
-
-        overwrite = config.setting["save_images_overwrite"]
-        ext = self.extension
-        image_filename = filename
-        if counters[filename] > 0:
-            image_filename = "%s (%d)" % (filename, counters[filename])
-        counters[filename] = counters[filename] + 1
-        while os.path.exists(image_filename + ext) and not overwrite:
-            if os.path.getsize(image_filename + ext) == self.datalength:
-                log.debug("Identical file size, not saving %r", image_filename)
-                break
-            image_filename = "%s (%d)" % (filename, counters[filename])
-            counters[filename] = counters[filename] + 1
-        else:
-            new_filename = image_filename + ext
-            # Even if overwrite is enabled we don't need to write the same
-            # image multiple times
-            if (os.path.exists(new_filename) and
-                    os.path.getsize(new_filename) == self.datalength):
-                    log.debug("Identical file size, not saving %r", image_filename)
-                    return
-            log.debug("Saving cover images to %r", image_filename)
-            new_dirname = os.path.dirname(image_filename)
-            if not os.path.isdir(new_dirname):
-                os.makedirs(new_dirname)
-            shutil.copyfile(self._tempfile_filename, new_filename)
-
-    @property
-    def data(self):
-        """Reads the data from the temporary file created for this image. May
-        raise IOErrors or OSErrors.
-        """
-        with open(self._tempfile_filename, "rb") as imagefile:
-            return imagefile.read()
-
-    def _delete(self):
-        log.debug("Unlinking %s", self._tempfile_filename)
-        try:
-            unlink(self._tempfile_filename)
-        except OSError as e:
-            log.error(traceback.format_exc())
-
-
 class Metadata(dict):
 
     """List of metadata items with dict-like access."""
@@ -172,32 +70,9 @@ class Metadata(dict):
         self.images = []
         self.length = 0
 
-    def make_and_add_image(self, mime, data, filename=None, comment="",
-                           types=[u"front"], is_front=True):
-        """Build a new image object from ``data`` and adds it to this Metadata
-        object. If an image with the same MD5 hash has already been added to
-        any Metadata object, that file will be reused.
-
-        Arguments:
-        mime -- The mimetype of the image
-        data -- The image data
-        filename -- The image filename, without an extension
-        comment -- image description or comment, default to ''
-        types -- list of types, default to [u'front']
-        is_front -- mark image as front image
-        """
-        m = md5()
-        m.update(data)
-        datahash = m.hexdigest()
-        QObject.tagger.images.lock()
-        image = QObject.tagger.images[datahash]
-        if image is None:
-            image = Image(data, mime, types, comment, filename,
-                          datahash=datahash,
-                          is_front=is_front)
-            QObject.tagger.images[datahash] = image
-        QObject.tagger.images.unlock()
-        self.images.append(image)
+    def append_image(self, coverartimage):
+        assert(coverartimage is not None)
+        self.images.append(coverartimage)
 
     def remove_image(self, index):
         self.images.pop(index)

picard/tagger.py

@@ -195,7 +195,7 @@ class Tagger(QtGui.QApplication):
         self.albums = {}
         self.release_groups = {}
         self.mbid_redirects = {}
-        self.images = LockableDefaultDict(lambda: None)
+        self.images = LockableDefaultDict(lambda: (None, 0))
         self.unmatched_files = UnmatchedFiles()
         self.nats = None
         self.window = MainWindow()
@@ -248,12 +248,16 @@ class Tagger(QtGui.QApplication):
 
     def exit(self):
         log.debug("exit")
-        map(lambda i: i._delete(), self.images.itervalues())
         self.stopping = True
         self._acoustid.done()
         self.thread_pool.waitForDone()
         self.browser_integration.stop()
         self.xmlws.stop()
+        for filename, refcount in self.images.itervalues():
+            try:
+                os.unlink(filename)
+            except OSError:
+                pass
 
     def _run_init(self):
         if self._args:

picard/ui/coverartbox.py

@@ -18,9 +18,11 @@
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
 
 import os
+from functools import partial
 from PyQt4 import QtCore, QtGui, QtNetwork
 from picard import config, log
 from picard.album import Album
+from picard.coverartimage import CoverArtImage
 from picard.track import Track
 from picard.file import File
 from picard.util import webbrowser2, encode_filename
@@ -158,7 +160,8 @@ class CoverArtBox(QtGui.QGroupBox):
             if url.hasQuery():
                 path += '?' + url.encodedQuery()
             self.tagger.xmlws.get(url.encodedHost(), url.port(80), path,
-                                  self.on_remote_image_fetched, xml=False,
+                                  partial(self.on_remote_image_fetched, url),
+                                  xml=False,
                                   priority=True, important=True)
         elif url.scheme() == 'file':
             path = encode_filename(unicode(url.toLocalFile()))
@@ -167,12 +170,12 @@ class CoverArtBox(QtGui.QGroupBox):
                 mime = 'image/png' if path.lower().endswith('.png') else 'image/jpeg'
                 data = f.read()
                 f.close()
-                self.load_remote_image(mime, data)
+                self.load_remote_image(url, mime, data)
 
-    def on_remote_image_fetched(self, data, reply, error):
+    def on_remote_image_fetched(self, url, data, reply, error):
         mime = reply.header(QtNetwork.QNetworkRequest.ContentTypeHeader)
         if mime in ('image/jpeg', 'image/png'):
-            self.load_remote_image(mime, data)
+            self.load_remote_image(url, mime, data)
         elif reply.url().hasQueryItem("imgurl"):
             # This may be a google images result, try to get the URL which is encoded in the query
             url = QtCore.QUrl(reply.url().queryItemValue("imgurl"))
@@ -180,24 +183,29 @@ class CoverArtBox(QtGui.QGroupBox):
         else:
             log.warning("Can't load image with MIME-Type %s", mime)
 
-    def load_remote_image(self, mime, data):
+    def load_remote_image(self, url, mime, data):
         pixmap = QtGui.QPixmap()
         if not pixmap.loadFromData(data):
             log.warning("Can't load image")
             return
         self.__set_data([mime, data], pixmap=pixmap)
+        coverartimage = CoverArtImage(
+            url=url.toString(),
+            data=data,
+            mimetype=mime
+        )
         if isinstance(self.item, Album):
             album = self.item
-            album.metadata.make_and_add_image(mime, data)
+            album.metadata.append_image(coverartimage)
             for track in album.tracks:
-                track.metadata.make_and_add_image(mime, data)
+                track.metadata.append_image(coverartimage)
             for file in album.iterfiles():
-                file.metadata.make_and_add_image(mime, data)
+                file.metadata.append_image(coverartimage)
         elif isinstance(self.item, Track):
             track = self.item
-            track.metadata.make_and_add_image(mime, data)
+            track.metadata.append_image(coverartimage)
             for file in track.iterfiles():
-                file.metadata.make_and_add_image(mime, data)
+                file.metadata.append_image(coverartimage)
         elif isinstance(self.item, File):
             file = self.item
-            file.metadata.make_and_add_image(mime, data)
+            file.metadata.append_image(coverartimage)

picard/ui/infodialog.py

@@ -22,6 +22,7 @@ import cgi
 import traceback
 from PyQt4 import QtGui, QtCore
 from picard import log
+from picard.coverartarchive import translate_caa_type
 from picard.util import format_time, encode_filename, bytes2human
 from picard.ui import PicardDialog
 from picard.ui.ui_infodialog import Ui_InfoDialog
@@ -56,17 +57,23 @@ class InfoDialog(PicardDialog):
             except (OSError, IOError) as e:
                 log.error(traceback.format_exc())
                 continue
-            size = len(data)
+            size = image.datalength
             item = QtGui.QListWidgetItem()
             pixmap = QtGui.QPixmap()
             pixmap.loadFromData(data)
             icon = QtGui.QIcon(pixmap)
             item.setIcon(icon)
-            s = "%s (%s)\n%d x %d" % (bytes2human.decimal(size),
-                                      bytes2human.binary(size),
-                                      pixmap.width(),
-                                      pixmap.height())
+            s = u"%s (%s)\n%d x %d\n%s" % (
+                bytes2human.decimal(size),
+                bytes2human.binary(size),
+                pixmap.width(),
+                pixmap.height(),
+                ','.join([translate_caa_type(t) for t in image.types])
+            )
+            if image.comment:
+                s += u"\n%s" % image.comment
             item.setText(s)
+            item.setToolTip(image.source)
             self.ui.artwork_list.addItem(item)
 
     def tab_hide(self, widget):

test/test_formats.py

@@ -7,6 +7,7 @@ import shutil
 from PyQt4 import QtCore
 from picard.util import LockableDefaultDict
 from picard import config, log
+from picard.coverartimage import CoverArtImage
 from picard.metadata import Metadata
 from tempfile import mkstemp
 
@@ -36,7 +37,7 @@ class FakeTagger(QtCore.QObject):
         QtCore.QObject.config = config
         QtCore.QObject.log = log
         self.tagger_stats_changed.connect(self.emit)
-        self.images = LockableDefaultDict(lambda: None)
+        self.images = LockableDefaultDict(lambda: (None, 0))
 
     def emit(self, *args):
         pass
@@ -508,9 +509,56 @@ class TestCoverArt(unittest.TestCase):
         QtCore.QObject.tagger = FakeTagger()
 
     def _tear_down(self):
-        map(lambda i: i._delete(), QtCore.QObject.tagger.images.itervalues())
+        for filename, refcount in QtCore.QObject.tagger.images.itervalues():
+            if refcount:
+                os.unlink(filename)
         os.unlink(self.filename)
 
+    def test_coverartimage(self):
+        dummyload = "x" * 1024 * 128
+        tests = {
+            'jpg': {
+                'mime': 'image/jpeg',
+                'head': 'JFIF'
+            },
+            'png': {
+                'mime': 'image/png',
+                'head': 'PNG'
+            },
+        }
+        tmp_files = []
+        for t in tests:
+            imgdata = tests[t]['head'] + dummyload
+            imgdata2 = imgdata + 'xxx'
+            # set data once
+            coverartimage = CoverArtImage(
+                data=imgdata2,
+                mimetype=tests[t]['mime']
+            )
+            tmp_file = coverartimage.tempfile_filename
+            tmp_files.append(tmp_file)
+            l = os.path.getsize(tmp_file)
+            # ensure file was written, and check its length
+            self.assertEqual(l, len(imgdata2))
+            self.assertEqual(coverartimage.data, imgdata2)
+            # delete file (and data)
+            coverartimage.delete_data()
+            self.assertEqual(coverartimage.data, None)
+            # set data again, with another payload
+            coverartimage.set_data(imgdata, tests[t]['mime'])
+            tmp_file = coverartimage.tempfile_filename
+            tmp_files.append(tmp_file)
+            l = os.path.getsize(tmp_file)
+            # check file length again
+            self.assertEqual(l, len(imgdata))
+            self.assertEqual(coverartimage.data, imgdata)
+            # delete the object, file should be deleted too
+            del coverartimage
+
+        # check if all files were deleted
+        for f in tmp_files:
+            self.assertEqual(os.path.isfile(f), False)
+
     def test_asf(self):
         self._test_cover_art(os.path.join('test', 'data', 'test.wma'))
 
@@ -549,7 +597,12 @@ class TestCoverArt(unittest.TestCase):
                 f = picard.formats.open(self.filename)
                 metadata = Metadata()
                 imgdata = tests[t]['head'] + dummyload
-                metadata.make_and_add_image(tests[t]['mime'], imgdata)
+                metadata.append_image(
+                    CoverArtImage(
+                        data=imgdata,
+                        mimetype=tests[t]['mime']
+                    )
+                )
                 f._save(self.filename, metadata)
 
                 f = picard.formats.open(self.filename)