Add setting "min_art_size" to prevent generating very small covers #68
Conversation
For instance I see sometimes empty covers that are actually just a white pixel. They shouldn't be considered as album covers.
|
PS: it's mostly to get my hands on the code, but I find that enhancement useful anyway |
README.rst
Outdated
| @@ -181,6 +184,13 @@ Project resources | |||
| - `Changelog <https://github.com/mopidy/mopidy-local/releases>`_ | |||
|
|
|||
|
|
|||
| Build locally and run | |||
There was a problem hiding this comment.
We have development envinronment guidelines at https://docs.mopidy.com/en/latest/devenv/#working-on-extensions and this doesn't match that. We don't want to maintain instructions for each extension when they can just as well use the standard setup.
There was a problem hiding this comment.
thanks! I was looking for such doc directly in the plugin's readme .. may I suggest to add a link from there?
There was a problem hiding this comment.
Yes, it seems we can never have enough links back to the docs. We might as well add it to https://github.com/mopidy/cookiecutter-mopidy-ext at some point
There was a problem hiding this comment.
by the way, this is an amazing doc that you have, this is rare enough to be reported! really, congrats
(and shame on me for not having seen it before)
README.rst
Outdated
| @@ -181,6 +184,13 @@ Project resources | |||
| - `Changelog <https://github.com/mopidy/mopidy-local/releases>`_ | |||
|
|
|||
|
|
|||
| Build locally and run | |||
There was a problem hiding this comment.
Yes, it seems we can never have enough links back to the docs. We might as well add it to https://github.com/mopidy/cookiecutter-mopidy-ext at some point
mopidy_local/ext.conf
Outdated
| @@ -44,3 +44,6 @@ use_artist_sortname = false | |||
| # a list of file names to check for when searching for external album | |||
| # art; may contain UNIX shell patterns, i.e. "*", "?", etc. | |||
| album_art_files = *.jpg, *.jpeg, *.png | |||
|
|
|||
| # minimum required size, in pixels, for an image to be picked up as an album cover | |||
| min_art_size = 32 | |||
There was a problem hiding this comment.
Part of me think it's not totally obvious this means the width and height, and not the total pixel count. We should also say this new feature is disabled by setting the value to 0, or making it optional. And arguably 0 should be the default value else this is a breaking change.
And I think we should call it album_art_min_size to be more in line with the existing setting names.
There was a problem hiding this comment.
maybe "dimension" would be less ambiguous than "size" ?
There was a problem hiding this comment.
Perhaps just make the comment clearer. And it needs a corresponding entry in the configuration section of the readme
There was a problem hiding this comment.
comment updated (and name also)
It's already documented in the readme too
btw, two tests added
|
Thanks for the review @kingosticks
Yes, it looks so 👀 |
Co-authored-by: Nick Steel <[email protected]>
Add tests of album covers processing
For instance I see sometimes empty covers that are actually just a white
pixel. They shouldn't be considered as album covers.