diff --git a/picard/coverart.py b/picard/coverart.py index a3e81b439..3a99951dd 100644 --- a/picard/coverart.py +++ b/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. diff --git a/picard/coverartimage.py b/picard/coverartimage.py new file mode 100644 index 000000000..90bbc2f55 --- /dev/null +++ b/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) diff --git a/picard/formats/apev2.py b/picard/formats/apev2.py index e358e5a1c..188087ac0 100644 --- a/picard/formats/apev2.py +++ b/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 diff --git a/picard/formats/asf.py b/picard/formats/asf.py index 75e95d627..1747f341d 100644 --- a/picard/formats/asf.py +++ b/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 diff --git a/picard/formats/id3.py b/picard/formats/id3.py index 7aacfc89f..e70c1561d 100644 --- a/picard/formats/id3.py +++ b/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: diff --git a/picard/formats/mp4.py b/picard/formats/mp4.py index 576f92cf4..77ff2202f 100644 --- a/picard/formats/mp4.py +++ b/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 diff --git a/picard/formats/vorbis.py b/picard/formats/vorbis.py index 1e42f9bca..1aff8aa4e 100644 --- a/picard/formats/vorbis.py +++ b/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) diff --git a/picard/metadata.py b/picard/metadata.py index 4b336f244..f370fe021 100644 --- a/picard/metadata.py +++ b/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) diff --git a/picard/tagger.py b/picard/tagger.py index 8a1a43f61..61ee173a3 100644 --- a/picard/tagger.py +++ b/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: diff --git a/picard/ui/coverartbox.py b/picard/ui/coverartbox.py index f20aba56d..dba4f42ff 100644 --- a/picard/ui/coverartbox.py +++ b/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) diff --git a/picard/ui/infodialog.py b/picard/ui/infodialog.py index c775c48fb..97ac89f59 100644 --- a/picard/ui/infodialog.py +++ b/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): diff --git a/test/test_formats.py b/test/test_formats.py index e880f81d9..336a525fb 100644 --- a/test/test_formats.py +++ b/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)