Add Embedded Art support to `fetchart` plugin by overhacked · Pull Request #6829 · beetbox/beets

overhacked · 2026-07-12T20:32:10Z

Description

Add embedded source to extract embedded art from files. Add skip_embedded option to detect embedded art and skip fetching from other sources (if extracting isn't the desired end state).

If skip_embedded=yes, then files with embedded art are considered "done" and skipped by FetchArt. If the embedded source is enabled, then embedded artwork is always extracted. The two settings are independent of each other.

Fixes #1130.

To Do

Documentation.
Changelog.
Tests.

codecov · 2026-07-12T20:38:00Z

Codecov Report

❌ Patch coverage is 54.54545% with 20 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.89%. Comparing base (4775599) to head (aac06ee).
⚠️ Report is 70 commits behind head on master.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
beetsplug/fetchart.py	54.76%	14 Missing and 5 partials ⚠️
beetsplug/_utils/art.py	50.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6829      +/-   ##
==========================================
- Coverage   74.92%   74.89%   -0.04%     
==========================================
  Files         163      163              
  Lines       21174    21211      +37     
  Branches     3341     3347       +6     
==========================================
+ Hits        15864    15885      +21     
- Misses       4544     4557      +13     
- Partials      766      769       +3

Files with missing lines	Coverage Δ
beetsplug/_utils/art.py	`58.51% <50.00%> (ø)`
beetsplug/fetchart.py	`73.31% <54.76%> (-0.84%)`	⬇️

🚀 New features to boost your workflow:

📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

overhacked · 2026-07-13T00:15:12Z

Taking this to draft to do a little work to improve the test coverage.

Add `embedded` source to extract embedded art from files. Add `skip_embedded` option to detect embedded art and skip fetching from other sources (if extracting isn't the desired end state). If `skip_embedded=yes`, then files with embedded art are considered "done" and skipped by FetchArt. If the `embedded` source is enabled, then embedded artwork is always extracted. The two settings are independent of each other. Fix variable shadowing lint due to added import of `_utils.art`. Signed-off-by: Ross Williams <ross@ross-williams.net>

Add unit tests for Embedded source of FetchArt plugin. Add integration tests for FetchArt plugin's handling of files with embedded artwork. Signed-off-by: Ross Williams <ross@ross-williams.net>

Signed-off-by: Ross Williams <ross@ross-williams.net>

Two messages were emitted in the lowest-level helper functions, which meant that callers who expected a silent pass/fail result to validate whether art is contained in media files would spam the CLI with info-level messages. This commit moves the emission-site of the log messages one level up the call stack so that the messages are only emitted by a function that is called when the user requests that art is extracted and needs success/skip/failure logged to the CLI. Also add art extraction logging to fetchart. Signed-off-by: Ross Williams <ross@ross-williams.net>

overhacked · 2026-07-15T13:30:24Z

@snejus, this is ready whenever you have time to take a look. There are some test coverage places I'd like to address, but when investigating I found that more test harness refactoring around when plugins read their configuration (e.g. @cached_property and PluginMixin.preload_plugin vs .configure_plugin()) would be necessary to get more coverage; also, the coverage gaps--while technically part of this PR's diff--already existed in the fetchart CLI test suite, so I'd be adding tests that don't test the Embedded source directly.

semohr

Thanks! Looks mostly good from my side 👍

semohr · 2026-07-29T08:51:50Z

    art = get_art(log, item)
    outpath = bytestring_path(outpath)
    if not art:
-        log.info("No album art present in {}, skipping.", item)


Any specific reason for removing the logs? Lets keep the git diff minimal.

From aac06ee:

Two messages were emitted in the lowest-level helper functions, which
meant that callers who expected a silent pass/fail result to validate
whether art is contained in media files would spam the CLI with
info-level messages. This commit moves the emission-site of the log
messages one level up the call stack so that the messages are only
emitted by a function that is called when the user requests that art is
extracted and needs success/skip/failure logged to the CLI.

Thanks, did not check the commits. Nonetheless, please revert the changes here again. I get that it is currently spamy but the change is not in scope for this PR. We can decrease the level to debug if you have a strong opinion but we should not remove it completely.

We have planned to increase the default log level to Warning with #6773 or a following PR so this will likely be a none issue or can be evaluated again afterwards.

semohr · 2026-07-29T08:52:01Z

    outpath += bytestring_path(f".{ext}")

-    log.info(
-        "Extracting album art from: {} to: {}", item, displayable_path(outpath)


see https://github.com/beetbox/beets/pull/6829/changes#r3672527806

semohr · 2026-07-29T08:52:17Z

    for item in items:
        real_path = extract(log, outpath, item)
        if real_path:
+            log.info(


see https://github.com/beetbox/beets/pull/6829/changes#r3672527806

semohr · 2026-07-29T08:52:43Z

        matches = []
        # can there be more than one releasegroupid per response?
-        for mbid, art in data.get("albums", {}).items():
+        for mbid, images in data.get("albums", {}).items():


Why was this renamed from art to images ?

_utils.art is now imported, so this line triggers a variable-shadowing lint. I changed the loop variable name so as not to add to the number of linter warnings.

snejus · 2026-07-29T21:27:15Z

+        for album in self.albums:
+            candidate = next(source.get(album, settings, None))


Do we need to check every album? Would it be enough to get the first candidate instead?

This does get the first candidate for each album (next()) created in setup_beets(), but loops over all the albums to test embedded art extraction in all of the media formats. I was going to use pytest parameterization when I first wrote this, but it ended up being less complex code to just create mock albums for each format, IIRC.

snejus · 2026-07-29T21:29:12Z

+    For test safety to ensure no changes cause tests to make
+    real, unmocked requests for cover art.
+    """
+    return


Do we actually want to make real requests here?

Suggested change

return

yield

There is no setup or teardown in this fixture, so yield is unnecessary, and it triggers the lint No teardown in fixture... use 'return' instead of 'yield'.

We don't want to make make real requests in test_fetchart.py, because all its tests are of the import and fetchart CLI logic of the plugin, so we want to fail a test if we've accidentally failed to mock something. The real art fetching tests are in test_art.py.

Signed-off-by: Ross Williams <ross@ross-williams.net>

overhacked requested a review from a team as a code owner July 12, 2026 20:32

github-actions Bot added the fetchart fetchart plugin label Jul 12, 2026

overhacked marked this pull request as draft July 13, 2026 00:14

overhacked force-pushed the overhacked/fetchart_embedded branch from 02b1a4f to 18cd2e7 Compare July 14, 2026 18:00

overhacked added 4 commits July 15, 2026 09:25

test: FetchArt plugin embedded art

c6d4ce9

Add unit tests for Embedded source of FetchArt plugin. Add integration tests for FetchArt plugin's handling of files with embedded artwork. Signed-off-by: Ross Williams <ross@ross-williams.net>

Changelog entry for FetchArt embedded features

8fda3ee

Signed-off-by: Ross Williams <ross@ross-williams.net>

overhacked force-pushed the overhacked/fetchart_embedded branch from 18cd2e7 to aac06ee Compare July 15, 2026 13:25

overhacked marked this pull request as ready for review July 15, 2026 13:25

semohr reviewed Jul 29, 2026

View reviewed changes

snejus requested changes Jul 29, 2026

View reviewed changes

snejus reviewed Jul 29, 2026

View reviewed changes

overhacked added 2 commits July 30, 2026 10:26

Add return type for album_has_art

fadb6bb

Signed-off-by: Ross Williams <ross@ross-williams.net>

docs: add link directly to embedded-art-source

8f46712

Signed-off-by: Ross Williams <ross@ross-williams.net>

		for album in self.albums:
		candidate = next(source.get(album, settings, None))

Uh oh!

Conversation

overhacked commented Jul 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

To Do

Uh oh!

codecov Bot commented Jul 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

overhacked commented Jul 13, 2026

Uh oh!

overhacked commented Jul 15, 2026

Uh oh!

semohr left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

overhacked commented Jul 12, 2026 •

edited

Loading

codecov Bot commented Jul 12, 2026 •

edited

Loading