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

CoverArt Post Processing #347

Open
wants to merge 4 commits into
base: 2.0
Choose a base branch
from
Open
Changes from 2 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
77 changes: 77 additions & 0 deletions plugins/coverart_processing/coverart_processing.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
PLUGIN_NAME = "CoverArt Processing"
PLUGIN_AUTHOR = "Pranay"
PLUGIN_DESCRIPTION = """
Post Processing Features for the CoverArt Images.
Ignore images smaller than specified width and height.
Resize the image if larger than specified dimensions.
"""
PLUGIN_VERSION = '0.1'
PLUGIN_API_VERSIONS = ['2.2']
PLUGIN_LICENSE = "GPL-2.0"
PLUGIN_LICENSE_URL = "https://www.gnu.org/licenses/gpl-2.0.html"

from PIL import Image
Copy link
Contributor

@rdswift rdswift Mar 27, 2023

Choose a reason for hiding this comment

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

Does the PIL library need to be installed separately? If so, how will this be handled on systems (e.g. Windows) where Picard has been installed using a binary installer and the system does not have Python installed?

Copy link
Author

@Pranay-Pandey Pranay-Pandey Apr 2, 2023

Choose a reason for hiding this comment

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

Should I package the PIL library along with the plugin?

Copy link
Contributor

Choose a reason for hiding this comment

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

You need to handle several use cases, where the location of the PIL library may vary and so the import statement may need to vary:

  1. Picard executable downloading the plugin - so you need either to include PIL or add code to download it.

  2. Picard running from source - PIL not already installed - as 1.

  3. Picard running from source + Python install already incudes PIL - you will need to decide which PIL to call.

from io import BytesIO
from picard.util import imageinfo
from picard import log
from picard.metadata import register_track_metadata_processor
from picard.plugin import PluginPriority

MAX_DIMENSION = 600 # Set maximum allowable dimensions of image in pixels
MIN_DIMENSION = 400 # Set minimum allowable dimensions of image in pixels
Copy link
Collaborator

Choose a reason for hiding this comment

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

Those should be made user-configurable as plugin config options.

Copy link
Author

Choose a reason for hiding this comment

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

Should a window open every time the user opens the Picard application?

image

Copy link
Contributor

Choose a reason for hiding this comment

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

No. It should be added to the config settings pages. See the code in https://github.com/metabrainz/picard-plugins/tree/2.0/plugins/format_performer_tags for an example of how this can be done.


def ignore_image(img_data):
"""Ignore The image file if the dimensions are smaller than a predefined"""
(width, height, _, _, _) = imageinfo.identify(img_data)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Picard is using _() for gettext translations, so it isn't recommended to use _ for unused variables,.

if width < MIN_DIMENSION or height < MIN_DIMENSION:
return True
return False

def resize_image(image_data, max_size=MAX_DIMENSION):
"""Resize the image to max_size and center crop if larger than max_size."""
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps a user-configurable option could be to determine whether the image should be center cropped or filled (with a specific color) to provide a square image?

with Image.open(BytesIO(image_data)) as img:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would be extra careful about potential exceptions here, we do not want the plugin to crash the whole app in cased of malformed images.
Also I wonder about performance impact (and threading).

width, height = img.size
if width > max_size or height > max_size:
if width > height:
new_width = max_size
new_height = int(height * (max_size / width))
else:
new_height = max_size
new_width = int(width * (max_size / height))
img = img.resize((new_width, new_height), resample=Image.LANCZOS)
left = (new_width - max_size) / 2
top = (new_height - max_size) / 2
right = left + max_size
bottom = top + max_size
img = img.crop((left, top, right, bottom))
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps first check if new_width or new_height is greater than max_size and skip this processing if not?

output_buffer = BytesIO()
img.save(output_buffer, format='JPEG')
return output_buffer.getvalue()

Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't the original image be returned if it is within the specified size parameters? Otherwise there could be a problem in Lines 64-65.

def Track_images(album, metadata, track, release):
try:
for ids,image in enumerate(metadata.images):
image_data = image.data
if image_data is not None:
if ignore_image(image_data):
# Removing the image
log.debug("Cover art image removed from metadata: %r [%s]" % (
image,
image.imageinfo_as_string())
)
metadata.images.pop(ids)
else:
img_data_edited = resize_image(image_data)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess some levels of caching would avoid extra processing.
This approach is rather simplistic, do the plugin resize multiple times the same image?

metadata.images[ids].set_data(img_data_edited)

log.debug("Cover art image processed: %r [%s]" % (
image,
image.imageinfo_as_string())
)
except Exception as ex:
log.error("{0}: Error: {1}".format(PLUGIN_NAME, ex,))


# Register the plugin to run at a HIGH priority so that other plugins will
# not have an opportunity to modify the contents of the metadata provided.
register_track_metadata_processor(Track_images, priority=PluginPriority.HIGH)