Skip to content

Allow fetchart to manipulate cover art candidates using all selected options#5440

Closed
mlbaquerizo wants to merge 11 commits intobeetbox:masterfrom
mbaquerizo:unify-fetchart-convert-options
Closed

Allow fetchart to manipulate cover art candidates using all selected options#5440
mlbaquerizo wants to merge 11 commits intobeetbox:masterfrom
mbaquerizo:unify-fetchart-convert-options

Conversation

@mlbaquerizo
Copy link

Description

Fixes #4452.

Also, addresses conversations around candidate validation and refactoring ArtResizer to manipulate cover art in one go.
See #4133 (comment)

I felt that the cover art candidate validation was doing more than it should. In no instance where a candidate was considered for any manipulation was CANDIDATE_BAD returned, so I interpreted that as the candidate being valid. I adapted the work done in #4606 along with some of the feedback it received and used the pattern of returning True or False. I also interpreted CANDIDATE_EXACT to mean that no further checks are needed, so returned straight away in cases where CANDIDATE_EXACT was returned before.

Now, the Candidate convert method is responsible for determining if any manipulation is needed, puts all those options into one object, and passes them to ArtResizer.convert if manipulation is needed.

I tried to make the new convert functions behave the same way that resize, convert_format, and deinterlace worked while combining their functionality. Reviewers should double check this. The only difference is that I no longer explicitly deinterlace unless the deinterlace option is set.

To Do

  • Documentation (bugfix and refactor)
  • Changelog.
  • Tests.

@stale
Copy link

stale bot commented Apr 26, 2025

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Apr 26, 2025
@mlbaquerizo
Copy link
Author

@wisp3rwind I would love a set of eyes on this. I think its still relevant. Tagged you because you were part of some related conversations in related issues/PRs

@snejus snejus removed the stale label May 4, 2025
@snejus snejus requested a review from a team as a code owner October 14, 2025 22:30
@JOJ0 JOJ0 added plugin Pull requests that are plugins related and removed review-needed labels Jan 10, 2026
@snejus
Copy link
Member

snejus commented Feb 23, 2026

I've only now noticed this PR, apologies @mlbaquerizo. Are you happy to rebase it?

@mlbaquerizo
Copy link
Author

I've only now noticed this PR, apologies @mlbaquerizo. Are you happy to rebase it?

hey @snejus thanks for taking a look! I'm in the process of rebasing now. Some tests have changed so I need to address that. Almost there though, just letting you know.

This consolidates options passed to the fetchart plugin that would
require ImageMagick to adjust the cover art into a single function.
Candidate now returns False if Candidate is not valid and True if
it is. It is no longer concerned about any adjustments the plugin
needs to make to the Candidate.
The convert function will determine if the maxwidth, max_filesize,
cover_format, or deinterlace options should be applied and passes
all applicable options to ArtResizer's convert.
A test for cover format was also added to ensure that an image
can be converted from png to jpg with deinterlacing
These functions have been replaced by `convert`
@mlbaquerizo mlbaquerizo force-pushed the unify-fetchart-convert-options branch from 90d026a to fce8dfe Compare March 13, 2026 04:40
@mlbaquerizo
Copy link
Author

Looks like i have some work to do. Feel free to close and I can reopen if/when I fix everything? Lots of changes and I haven't looked at this project in a while

@snejus
Copy link
Member

snejus commented Mar 13, 2026

Looks like i have some work to do. Feel free to close and I can reopen if/when I fix everything? Lots of changes and I haven't looked at this project in a while

Yeah this makes sense, thanks!

@snejus snejus closed this Mar 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

plugin Pull requests that are plugins related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fetchart cover_format does not seem to be working (PNG => JPEG)

4 participants