From e94b74edd43cb3ac8625a0c55e1f41231724b465 Mon Sep 17 00:00:00 2001 From: Alexander Capehart Date: Wed, 8 Jan 2025 11:11:48 -0700 Subject: [PATCH] musikr: do custom picture handling TagLib's picture handling is inadequate for our use case. --- musikr/src/main/cpp/JVMMetadataBuilder.cpp | 68 ++++++++++++++++------ musikr/src/main/cpp/JVMMetadataBuilder.h | 8 +-- musikr/src/main/cpp/taglib_jni.cpp | 3 +- 3 files changed, 57 insertions(+), 22 deletions(-) diff --git a/musikr/src/main/cpp/JVMMetadataBuilder.cpp b/musikr/src/main/cpp/JVMMetadataBuilder.cpp index 6f2630177..e6ecf0bc3 100644 --- a/musikr/src/main/cpp/JVMMetadataBuilder.cpp +++ b/musikr/src/main/cpp/JVMMetadataBuilder.cpp @@ -22,6 +22,7 @@ #include #include +#include #include @@ -33,7 +34,10 @@ void JVMMetadataBuilder::setMimeType(const std::string_view type) { this->mimeType = type; } -void JVMMetadataBuilder::setId3v2(const TagLib::ID3v2::Tag &tag) { +void JVMMetadataBuilder::setId3v2(TagLib::ID3v2::Tag &tag) { + // We want to ideally find the front cover, fall back to the first picture otherwise. + std::optional firstPic; + std::optional frontCoverPic; for (auto frame : tag.frameList()) { if (auto txxxFrame = dynamic_cast(frame)) { @@ -50,18 +54,37 @@ void JVMMetadataBuilder::setId3v2(const TagLib::ID3v2::Tag &tag) { TagLib::String key = frame->frameID(); TagLib::StringList frameText = textFrame->fieldList(); id3v2.add_id(key, frameText); + } else if (auto pictureFrame = + dynamic_cast(frame)) { + if (!firstPic) { + firstPic = pictureFrame; + } + if (!frontCoverPic + && pictureFrame->type() + == TagLib::ID3v2::AttachedPictureFrame::FrontCover) { + frontCoverPic = pictureFrame; + } } else { continue; } } + if (frontCoverPic) { + auto pic = *frontCoverPic; + cover = pic->picture(); + } else if (firstPic) { + auto pic = *firstPic; + cover = pic->picture(); + } } -void JVMMetadataBuilder::setXiph(const TagLib::Ogg::XiphComment &tag) { +void JVMMetadataBuilder::setXiph(TagLib::Ogg::XiphComment &tag) { for (auto field : tag.fieldListMap()) { auto key = field.first.upper(); auto values = field.second; xiph.add_custom(key, values); } + auto pics = tag.pictureList(); + setFlacPictures(pics); } template @@ -77,11 +100,27 @@ void mp4AddImpl(JVMTagMap &map, TagLib::String &itemName, T itemValue) { } } -void JVMMetadataBuilder::setMp4(const TagLib::MP4::Tag &tag) { +void JVMMetadataBuilder::setMp4(TagLib::MP4::Tag &tag) { auto map = tag.itemMap(); + std::optional < TagLib::MP4::CoverArt > firstCover; for (auto item : map) { auto itemName = item.first; auto itemValue = item.second; + if (itemName == "covr") { + // Special cover case. + // MP4 has no types, so just prioritize easier to decode covers (PNG, JPEG) + auto pics = itemValue.toCoverArtList(); + for (auto &pic : pics) { + auto format = pic.format(); + if (format == TagLib::MP4::CoverArt::PNG + || format == TagLib::MP4::CoverArt::JPEG) { + cover = pic.data(); + return; + } + } + cover = pics.front().data(); + return; + } auto type = itemValue.type(); std::string serializedValue; switch (type) { @@ -114,23 +153,18 @@ void JVMMetadataBuilder::setMp4(const TagLib::MP4::Tag &tag) { } } -void JVMMetadataBuilder::setCover( - const TagLib::List covers) { - if (covers.isEmpty()) { - return; - } - // Find the cover with a "front cover" type - for (auto cover : covers) { - auto type = cover["pictureType"].toString(); - if (type == "Front Cover") { - this->cover = cover["data"].toByteVector(); +void JVMMetadataBuilder::setFlacPictures( + TagLib::List &pics) { + // Find the front cover image. If it doesn't exist, fall back to the first image. + for (auto pic : pics) { + if (pic->type() == TagLib::FLAC::Picture::FrontCover) { + cover = pic->data(); return; } } - // No front cover, just pick first. - // TODO: Consider having cascading fallbacks to increasingly less - // relevant covers perhaps - this->cover = covers.front()["data"].toByteVector(); + if (!pics.isEmpty()) { + cover = pics.front()->data(); + } } void JVMMetadataBuilder::setProperties(TagLib::AudioProperties *properties) { diff --git a/musikr/src/main/cpp/JVMMetadataBuilder.h b/musikr/src/main/cpp/JVMMetadataBuilder.h index df2a7f8bf..b093dd01d 100644 --- a/musikr/src/main/cpp/JVMMetadataBuilder.h +++ b/musikr/src/main/cpp/JVMMetadataBuilder.h @@ -35,10 +35,10 @@ public: JVMMetadataBuilder(JNIEnv *env); void setMimeType(const std::string_view type); - void setId3v2(const TagLib::ID3v2::Tag &tag); - void setXiph(const TagLib::Ogg::XiphComment &tag); - void setMp4(const TagLib::MP4::Tag &tag); - void setCover(const TagLib::List covers); + void setId3v2(TagLib::ID3v2::Tag &tag); + void setXiph(TagLib::Ogg::XiphComment &tag); + void setMp4(TagLib::MP4::Tag &tag); + void setFlacPictures(TagLib::List &pics); void setProperties(TagLib::AudioProperties *properties); jobject build(); diff --git a/musikr/src/main/cpp/taglib_jni.cpp b/musikr/src/main/cpp/taglib_jni.cpp index 7e476a84e..8590cbb25 100644 --- a/musikr/src/main/cpp/taglib_jni.cpp +++ b/musikr/src/main/cpp/taglib_jni.cpp @@ -66,6 +66,8 @@ Java_org_oxycblt_musikr_metadata_TagLibJNI_openNative(JNIEnv *env, if (xiphComment != nullptr) { builder.setXiph(*xiphComment); } + auto pics = flacFile->pictureList(); + builder.setFlacPictures(pics); } else if (auto *opusFile = dynamic_cast(file)) { builder.setMimeType("audio/opus"); auto tag = opusFile->tag(); @@ -92,7 +94,6 @@ Java_org_oxycblt_musikr_metadata_TagLibJNI_openNative(JNIEnv *env, } builder.setProperties(file->audioProperties()); - builder.setCover(file->tag()->complexProperties("PICTURE")); return builder.build(); } catch (std::runtime_error e) { LOGE("Error opening file: %s", e.what());