beets-alternatives icon indicating copy to clipboard operation
beets-alternatives copied to clipboard

Copy album art

Open pkel opened this issue 3 years ago • 1 comments

This is my first take on #43 . It's the first time I look into beets (plugin) code. I will need your input to bring this into shape.

I'm confused by the implementation of sync_art / embed. From reading the code it seems that album art is embedded even if convert.embed is set to false. More specifically, I think SYNC_ART is set independent of the configuration option.

PS: Thanks for developing this awesome plugin.

pkel avatar Oct 11 '20 20:10 pkel

Thanks for tackling this, @pkel! This is already a great start. The next step woulds be to document this feature in README.md and write some tests.

The documentation should mention the fine points of this implementation:

  • We only add album art if the whole album matches the selector.
  • We base the location on the directory of the first track on the album
  • We potentially override album art when multiple songs from the same album share a directory
  • We never clean up cover art when the album is removed from the collection.

The tests should include the following test cases

  • Album art is copied if album is matched
  • Album art is not copied if album is not matched
  • Album art is not copied for single tracks where album is not matched
  • Album art copy works even if albums share the same directory
  • Album art copy does nothing for symlink view (This is not yet implemented)

After this we can tweak the implementation.

If you need more help, feel free to ask for it.

From reading the code it seems that album art is embedded even if convert.embed is set to false.

Yes, you’re right. Do you expect covert.embed to control that behavior (even though it configures a different plugin)? Do we need our own configuration setting for this? Feel free to discuss this in an issue.

geigerzaehler avatar Oct 12 '20 13:10 geigerzaehler