search

strawberrymusicplayer / strawberry

:strawberry: Strawberry Music Player
https://www.strawberrymusicplayer.org/
GNU General Public License v3.0
2.66k stars 183 forks source link

Unable to set Performer/Compilation tags for some files #1076

Closed posita closed 1 year ago

posita commented 1 year ago

I can't quite figure out why, but editing tags on some .m4a (AAC) files does not register changes to the Compilation boolean flag or the Performer tag. For others, blanking the Performer tag ends up with it being populated with whatever is in the Artist field at the time of saving. I can provide (copyrighted) material via another channel for reproducing this, if helpful.

Steps to reproduce files to which this applies:

  1. Edit the track information
  2. Check (or uncheck) the Compilation box
  3. If the Performer field is set, clear it; if it is blank, set it
  4. Click save

The Compilation flag will not remain set. If the Performer field was clear and you set it, it will be blank. If the Performer field was set and you cleared it, it will be set to whatever was in the Artist field.

The above happens with 1.0.10.

posita commented 1 year ago

Files that can be used to reproduce this behavior with 1.0.10 can be found by reconstruction the appropriate URL from gist.github.com ... posita ... 23d9dbe4e8439bbcaf1088812a2020cb.

jonaski commented 1 year ago

Strawberry does not read or write performer tag for MP4, I'm not sure if MP4 supports that tag at all. As for compilation, it looks like we save the tag, but does not read it. That looks like a bug to me.

posita commented 1 year ago

Perhaps the bug is that the Performer tag gets populated at all (at least in the UI), but not consistently?

jonaski commented 1 year ago

No, it shouldn't read or write performer at all for MP4, so not sure how that happened, I can check if it's possible to implement. Compilation read/write is fixed now https://github.com/strawberrymusicplayer/strawberry/commit/67b503da445a2a814b368639204e26452105524c It was not reading it at all, it was writing it, but incorrectly.

jonaski commented 1 year ago

Performer isn't supported for MP4. I also checked Kid3 and Amarok, they don't have it either. What audio format was the songs that you had the issue with performer on?

posita commented 1 year ago

Upon further examination, you're right: Performer is only persisted with MP3 files, not M4A/AAC files. ~I'm not sure if it's intentional that blanking/clearing the Performer field for MP3s should mean that it should take on the value of the Artist field, though. Is that intentional?~ _Correction: It looks like ID3v2 treats those fields as synonymous. Any idea why the UI considers them separate? Are they distinct in some other format? Their being linked for MP3 files is kind of confusing from someone who doesn't understand their connection. Maybe they should be hidden for both M4As and MP3s?_

jonaski commented 1 year ago

TPE1 "Lead artist(s)/Lead performer(s)/Soloist(s)/Performing group" is artist and TOPE "Original artist(s)/performer(s)" is performer. However, Strawberry is incorrectly using TPE1, also for performer if TOPE is missing. I'm pretty sure that is something I've done unintentionally while reading the ID3v2 specifications as they call what everyone else calls artist performer, which is confusing. I'll fix that. Most formats have performer, ie Vorbis comment tags used for ogg/flac, etc, the exception is MP4, AIFF and ASF. We should just disable the performer field for formats that don't support it.

posita commented 1 year ago

We should just disable the performer field for formats that don't support it.

Would it be helpful if I filed a separate issue to track that? If so, in what form would be best?