beetbox / beets

music library manager and MusicBrainz tagger
http://beets.io/
MIT License
12.69k stars 1.81k forks source link

Disable multiple tag names for free-form tag formats #350

Open euri10 opened 11 years ago

euri10 commented 11 years ago

I don't know if this comes from beets or if it's me configuring ncmpcpp poorly. I got this line in .ncmpcpp/config : song_columns_list_format = "(5)[red]{n}(5f)[green]{l} (25)[cyan]{a} (40)[]{t|f} (25)[red]{b}" This is the default way to display the songs in the column format in ncmpcpp except that I added the track number: (5)[red]{n} The issue is that tracks are displayed that way : track If I go into the ncmpcpp tag editor : tags Of course all those tracks were imported with beets, it seems like a strange format. Is that a bug from beets mistagging the tracks or a ncmcpp rendering issue ?

sampsyo commented 11 years ago

I'm not overly familiar with ncmpcpp (i.e., I don't know what that configuration string means) -- does the information appear the same in other tools the same way? Beets doesn't do anything funky with track number tags, so it's probably something on their end. Have you tried asking any of the developers on that project?

euri10 commented 11 years ago

I asked on the ncmpcpp bug tracker (http://bugs.musicpd.org/view.php?id=3804) I'll let you know what comes from that.

edit : well dev there says I have twice the track number as a tag ("You just have two the same track tags in your files, that's why they're displayed two times. "), so might come from beets since it's the only thing that touches tags no ? what can I test to see if this comes or not from here ?

sampsyo commented 10 years ago

Thanks for trying to coordinate between these two projects! :smile:

To get to the bottom of this, though, we're going to need a little more information.

Making a few assumptions, here's my best bet as to what is going on. You're using Ogg Vorbis or FLAC or another format that uses Vorbis Comments-like tag names. For the track number field (and a few others), beets maximizes compatibility by writing the data under two names. Most software in the world only looks at one of these tags. For some reason, ncmpcpp is inspecting the files themselves rather than communicating over the MPD protocol, looking at both tag names, and displaying both values separated by a comma.

If this is the case (again, I'm working with limited information here, so I could be wrong), I think ncmpcpp should be fixed to display only one of the tags. I'm not sure why it would want to display both.

euri10 commented 10 years ago

I'll try to answer as best as I can though I'm not familiar with the internals of ncmpcpp. All I can say is that it's a mpd client, I don't know if it reads the metadata directoy or not. You're absolutely right that it doesn't show up on mp3, only on flac files which represents 99% of my library, hence why I didn't notice. I'll try to reopen the ncmppcpp issue then I think !:)

On Mon, Sep 16, 2013 at 12:20 AM, Adrian Sampson notifications@github.comwrote:

Thanks for trying to coordinate between these two projects! [image: :smile:]

To get to the bottom of this, though, we're going to need a little more information.

  • It's not really clear what unK means by "two the same track tags". As far as I can tell, beets does not duplicate any tags on any files I have.
  • Is ncmpcpp reading the metadata directly or communicating through MPD? (This affects which project(s) have the bug that needs to be fixed.)
  • Which audio format does this show up in?

Making a few assumptions, here's my best bet as to what is going on. You're using Ogg Vorbis or FLAC or another format that uses Vorbis Comments-like tag names. For the track number field (and a few others), beets maximizes compatibility by writing the data under two nameshttps://github.com/sampsyo/beets/blob/master/beets/mediafile.py#L986. Most software in the world only looks at one of these tags. For some reason, ncmpcpp is inspecting the files themselves rather than communicating over the MPD protocol, looking at both tag names, and displaying both values separated by a comma.

If this is the case (again, I'm working with limited information here, so I could be wrong), I think ncmpcpp should be fixed to display only one of the tags. I'm not sure why it would want to display both.

— Reply to this email directly or view it on GitHubhttps://github.com/sampsyo/beets/issues/350#issuecomment-24481807 .

sampsyo commented 10 years ago

This came up in out-of-band reports by a couple of others (@mineo and @mfiano), so here's a brief summary of the problem: MPD (apparently not ncmpcpp itself) interprets multiple keys to mean the track number. Instead of picking just one and trusting it, it instead reports both to the clients. ncmpcpp takes both of these values and displays them. In my view, both policies are somewhat unfortunate, but beets should be able to work around them—hence this issue.

@mineo has a nice fix that makes tags use only standard Vorbis Comments fields. I want to make this slightly more general, since it's also been pointed out that MPD does the same thing for Musepack (i.e., APE tags). So here's what I'm thinking: a new configuration option (called strict_tags, maybe) that causes MediaFile to only write the first of the StorageStyles for each of the tag fields. We'll take care to make the first-listed field name the standard one. This way, we'll maximize compatibility by default but ncmpcpp users can work around the strange display.

Sound right to everybody?

mfiano commented 10 years ago

I just thought I would point out that in at least one case it affected the "year" fields of APEv2 Musepack audio.

Sounds right to me.

mineo commented 10 years ago

If the first field of all storage styles really is the standard one for all formats, then that sounds like a good plan.

deonspengler commented 10 years ago

Hi, has this been implemented yet?

sampsyo commented 10 years ago

@deonspengler No, sorry. If you're interested in helping, there are two ways you could get involved:

carnager commented 9 years ago

this is actually a mpd issue... (at least in parts)

ok, 2 releases by "4 Non Blondes" an album and a single. the album i tagged with both "albumartist" and "albumartist" see what happens: carnager@caprica ~ > mppc search albumartist "4 Non Blondes" --format '{albumartist}' 4 Non Blondes, 4 Non Blondes 4 Non Blondes, 4 Non Blondes 4 Non Blondes, 4 Non Blondes 4 Non Blondes, 4 Non Blondes 4 Non Blondes, 4 Non Blondes 4 Non Blondes, 4 Non Blondes 4 Non Blondes, 4 Non Blondes 4 Non Blondes, 4 Non Blondes 4 Non Blondes, 4 Non Blondes 4 Non Blondes, 4 Non Blondes 4 Non Blondes, 4 Non Blondes 4 Non Blondes 4 Non Blondes 4 Non Blondes 4 Non Blondes

dennis-hamester commented 9 years ago

This really bugged me. I wrote a patch that fixed this in ncmpcpp: arybczak/ncmpcpp#54

sampsyo commented 9 years ago

Awesome! Here's hoping that gets merged.

repomaa commented 8 years ago

i tried my luck contributing to mpd and fixing the issue "at the root" but it didn't go particularily well... https://github.com/MaxKellermann/MPD/pull/2

sampsyo commented 8 years ago

Wow, what a ridiculously hostile developer. :cry:

What about, for example, just using the first tag found? I guess I just don't understand why MPD would ever want to show multiple track tags, even if they're different.

dennis-hamester commented 8 years ago

I tried something similar some time ago: http://thread.gmane.org/gmane.comp.audio.musicpd.devel/3377 https://github.com/dennishamester/MPD/commit/cbb5d2d9a51110d6b322d7dd4f50a8c19289f273

But there was no response from the developer at all. Since then I've given up getting this upstream.

sampsyo commented 8 years ago

Wow; that's really depressing. I suppose this is just another instance of different open-source projects trying to blame each other. :confused:

I guess we're left with trying to invent a config option for beets: compatible_tags: no or some such thing.

carnager commented 8 years ago

maybe a pure musicbrainz config option :smile:

carnager commented 8 years ago

What about, for example, just using the first tag found? I guess I just don't understand why MPD would ever want to show multiple track tags, even if they're different.

this would not be a good solution, since many users use multiple artist tags for collaborations.

sampsyo commented 8 years ago

@carnager, what do you mean by "pure MusicBrainz"?

carnager commented 8 years ago

@sampsyo https://picard.musicbrainz.org/docs/mappings/ (might be outdated)

sampsyo commented 8 years ago

Ah, so exactly matching what Picard in particular uses. I'm not sure if that exact replication is really necessary—to fix the issue at hand, all we need to do is stop writing multiple versions of a tag for compatibility's sake.

sentriz commented 8 years ago

ah sorry @sampsyo - I never found this issue when I searched.

hopefully either mpd, ncmpcpp, or beets comes up with something - it would be great to see this fixed.
when I open up one of the tracks that has the problem in foobar2000, it shows two fields: "tracknumber" and "trackc" - both with the same value, just like ncmpcpp. that was after I rewrote my tags with beets. is it just beets and foobar2000, ncmpcpp and others not agreeing on a standard?

sampsyo commented 8 years ago

No problem!

Here's a brief summary of the current state of things:

We have two bad options:

I think the only option for us is to add a configuration option specifically for people who want to use ncmpcpp.

dennis-hamester commented 8 years ago

Just to clarify, this is not a pure ncmpcpp issue. MPD reads the files and stores the same tracknumber twice. ncmpcpp just happens to be the client that displays two tracknumbers iff mpd sends two or more. I also never got the impression that the ncmpcpp maintainers are hostile towards changes. The MPD maintainer however, doesn't even want to discuss the matter.

My suggestion is to go with a new configuration option in beets, even if this seems more like a workaround than an actual solution.

sampsyo commented 8 years ago

Yes, thanks for clarifying -- and my apologies for remembering wrong; it was the MPD developer who shut down the conversation, not the ncmpcpp people at all.

carnager commented 8 years ago

to be fair, it's pretty easy for mpd client devs to filter out duplicate tags. python-mpd2 for example will create a list, if there are more than one tag for a tagtype.

sentriz commented 8 years ago

Would it be possible to stop writing one of the two tags with a plugin like Zero? or does beets consider both trackc and tracknumber the same tag?

sampsyo commented 8 years ago

Alas, no—it's just as you suspected; zero sees them as one logical tag.

ArchangeGabriel commented 7 years ago

Hey there, new here but definitively affected by this and wanted to share my thoughts on it:

So, yes, please add a strict_tags option or a way to disable some tags to be written (since this can’t be done in current zero plugin). This is actually a blocking issue for me.

mineo commented 7 years ago

Since it provides tag editing interface for client, mpd should show all existing tags

MPD doesn't provide a tag editing interface, ncmpcpp does that via taglib - that's why you have to configure mpd_music_dir in ncmpcpp for the tag editor to work.

ArchangeGabriel commented 7 years ago

Hum, OK. So must do my other MPD clients I suppose then. Then I suppose MPD should not care about non strict tags too or only consider one for each type, but we know this is not going to happen (could in ncmpcpp though).

Still, my second (and main for me) point is still valid.

sampsyo commented 7 years ago

Any help to add a mode like this would be greatly appreciated!

ArchangeGabriel commented 7 years ago

Where in the code does beets/MediaFile write tags?

sampsyo commented 7 years ago

Well, that's pretty much the job of MediaFile—it's our library for reading and writing tags. Specifically, you might want to start with the store methods.

dinojr commented 7 years ago

@jreinert I'm trying to use your fork of MaxKellerman/MPD but it won't build: I get

In file included from src/decoder/DecoderBuffer.cxx:21:0:
src/decoder/DecoderBuffer.hxx:41:20: error: ‘uint8_t’ was not declared in this scope
  DynamicFifoBuffer<uint8_t> buffer;

I've tried reporting the issue at your github fork but it seems you have disabled the issue tracking.

repomaa commented 7 years ago

@dinojr i completely forgot about my MPD fork. I merged the upstream master into my fork and pushed the changes. It builds at least.

jtpavlock commented 7 years ago

See https://github.com/arybczak/ncmpcpp/commit/6ebf00eb5d3e6a66ad567aa09ca575f5e0c9a982 for those just worried about ncmpcpp

sampsyo commented 7 years ago

Great news!

carnager commented 7 years ago

yeah for relaying an actual issue to downstream...