New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
euri10 opened this Issue Jul 29, 2013 · 39 comments

Comments

Projects
None yet
@euri10
Copy link
Contributor

euri10 commented Jul 29, 2013

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

This comment has been minimized.

Copy link
Member

sampsyo commented Jul 29, 2013

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

This comment has been minimized.

Copy link
Contributor Author

euri10 commented Jul 30, 2013

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 ?

@euri10 euri10 closed this Jul 30, 2013

@euri10 euri10 reopened this Sep 14, 2013

@sampsyo

This comment has been minimized.

Copy link
Member

sampsyo commented Sep 15, 2013

Thanks for trying to coordinate between these two projects! 😄

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 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

This comment has been minimized.

Copy link
Contributor Author

euri10 commented Sep 16, 2013

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:
😄]

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//issues/350#issuecomment-24481807
.

@sampsyo

This comment has been minimized.

Copy link
Member

sampsyo commented Oct 9, 2013

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

This comment has been minimized.

Copy link

mfiano commented Oct 9, 2013

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

This comment has been minimized.

Copy link
Contributor

mineo commented Oct 17, 2013

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

@deonspengler

This comment has been minimized.

Copy link

deonspengler commented Mar 8, 2014

Hi, has this been implemented yet?

@sampsyo

This comment has been minimized.

Copy link
Member

sampsyo commented Mar 8, 2014

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

  • Implement the config option described a few comments up and open a pull request.
  • Contact the authors of ncmpcpp and advocate for changing this behavior. If DISC and DISCNUMBER are both set on a file, it should only display one of them.
@carnager

This comment has been minimized.

Copy link

carnager commented Oct 24, 2014

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

@dennishamester

This comment has been minimized.

Copy link

dennishamester commented Aug 1, 2015

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

@sampsyo

This comment has been minimized.

Copy link
Member

sampsyo commented Aug 1, 2015

Awesome! Here's hoping that gets merged.

@jreinert

This comment has been minimized.

Copy link

jreinert commented Mar 1, 2016

i tried my luck contributing to mpd and fixing the issue "at the root" but it didn't go particularily well... MusicPlayerDaemon/MPD#2

@sampsyo

This comment has been minimized.

Copy link
Member

sampsyo commented Mar 2, 2016

Wow, what a ridiculously hostile developer. 😢

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.

@dennishamester

This comment has been minimized.

Copy link

dennishamester commented Mar 2, 2016

I tried something similar some time ago:
http://thread.gmane.org/gmane.comp.audio.musicpd.devel/3377
dennishamester/MPD@cbb5d2d

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

@sampsyo

This comment has been minimized.

Copy link
Member

sampsyo commented Mar 2, 2016

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

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

@carnager

This comment has been minimized.

Copy link

carnager commented Mar 2, 2016

maybe a pure musicbrainz config option 😄

@carnager

This comment has been minimized.

Copy link

carnager commented Mar 2, 2016

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

This comment has been minimized.

Copy link
Member

sampsyo commented Mar 2, 2016

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

@carnager

This comment has been minimized.

Copy link

carnager commented Mar 3, 2016

@sampsyo

This comment has been minimized.

Copy link
Member

sampsyo commented Mar 3, 2016

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

This comment has been minimized.

Copy link

sentriz commented Jun 23, 2016

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

This comment has been minimized.

Copy link
Member

sampsyo commented Jun 23, 2016

No problem!

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

  • Tools disagree about which of the two names tracknumber and trackc should be used for this data.
  • So, to maximize compatibility, beets (really, MediaFile) uses a "read any, write all" policy.
  • This works great for 99% of software, which reads one or the other but doesn't care if both are set.
  • ncmpcpp is the 1% that reads and displays both fields, even when they hold the same value.

We have two bad options:

  • Status quo: ncmpcpp breaks. (And it sounds like the ncmpcpp developers are hostile toward any changes in this regard.)
  • Remove one or the other: we break unknown software that has been silently working fine so far.

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

@dennishamester

This comment has been minimized.

Copy link

dennishamester commented Jun 23, 2016

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

This comment has been minimized.

Copy link
Member

sampsyo commented Jun 23, 2016

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

This comment has been minimized.

Copy link

carnager commented Jun 23, 2016

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

This comment has been minimized.

Copy link

sentriz commented Jun 24, 2016

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

This comment has been minimized.

Copy link
Member

sampsyo commented Jun 24, 2016

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

@ArchangeGabriel

This comment has been minimized.

Copy link

ArchangeGabriel commented Nov 22, 2016

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

  • According to me and my understanding of thing, mpd has the right behaviour. Since it provides tag editing interface for client, mpd should show all existing tags (unless this interface is accessed differently than the playing one?). What can be argued however is that ncmpcpp should only show one when multiple are presents in the playing mode, while displaying them all in the editing mode. Fun fact is I didn’t even tried ncmpcpp on my beets collection yet, thought this is in fact my current player, but came here because of my second point.

  • This is not just an mpd (world) issue: this is an issue for anyone editing tags with other things than beets, they would look at files cluttered by numerous similar tags (and that’s my case, because beets can’t — or at least I don’t know how — fill all my needs, so I also use ExFalso aside). Or even just look at them with things like ffprobe or the like. And you can’t say or convince me that this is not the right behaviour for these tools, yet you might not want to agree with beets maximum compatibility policy and so not having extra tags for those things.

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

This comment has been minimized.

Copy link
Contributor

mineo commented Nov 22, 2016

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

This comment has been minimized.

Copy link

ArchangeGabriel commented Nov 22, 2016

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

This comment has been minimized.

Copy link
Member

sampsyo commented Nov 22, 2016

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

@ArchangeGabriel

This comment has been minimized.

Copy link

ArchangeGabriel commented Nov 22, 2016

Where in the code does beets/MediaFile write tags?

@sampsyo

This comment has been minimized.

Copy link
Member

sampsyo commented Nov 23, 2016

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

This comment has been minimized.

Copy link

dinojr commented Apr 6, 2017

@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.

@jreinert

This comment has been minimized.

Copy link

jreinert commented Apr 16, 2017

@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

This comment has been minimized.

Copy link

jtpavlock commented Jun 24, 2017

See arybczak/ncmpcpp@6ebf00e for those just worried about ncmpcpp

@sampsyo

This comment has been minimized.

Copy link
Member

sampsyo commented Jun 24, 2017

Great news!

@carnager

This comment has been minimized.

Copy link

carnager commented Jun 24, 2017

yeah for relaying an actual issue to downstream...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment