From b1546eeaa2e80f6d6a655c598764201c26cd850f Mon Sep 17 00:00:00 2001 From: Leni Kadali <52788034+lenikadali@users.noreply.github.com> Date: Sat, 14 Sep 2024 12:44:41 +0300 Subject: [PATCH 1/3] Add release date to track metadata Fixes #185 This commit adds the DATE metadata field to the transcoded MP3 and FLAC files. The FLAC documentation we can find seems to suggest using DATE for this purpose[1]. For MP3, `date` also seems to be an acceptable field name[2]. We've verified that the data shows up in the metadata displayed by VLC[3] for both formats. [1] https://xiph.org/vorbis/doc/v-comment.html#fieldnames [2] https://docs.mp3tag.de/format/ [3] https://www.videolan.org/vlc/ Co-authored-by: Chris Lowis --- app/models/track.rb | 5 ++++- app/models/transcode_command.rb | 6 ++++-- test/models/track_test.rb | 10 ++++++++++ test/models/transcode_command_test.rb | 8 ++++++-- 4 files changed, 24 insertions(+), 5 deletions(-) diff --git a/app/models/track.rb b/app/models/track.rb index 245d76a1..788ccd74 100644 --- a/app/models/track.rb +++ b/app/models/track.rb @@ -27,12 +27,15 @@ def preview_duration end def metadata - { + data = { track_title: title, track_number: number, album_title: album.title, artist_name: artist.name } + + data[:release_date] = album.released_on if album.released_on + data end def transcode diff --git a/app/models/transcode_command.rb b/app/models/transcode_command.rb index 5854fde5..3c42d8c6 100644 --- a/app/models/transcode_command.rb +++ b/app/models/transcode_command.rb @@ -5,14 +5,16 @@ class TranscodeCommand track_title: 'TIT2', track_number: 'TRCK', album_title: 'TALB', - artist_name: 'TPE1' + artist_name: 'TPE1', + release_date: 'DATE' }.freeze METADATA_KEYS_VS_FLAC_TAGS = { track_title: 'TITLE', track_number: 'TRACKNUMBER', album_title: 'ALBUM', - artist_name: 'ARTIST' + artist_name: 'ARTIST', + release_date: 'DATE' }.freeze def initialize(input, output, format, metadata = {}, image = nil) diff --git a/test/models/track_test.rb b/test/models/track_test.rb index 3782c642..b9342dd0 100644 --- a/test/models/track_test.rb +++ b/test/models/track_test.rb @@ -103,6 +103,16 @@ class TrackTest < ActiveSupport::TestCase assert_equal track.number, metadata[:track_number] assert_equal track.album.title, metadata[:album_title] assert_equal track.album.artist.name, metadata[:artist_name] + assert_not metadata.key?(:release_date) + end + + test '#metadata when album has released_on' do + album = create(:album, released_on: Time.zone.today) + track = create(:track, album:) + + metadata = track.metadata + + assert_equal track.album.released_on, metadata[:release_date] end test '#number' do diff --git a/test/models/transcode_command_test.rb b/test/models/transcode_command_test.rb index 26a3593d..11508446 100644 --- a/test/models/transcode_command_test.rb +++ b/test/models/transcode_command_test.rb @@ -47,7 +47,8 @@ class TranscodeCommandTest < ActiveSupport::TestCase track_title: 'track-title', track_number: 'track-number', album_title: 'album-title', - artist_name: 'artist-name' + artist_name: 'artist-name', + release_date: 'release-date' } command_string = TranscodeCommand.new(@input, @output, :mp3v0, metadata).generate assert_contains_pair(command_string, ['-write_id3v2', '1']) @@ -56,6 +57,7 @@ class TranscodeCommandTest < ActiveSupport::TestCase assert_contains_pair(command_string, ['-metadata', 'TRCK="track-number"']) assert_contains_pair(command_string, ['-metadata', 'TALB="album-title"']) assert_contains_pair(command_string, ['-metadata', 'TPE1="artist-name"']) + assert_contains_pair(command_string, ['-metadata', 'DATE="release-date"']) end test 'adds metadata tags for FLAC format' do @@ -63,13 +65,15 @@ class TranscodeCommandTest < ActiveSupport::TestCase track_title: 'track-title', track_number: 'track-number', album_title: 'album-title', - artist_name: 'artist-name' + artist_name: 'artist-name', + release_date: 'release-date' } command_string = TranscodeCommand.new(@input, @output, :flac, metadata).generate assert_contains_pair(command_string, ['-metadata', 'TITLE="track-title"']) assert_contains_pair(command_string, ['-metadata', 'TRACKNUMBER="track-number"']) assert_contains_pair(command_string, ['-metadata', 'ALBUM="album-title"']) assert_contains_pair(command_string, ['-metadata', 'ARTIST="artist-name"']) + assert_contains_pair(command_string, ['-metadata', 'DATE="release-date"']) end test 'transcodes audio to mp3 using libmp3lame codec highest audio quality' do From e255eeb8441ec502c13118bcf4bf0977c03112ee Mon Sep 17 00:00:00 2001 From: Chris Lowis Date: Wed, 2 Oct 2024 16:07:09 +0100 Subject: [PATCH 2/3] Add an end-to-end test for adding metadata to transcodes This test runs a WAV file through the TranscodeJob and then ensures that the correct metadata has been added to the generated MP3 and FLAC file using `ffprobe`. --- test/jobs/transcode_job_test.rb | 36 +++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/test/jobs/transcode_job_test.rb b/test/jobs/transcode_job_test.rb index ac69e9a7..d8f8d1cf 100644 --- a/test/jobs/transcode_job_test.rb +++ b/test/jobs/transcode_job_test.rb @@ -51,4 +51,40 @@ class TranscodeJobTest < ActiveJob::TestCase TranscodeJob.perform_now(build(:track)) end + + test 'it adds metadata to the generated MP3 file' do + album = create(:album, released_on: Time.zone.today) + track = create(:track, album:) + TranscodeJob.perform_now(track) + + track.transcodes.mp3v0.first.file.open do |file| + cmd = "ffprobe -i #{file.path} -v quiet -print_format json -show_format" + std_out, _status = Open3.capture2(cmd) + metadata = JSON.parse(std_out) + + assert_equal track.title, metadata['format']['tags']['title'] + assert_equal '01', metadata['format']['tags']['track'] + assert_equal album.title, metadata['format']['tags']['album'] + assert_equal album.artist.name, metadata['format']['tags']['artist'] + assert_equal album.released_on.to_s, metadata['format']['tags']['date'] + end + end + + test 'it adds metadata to the generated flac file' do + album = create(:album, released_on: Time.zone.today) + track = create(:track, album:) + TranscodeJob.perform_now(track, format: :flac) + + track.transcodes.flac.first.file.open do |file| + cmd = "ffprobe -i #{file.path} -v quiet -print_format json -show_format" + std_out, _status = Open3.capture2(cmd) + metadata = JSON.parse(std_out) + + assert_equal track.title, metadata['format']['tags']['TITLE'] + assert_equal '01', metadata['format']['tags']['track'] + assert_equal album.title, metadata['format']['tags']['ALBUM'] + assert_equal album.artist.name, metadata['format']['tags']['ARTIST'] + assert_equal album.released_on.to_s, metadata['format']['tags']['DATE'] + end + end end From 633df3fc7368fc54be0b40beecd194084f0bb1e9 Mon Sep 17 00:00:00 2001 From: Chris Lowis Date: Thu, 3 Oct 2024 10:48:55 +0100 Subject: [PATCH 3/3] Use TORY/DATE tags for release year Our best understanding is that the MP3 ID3 tag TORY[1] should be used to store the *year* (i.e. as a 4-digit number) the music was released. The convention for FLAC tags seems to have even less consensus/real-world usage, so for simplicity in the code, we've decided to use the four digit year in the DATE tag. [1] https://community.mp3tag.de/t/how-do-i-add-the-tag-for-when-my-music-was-released/55319 Co-authored-by: James Mead --- app/models/track.rb | 2 +- app/models/transcode_command.rb | 4 ++-- test/jobs/transcode_job_test.rb | 8 ++++---- test/models/track_test.rb | 2 +- test/models/transcode_command_test.rb | 8 ++++---- 5 files changed, 12 insertions(+), 12 deletions(-) diff --git a/app/models/track.rb b/app/models/track.rb index 788ccd74..b5bf99c0 100644 --- a/app/models/track.rb +++ b/app/models/track.rb @@ -34,7 +34,7 @@ def metadata artist_name: artist.name } - data[:release_date] = album.released_on if album.released_on + data[:release_year] = album.released_on.year if album.released_on data end diff --git a/app/models/transcode_command.rb b/app/models/transcode_command.rb index 3c42d8c6..e83f85d3 100644 --- a/app/models/transcode_command.rb +++ b/app/models/transcode_command.rb @@ -6,7 +6,7 @@ class TranscodeCommand track_number: 'TRCK', album_title: 'TALB', artist_name: 'TPE1', - release_date: 'DATE' + release_year: 'TORY' }.freeze METADATA_KEYS_VS_FLAC_TAGS = { @@ -14,7 +14,7 @@ class TranscodeCommand track_number: 'TRACKNUMBER', album_title: 'ALBUM', artist_name: 'ARTIST', - release_date: 'DATE' + release_year: 'DATE' }.freeze def initialize(input, output, format, metadata = {}, image = nil) diff --git a/test/jobs/transcode_job_test.rb b/test/jobs/transcode_job_test.rb index d8f8d1cf..88b14e32 100644 --- a/test/jobs/transcode_job_test.rb +++ b/test/jobs/transcode_job_test.rb @@ -53,7 +53,7 @@ class TranscodeJobTest < ActiveJob::TestCase end test 'it adds metadata to the generated MP3 file' do - album = create(:album, released_on: Time.zone.today) + album = create(:album, released_on: Date.parse('2024-10-03')) track = create(:track, album:) TranscodeJob.perform_now(track) @@ -66,12 +66,12 @@ class TranscodeJobTest < ActiveJob::TestCase assert_equal '01', metadata['format']['tags']['track'] assert_equal album.title, metadata['format']['tags']['album'] assert_equal album.artist.name, metadata['format']['tags']['artist'] - assert_equal album.released_on.to_s, metadata['format']['tags']['date'] + assert_equal '2024', metadata['format']['tags']['TORY'] end end test 'it adds metadata to the generated flac file' do - album = create(:album, released_on: Time.zone.today) + album = create(:album, released_on: Date.parse('2024-10-03')) track = create(:track, album:) TranscodeJob.perform_now(track, format: :flac) @@ -84,7 +84,7 @@ class TranscodeJobTest < ActiveJob::TestCase assert_equal '01', metadata['format']['tags']['track'] assert_equal album.title, metadata['format']['tags']['ALBUM'] assert_equal album.artist.name, metadata['format']['tags']['ARTIST'] - assert_equal album.released_on.to_s, metadata['format']['tags']['DATE'] + assert_equal '2024', metadata['format']['tags']['DATE'] end end end diff --git a/test/models/track_test.rb b/test/models/track_test.rb index b9342dd0..cda570b0 100644 --- a/test/models/track_test.rb +++ b/test/models/track_test.rb @@ -112,7 +112,7 @@ class TrackTest < ActiveSupport::TestCase metadata = track.metadata - assert_equal track.album.released_on, metadata[:release_date] + assert_equal track.album.released_on.year, metadata[:release_year] end test '#number' do diff --git a/test/models/transcode_command_test.rb b/test/models/transcode_command_test.rb index 11508446..7235e4b4 100644 --- a/test/models/transcode_command_test.rb +++ b/test/models/transcode_command_test.rb @@ -48,7 +48,7 @@ class TranscodeCommandTest < ActiveSupport::TestCase track_number: 'track-number', album_title: 'album-title', artist_name: 'artist-name', - release_date: 'release-date' + release_year: 'release-year' } command_string = TranscodeCommand.new(@input, @output, :mp3v0, metadata).generate assert_contains_pair(command_string, ['-write_id3v2', '1']) @@ -57,7 +57,7 @@ class TranscodeCommandTest < ActiveSupport::TestCase assert_contains_pair(command_string, ['-metadata', 'TRCK="track-number"']) assert_contains_pair(command_string, ['-metadata', 'TALB="album-title"']) assert_contains_pair(command_string, ['-metadata', 'TPE1="artist-name"']) - assert_contains_pair(command_string, ['-metadata', 'DATE="release-date"']) + assert_contains_pair(command_string, ['-metadata', 'TORY="release-year"']) end test 'adds metadata tags for FLAC format' do @@ -66,14 +66,14 @@ class TranscodeCommandTest < ActiveSupport::TestCase track_number: 'track-number', album_title: 'album-title', artist_name: 'artist-name', - release_date: 'release-date' + release_year: 'release-year' } command_string = TranscodeCommand.new(@input, @output, :flac, metadata).generate assert_contains_pair(command_string, ['-metadata', 'TITLE="track-title"']) assert_contains_pair(command_string, ['-metadata', 'TRACKNUMBER="track-number"']) assert_contains_pair(command_string, ['-metadata', 'ALBUM="album-title"']) assert_contains_pair(command_string, ['-metadata', 'ARTIST="artist-name"']) - assert_contains_pair(command_string, ['-metadata', 'DATE="release-date"']) + assert_contains_pair(command_string, ['-metadata', 'DATE="release-year"']) end test 'transcodes audio to mp3 using libmp3lame codec highest audio quality' do