fix(taglib): read cover art from dsf, wavpak, fix wma test #4296

kgarner7 · 2025-07-02T15:31:52Z

Description

DSF/DSD (or, at least Picard) appears to store cover art in APIC field (ID3). Extract this in taglib for has_cover

Also make reading from wma a bit safer, fix the tests, and read from WavPack (APE tags only, not ID3v1).

Related Issues

Fixes #4278

Type of Change

Checklist

Please review and check all that apply:

My code follows the project’s coding style
I have tested the changes locally
I have added or updated documentation as needed
I have added tests that prove my fix/feature works (or explain why not)
All existing and new tests pass

How to Test

I could not for the life of me figure out how to trim a DSF file to 1 second. So, to test:

Get a DSD file from https://www.oppodigital.com/hra/dsd-by-davidelias.aspx
Use Picard to embed cover art
Verify navidrome inspect $file shows "hasCoverArt": true,
Verify that Navidrome, with Agents set to an empty string, shows the embedded cover art for the album and song

For the other formats, just make test PKG=./adapters/taglib

Screenshots / Demos (if applicable)

Inspect output:

inspect_output.json

Embedding a completely unrelated screenshot into the album:

Additional Notes

gemini-code-assist

Summary of Changes

Hello @kgarner7, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses a bug where cover art embedded in DSF/DSD audio files was not being recognized by the taglib wrapper. It introduces specific logic to correctly identify and extract cover art from these files, ensuring that applications utilizing this wrapper can now display embedded artwork for DSF content, resolving the issue described in #4278.

Highlights

Bug Fix: DSF Cover Art Detection: I've added specific handling within the has_cover function in taglib_wrapper.cpp to correctly identify embedded cover art in DSF/DSD audio files. This is achieved by checking for the presence of the "APIC" frame within the ID3 tag structure of DSF files.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request adds support for detecting cover art in DSF files. A null pointer dereference issue was identified and a suggestion to add a null check has been provided.

gemini-code-assist · 2025-07-02T15:33:29Z

adapters/taglib/taglib_wrapper.cpp

+    const auto& frameListMap = dsffile->tag()->frameListMap();
+    hasCover = !frameListMap["APIC"].isEmpty();


The dsffile->tag() method can return a nullptr if the file does not contain any tags. Dereferencing a null pointer by calling ->frameListMap() will lead to a crash. Add a null check before accessing the tag's methods.

Suggested change

const auto& frameListMap = dsffile->tag()->frameListMap();

hasCover = !frameListMap["APIC"].isEmpty();

const auto* tag = dsffile->tag();

hasCover = tag && !tag->frameListMap()["APIC"].isEmpty();

github-actions · 2025-07-02T15:40:52Z

kgarner7 · 2025-07-02T16:48:41Z

/gemini review

gemini-code-assist

Code Review

This pull request adds support for detecting cover art in DSF and WavPack files, and improves tag handling for WMA files by supporting multi-value tags. The changes include updates to the C++ wrapper to implement the new logic, and corresponding updates to Go tests to verify the new functionality. The implementation looks correct, but I have a couple of suggestions to improve the C++ code quality by using const_cast instead of C-style casts for better type safety and maintainability.

gemini-code-assist · 2025-07-02T16:50:00Z

adapters/taglib/taglib_wrapper.cpp

@@ -174,7 +174,14 @@ int taglib_read(const FILENAME_CHAR_T *filename, unsigned long id) {
    const TagLib::ASF::Tag *asfTags{asfFile->tag()};
    const auto itemListMap = asfTags->attributeListMap();
    for (const auto item : itemListMap) {
-      tags.insert(item.first, item.second.front().toString());
+      char *key = (char *)item.first.toCString(true);


Using a C-style cast (char *) to remove the const qualifier is discouraged in C++. It's better to use const_cast which is type-safe and clearly expresses the intent of casting away const-ness. This also makes the code easier to maintain and search for potentially dangerous casts.

Suggested change

char *key = (char *)item.first.toCString(true);

char *key = const_cast<char *>(item.first.toCString(true));

gemini-code-assist · 2025-07-02T16:50:00Z

adapters/taglib/taglib_wrapper.cpp

+      for (auto j = item.second.begin();
+           j != item.second.end(); ++j) {
+
+        char *val = (char *) j->toString().toCString(true);


Similar to the comment above, please use const_cast here instead of a C-style cast for better type safety and code clarity.

Suggested change

char *val = (char *) j->toString().toCString(true);

char *val = const_cast<char *>(j->toString().toCString(true));

fix(taglib): read cover art from dsf

340b374

gemini-code-assist bot reviewed Jul 2, 2025

View reviewed changes

kgarner7 marked this pull request as draft July 2, 2025 16:20

address feedback and alsi realize wma/wavpack are missing

ac6625d

kgarner7 changed the title ~~fix(taglib): read cover art from dsf~~ fix(taglib): read cover art from dsf, wavpak, fix wma test Jul 2, 2025

kgarner7 marked this pull request as ready for review July 2, 2025 16:46

gemini-code-assist bot reviewed Jul 2, 2025

View reviewed changes

kgarner7 added 2 commits July 2, 2025 09:52

feedback

6bb73e9

more const char and remove unused import

14eb2e4

deluan merged commit d4f8691 into navidrome:master Jul 3, 2025
31 checks passed

BrewTestBot mentioned this pull request Jul 28, 2025

navidrome 0.58.0 Homebrew/homebrew-core#231529

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(taglib): read cover art from dsf, wavpak, fix wma test #4296

fix(taglib): read cover art from dsf, wavpak, fix wma test #4296

Uh oh!

kgarner7 commented Jul 2, 2025 •

edited

Loading

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot Jul 2, 2025

Uh oh!

github-actions bot commented Jul 2, 2025 •

edited

Loading

Uh oh!

kgarner7 commented Jul 2, 2025

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot Jul 2, 2025

Uh oh!

gemini-code-assist bot Jul 2, 2025

Uh oh!

Uh oh!

Uh oh!

		const auto& frameListMap = dsffile->tag()->frameListMap();
		hasCover = !frameListMap["APIC"].isEmpty();

	char key = (char )item.first.toCString(true);
	char key = const_cast<char >(item.first.toCString(true));

	char val = (char ) j->toString().toCString(true);
	char val = const_cast<char >(j->toString().toCString(true));

Uh oh!

fix(taglib): read cover art from dsf, wavpak, fix wma test #4296

fix(taglib): read cover art from dsf, wavpak, fix wma test #4296

Uh oh!

Conversation

kgarner7 commented Jul 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issues

Type of Change

Checklist

How to Test

Screenshots / Demos (if applicable)

Additional Notes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Jul 2, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jul 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kgarner7 commented Jul 2, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Jul 2, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jul 2, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

kgarner7 commented Jul 2, 2025 •

edited

Loading

github-actions bot commented Jul 2, 2025 •

edited

Loading