Ignore list for scrub plugin

[m-urban]

I would like to propose a first attempt to the issues described in #1514 and #571. The idea is to have an additional config parameter for the scrub plugin which describes a list of tags that should not be scrubbed during import and other write operations.

scrub:
    auto: yes
    preserve: art year

Generally, the approach is identical to how we currently preserve the album art on a manual scrub execution. Save listed tags → clear tag frame → restore preserved tags.

I am not sure if the code invokes unwanted side effects, but I'd like to throw it out there for further discussion. Presumably, we'd be able to extend the used functions to cover the already implemented restoration of album art as well.

[l-t-k]

Something is wrong on this line. Not sure what it is, but if I remove this all my tests were successful.
http://pastebin.com/QxFuXeTY

[sampsyo]

Thanks for looking into this. The implementation seems fine, but I'm a little uncertain still about the purpose—are we sure that this is the right way to facilitate interaction with your track-year plugin?

In general, I'm a bit worried that users will see this configuration option and be tempted to use it instead of zero, which usually is the right tool for what they want to do. If necessary, we can leave it as an undocumented option specifically for avoiding bad interactions with other plugins.

[arcresu]

I'm inclined to agree that this change is solving the wrong problem. It's adding complexity to the scrub plugin as a workaround to a separate issue. @m-urban are you still interested in pursuing this change or can we close it?

[sandreas]

I would love to see that as new feature (even if it would be undocumented). At the moment i'm using the grouping Tag (GRP1) to store my iTunes-Rating in the mp3-Files and scrub deletes this tag on import - so i also locally patched my scrub.py to skip this tag.

scrub is still the feature i like to use, but i just want to preserve this single Tag grouping - nulling tags via zero is not my favourite option

Another long term possibility would be merging zero, scrub and --from-scratch into one plugin and adjust all the settings to result in the expected behaviour. That would be MUCH clearer in my opinion and would also prevent misunderstandings.

[sampsyo]

I know the current landscape is pretty confusing, and it's true that zero and --from-scratch could probably be reconciled somewhat. But I think it's important to note that scrub is actually very different from these other two bits of functionality. The fundamental difference is that the former deal with beets fields, which are present in the database and use beets-specific names, while the latter deals with file tags, which do not have beets-specific or cross-file-format names.

So while it's possible to do something like this, it's important to keep in mind that there's no way around using format-specific tag names and details (as this PR does). Another way around this is to use extensible MediaFile fields to actually reflect these extra tags in the database, which will naturally preserve their value—a clean way to do that might be more complete solution to this problem. (Not to close the door on this proposal here, which is certainly an easier hack to implement.)

[sandreas]

Thank you for this detailed answer... but let me ask a question - How would you deal with this problem?:

• A bunch of mp3 files
• A bunch of m4a files
• Itunes stores an Tag visible as Grouping to GRP1 in MP3 and to grouping in M4A
• Now i would like beets to remove all tags not supported but only preserve these Grouping Tags

Would this be possible via zero plugin?

Actuallly i am patching the scrub.py manually to preserve GRP1 and grouping keys, which works...

Another nice thing would be, that convert would map these tags from m4a to mp3, which does not work for me at the moment, but i think this can be solved by adding -map_metadata 0 (eventually -movflags use_metadata_tags) to the ffmpeg conversion instructions...

See https://superuser.com/questions/1208273/add-new-and-non-defined-metadata-to-a-mp4-file for details.

[sampsyo]

Indeed, that's what a feature like this would be for. But it sounds like you actually want to preserve the data in that field—including when doing a convert—in which case just modifying scrub won't cut it. What you really want is to add a new metadata field to beets, as I briefly mentioned above:

Another way around this is to use extensible MediaFile fields to actually reflect these extra tags in the database, which will naturally preserve their value—a clean way to do that might be more complete solution to this problem.

I'm referring to this feature available to plugins, which admittedly is pretty hacky by itself. But it does what you want: extend beets' ability to read and write a new tag.

---

Separately, however, I'm intrigued by the fact that you mentioned that this comes from an iTunes field. Back when we were constructing our set of built-in fields, we also tested with what iTunes labeled as "grouping," and came up with this tag mapping:
https://github.com/beetbox/mediafile/blob/deb597f747b0959350bb74cabcea177d2ea1de3c/mediafile.py#L1651-L1656

That is, the ID3 frame is TIT1, and the MPEG-4 tag is ©grp. Has iTunes changed its behavior since then?

[sandreas]

Well, this is getting interesting. Thank you very much for pointing out the MediaFile extension possibility, perhaps this would make things a lot easier without having to hack the scrub plugin.

TLDR: If you change the "Grouping" on iTunes, it changes GRP1 for mp3 files and ©grp for m4a files. I don't know, if this behavior has changed, but this is how it is in iTunes Version 12.9.5.7.

Should I add an issue for this?

More information:

I downloaded the files empty.mp3, empty.m4a and empty.alac.m4a (flac, etc. are not supported) from beets/test/rsrc/ added them to a iTunes Version 12.9.5.7 on Windows (!), because i wanted to provide sample files for you. However, changing the Grouping in iTunes, did not result in any change when dumping metadata with ffmpeg / ffplay or mid3v2 (python-mutagen) on ubuntu - BUT the files definitely changed (i checked the hash).

To ensure, that this is not a windows specific problem, i changed the grouping for the same files on my Mac with iTunes, same result with ffmpeg / ffplay or mid3v2.

Using Mp3tag v.2.9.1 (https://www.mp3tag.de/en/index.html) shows, that following Mp3tag-internal tags have changed:

empty.mp3 => GROUPING
empty.m4a => CONTENTGROUP
empty.alac.m4a => CONTENTGROUP

Regarding to the mapping table on https://help.mp3tag.de/main_tags.html:

GROUPING maps to raw tag GRP1 for mp3 but has no valid raw mapping for mp4 / m4a.
CONTENTGROUP maps to raw tag TIT1 (Work name) for mp3 and the ©grp (Grouping) for mp4 / m4a.

See the attached files for details: itunes_grouping.zip

[sampsyo]

Thanks for investigating! Here are some things I found:

• TIT1 is in the ID3 standard; GRP1 is not.
• This mp3tag-related bug suggests that iTunes, in some 12.x version, changed the grouping tag to map to GRP1, leaving TIT1 for the work name. More on the mp3tag forum.
• In Picard's tag mapping, there's a special "save iTunes compatible grouping and work" config option that hacks around exactly this problem.

Seems like something we need to address! I've filed an issue at beetbox/mediafile#15.

[sandreas]

Seems like something we need to address! I've filed an issue at beetbox/mediafile#15.

Nice... thank you. I'll treat this as fixed for now and keep track of the other issue...

[m-urban]

Regarding the original PR from four years ago, I do need to admit that it is a workaround for an entirely different problem. I'm happy that it fostered more discussions, though.
If it was just for me, we can close the PR. If it's still useful to others, feel free to continue working on it.

[sandreas]

For me fixing issue beetbox/mediafile#15 does resolve the main problem. If beets does support the GRP1 on import and convert, there is nothing left to do for me :-)

I'm also happy that the pull request exists, because it made many things much clearer.

But I still think, that the cleanest solution would be to merge the scrub and the zero plugin... an example:

zero:
    default_behaviour: scrub # scrub|zero
    fields: month day genre comments !grouping
    # keep_fields: !!! this option would be obsolete, a much cleaner approach I think !!! 
    # comments: [EAC, LAME, from.+collection, 'ripped by']
    # genre: [rnb, 'power metal']
    update_database: true

• If default behaviour is zero, which would be the default setting, zero would behave like it does at the moment.
• If default behaviour is scrub, zero plugin would behave like the scrub plugin, except the given fields, they will be zeroed - if there is no field configured, it would be the scrub plugin
• If a field name contains a !, it would be treated as keep_fields - I don't like the fact, that it is possible, to configure fields and keep_fields at the same time, although this would result in unexpected behaviour: Remember to set only fields or keep_fields—not both!

What do you think about this idea?

[sampsyo]

Hmm… I get the idea here to merge these two things, but it sort of seems like your idea for a default_behaviour option boils down to saying "new plugin, please behave either like the old 'zero' plugin or the old 'scrub' plugin." So while that nominally brings the two together, from my perspective, it doesn't seem to be any less confusing than the current situation. Does that make sense?

[sandreas]

boils down to saying "new plugin, please behave either like the old 'zero' plugin or the old 'scrub' plugin."

Good point... well, in my opinion zero, scrub and --from-scratch all exist to remove unwanted information, which is kind of ambiguous...

The problem is, that scrub is a blacklist (scrub all but the known internal fields) applied automatically, that cannot be modified, while zero is a is whitelist applied to just these fields, that are explicitly defined in the beets config.

After your comment I really think these two approaches are difficult to merge.

rsync for example provides both approaches by the --include and --exclude parameters, where include is automatically whitelisting, while exclude only is a blacklisting. This is a very nice way to do this - unfortunately also a bit confusing.

But I'm wasting your time... preserve would be nice for scrub, but for me it is not necessary any more, once the issue with GRP1 is fixed. Since there seems to be no elegant solution for this problem, just keep up the good work, for my purposes this issue can be closed.

Thank you for the kind and instructive discussion...

[jtpavlock]

Closing since the original author has abandoned and it's current implementation won't get merged. Any further discussion of the issue or feature request should be directed to #1514