-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
api/soundcloud: add more metadata support #1313
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please add more optional chaining operators (?.
), because else an error will be thrown instead of an empty metadata field
title: json.title.trim(), | ||
artist: json.user.username.trim(), | ||
title, | ||
album: json.publisher_metadata.album_title?.trim() ?? title, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i don't think the fall back to song title makes sense here, wouldn't it be misleading most of the time?
album_artist: artist, | ||
composer: json.publisher_metadata.writer_composer?.trim(), | ||
genre: json.genre.trim(), | ||
date: json.display_date.slice(0, 4), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is the date sliced here in such a way?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it only keeps the release year, not the full date (which is what onthespot does)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
during research I also found this gist, which lists the date
as just the year, https://gist.github.com/eyecatchup/0757b3d8b989fe433979db2ea7d95a01
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cobalt keeps the full date for songs from youtube music, so i think it should be this way here too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ffmpeg should be able to write the full date into the TDRC
tag supported in id3v2.4:
https://github.com/FFmpeg/FFmpeg/blob/a5103808f440323109304045fe012f881c36e6c1/libavformat/id3v2.c#L66-L67
if you can't find it, then it's probably not there. i wouldn't add something you're not sure in existence of. you can always add it later! |
turns out I just didn't look hard enough, this song by Imogen Heap has proper credits {
"id": 1978104099,
"urn": "soundcloud:tracks:1978104099",
"artist": "Imogen Heap",
// ........
"writer_composer": "Imogen Heap, Guy Sigsworth",
"release_title": "Let Go (on the hang in the park)"
} |
0afa2b1
to
88e8b52
Compare
@wukko reminder to review again pls |
this PR adds and documents the
composer
,genre
andalbum_artist
metadata tags. it also adds more metadata parsing to the soundcloud service:album
album_artist
composer
genre
date
copyright
why?
having more metadata is always nice. this is a comparison with onthespot in foobar2000
PS: invest in a formatter. please