From d02128927975971db1462e577357f728b9d7911c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Deluan=20Quint=C3=A3o?= Date: Sun, 26 Oct 2025 19:36:44 -0400 Subject: [PATCH 1/2] fix: enable multi-valued releasetype in smart playlists (#4621) * fix: prevent infinite loop in Type filter autocomplete Fixed an infinite loop issue in the album Type filter caused by an inline arrow function in the optionText prop. The inline function created a new reference on every render, causing React-Admin's AutocompleteInput to continuously re-fetch data from the /api/tag endpoint. The solution extracts the formatting function outside the component scope as formatReleaseType, ensuring a stable function reference across renders. This prevents unnecessary re-renders and API calls while maintaining the humanized display format for release type values. * fix: enable multi-valued releasetype in smart playlists Smart playlists can now match all values in multi-valued releasetype tags. Previously, the albumtype field was mapped to the single-valued mbz_album_type database field, which only stored the first value from tags like album; soundtrack. This prevented smart playlists from matching albums with secondary release types like soundtrack, live, or compilation when tagged by MusicBrainz Picard. The fix removes the direct database field mapping and allows both albumtype and releasetype to use the multi-valued tag system. The albumtype field is now an alias that points to the releasetype tag field, ensuring both query the same JSON path in the tags column. This maintains backward compatibility with the documented albumtype field while enabling proper multi-value tag matching. Added tests to verify both releasetype and albumtype correctly generate multi-valued tag queries. Fixes #4616 * fix: resolve albumtype alias for all operators and sorting Codex correctly identified that the initial fix only worked for Contains/StartsWith/EndsWith operators. The alias resolution was happening too late in the code path. Fixed by resolving the alias in two places: 1. tagCond.ToSql() - now uses the actual field name (releasetype) in the JSON path 2. Criteria.OrderBy() - now uses the actual field name when building sort expressions Added tests for Is/IsNot operators and sorting to ensure complete coverage. --- model/criteria/criteria.go | 7 ++++++- model/criteria/criteria_test.go | 10 ++++++++++ model/criteria/fields.go | 18 ++++++++++++----- model/criteria/operators_test.go | 34 ++++++++++++++++++++++++++++++++ ui/src/album/AlbumList.jsx | 7 ++++--- 5 files changed, 67 insertions(+), 9 deletions(-) diff --git a/model/criteria/criteria.go b/model/criteria/criteria.go index fa92c5aca..54ac59697 100644 --- a/model/criteria/criteria.go +++ b/model/criteria/criteria.go @@ -61,7 +61,12 @@ func (c Criteria) OrderBy() string { if f.order != "" { mapped = f.order } else if f.isTag { - mapped = "COALESCE(json_extract(media_file.tags, '$." + sortField + "[0].value'), '')" + // Use the actual field name (handles aliases like albumtype -> releasetype) + tagName := sortField + if f.field != "" { + tagName = f.field + } + mapped = "COALESCE(json_extract(media_file.tags, '$." + tagName + "[0].value'), '')" } else if f.isRole { mapped = "COALESCE(json_extract(media_file.participants, '$." + sortField + "[0].name'), '')" } else { diff --git a/model/criteria/criteria_test.go b/model/criteria/criteria_test.go index 3792264a5..032ead5c8 100644 --- a/model/criteria/criteria_test.go +++ b/model/criteria/criteria_test.go @@ -118,6 +118,16 @@ var _ = Describe("Criteria", func() { ) }) + It("sorts by albumtype alias (resolves to releasetype)", func() { + AddTagNames([]string{"releasetype"}) + goObj.Sort = "albumtype" + gomega.Expect(goObj.OrderBy()).To( + gomega.Equal( + "COALESCE(json_extract(media_file.tags, '$.releasetype[0].value'), '') asc", + ), + ) + }) + It("sorts by random", func() { newObj := goObj newObj.Sort = "random" diff --git a/model/criteria/fields.go b/model/criteria/fields.go index 3699eb14a..70719cd6f 100644 --- a/model/criteria/fields.go +++ b/model/criteria/fields.go @@ -32,7 +32,6 @@ var fieldMap = map[string]*mappedField{ "sortalbum": {field: "media_file.sort_album_name"}, "sortartist": {field: "media_file.sort_artist_name"}, "sortalbumartist": {field: "media_file.sort_album_artist_name"}, - "albumtype": {field: "media_file.mbz_album_type", alias: "releasetype"}, "albumcomment": {field: "media_file.mbz_album_comment"}, "catalognumber": {field: "media_file.catalog_num"}, "filepath": {field: "media_file.path"}, @@ -55,6 +54,9 @@ var fieldMap = map[string]*mappedField{ "mbz_release_group_id": {field: "media_file.mbz_release_group_id"}, "library_id": {field: "media_file.library_id", numeric: true}, + // Backward compatibility: albumtype is an alias for releasetype tag + "albumtype": {field: "releasetype", isTag: true}, + // special fields "random": {field: "", order: "random()"}, // pseudo-field for random sorting "value": {field: "value"}, // pseudo-field for tag and roles values @@ -154,13 +156,19 @@ type tagCond struct { func (e tagCond) ToSql() (string, []any, error) { cond, args, err := e.cond.ToSql() - // Check if this tag is marked as numeric in the fieldMap - if fm, ok := fieldMap[e.tag]; ok && fm.numeric { - cond = strings.ReplaceAll(cond, "value", "CAST(value AS REAL)") + // Resolve the actual tag name (handles aliases like albumtype -> releasetype) + tagName := e.tag + if fm, ok := fieldMap[e.tag]; ok { + if fm.field != "" { + tagName = fm.field + } + if fm.numeric { + cond = strings.ReplaceAll(cond, "value", "CAST(value AS REAL)") + } } cond = fmt.Sprintf("exists (select 1 from json_tree(tags, '$.%s') where key='value' and %s)", - e.tag, cond) + tagName, cond) if e.not { cond = "not " + cond } diff --git a/model/criteria/operators_test.go b/model/criteria/operators_test.go index ee716a9cd..4c1db1303 100644 --- a/model/criteria/operators_test.go +++ b/model/criteria/operators_test.go @@ -105,6 +105,40 @@ var _ = Describe("Operators", func() { gomega.Expect(sql).To(gomega.BeEmpty()) gomega.Expect(args).To(gomega.BeEmpty()) }) + It("supports releasetype as multi-valued tag", func() { + AddTagNames([]string{"releasetype"}) + op := Contains{"releasetype": "soundtrack"} + sql, args, err := op.ToSql() + gomega.Expect(err).ToNot(gomega.HaveOccurred()) + gomega.Expect(sql).To(gomega.Equal("exists (select 1 from json_tree(tags, '$.releasetype') where key='value' and value LIKE ?)")) + gomega.Expect(args).To(gomega.HaveExactElements("%soundtrack%")) + }) + It("supports albumtype as alias for releasetype", func() { + AddTagNames([]string{"releasetype"}) + op := Contains{"albumtype": "live"} + sql, args, err := op.ToSql() + gomega.Expect(err).ToNot(gomega.HaveOccurred()) + gomega.Expect(sql).To(gomega.Equal("exists (select 1 from json_tree(tags, '$.releasetype') where key='value' and value LIKE ?)")) + gomega.Expect(args).To(gomega.HaveExactElements("%live%")) + }) + It("supports albumtype alias with Is operator", func() { + AddTagNames([]string{"releasetype"}) + op := Is{"albumtype": "album"} + sql, args, err := op.ToSql() + gomega.Expect(err).ToNot(gomega.HaveOccurred()) + // Should query $.releasetype, not $.albumtype + gomega.Expect(sql).To(gomega.Equal("exists (select 1 from json_tree(tags, '$.releasetype') where key='value' and value = ?)")) + gomega.Expect(args).To(gomega.HaveExactElements("album")) + }) + It("supports albumtype alias with IsNot operator", func() { + AddTagNames([]string{"releasetype"}) + op := IsNot{"albumtype": "compilation"} + sql, args, err := op.ToSql() + gomega.Expect(err).ToNot(gomega.HaveOccurred()) + // Should query $.releasetype, not $.albumtype + gomega.Expect(sql).To(gomega.Equal("not exists (select 1 from json_tree(tags, '$.releasetype') where key='value' and value = ?)")) + gomega.Expect(args).To(gomega.HaveExactElements("compilation")) + }) }) Describe("Custom Roles", func() { diff --git a/ui/src/album/AlbumList.jsx b/ui/src/album/AlbumList.jsx index 40b927a89..f10f8dbd3 100644 --- a/ui/src/album/AlbumList.jsx +++ b/ui/src/album/AlbumList.jsx @@ -42,6 +42,9 @@ const useStyles = makeStyles({ }, }) +const formatReleaseType = (record) => + record?.tagValue ? humanize(record?.tagValue) : '-- None --' + const AlbumFilter = (props) => { const classes = useStyles() const translate = useTranslate() @@ -142,9 +145,7 @@ const AlbumFilter = (props) => { > - record?.tagValue ? humanize(record?.tagValue) : '-- None --' - } + optionText={formatReleaseType} /> From cce11c5416f9321942748626c217a4f0d1d3a445 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Deluan=20Quint=C3=A3o?= Date: Sun, 26 Oct 2025 19:38:34 -0400 Subject: [PATCH 2/2] fix(scanner): restore basic tag extraction fallback mechanism for improved metadata parsing (#4401) * feat: add basic tag extraction fallback mechanism Added basic tag extraction from TagLib's generic Tag interface as a fallback when PropertyMap doesn't contain standard metadata fields. This ensures that essential tags like title, artist, album, comment, genre, year, and track are always available even when they're not present in format-specific property maps. Changes include: - Extract basic tags (__title, __artist, etc.) in C++ wrapper - Add parseBasicTag function to process basic tags in Go extractor - Refactor parseProp function to be reusable across property parsing - Ensure basic tags are preferred over PropertyMap when available * feat(taglib): update tag parsing to use double underscores for properties Signed-off-by: Deluan --------- Signed-off-by: Deluan --- adapters/taglib/taglib.go | 57 ++++++++++++++++++++--------- adapters/taglib/taglib_wrapper.cpp | 58 +++++++++++++++++++++++------- 2 files changed, 85 insertions(+), 30 deletions(-) diff --git a/adapters/taglib/taglib.go b/adapters/taglib/taglib.go index 62a949d85..d32adf4ed 100644 --- a/adapters/taglib/taglib.go +++ b/adapters/taglib/taglib.go @@ -43,23 +43,21 @@ func (e extractor) extractMetadata(filePath string) (*metadata.Info, error) { // Parse audio properties ap := metadata.AudioProperties{} - if length, ok := tags["_lengthinmilliseconds"]; ok && len(length) > 0 { - millis, _ := strconv.Atoi(length[0]) - if millis > 0 { - ap.Duration = (time.Millisecond * time.Duration(millis)).Round(time.Millisecond * 10) - } - delete(tags, "_lengthinmilliseconds") - } - parseProp := func(prop string, target *int) { - if value, ok := tags[prop]; ok && len(value) > 0 { - *target, _ = strconv.Atoi(value[0]) - delete(tags, prop) - } - } - parseProp("_bitrate", &ap.BitRate) - parseProp("_channels", &ap.Channels) - parseProp("_samplerate", &ap.SampleRate) - parseProp("_bitspersample", &ap.BitDepth) + ap.BitRate = parseProp(tags, "__bitrate") + ap.Channels = parseProp(tags, "__channels") + ap.SampleRate = parseProp(tags, "__samplerate") + ap.BitDepth = parseProp(tags, "__bitspersample") + length := parseProp(tags, "__lengthinmilliseconds") + ap.Duration = (time.Millisecond * time.Duration(length)).Round(time.Millisecond * 10) + + // Extract basic tags + parseBasicTag(tags, "__title", "title") + parseBasicTag(tags, "__artist", "artist") + parseBasicTag(tags, "__album", "album") + parseBasicTag(tags, "__comment", "comment") + parseBasicTag(tags, "__genre", "genre") + parseBasicTag(tags, "__year", "year") + parseBasicTag(tags, "__track", "tracknumber") // Parse track/disc totals parseTuple := func(prop string) { @@ -107,6 +105,31 @@ var tiplMapping = map[string]string{ "DJ-mix": "djmixer", } +// parseProp parses a property from the tags map and sets it to the target integer. +// It also deletes the property from the tags map after parsing. +func parseProp(tags map[string][]string, prop string) int { + if value, ok := tags[prop]; ok && len(value) > 0 { + v, _ := strconv.Atoi(value[0]) + delete(tags, prop) + return v + } + return 0 +} + +// parseBasicTag checks if a basic tag (like __title, __artist, etc.) exists in the tags map. +// If it does, it moves the value to a more appropriate tag name (like title, artist, etc.), +// and deletes the basic tag from the map. If the target tag already exists, it ignores the basic tag. +func parseBasicTag(tags map[string][]string, basicName string, tagName string) { + basicValue := tags[basicName] + if len(basicValue) == 0 { + return + } + delete(tags, basicName) + if len(tags[tagName]) == 0 { + tags[tagName] = basicValue + } +} + // parseTIPL parses the ID3v2.4 TIPL frame string, which is received from TagLib in the format: // // "arranger Andrew Powell engineer Chris Blair engineer Pat Stapley producer Eric Woolfson". diff --git a/adapters/taglib/taglib_wrapper.cpp b/adapters/taglib/taglib_wrapper.cpp index 224642c6d..2985e8f18 100644 --- a/adapters/taglib/taglib_wrapper.cpp +++ b/adapters/taglib/taglib_wrapper.cpp @@ -45,31 +45,63 @@ int taglib_read(const FILENAME_CHAR_T *filename, unsigned long id) { // Add audio properties to the tags const TagLib::AudioProperties *props(f.audioProperties()); - goPutInt(id, (char *)"_lengthinmilliseconds", props->lengthInMilliseconds()); - goPutInt(id, (char *)"_bitrate", props->bitrate()); - goPutInt(id, (char *)"_channels", props->channels()); - goPutInt(id, (char *)"_samplerate", props->sampleRate()); + goPutInt(id, (char *)"__lengthinmilliseconds", props->lengthInMilliseconds()); + goPutInt(id, (char *)"__bitrate", props->bitrate()); + goPutInt(id, (char *)"__channels", props->channels()); + goPutInt(id, (char *)"__samplerate", props->sampleRate()); + // Extract bits per sample for supported formats + int bitsPerSample = 0; if (const auto* apeProperties{ dynamic_cast(props) }) - goPutInt(id, (char *)"_bitspersample", apeProperties->bitsPerSample()); - if (const auto* asfProperties{ dynamic_cast(props) }) - goPutInt(id, (char *)"_bitspersample", asfProperties->bitsPerSample()); + bitsPerSample = apeProperties->bitsPerSample(); + else if (const auto* asfProperties{ dynamic_cast(props) }) + bitsPerSample = asfProperties->bitsPerSample(); else if (const auto* flacProperties{ dynamic_cast(props) }) - goPutInt(id, (char *)"_bitspersample", flacProperties->bitsPerSample()); + bitsPerSample = flacProperties->bitsPerSample(); else if (const auto* mp4Properties{ dynamic_cast(props) }) - goPutInt(id, (char *)"_bitspersample", mp4Properties->bitsPerSample()); + bitsPerSample = mp4Properties->bitsPerSample(); else if (const auto* wavePackProperties{ dynamic_cast(props) }) - goPutInt(id, (char *)"_bitspersample", wavePackProperties->bitsPerSample()); + bitsPerSample = wavePackProperties->bitsPerSample(); else if (const auto* aiffProperties{ dynamic_cast(props) }) - goPutInt(id, (char *)"_bitspersample", aiffProperties->bitsPerSample()); + bitsPerSample = aiffProperties->bitsPerSample(); else if (const auto* wavProperties{ dynamic_cast(props) }) - goPutInt(id, (char *)"_bitspersample", wavProperties->bitsPerSample()); + bitsPerSample = wavProperties->bitsPerSample(); else if (const auto* dsfProperties{ dynamic_cast(props) }) - goPutInt(id, (char *)"_bitspersample", dsfProperties->bitsPerSample()); + bitsPerSample = dsfProperties->bitsPerSample(); + + if (bitsPerSample > 0) { + goPutInt(id, (char *)"__bitspersample", bitsPerSample); + } // Send all properties to the Go map TagLib::PropertyMap tags = f.file()->properties(); + // Make sure at least the basic properties are extracted + TagLib::Tag *basic = f.file()->tag(); + if (!basic->isEmpty()) { + if (!basic->title().isEmpty()) { + tags.insert("__title", basic->title()); + } + if (!basic->artist().isEmpty()) { + tags.insert("__artist", basic->artist()); + } + if (!basic->album().isEmpty()) { + tags.insert("__album", basic->album()); + } + if (!basic->comment().isEmpty()) { + tags.insert("__comment", basic->comment()); + } + if (!basic->genre().isEmpty()) { + tags.insert("__genre", basic->genre()); + } + if (basic->year() > 0) { + tags.insert("__year", TagLib::String::number(basic->year())); + } + if (basic->track() > 0) { + tags.insert("__track", TagLib::String::number(basic->track())); + } + } + TagLib::ID3v2::Tag *id3Tags = NULL; // Get some extended/non-standard ID3-only tags (ex: iTunes extended frames)