-
Notifications
You must be signed in to change notification settings - Fork 2
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
Use FLAC tag names in FLAC metadata #184
Conversation
9fcce16
to
ddf92b9
Compare
This is an impressively fast PR! I don't understand what "encoded using the ID3v2 format" means. The encoding is described in the Vorbis comment doc you linked to. The code you have in production (as of when I bought that album yesterday) appears to be producing correctly encoded tags except for the tag names. Perhaps ffmpeg is quietly ignoring those ID3 related command line arguments when working with a non-MP3 file. |
@graue Thanks.
Yes, that seems like a likely explanation. I'd like to understand it a bit better before merging this, so I'll try to find some time to dig into it further when I can. |
Sorry for the delay in making progress on this. I've just done some experiments using both Download sample WAV file
Encode FLAC metadata using ffmpeg (without id3 options)
Encode FLAC metadata using ffmpeg (with id3 options)
Encode FLAC metadata using metaflac
ConclusionWe can continue to use |
Instead of the `format` param. This is more consistent with what we're doing in other methods on this class.
I'm planning on using this method for non-ID3 tags.
I'm planning on using this method for non-ID3 tags.
In #183 @graue pointed out that FLAC has its own tag names [1,2]. This changes the ffmpeg transcoding for FLAC tracks to use those names. After some experiments [3], I have determined that the `write_id3v2` & `id3v2_version` muxer options are not needed (they are ignored) when writing metadata to a FLAC file and so I have removed them from the generated `ffmpeg` command in that case in order to make the command less confusing. Fixes #183. [1]: https://xiph.org/flac/faq.html#general__tagging [2]: https://xiph.org/vorbis/doc/v-comment.html [3]: #184 (comment)
Addressed in 1ce8257. |
@graue I've just merged & deployed this change. However, for now it will only fix tracks which are transcoded after the change. If you point me at a few tracks, I can force re-transcoding for them and you can tell me whether it's working for you. Once we're happy it's working, we'll re-transcode all the tracks across the site. Thanks again for reporting the problem and for your patience! |
In #183 @graue pointed out that FLAC has its own tag names (FLAC FAQ; Vorbis comment docs). This changes the ffmpeg transcoding for FLAC tracks to use those names. However, it's unclear to me whether we also need to change the encoding of the metadata - currently I've left the
-write_id3v2 1
&-id3v2_version 3
ffmpeg options in place on the basis that @graue is already able to view the tags on a FLAC track; it was just that the tag names were incorrect.An initial search of the internet suggests that it might be better to use the
metaflac
tool to add metadata to a track, but that would be a moderately significant change and it would probably be worth changing MP3 tagging to work in a similar way, i.e. with a specialist command line tool rather than just using ffmpeg.Fixes #183.