beetbox / beets

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

Customizable left-padding of track and disc numbers #3352

Open ghost opened 5 years ago

ghost commented 5 years ago

I noticed that importing an album with >99 tracks will cause an inconsistency in left-padding track numbers.

'09 Enemy Approaching.opus'
'100 MEGALOVANIA.opus'
'101 Good Night.opus'
'10 Ghost Fight.opus'
'11 Determination.opus'
'12 Home.opus'

This also makes ncmpcpp think that Megalovania is the 10th track.

I think beets should make sure that the zero padding scales up beyond >99 tracks.

Setup

My configuration (output of beet config) is:

directory: ~/Music
library: ~/Music/beetslibrary.db
import:
  write: yes
  move: yes
  resume: no
match:
  distance_weights:
    missing_tracks: 0
    date: 10
    artist: 10
    title: 10
    length: 10
paths:
    default: $albumartist/$original_year $album%aunique{}/$track $title
    singleton: $artist/Non-Album/$original_year $title
    comp: Compilations/$album%aunique{}/$track $title
jackwilsdon commented 5 years ago

It seems that we set the track field to be a padded integer with a length of two - changing this would probably end up making a mess and most people probably don't need 3 digit track numbers.

Off the top of my head, we could maybe add a new template function for left padding text, such that the user can implement their own track padding:

paths:
    # usage: %pad{value,padding,length}
    default: $albumartist/$album%aunique{}/%pad{$track,0,3} $title
ghost commented 5 years ago

It seems that we set the track field to be a padded integer with a length of two - changing this would probably end up making a mess and most people probably don't need 3 digit track numbers.

An easy way to do this while preserving backwards compatability would be:

# of digits in total track count amount of left padding example
1 1 "01"..."09"
2 1 "01"..."29"
3 2 "001"..."109"
4 3 "0001"..."1009"

An album with >999 tracks probably doesn't exist, but I don't like the idea of hardcoding values. Simply taking the len() of the tracks and using zfill plus an if statement for the first case for backwards compatability I think isn't too complicated. I get the feeling that the assumption of leftpadding for 2 digits lends a lot of convenience for Beets and simplifies some internals, but I feel like there's something just plain wrong about not handling long albums nicely because it doesn't fit the assumption.

Off the top of my head, we could maybe add a new template function for left padding text, such that the user can implement their own track padding:

paths:
    # usage: %pad{value,padding,length}
    default: $albumartist/$album%aunique{}/%pad{$track,0,3} $title

I don't think I would want all of my tracks to have that much padding just because I have some albums that need it. Maybe make dynamic track numbers an option? Idk, that sounds even more complicated.

sampsyo commented 5 years ago

Yeah, the built-in padding amounts work most of the time but can break down in cases like this. I like @jackwilsdon's idea to add customizable padding with a function because it's so simple and would solve the problem.

Of course, we could also consider the "dynamic" option that uses the track count, but implementing that would be substantially more complicated—and it's not clear it's actually what most users would want (having different amounts of padding for different albums could be inconvenient for other reasons).

ghost commented 5 years ago

I had the feeling it would be complicated. In that case I like @jackwilsdon's idea. I can live with the zeroes.

jackwilsdon commented 5 years ago

Because the stack field is item level, I'm not 100% sure how we'd take the whole album into account whilst rendering track numbers in a template. Have you got any ideas on how this could be implemented @sampsyo?

sampsyo commented 5 years ago

Yeah, that's a problem—I was thinking that the way to do it would be to use the $tracktotal field. It wouldn't be totally reliable, because of course it's allowed to have a $tracktotal that "lies" with respect to individual tracks' $track fields, but it could be better than nothing.

RollingStar commented 5 years ago

OP knows this, but this is trivial in Python... if you don't care about the internal beets representation. For anyone who wants a hotfix:

item_fields:
    i_track: |
        # hotfix for beets' hardcoded 2-digit track numbers
        # https://github.com/beetbox/beets/issues/3352
        str_track = str(track)
        # pad based on length of highest track number (per disc).
        # may interact with per_disc_numbering, not sure
        pad_length = len(str(tracktotal))
        if pad_length < 2:
            pad_length = 2
        return str_track.zfill(pad_length)
dkanada commented 4 years ago

@sampsyo have you reconsidered the best way to solve this problem lately? I've run into the same issue a few times so I wouldn't mind fixing it myself if you had a method in mind.

sampsyo commented 4 years ago

Good question! To sum up the options:

  1. Add a new %pad{} function specifically for customizing padding.
  2. Add a new global configuration option to change the amount of padding.
  3. Use $tracktotal to automatically guess the appropriate amount of padding.

I think option 2 is probably not a great idea because it would need to apply uniformly to all relevant fields. Option 1 is nice and simple and wouldn't be too hard to implement. Finally, option 3 would be slightly more complete solution but would be substantially more complicated.

So, all in all, I think I like option 1, at least for now—and if folks still think that a "dynamic" version would be better, we can come back later to something in the option 3 category.

Thanks for looking into this!

dkanada commented 4 years ago

I seem to recall another issue somewhere else about editing track fields in bulk but I can't seem to find it anymore. I was also interested in that, specifically to handle missing disc numbers, so do you know where that issue went?

sampsyo commented 4 years ago

Hmm, I'm afraid I don't know the issue you're thinking of, no…

dkanada commented 4 years ago

Just to make sure, should I include both a global setting and some specific field options? Track number is the obvious issue here, but a global setting of three wouldn't be good for years or disc numbers.

sampsyo commented 4 years ago

To clarify, I was arguing above against adding configuration options, for exactly that reason—it's not clear how to make this cleanly apply to different fields. For that reason, I think a %pad function as @jackwilsdon proposed above would be a more modular solution.

EDIT: I realized I mistyped an option number above. Oops!! Fixed. 😳

dkanada commented 4 years ago

I searched again and still can't find the issue I referenced above. Basically, I would like to be able to apply disc number to an entire album when it's missing, or modify the artist name for all given tracks in an album when they're the same. I recall an issue where you were talking about track-level fields and why it would be difficult to implement a fix, but you did mention it might be possible to query all tracks and display the field when it was the same for all of them. Does any of that sound familiar?

sampsyo commented 4 years ago

Hmm, not exactly, unfortunately. To clarify, it sounds like the idea would be to do something like beet modify -a but for affecting track-level fields? Like, right now, I don't think it's possible to do beet modify -a revolver artist='The Beatles' because $artist is a track-level field. (You can do this with $albumartist, of course.) If that's not possible, I can see the argument for making it possible, and it seems like a pretty reasonable idea! The only hairy issue would be naming conflicts between album-level flexible attributes and item-level fields, so perhaps a new flag would be necessary.

I know from experience, however, that I do very often forget issues. There are just too many of them for my tiny brain! So it could exist somewhere…

dkanada commented 4 years ago

naming conflicts between album-level flexible attributes and item-level fields

Could you expand on what might cause problems here?

Also, the %pad function wouldn't apply to the tags themselves would it? That would be the main goal for me, especially because ignoring padding for tags would make the existing tags more transparent.

sampsyo commented 4 years ago

Could you expand on what might cause problems here?

Yes—currently, I think beet modify -a revolver artist='The Beatles' would create an album-level flexattr named artist. In general, the user might actually intend to do that. So just interpreting that command as "please set the item-level fields" could be surprising/problematic.

Also, the %pad function wouldn't apply to the tags themselves would it? That would be the main goal for me, especially because ignoring padding for tags would make the existing tags more transparent.

Aha—yes, that's correct. But, to clarify, numbers in tags are never padded at all, unless I'm mistaken. (If you're seeing padding, it's probably on the "client side.") For some formats, the values are stored as proper integers, not strings—so padding is actually impossible.

dkanada commented 4 years ago

But, to clarify, numbers in tags are never padded at all, unless I'm mistaken.

Ah okay, so they are only displayed padded. What about a situation where some tags (the ones stored as strings) are padded but others aren't though? Are tags always rewritten without padding, and is that done on import or only when editing a specific track?

I wouldn't mind adding beet edit -b artist='The Beatles' to group and edit track level fields. It would preserve the existing behavior and allow users to change track level fields on an album level basis.

sampsyo commented 4 years ago

Ah okay, so they are only displayed padded. What about a situation where some tags (the ones stored as strings) are padded but others aren't though? Are tags always rewritten without padding, and is that done on import or only when editing a specific track?

That could be worth considering, but I'm curious about the use case—do you use a player/browser that displays the string tags "as is" and uses alphabetical sorting on track fields, for example? If so, I'd recommend to the developer of whatever that software is that they consider interpreting the tags as integers, even when they are required to be stored as strings. :smiley:

FWIW, this change would have to happen in MediaFile rather than beets.

I wouldn't mind adding beet edit -b artist='The Beatles' to group and edit track level fields. It would preserve the existing behavior and allow users to change track level fields on an album level basis.

Seems good to me! If you're interested, please open a new issue with details and we can go from there.