Create arranger_sort, lyricist_sort, performer and performer_sort tags

[dosoe]

In the same idea than #2529 , but since those tags are not in the MB-Picard tag mapping, I didn't do anything to write the tags to the files.
Else everything is pretty much copy-paste from #2529 . It's all following the philosophy that where there is an artist name, there should be his sort name.

I have an issue with the 'arranger' tag: in MusicBrainz there are two arranger tags: one that applies to recordings, and one that applies to works. So far we a only fetching the one that is about recordings, but especially for classical music the work-arranger is more used. The question is: should we make two separate tags, one work-arranger and a recording-arranger?

I also have an issue about the 'lyricist' tag: in classical music and especially in opera, the usual term is 'librettist' and MB has a separate tag for that. Should we keep them separate as well or just merge the two terms?

[mention-bot]

@dosoe, thanks for your PR! By analyzing the history of the files in this pull request, we identified @sampsyo, @geigerzaehler and @brunal to be potential reviewers.

[sampsyo]

Looking good so far! Please try running the tests, though—looks like they're complaining about a missing field on TrackInfo. https://travis-ci.org/beetbox/beets/jobs/232782949

[dosoe]

It's strange the test is only complaining about lyricist_sort, since I did exactly the same with lyricist_sort than with arranger_sort.

[dosoe]

Hi! So I don't get what's wrong and I don't see where the problem is according to the logs. I could try out some stuff, but is there a way I can run the tests before uploading and committing?

[sampsyo]

Yeah, absolutely! There's background on the testing setup and instructions for running them on the wiki: https://github.com/beetbox/beets/wiki/Testing

Weird that the test failure is coming from the edit plugin. Maybe it would be worth running the edit command interactively to see if something strange happens with those fields? I'm guessing there's a hard-coded list of fields somewhere shameful that needs constant updating, which is somewhat sad…

[dosoe]

Well, there is one in test.test_mediafile.py and in test._common.py, but adding it there still makes the tests fail.

[dosoe]

reimporting files and making 'beet -vv edit --all' doesn't raise any problems, and the lyricist_sort is fine (in the recording I tested there is no arranger). However, the tests still fail. I cannot really see why because 'python setup.py test' is overly verbose.

[dosoe]

Could all the problems be related to the ReadWriteTestBase class in test.test_mediafile.py ?

[sampsyo]

OK! I'll investigate further, but FWIW, you can get compact test output for a specific test file by running nosetests test/test_edit.py.

[sampsyo]

OK! I've addressed the test problem in #2605, so it should be possible for this PR to move forward. It looks like everything's mostly in place, except for a changelog entry?

[dosoe]

Great! Thank you very much.

[dosoe]

Ok, I synched up my fork with the main repo in the hope that your fix would clear the problems, but it seems like the tests are still failing. Apparently, there is a list of tags that I missed to update.

[sampsyo]

Ah, cool—I think the problem is just that there are no tag definitions for MediaFile, but you've added the tags to the list of fields to be tested in test_mediafile.py. So we can either add those MediaFile tags, or we can remove them from test_mediafile.py and let them remain database-only tags.

[dosoe]

Ok I tried to work through it, it seems like the release dicts that are in input of track_info contain the arranger and lyricist but not the performers. This doesn't make sense to me, I guess I'm not getting something. Worst case would be that we need to make a supplementary api request but that shouldn't be necessary normally.

[dosoe]

It's all very confusing to me. Sometimes this code gets the performer just fine, sometimes it doesn't get anything.

[dosoe]

I finally sorted it out. When I hit beet update or beet import -L the performers don't get fetched, but with beet mbsync they get fetched all fine. So I would declare this pull request ready to merge.

[dosoe]

Hi @sampsyo , as I said a few posts earlier, I can add a bootload of tags from MusicBrainz like album performers, recording date (actually an intervall), recording place, DJ and so on. The question is now if I should do this. What do you want from beets? Do you just want it to have the necessary information to sort your music or do you want it to contain all kind of relevant information about your music, becoming some kind of offline-musicbrainz? I can add as many tags as I can from MB without problem but maybe at some point there will be too many tags (DJ, DJ_sort, Mixer, Mixer_sort, Producer, Producer_sort, Engineer, Engineer_sort, Recording_place, Recording_date, Album_performer, Album_performer_sort, Recording_area and all the respective mb_ids...) and make beets less efficient and usable because of the flood of tags (it shouldn't make it slower however because it doesn't use any more API requests), on the other hand if maybe some day other programs use the beets database (as I would imagine to be possible on linux at least) this could be interesting.
So what is your opinion on this?
I am very comfortable with having a lot of information in the beets database and would keep adding tags since it is not much work and potentially useful.

[dosoe]

Actually it would be great if amarok and clementine were able to read the beets databases.

[sampsyo]

I actually don't mind the idea of being "omnivorous" with the metadata that we fetch from MB. It doesn't really cost us anything to store extra flexible fields, as long as we don't endeavor to invent on-disk storage formats (e.g., ID3 tags) for every new field. A couple of contingencies come to mind:

• To make this work easier, I think it would be useful to start by refactoring the AlbumInfo and TrackInfo classes, as described in the thread for Extra fields from MusicBrainz (missing fields) #1547. The idea would be that we shouldn't need to add new explicit fields to those structures; they should dynamically expand to carry whatever metadata we throw at them.
• For credits like DJ, mixer, producer, engineer, etc., we should think carefully ahead of time about how to give them a consistent structure. This could be as little as deciding on a good naming convention, or as much as deciding that we actually want a generic "artist credit" tag with different "flavors" of some kind for each role.

[dosoe]

Concerning the consistent structure, I would just take the one from MusicBrainz, since they had time, competence and interest to deal with it, but it's not too hard changing it.

[dosoe]

I am not a programmer, so I don't fully understand what is in #1547 . Would it just be to make a dictionary out of TrackInfo and AlbumInfo and then adapt autotag/hooks.py? This would simplify the structure a lot: for each tracks, make a performer dictionary and for each performer make a dict with name, sort_name, credit, mb_id, instrument, role, disambig and similarly for all the other tags. Then I would need to understand more in depth what autotag/hooks.py and especially autotag/__init__.py work but I could definitely get going.
Would all the plugins that fetch metadata (like parentwork) have to be rewritten accordingly or would there be a way around?

[bearcatsandor]

Personally, i'd rather have too much information in my beets database than not enough. I mean beets is the music tagger for OCD people, right? It might as well live up to it's name.

[dosoe]

Also my opinion. But redesigning the storing of the metadata would be interesting as well. By the way, @sampsyo is there a reason my 2 pull requests are still pending else than "too much to do IRL" (i. e. is there something wrong with them)?

[dosoe]

a little merge to keep my fork up to date

[sampsyo]

Sorry for taking so long, and no, there's nothing wrong on your end! It is indeed IRL intrusions on my end; I'll get to this very soon, I hope.

[Holzhaus]

What is the status of this PR? Will fixing the merge conflict suffice to get this merged or are the more problems?

[cole-miller]

I'd find the performer tag in particular very useful; can this be resurrected?

[Holzhaus]

Yup, I hope that beets will support that field, too. If the merge conflics are the only thing blocking this I'd volunteer to fix them.

[Holzhaus]

@dosoe ping

[dosoe]

Hi! There was a discussion on #1547 that basically concluded that we first need to change the way TrackInfo and AlbumInfo works before we can add big batches of new tags. Now this was a long time ago, I don't know if this has been solved since, I know of two attempts that are stalled but maybe I missed something, @sampsyo or @arcresu do you know about this? There is also a more fundamental decision to be made: Do we make a parformer (and the corresponding performer_sort) tag or do we make a tag for each performer (so a conductor, orchestra, violin tag and all the corresponding sort tags)? The first solution is probably simpler to implement, but we need a way to add the instruments of everyone in a proper way. The second solution is more logical, but we would need flexible tags (unless we create a tag for all the + instruments of musicbrainz and then keep almost all of them empty all the time, which can't be a good solution) and we will run into complications for file naming (for using all the performers in the file naming, we would need to check all possible instruments). One solution would be to have kind of a meta-tag 'performer' and then sub-tags that fall in this category (like a 'performer' dictionary that has names and sort names associated to an instrument key). I don't know if this is feasible or even a good idea. If it is feasible, and the path naming stuff gets adapted, then I think it's the better solution.
A minor issue would be how to handle the performers that are not attached to a recording but to an album, should we attach them to every track of the album or should be create an album_performer tag? The latter one seems silly to me.
I'm in my last year of PhD, I don't expect to have a lot of time to work on this though, but if you're familiar with musicbrainz, the fetching of the data isn't that difficult, it's the organization of this data in beets that will be the tough part.

[sampsyo]

Yes, that's the way to proceed! I don't think it should be too hard. I believe there may be a stalled effort like that around somewhere but I couldn't find it easily---maybe it doesn't actually exist.

I would start with the simple, single-tag approach. I think the per-instrument tags are likely to be a pretty specialized use case. And I don't really see how to proceed with a "meta-tag" approach.

The "start simple" approach also applies to album performers. Let's start with just tracks and reassess after that.

[Holzhaus]

IMHO the best way would be to check what Picard does and do the same instead of reinventing the wheel. This seems to be pretty well supported by audio players. When I look at the info of a track tagged with Picard in ncmpcpp, I can see all performers.

[jtpavlock]

So to get this straight, this PR won't be merged until a proper implementation of the ideas proposed in #1547 exists?

[sampsyo]

The relevant bits there have actually been addressed, in #3568, I think! It is now possible to easily add new fields that we extract from MB without fiddling with the TrackInfo and AlbumInfo classes.

[dosoe]

Well, I stopped working on this PR, as with the flexible attributes another approach is better in my opinion. I'm trying to do it in #3589

[jtpavlock]

Sounds good. I'll close this in favor of #3589 then