support parsing MBIDs for roles (from the https://github.com/kgarner7/picard-all-mbids plugin) (#3698)

* parse standard roles, vorbis/m4a work for now

* fix djmixer

* working roles, use DJ-mix

* add performers to file

* map mbids

* add a few more tests

* add test

Signed-off-by: Deluan <deluan@navidrome.org>

* try to simplify the performers logic

Signed-off-by: Deluan <deluan@navidrome.org>

* stylistic changes

---------

Signed-off-by: Deluan <deluan@navidrome.org>
Co-authored-by: Deluan <deluan@navidrome.org>
This commit is contained in:
Kendall Garner 2025-02-17 03:14:15 +00:00 committed by GitHub
parent 2188c9ea30
commit 097fec7d6e
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
18 changed files with 374 additions and 44 deletions

View file

@ -0,0 +1,154 @@
package taglib
import (
"io/fs"
"os"
"time"
"github.com/djherbis/times"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/model/metadata"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)
type testFileInfo struct {
fs.FileInfo
}
func (t testFileInfo) BirthTime() time.Time {
if ts := times.Get(t.FileInfo); ts.HasBirthTime() {
return ts.BirthTime()
}
return t.FileInfo.ModTime()
}
var _ = Describe("Extractor", func() {
toP := func(name, sortName, mbid string) model.Participant {
return model.Participant{
Artist: model.Artist{Name: name, SortArtistName: sortName, MbzArtistID: mbid},
}
}
roles := []struct {
model.Role
model.ParticipantList
}{
{model.RoleComposer, model.ParticipantList{
toP("coma a", "a, coma", "bf13b584-f27c-43db-8f42-32898d33d4e2"),
toP("comb", "comb", "924039a2-09c6-4d29-9b4f-50cc54447d36"),
}},
{model.RoleLyricist, model.ParticipantList{
toP("la a", "a, la", "c84f648f-68a6-40a2-a0cb-d135b25da3c2"),
toP("lb", "lb", "0a7c582d-143a-4540-b4e9-77200835af65"),
}},
{model.RoleArranger, model.ParticipantList{
toP("aa", "", "4605a1d4-8d15-42a3-bd00-9c20e42f71e6"),
toP("ab", "", "002f0ff8-77bf-42cc-8216-61a9c43dc145"),
}},
{model.RoleConductor, model.ParticipantList{
toP("cona", "", "af86879b-2141-42af-bad2-389a4dc91489"),
toP("conb", "", "3dfa3c70-d7d3-4b97-b953-c298dd305e12"),
}},
{model.RoleDirector, model.ParticipantList{
toP("dia", "", "f943187f-73de-4794-be47-88c66f0fd0f4"),
toP("dib", "", "bceb75da-1853-4b3d-b399-b27f0cafc389"),
}},
{model.RoleEngineer, model.ParticipantList{
toP("ea", "", "f634bf6d-d66a-425d-888a-28ad39392759"),
toP("eb", "", "243d64ae-d514-44e1-901a-b918d692baee"),
}},
{model.RoleProducer, model.ParticipantList{
toP("pra", "", "d971c8d7-999c-4a5f-ac31-719721ab35d6"),
toP("prb", "", "f0a09070-9324-434f-a599-6d25ded87b69"),
}},
{model.RoleRemixer, model.ParticipantList{
toP("ra", "", "c7dc6095-9534-4c72-87cc-aea0103462cf"),
toP("rb", "", "8ebeef51-c08c-4736-992f-c37870becedd"),
}},
{model.RoleDJMixer, model.ParticipantList{
toP("dja", "", "d063f13b-7589-4efc-ab7f-c60e6db17247"),
toP("djb", "", "3636670c-385f-4212-89c8-0ff51d6bc456"),
}},
{model.RoleMixer, model.ParticipantList{
toP("ma", "", "53fb5a2d-7016-427e-a563-d91819a5f35a"),
toP("mb", "", "64c13e65-f0da-4ab9-a300-71ee53b0376a"),
}},
}
var e *extractor
BeforeEach(func() {
e = &extractor{}
})
Describe("Participants", func() {
DescribeTable("test tags consistent across formats", func(format string) {
path := "tests/fixtures/test." + format
mds, err := e.Parse(path)
Expect(err).ToNot(HaveOccurred())
info := mds[path]
fileInfo, _ := os.Stat(path)
info.FileInfo = testFileInfo{FileInfo: fileInfo}
metadata := metadata.New(path, info)
mf := metadata.ToMediaFile(1, "folderID")
for _, data := range roles {
role := data.Role
artists := data.ParticipantList
actual := mf.Participants[role]
Expect(actual).To(HaveLen(len(artists)))
for i := range artists {
actualArtist := actual[i]
expectedArtist := artists[i]
Expect(actualArtist.Name).To(Equal(expectedArtist.Name))
Expect(actualArtist.SortArtistName).To(Equal(expectedArtist.SortArtistName))
Expect(actualArtist.MbzArtistID).To(Equal(expectedArtist.MbzArtistID))
}
}
if format != "m4a" {
performers := mf.Participants[model.RolePerformer]
Expect(performers).To(HaveLen(8))
rules := map[string][]string{
"pgaa": {"2fd0b311-9fa8-4ff9-be5d-f6f3d16b835e", "Guitar"},
"pgbb": {"223d030b-bf97-4c2a-ad26-b7f7bbe25c93", "Guitar", ""},
"pvaa": {"cb195f72-448f-41c8-b962-3f3c13d09d38", "Vocals"},
"pvbb": {"60a1f832-8ca2-49f6-8660-84d57f07b520", "Vocals", "Flute"},
"pfaa": {"51fb40c-0305-4bf9-a11b-2ee615277725", "", "Flute"},
}
for name, rule := range rules {
mbid := rule[0]
for i := 1; i < len(rule); i++ {
found := false
for _, mapped := range performers {
if mapped.Name == name && mapped.MbzArtistID == mbid && mapped.SubRole == rule[i] {
found = true
break
}
}
Expect(found).To(BeTrue(), "Could not find matching artist")
}
}
}
},
Entry("FLAC format", "flac"),
Entry("M4a format", "m4a"),
Entry("OGG format", "ogg"),
Entry("WMA format", "wv"),
Entry("MP3 format", "mp3"),
Entry("WAV format", "wav"),
Entry("AIFF format", "aiff"),
)
})
})

View file

@ -103,7 +103,7 @@ var tiplMapping = map[string]string{
"engineer": "engineer",
"producer": "producer",
"mix": "mixer",
"dj-mix": "djmixer",
"DJ-mix": "djmixer",
}
// parseTIPL parses the ID3v2.4 TIPL frame string, which is received from TagLib in the format:
@ -122,7 +122,7 @@ func parseTIPL(tags map[string][]string) {
addRole := func(currentRole string, currentValue []string) {
if currentRole != "" && len(currentValue) > 0 {
role := tiplMapping[currentRole]
tags[role] = append(tags[currentRole], strings.Join(currentValue, " "))
tags[role] = append(tags[role], strings.Join(currentValue, " "))
}
}

View file

@ -255,11 +255,11 @@ var _ = Describe("Extractor", func() {
Context("when the TIPL string is populated", func() {
It("correctly parses roles and names", func() {
tags["tipl"] = []string{"arranger Andrew Powell dj-mix François Kevorkian engineer Chris Blair"}
tags["tipl"] = []string{"arranger Andrew Powell DJ-mix François Kevorkian DJ-mix Jane Doe engineer Chris Blair"}
parseTIPL(tags)
Expect(tags["arranger"]).To(ConsistOf("Andrew Powell"))
Expect(tags["engineer"]).To(ConsistOf("Chris Blair"))
Expect(tags["djmixer"]).To(ConsistOf("François Kevorkian"))
Expect(tags["djmixer"]).To(ConsistOf("François Kevorkian", "Jane Doe"))
})
It("handles multiple names for a single role", func() {

View file

@ -17,27 +17,35 @@ type roleTags struct {
}
var roleMappings = map[model.Role]roleTags{
model.RoleComposer: {name: model.TagComposer, sort: model.TagComposerSort},
model.RoleLyricist: {name: model.TagLyricist, sort: model.TagLyricistSort},
model.RoleConductor: {name: model.TagConductor},
model.RoleArranger: {name: model.TagArranger},
model.RoleDirector: {name: model.TagDirector},
model.RoleProducer: {name: model.TagProducer},
model.RoleEngineer: {name: model.TagEngineer},
model.RoleMixer: {name: model.TagMixer},
model.RoleRemixer: {name: model.TagRemixer},
model.RoleDJMixer: {name: model.TagDJMixer},
model.RoleComposer: {name: model.TagComposer, sort: model.TagComposerSort, mbid: model.TagMusicBrainzComposerID},
model.RoleLyricist: {name: model.TagLyricist, sort: model.TagLyricistSort, mbid: model.TagMusicBrainzLyricistID},
model.RoleConductor: {name: model.TagConductor, mbid: model.TagMusicBrainzConductorID},
model.RoleArranger: {name: model.TagArranger, mbid: model.TagMusicBrainzArrangerID},
model.RoleDirector: {name: model.TagDirector, mbid: model.TagMusicBrainzDirectorID},
model.RoleProducer: {name: model.TagProducer, mbid: model.TagMusicBrainzProducerID},
model.RoleEngineer: {name: model.TagEngineer, mbid: model.TagMusicBrainzEngineerID},
model.RoleMixer: {name: model.TagMixer, mbid: model.TagMusicBrainzMixerID},
model.RoleRemixer: {name: model.TagRemixer, mbid: model.TagMusicBrainzRemixerID},
model.RoleDJMixer: {name: model.TagDJMixer, mbid: model.TagMusicBrainzDJMixerID},
}
func (md Metadata) mapParticipants() model.Participants {
participants := make(model.Participants)
// Parse track artists
artists := md.parseArtists(model.TagTrackArtist, model.TagTrackArtists, model.TagTrackArtistSort, model.TagTrackArtistsSort, model.TagMusicBrainzArtistID)
artists := md.parseArtists(
model.TagTrackArtist, model.TagTrackArtists,
model.TagTrackArtistSort, model.TagTrackArtistsSort,
model.TagMusicBrainzArtistID,
)
participants.Add(model.RoleArtist, artists...)
// Parse album artists
albumArtists := md.parseArtists(model.TagAlbumArtist, model.TagAlbumArtists, model.TagAlbumArtistSort, model.TagAlbumArtistsSort, model.TagMusicBrainzAlbumArtistID)
albumArtists := md.parseArtists(
model.TagAlbumArtist, model.TagAlbumArtists,
model.TagAlbumArtistSort, model.TagAlbumArtistsSort,
model.TagMusicBrainzAlbumArtistID,
)
if len(albumArtists) == 1 && albumArtists[0].Name == consts.UnknownArtist {
if md.Bool(model.TagCompilation) {
albumArtists = md.buildArtists([]string{consts.VariousArtists}, nil, []string{consts.VariousArtistsMbzId})
@ -58,22 +66,58 @@ func (md Metadata) mapParticipants() model.Participants {
}
}
titleCaser := cases.Title(language.Und)
rolesMbzIdMap := md.buildRoleMbidMaps()
md.processPerformers(participants, rolesMbzIdMap)
md.syncMissingMbzIDs(participants)
// Parse performers
for _, performer := range md.Pairs(model.TagPerformer) {
name := performer.Value()
id := md.artistID(name)
orderName := str.SanitizeFieldForSortingNoArticle(name)
subRole := titleCaser.String(performer.Key())
participants.AddWithSubRole(model.RolePerformer, subRole, model.Artist{
ID: id,
Name: name,
OrderArtistName: orderName,
})
return participants
}
// buildRoleMbidMaps creates a map of roles to MBZ IDs
func (md Metadata) buildRoleMbidMaps() map[string][]string {
titleCaser := cases.Title(language.Und)
rolesMbzIdMap := make(map[string][]string)
for _, mbid := range md.Pairs(model.TagMusicBrainzPerformerID) {
role := titleCaser.String(mbid.Key())
rolesMbzIdMap[role] = append(rolesMbzIdMap[role], mbid.Value())
}
// Create a map to store the MbzArtistID for each artist name
return rolesMbzIdMap
}
func (md Metadata) processPerformers(participants model.Participants, rolesMbzIdMap map[string][]string) {
// roleIdx keeps track of the index of the MBZ ID for each role
roleIdx := make(map[string]int)
for role := range rolesMbzIdMap {
roleIdx[role] = 0
}
titleCaser := cases.Title(language.Und)
for _, performer := range md.Pairs(model.TagPerformer) {
name := performer.Value()
subRole := titleCaser.String(performer.Key())
artist := model.Artist{
ID: md.artistID(name),
Name: name,
OrderArtistName: str.SanitizeFieldForSortingNoArticle(name),
MbzArtistID: md.getPerformerMbid(subRole, rolesMbzIdMap, roleIdx),
}
participants.AddWithSubRole(model.RolePerformer, subRole, artist)
}
}
// getPerformerMbid returns the MBZ ID for a performer, based on the subrole
func (md Metadata) getPerformerMbid(subRole string, rolesMbzIdMap map[string][]string, roleIdx map[string]int) string {
if mbids, exists := rolesMbzIdMap[subRole]; exists && roleIdx[subRole] < len(mbids) {
defer func() { roleIdx[subRole]++ }()
return mbids[roleIdx[subRole]]
}
return ""
}
// syncMissingMbzIDs fills in missing MBZ IDs for artists that have been previously parsed
func (md Metadata) syncMissingMbzIDs(participants model.Participants) {
artistMbzIDMap := make(map[string]string)
for _, artist := range append(participants[model.RoleArtist], participants[model.RoleAlbumArtist]...) {
if artist.MbzArtistID != "" {
@ -81,24 +125,21 @@ func (md Metadata) mapParticipants() model.Participants {
}
}
if len(artistMbzIDMap) > 0 {
// For each artist in each role, try to figure out their MBID from the
// track/album artists (the only roles that have MBID in MusicBrainz)
for role, list := range participants {
for i, participant := range list {
if participant.MbzArtistID == "" {
if mbzID, found := artistMbzIDMap[participant.Name]; found {
participants[role][i].MbzArtistID = mbzID
}
for role, list := range participants {
for i, artist := range list {
if artist.MbzArtistID == "" {
if mbzID, exists := artistMbzIDMap[artist.Name]; exists {
participants[role][i].MbzArtistID = mbzID
}
}
}
}
return participants
}
func (md Metadata) parseArtists(name model.TagName, names model.TagName, sort model.TagName, sorts model.TagName, mbid model.TagName) []model.Artist {
func (md Metadata) parseArtists(
name model.TagName, names model.TagName, sort model.TagName,
sorts model.TagName, mbid model.TagName,
) []model.Artist {
nameValues := md.getArtistValues(name, names)
sortValues := md.getArtistValues(sort, sorts)
mbids := md.Strings(mbid)

View file

@ -518,4 +518,76 @@ var _ = Describe("Participants", func() {
Expect(producers[1].MbzArtistID).To(Equal(mbid1))
})
})
Describe("Non-standard MBID tags", func() {
var allMappings = map[model.Role]model.TagName{
model.RoleComposer: model.TagMusicBrainzComposerID,
model.RoleLyricist: model.TagMusicBrainzLyricistID,
model.RoleConductor: model.TagMusicBrainzConductorID,
model.RoleArranger: model.TagMusicBrainzArrangerID,
model.RoleDirector: model.TagMusicBrainzDirectorID,
model.RoleProducer: model.TagMusicBrainzProducerID,
model.RoleEngineer: model.TagMusicBrainzEngineerID,
model.RoleMixer: model.TagMusicBrainzMixerID,
model.RoleRemixer: model.TagMusicBrainzRemixerID,
model.RoleDJMixer: model.TagMusicBrainzDJMixerID,
}
It("should handle more artists than mbids", func() {
for key := range allMappings {
mf = toMediaFile(map[string][]string{
key.String(): {"a", "b", "c"},
allMappings[key].String(): {"f634bf6d-d66a-425d-888a-28ad39392759", "3dfa3c70-d7d3-4b97-b953-c298dd305e12"},
})
participants := mf.Participants
Expect(participants).To(HaveKeyWithValue(key, HaveLen(3)))
roles := participants[key]
Expect(roles[0].Name).To(Equal("a"))
Expect(roles[1].Name).To(Equal("b"))
Expect(roles[2].Name).To(Equal("c"))
Expect(roles[0].MbzArtistID).To(Equal("f634bf6d-d66a-425d-888a-28ad39392759"))
Expect(roles[1].MbzArtistID).To(Equal("3dfa3c70-d7d3-4b97-b953-c298dd305e12"))
Expect(roles[2].MbzArtistID).To(Equal(""))
}
})
It("should handle more mbids than artists", func() {
for key := range allMappings {
mf = toMediaFile(map[string][]string{
key.String(): {"a", "b"},
allMappings[key].String(): {"f634bf6d-d66a-425d-888a-28ad39392759", "3dfa3c70-d7d3-4b97-b953-c298dd305e12"},
})
participants := mf.Participants
Expect(participants).To(HaveKeyWithValue(key, HaveLen(2)))
roles := participants[key]
Expect(roles[0].Name).To(Equal("a"))
Expect(roles[1].Name).To(Equal("b"))
Expect(roles[0].MbzArtistID).To(Equal("f634bf6d-d66a-425d-888a-28ad39392759"))
Expect(roles[1].MbzArtistID).To(Equal("3dfa3c70-d7d3-4b97-b953-c298dd305e12"))
}
})
It("should refuse duplicate names if no mbid specified", func() {
for key := range allMappings {
mf = toMediaFile(map[string][]string{
key.String(): {"a", "b", "a", "a"},
})
participants := mf.Participants
Expect(participants).To(HaveKeyWithValue(key, HaveLen(2)))
roles := participants[key]
Expect(roles[0].Name).To(Equal("a"))
Expect(roles[0].MbzArtistID).To(Equal(""))
Expect(roles[1].Name).To(Equal("b"))
Expect(roles[1].MbzArtistID).To(Equal(""))
}
})
})
})

View file

@ -132,11 +132,12 @@ func (p Participants) Merge(other Participants) {
func (p Participants) add(role Role, participants ...Participant) {
seen := make(map[string]struct{}, len(p[role]))
for _, artist := range p[role] {
seen[artist.ID] = struct{}{}
seen[artist.ID+artist.SubRole] = struct{}{}
}
for _, participant := range participants {
if _, ok := seen[participant.ID]; !ok {
seen[participant.ID] = struct{}{}
key := participant.ID + participant.SubRole
if _, ok := seen[key]; !ok {
seen[key] = struct{}{}
p[role] = append(p[role], participant)
}
}

View file

@ -78,6 +78,23 @@ var _ = Describe("Participants", func() {
RoleArtist: []Participant{_p("1", "Artist1"), _p("2", "Artist2")},
}))
})
It("adds the artist with and without subrole", func() {
participants = Participants{}
participants.Add(RolePerformer, Artist{ID: "3", Name: "Artist3"})
participants.AddWithSubRole(RolePerformer, "SubRole", Artist{ID: "3", Name: "Artist3"})
artist3 := _p("3", "Artist3")
artist3WithSubRole := artist3
artist3WithSubRole.SubRole = "SubRole"
Expect(participants[RolePerformer]).To(HaveLen(2))
Expect(participants).To(Equal(Participants{
RolePerformer: []Participant{
artist3,
artist3WithSubRole,
},
}))
})
})
Describe("Merge", func() {

View file

@ -241,4 +241,16 @@ const (
TagMusicBrainzAlbumArtistID TagName = "musicbrainz_albumartistid"
TagMusicBrainzAlbumID TagName = "musicbrainz_albumid"
TagMusicBrainzReleaseGroupID TagName = "musicbrainz_releasegroupid"
TagMusicBrainzComposerID TagName = "musicbrainz_composerid"
TagMusicBrainzLyricistID TagName = "musicbrainz_lyricistid"
TagMusicBrainzDirectorID TagName = "musicbrainz_directorid"
TagMusicBrainzProducerID TagName = "musicbrainz_producerid"
TagMusicBrainzEngineerID TagName = "musicbrainz_engineerid"
TagMusicBrainzMixerID TagName = "musicbrainz_mixerid"
TagMusicBrainzRemixerID TagName = "musicbrainz_remixerid"
TagMusicBrainzDJMixerID TagName = "musicbrainz_djmixerid"
TagMusicBrainzConductorID TagName = "musicbrainz_conductorid"
TagMusicBrainzArrangerID TagName = "musicbrainz_arrangerid"
TagMusicBrainzPerformerID TagName = "musicbrainz_performerid"
)

View file

@ -135,6 +135,36 @@ main:
musicbrainz_releasegroupid:
aliases: [ txxx:musicbrainz release group id, musicbrainz_releasegroupid, ----:com.apple.itunes:musicbrainz release group id, musicbrainz/release group id ]
type: uuid
musicbrainz_composerid:
aliases: [ txxx:musicbrainz composer id, musicbrainz_composerid, musicbrainz_composer_id, ----:com.apple.itunes:musicbrainz composer id, musicbrainz/composer id ]
type: uuid
musicbrainz_lyricistid:
aliases: [ txxx:musicbrainz lyricist id, musicbrainz_lyricistid, musicbrainz_lyricist_id, ----:com.apple.itunes:musicbrainz lyricist id, musicbrainz/lyricist id ]
type: uuid
musicbrainz_directorid:
aliases: [ txxx:musicbrainz director id, musicbrainz_directorid, musicbrainz_director_id, ----:com.apple.itunes:musicbrainz director id, musicbrainz/director id ]
type: uuid
musicbrainz_producerid:
aliases: [ txxx:musicbrainz producer id, musicbrainz_producerid, musicbrainz_producer_id, ----:com.apple.itunes:musicbrainz producer id, musicbrainz/producer id ]
type: uuid
musicbrainz_engineerid:
aliases: [ txxx:musicbrainz engineer id, musicbrainz_engineerid, musicbrainz_engineer_id, ----:com.apple.itunes:musicbrainz engineer id, musicbrainz/engineer id ]
type: uuid
musicbrainz_mixerid:
aliases: [ txxx:musicbrainz mixer id, musicbrainz_mixerid, musicbrainz_mixer_id, ----:com.apple.itunes:musicbrainz mixer id, musicbrainz/mixer id ]
type: uuid
musicbrainz_remixerid:
aliases: [ txxx:musicbrainz remixer id, musicbrainz_remixerid, musicbrainz_remixer_id, ----:com.apple.itunes:musicbrainz remixer id, musicbrainz/remixer id ]
type: uuid
musicbrainz_djmixerid:
aliases: [ txxx:musicbrainz djmixer id, musicbrainz_djmixerid, musicbrainz_djmixer_id, ----:com.apple.itunes:musicbrainz djmixer id, musicbrainz/djmixer id ]
type: uuid
musicbrainz_conductorid:
aliases: [ txxx:musicbrainz conductor id, musicbrainz_conductorid, musicbrainz_conductor_id, ----:com.apple.itunes:musicbrainz conductor id, musicbrainz/conductor id ]
type: uuid
musicbrainz_arrangerid:
aliases: [ txxx:musicbrainz arranger id, musicbrainz_arrangerid, musicbrainz_arranger_id, ----:com.apple.itunes:musicbrainz arranger id, musicbrainz/arranger id ]
type: uuid
releasetype:
aliases: [ txxx:musicbrainz album type, releasetype, musicbrainz_albumtype, ----:com.apple.itunes:musicbrainz album type, musicbrainz/album type ]
album: true
@ -154,6 +184,9 @@ main:
performer:
aliases: [performer]
type: pair
musicbrainz_performerid:
aliases: [ txxx:musicbrainz performer id, musicbrainz_performerid, musicbrainz_performer_id, ----:com.apple.itunes:musicbrainz performer id, musicbrainz/performer id ]
type: pair
explicitstatus:
aliases: [ itunesadvisory, rtng ]

Binary file not shown.

Binary file not shown.

Binary file not shown.

Binary file not shown.

Binary file not shown.

Binary file not shown.

Binary file not shown.

Binary file not shown.

BIN
tests/fixtures/test.wv vendored

Binary file not shown.