beetbox / beets

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

Choosing "Use as-is" to resolve a match prevent the writing of tags via plugins #726

Open Kraymer opened 10 years ago

Kraymer commented 10 years ago

I spent lot of time figuring why sometimes lyrics and genre fetched via lyrics and lastgenre plugins were not written in the files metadata. After reading the docs I figured that the "Use as-is" option was the culprit.
It's not intuitive for me as the [A]pply, More candidates, Skip, Use as-is... choice exists to correct track tags. When I pick Use as-is that's equivalent for me to "i know that my titles/album/artist/track numbers are correct, please don't get them from musicbrainz". Yet, genre and lyrics have nothing to do with track identification, and I expected the behaviour of these plugins to not be impacted by my previous choice of identification method : I want lyrics and genres to be fetched no matter what. (and if i wanted to keep original lyrics/genre when they exist I would use the plugins force option)

Does that make sense to decorrelate the 'use as-is' pick from lyrics/genres writing or am i missing something?

sampsyo commented 10 years ago

This is a good point; it's certainly confusing behavior, that I've run into also, when you notice that genres are missing (for example) just because you couldn't find a match in MusicBrainz.

But there's a tension here between expectations: at least it's easy to explain that "as-is" really means your files are added as is; no modifications at all are made to the files.

It's worth asking everyone who's seeing these issues: what do you think? Which is more surprising when choosing as-is: not having plugin-provided metadata applied to files or not having "as-is" guaranteed to avoid touching files altogether?

My guess is that @KraYmer's position is right and that we should make tags get written for as-is imports. If we do this, we also need to be careful with non-autotagged imports (-A), which are currently handled the same way as the "as-is" selection. Changing one with affect both unless we do extra work to disambiguate.

Kraymer commented 10 years ago

Agree about the risk of surprising by not having "as-is" guaranteed to avoid touching files. Is it possible to reword "as-is" ? Something in the vein of "Keep core tags" ... but hopefully better phrased :smirk:

rahedges commented 10 years ago

I have been struggling with the "use-as-is" option as well. I have a large FLAC library that I have assembled over the years. Almost all of the files were tagged when I ripped them. I have some obscure titles (African drumming) that won't get a match from the taggers. I want to be able to say use this as is, BUT to have beets move and organize the files based on my tags. Since I am using the LastGenre plugin and use genre in my path, all of my 'as-is' files are but in the 'unknown' category regardless of the genre that is in the flac metadata.

Could beets make use of the tags already in files if the "use-as-is" options is selected?

sampsyo commented 10 years ago

@djibi2 Something must be going wrong—that's exactly what "use as-is" is supposed to do. It uses the tags as they are on your files. (This issue is about running plugin actions.)

Are you finding that beets is ignoring your genres on import? If this is reproducible, maybe we need a separate ticket.

rahedges commented 10 years ago

I will doublecheck the behaviour and create a new item if nescessary, it may be related to the lastgenre plugin.

On Sat, Aug 23, 2014 at 11:40 AM, Adrian Sampson notifications@github.com wrote:

@djibi2 https://github.com/djibi2 Something must be going wrong—that's exactly what "use as-is" is supposed to do. It uses the tags as they are on your files. (This issue is about running plugin actions.)

Are you finding that beets is ignoring your genres on import? If this is reproducible, maybe we need a separate ticket.

— Reply to this email directly or view it on GitHub https://github.com/sampsyo/beets/issues/726#issuecomment-53158101.

rahedges commented 10 years ago

After checking, I should clarify my comment, as I do think it relates to the lastGenre plugin. When I use the as-is option it does organize the files approriately from the existing tags. However, the genre information isn't used and so the lastGenre plugin uses the fallback "Unknown" even if the existing tag has a genre.

sampsyo commented 10 years ago

Sounds like a separate issue is indeed in order.

robotanarchy commented 9 years ago

I'm also affected by this issue, in my case I'd expect beets to apply replay gain for the "use as is" imported files.

How about setting a config variable for what can be kept (eg. replaygain lyrics ...). The user would then see [U]se as is (but apply replaygain, lyrics, ...) - and it would just do that.

What do you guys think?

Kraymer commented 9 years ago

How about setting a config variable for what can be kept (eg. replaygain lyrics ...).

Good point. :zap: :+1: Everyone has different expectations concerning the Use as-is scope, being able to list the plugins to execute seems like the right approach.

robotanarchy commented 9 years ago

Thanks! @sampsyo what do you think about this?

sampsyo commented 9 years ago

This sounds like an acceptable interface, but, unfortunately, it's not 100% obvious how the option would work. Some questions:

It might still be worth considering the simpler solution: just enable this kind of plugin for all as-is imports (no config option). If someone really wants to leave their files untouched, they can use the -A flag. This might end up being more predictable than the current solution. How would people feel about that?

robotanarchy commented 9 years ago

My personal opinion: The simple solution you have suggested doesn't work with the lyrics or lastgenre plugins (because we certainly don't want them to be default for use-as-is, without any config, right?), so I'd favor the one with the config (I know it is harder to implement).

For the bullet points, i'd say:

Again, this is just my opinion, from a users point of view. I don't really know the beets code and can't tell if this makes sense on the implementation side.

Kraymer commented 9 years ago

The simple solution you have suggested doesn't work with the lyrics or lastgenre plugins (because we certainly don't want them to be default for use-as-is, without any config, right?)

the use-as-is should be reworded anyway. if you consider that "use as is" means "use tags used for identification as is" (trackname, track number, artist, album -- i think?) then @sampsyo simpler solution is fine imo.

sampsyo commented 9 years ago

To summarize the decision we're trying to make, here's what I'd like to ask users:

What do you think the "as-is" import selection should do? (Assuming you have tag-writing enabled and some import-time plugins running.)

  1. Take the files byte-for-byte, exactly as they are, and record their metadata without modification. No import-time plugins run. (For example, no lyrics are fetched and no ReplayGain analysis happens.)
  2. Take the files how they are, but do run import-time plugins to keep information in the database. (For example, fetch the lyrics, but only keep them in the database—do not write them to ID3 tags.)
  3. Work like other import modes, but without matched (i.e., MusicBrainz) metadata application: that is, run import-time plugins and write the resulting tags. (For example, fetch lyrics/genres/etc. and write them to ID3 tags.)

Feel free to comment to vote for one of these three options according to your expectation/desire (or explain why you expect something else).

oldtopman commented 9 years ago

One of the main features of beets is its ability to maintain differing tags between it's internal library and the actual tags on the files; despite that, your hard-separated tags are only one careless beet write away from certain doom.

To me, the most important part of the import process is to have both beet's library and my songs to have matching tags. Sometimes I need the tags to take top priority, with everything else completely ignored, but not necessarily without import-time plugins missing the opportunity to add/change/tweak new songs. I vote option 1.

Kraymer commented 9 years ago

I'm interested by n°3 behaviour for my personal use case. n°1 would be less surprising though if it keeps being named Use as-is.

If we go for n°1, we could add an always present entry into the More candidates screen that would correspond to file identification metadata. To sum up :

daks commented 9 years ago

n°1 seems the more logical for me, given the 'as-is' name, but it may exists cases where you want to run plugins or modify tags, and then i think it could be great to have an option for it.

elwoodpd commented 9 years ago

I can't imagine anyone wanting anything other than 1 or 3. If they wanted something like 2 to happen they would have run beets in the mode where it doesn't tag any files at all.

Maybe have another option at run time: don't touch. Then call as-is something like don't tag. The fact a don't touch option was there would make it obvious that don't tag will touch the files (or the db entry). I am concious these two start with the same letters but I am sure you could think of something better.

This might confuse people who have their set up to never touch id3 tags but if the documentation explained the language in the same place it explained the write options I think it would be OK.

robotanarchy commented 9 years ago

At the very least, replaygain should always be done. Because the point is, that all files have replaygain. Otherwise replaygain is useless. Also the mpdupdate and importfeeds should always run.

Depending on how much work anyone is willing to put into this issue, I'd like to see the following: