-
-
Notifications
You must be signed in to change notification settings - Fork 799
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
UI doesn't seem to display comments from DESCRIPTION tag #963
Comments
Thanks for the report. Are you using the (default) |
I'm using the default one. Update: I don't know what I did with easytags. But upon editing another album, it did use capital case. However comment is set under |
Weird thing is that it is.... Can you attach a problematic file so I can take a look? I don't have easytag installed, could be another issue with tags created by it |
@deluan github won't let me upload .flac, but you can replicate with
and
In my testing it turns out navidrome actually does read both However, it looks like easytag actually sets |
|
Is |
Well Vorbis doesn’t have any formal standard what fields are allowed or what name they should have, so all fields are “custom”. If some taggers or players use DESCRIPTION, and it catches on, then that’s just as valid as any other Vorbis field. id3v2 does have an official “subtitle/description” (TIT3). The original idea with id3 back in 1999/2000 was:
That was the theory at least, but in practice almost no players support TIT1 or TIT3, although Beatport uses TIT3 to store remix names, and as a result some DJ mixing apps use it. And then Apple came in six years ago and introduced a fourth title field, Movement (MVNM, for the individual pieces in a Work). Why they didn’t just use TIT2 for that is anyone’s guess, but hey, it’s a thing now. And they also added yet another field, Grouping (GRP1), the purpose of which isn’t totally clear to anyone but everybody just seems to use it as a generic extra field. Anyway, would be nice if Navidrome supported all these fields, although how you’d display all this stuff in the web UI is probably requires some thinking. The Subsonic API has only one song title field anyway, so even if ND supported a hundred fields, it would be no use to 3rd party clients. |
Yeah, but the main question is: Should we import "Description" as "Comment"? What happens if the file has both? |
It seems like using description as comment is nonstandard.
https://www.xiph.org/vorbis/doc/v-comment.html If both are set, one could put the description text above where the comment text is. |
example
|
Supporting Description is fine, but it is track-level metadata, it should be a column in the tracks list, not one field at the album level. What do you do with two tracks on one album, with different Description fields? I feel the same about the Comments field btw, I think it’s a mistake to use it in Navidrome as “album comments”, since each track can have its own Comments. |
@certuna navidrome already shows the comment for each track in the detailed view. Description could easily go there as well if it's not the same across tracks. In both of the source I linked above, The most common one being from bandcamp, but others contain the cd release id, ripping/transcode information, etc.
|
I wonder why they're using But the thing is, the Comments frame in id3v2 (
Vorbis/FLAC as usual, has nothing strictly defined, so there too The Xiph recommendations say though: Like every field in Vorbis, you can store multiple frames of the same field, so you're allowed to store TLDR; "Description" was supposed to be song metadata (but people may store album metadata there in practice), and "Comments" may refer to album metadata, or song metadata, as a music player you cannot know in advance. I would probably suggest that Navidrome treats both as |
I don't know, but it's the same for any album downloaded from bandcamp. I can send them an email about it. |
I think we discussed this before, but for comment, we could consider it an album-level comment if all comments from all songs are the same. If not, they only appear in the song details, and the album comment stays empty. Thoughts? |
Agree. |
Ok, will do this for
|
If we keep it, hiding the full comment until it's clicked like in the album page would be nice. See the previous image I uploaded for an example of what long comments end up looking like currently. Also, we should keep in mind that it's possible to have multiple COMMENT tags. The docs certuna linked state
It seems best to me to follow the "standard" tags as mentioned by certuna. It would be nice if the detailed song view supported these, like composer, arranger, etc. I also found more tags here: https://help.mp3tag.de/main_tags.html I don't know why easytag maps comment->description. (I think I should stop using easytag). Maybe if both comment and description are the same for all songs on an album, the description can go before the comment in the album view? |
I took a shot at implementing @deluan's suggestion:
Should we also remove the comments from the songs in that case? |
Hey @caiocotts, thanks for the PR, looks good. I think we should hide the comments from the songs in that case. In the current state, it may be challenging to hide the comments from songs in the UI as we don't have the album info everywhere we would need it (ex: in Any ideas @certuna ? |
This also has parallels with #846 : if any metadata is the same for all tracks, should the Navidrome UI then hide it (and move it to the album details section)? Because it doesn't end with comment, year or artist, we'll soon have the same discussion with things like Producer credits: if one producer did all tracks, should it then show as a credit for each individual track, or as an album-level credit? I don't think Would this be workable?
That way we have the maximum amount of information at the API level, and the most flexibility towards the future. |
Yes, this is workable. The issue is in the SongList and PlaylistSongs views : When listing tracks detached from their albums, there is no way to know if the comment in the track is an album-level or track-level comment as we don't have the album information, only the data that is part of the track. Should we ignore this fact and always show the comment? Example showing songs from the same album, with album-level comment: If we are ok repeating the comment in all tracks of an album where we don't have the album info (basically anywhere except in the Album view), then we can implement this on the UI side (in the Album view) as suggested. If not, we need a way to flag the comment as album-level or track-level. I'm ok with always showing all info as track level when the album info is not available. Just want to be sure we are doing The Right Thing ™️ @caiocotts Regardless of the approach we take from the options above, we will need to write a DB migration, to roll-up album comments for the music that is already imported. Let me know if you have any questions. |
@deluan please my PR and see if there's anything missing. |
@caiocotts, looks good, thanks! @certuna / @andrewzah, the PR makes What should we do re: |
If it is added, perhaps it could be shown underneath
|
If it is a nonstandard, not commonly used field, I rather not add support for it, as it is more code to maintain for such an edge case. I'll close this for now, but feel free to reopen if you sill have any concerns. |
I am late to this discussion, but came from the enhancements tracker. Let me know if there is a better way to express this: I like the way unified comments for an album are displayed. But I am hoping for an optional column to display track comments in the album view (like Rating, BPM, etc . . .). Perhaps the album comments can indicate that there are "varied" comments within the tracks or that comments are present for some songs. This is most applicable to albums with Bonus Discs or remixes/other special stuff tacked on at the end. For what it's worth, I just went through my mp3's ID3 tags and made sure the info I wanted was in the "COMM" section. If this isn't a good approach for Navidrome, then I can move track specific comments to the track title (TIT2).
|
This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Description
The UI doesn't show comments from flac tags.
Expected Behaviour
I expect the UI to show comments from flac tags.
Steps to reproduce
Platform information
0.41.0-SNAPSHOT (d5434d41)
78.8.0esr (64-bit)
Additional information
This album DOES show the comment.
metaflac output: https://gist.github.com/andrewzah/ed008321799651ded9c2cac32b511697
This album also does show the comment, but not the description:
The text was updated successfully, but these errors were encountered: