regosen / get_cover_art

Batch cover art downloader and embedder for audio files
MIT License
69 stars 8 forks source link

Cover art is not fetched when files contain embedded cover art #21

Open audiomuze opened 2 years ago

audiomuze commented 2 years ago

Calling get_cover_art --no-embed --art-dest-inline --art-size 9999 --external-art-filename xfolder.jpg --verbose results in cover art retrieval being skipped if files contain embedded image.

regosen commented 2 years ago

That is by design, and I believe running with --force should retrieve art regardless of the presence of embedded art. (If that works for you, I will also edit the description of force to be more clear about this.)

audiomuze commented 2 years ago

Apologies for the delay in getting back to you. Had a chance to try it today and --force has the desired outcome.

I noticed that even with --no-embed passed all music files in each folder are processed. From a performance perspective would it not be preferable not to iterate every file when --no-embed is passed?

regosen commented 2 years ago

By "processed" do you mean the file is actually modified? There are modes where you would still want the file read and artwork downloaded (but not embedded). And --clear is another example of when the file would be modified without art being embedded.

audiomuze commented 2 years ago

I meant every audio track in every folder in a tree is iterated over, as opposed to assuming every folder contains a distinct album. So if a folder contains one album that comprises 30 tracks, each track is iterated over as if to check the file for artist and album metadata, then search matching artwork, whereas, if I've understood correctly --no-embed implies a singular image file per folder making the iteration redundant (unless of course, each file has different artist and album metadata and the files are saved as "artist - album.jpg"

As I try to explain myself I wonder whether it's a feature request to be able to run in "folder mode" as opposed to file mode when using --no-embed so as to avoid unnecesarily iterating where each folder represents a distinct album.

regosen commented 2 years ago

Oh, I see. I personally have files with different artwork in the same folder, so I have made no assumptions that every folder's files contain the same artwork. --no-embed used to be called --test, it has nothing to do with folders vs files. It simply skips embedding artwork altogether.

Yes, there could be some kind of "folder mode", but in my opinion it would just make things tidy and not have a noticeable impact on performance, even if you're running with --no-embed just to download artwork. The biggest bottleneck of this workflow is the querying and downloading from Apple Music, and the script already avoids unnecessary network activity when corresponding artwork has already been found, based on the ID3 tags. I'd be happy to review a PR though if you want to take a stab at it. (But if you do, please make it opt-in via a new flag, rather than changing default behavior.)

audiomuze commented 2 years ago

Thanks, I'll take a look.