Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PICARD-1001 Rework local cover art handling #1934

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions picard/const/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@
DEFAULT_NUMBERED_SCRIPT_NAME = N_("My script %d")
DEFAULT_SCRIPT_NAME = N_("My script")
DEFAULT_COVER_IMAGE_FILENAME = "cover"
DEFAULT_LOCAL_COVER_ART_REGEX = r'^(?:cover|folder|albumart)(.*)\.(?:jpe?g|png|gif|tiff?|webp)$'
DEFAULT_NUMBERED_PROFILE_NAME = N_("My profile %d")
DEFAULT_PROFILE_NAME = N_("My profile")

Expand Down
3 changes: 3 additions & 0 deletions picard/coverart/image.py
Original file line number Diff line number Diff line change
Expand Up @@ -480,3 +480,6 @@ def __init__(self, filepath, types=None, comment='',
super().__init__(url=url, types=types, comment=comment)
self.support_types = support_types
self.support_multi_types = support_multi_types
path = self.url.toLocalFile()
with open(path, 'rb') as file:
self.set_data(file.read())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think loading the data here makes sense, but this needs fixing a couple of tests that currently fail due to this. The LocalFileCoverArtImageTest currently works without files, should be updated to work on temporary files.

2 changes: 0 additions & 2 deletions picard/coverart/providers/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
from picard.coverart.providers.caa_release_group import (
CoverArtProviderCaaReleaseGroup,
)
from picard.coverart.providers.local import CoverArtProviderLocal
from picard.coverart.providers.provider import ( # noqa: F401 # pylint: disable=unused-import
CoverArtProvider,
ProviderOptions,
Expand Down Expand Up @@ -92,7 +91,6 @@ def label(p):


__providers = [
CoverArtProviderLocal,
CoverArtProviderCaa,
CoverArtProviderUrlRelationships,
CoverArtProviderCaaReleaseGroup,
Expand Down
123 changes: 0 additions & 123 deletions picard/coverart/providers/local.py

This file was deleted.

52 changes: 52 additions & 0 deletions picard/file.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,11 @@
IS_MACOS,
IS_WIN,
)
from picard.coverart.image import (
CoverArtImageError,
LocalFileCoverArtImage
)
from picard.coverart.utils import CAA_TYPES
from picard.metadata import (
Metadata,
SimMatchTrack,
Expand Down Expand Up @@ -240,6 +245,11 @@ def _loading_finished(self, callback, result=None, error=None):
postprocessors = []
if config.setting["guess_tracknumber_and_title"]:
postprocessors.append(self._guess_tracknumber_and_title)
# If no cover art was loaded from file tags, try loading from a local file
# TODO: should this be a preference? should we load from both sources?
log.debug("load local cover art: %r", config.setting["load_local_cover_art"])
if config.setting["load_local_cover_art"] and len(result.images) == 0:
self._load_local_cover_art(result, config)
self._copy_loaded_metadata(result, postprocessors)
# use cached fingerprint from file metadata
if not config.setting["ignore_existing_acoustid_fingerprints"]:
Expand All @@ -250,6 +260,48 @@ def _loading_finished(self, callback, result=None, error=None):
self.update()
callback(self)

_types_split_re = re.compile('[^a-z0-9]', re.IGNORECASE)
_known_types = set([t['name'] for t in CAA_TYPES])
_default_types = ['front']
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be written directly as {t['name'] for t in CAA_TYPES}


def get_types(self, string):
found = set([x.lower() for x in self._types_split_re.split(string) if x])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can also be done with a set comprehension {x.lower() for x in self._types_split_re.split(string) if x}

return list(found.intersection(self._known_types))

def _load_local_cover_art(self, metadata, config):
log.debug("Attempting to load cover art from local files")
match_re = re.compile(config.setting['local_cover_regex'], re.IGNORECASE)
current_dir = os.path.dirname(self.filename)
for root, dirs, files in os.walk(current_dir):
for filename in files:
m = match_re.search(filename)
if not m:
continue
filepath = os.path.join(current_dir, root, filename)
if not os.path.exists(filepath):
continue
try:
type_from_filename = self.get_types(m.group(1))
except IndexError:
type_from_filename = []
try:
coverartimage = LocalFileCoverArtImage(
filepath,
types=type_from_filename or self._default_types,
support_types=True,
support_multi_types=False
)

log.debug("Loaded local cover art image: %r", coverartimage)
except OSError as exc:
(errnum, errmsg) = exc.args
log.error("Failed to read %r: %s (%d)" %
(filepath, errmsg, errnum))
except (CoverArtImageError) as e:
log.error('Cannot load image from %r: %s' % (filename, e))
else:
metadata.images.append(coverartimage)

def _copy_loaded_metadata(self, metadata, postprocessors=None):
metadata['~length'] = format_time(metadata.length)
if postprocessors:
Expand Down
7 changes: 6 additions & 1 deletion picard/ui/options/cover.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,10 @@
TextOption,
get_config,
)
from picard.const import DEFAULT_COVER_IMAGE_FILENAME
from picard.const import (
DEFAULT_COVER_IMAGE_FILENAME,
DEFAULT_LOCAL_COVER_ART_REGEX,
)
from picard.coverart.providers import cover_art_providers

from picard.ui.checkbox_list_item import CheckboxListItem
Expand Down Expand Up @@ -61,6 +64,8 @@ class CoverOptionsPage(OptionsPage):
BoolOption("setting", "save_images_overwrite", False),
BoolOption("setting", "save_only_one_front_image", False),
BoolOption("setting", "image_type_as_filename", False),
BoolOption("setting", "load_local_cover_art", False),
TextOption("setting", "local_cover_regex", DEFAULT_LOCAL_COVER_ART_REGEX),
ListOption("setting", "ca_providers", [
('Cover Art Archive', True),
('UrlRelationships', True),
Expand Down
83 changes: 0 additions & 83 deletions ui/provider_options_local.ui

This file was deleted.