From e22a1a75eb97715352602b012727081ea9aa65b1 Mon Sep 17 00:00:00 2001 From: Pascal Zumkehr Date: Mon, 21 Sep 2020 22:42:00 +0200 Subject: [PATCH] Still import recordings with gaps of a few minutes --- README.md | 2 +- app/services/import/broadcast_mapping.rb | 6 ++--- app/services/import/importer.rb | 25 ++++++++++++----- app/services/import/recording/composer.rb | 4 ++- doc/import.md | 3 ++- .../services/import/broadcast_mapping_test.rb | 7 +++++ test/services/import/importer_test.rb | 27 ++++++++++++++++++- .../import/recording/composer_test.rb | 16 +++++++++++ 8 files changed, 77 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index e8c4687..23a8a86 100644 --- a/README.md +++ b/README.md @@ -25,5 +25,5 @@ It consists of three main parts: ## License RAAR is released under the terms of the GNU Affero General Public License. -Copyright 2015-2019 Radio Rabe. +Copyright 2015-2020 Radio Rabe. See `LICENSE` for further information. diff --git a/app/services/import/broadcast_mapping.rb b/app/services/import/broadcast_mapping.rb index 61247c5..934de15 100644 --- a/app/services/import/broadcast_mapping.rb +++ b/app/services/import/broadcast_mapping.rb @@ -62,11 +62,11 @@ def add_recording_if_overlapping(recording) end # Do the assigned recordings cover the entire duration of the broadcast? - def complete? - finish = started_at + Recording::DURATION_TOLERANCE.seconds + def complete?(maximum_gap = Recording::DURATION_TOLERANCE) + finish = started_at + maximum_gap @recordings.sort_by(&:started_at).all? do |r| adjacent = r.started_at <= finish - new_finish = r.finished_at + Recording::DURATION_TOLERANCE.seconds + new_finish = r.finished_at + maximum_gap finish = new_finish if new_finish > finish adjacent end && finish >= finished_at diff --git a/app/services/import/importer.rb b/app/services/import/importer.rb index fe9951c..27458dd 100644 --- a/app/services/import/importer.rb +++ b/app/services/import/importer.rb @@ -6,6 +6,11 @@ module Import class Importer include Loggable + # Even if there are a few minutes missing, consider a mapping as complete + # after a 24 hours grace period when recordings could still show up. + # It's better to import what we have instead of nothing. + INCOMPLETE_MAPPING_TOLERANCE = 15.minutes + INCOMPLETE_MAPPING_GRACE_PERIOD = 24.hours attr_reader :mapping @@ -53,15 +58,23 @@ def mapping_imported? end def mapping_complete? - mapping.complete?.tap do |complete| - unless complete - inform("Broadcast #{mapping} is not imported, " \ - "as the following recordings do not cover the entire duration:\n" + - mapping.recordings.map(&:path).join("\n")) - end + tolerance = grace_period_over? ? INCOMPLETE_MAPPING_TOLERANCE : Recording::DURATION_TOLERANCE + mapping.complete?(tolerance).tap do |complete| + log_mapping_not_imported if !complete && mapping.finished_at < 1.hour.ago end end + def log_mapping_not_imported + log(grace_period_over? ? 'WARN' : 'INFO', + "Broadcast #{mapping} is not imported, " \ + "as the following recordings do not cover the entire duration:\n" + + mapping.recordings.map(&:path).join("\n")) + end + + def grace_period_over? + mapping.finished_at < INCOMPLETE_MAPPING_GRACE_PERIOD.ago + end + def broadcast_valid? broadcast = mapping.broadcast broadcast.valid?.tap do |valid| diff --git a/app/services/import/recording/composer.rb b/app/services/import/recording/composer.rb index c6bf206..64de44c 100644 --- a/app/services/import/recording/composer.rb +++ b/app/services/import/recording/composer.rb @@ -36,7 +36,9 @@ def compose private def check_arguments - raise(ArgumentError, 'broadcast mapping must be complete') unless mapping.complete? + unless mapping.complete?(Importer::INCOMPLETE_MAPPING_TOLERANCE) + raise(ArgumentError, 'broadcast mapping must be complete') + end if (recordings - mapping.recordings).present? raise(ArgumentError, 'recordings must be part of the broadcast mapping') end diff --git a/doc/import.md b/doc/import.md index a312195..84cd40f 100644 --- a/doc/import.md +++ b/doc/import.md @@ -22,7 +22,8 @@ The following steps are performed during the import process. The respective Clas 1. Based on the timestamps given in the recording file names, map the recordings to their respective broadcasts (`Import::BroadcastMapping::Builder#run`). Different strategies would be possible by implementing different `Import::BroadcastMapping::Builder`s. Currently, this data is fetched directly for an Airtime database (`Import::BroadcastMapping::Builder::AirtimeDb`). If no mappings are found but a `IMPORT_DEFAULT_SHOW_ID` is defined, broadcasts mappings for this show are created. 1. For each broadcast mapping, do the following (`Import::Importer#run`): 1. If the recordings do not cover the entire broadcast duration, cancel and retry later. -1. Select the best recording for a given time (`Import::Recording::Chooser#best`). +1. If one day after the broadcast the recordings do not cover the entire broadcast duration and the gaps are less than 15 minutes, continue anyways. It is better to import what is there than to import nothing. +1. Select the best recordings to work with (`Import::Recording::Chooser#best`). 1. If the recording duration does not match the broadcast duration, cut the audio file(s) accordingly to get one single master file for the broadcast (`Import::Recording::Composer#compose`). 1. Transcode the master file into the defined archive formats in the `ARCHIVE_HOME` directory and create the corresponding database entries (`Import::Archiver#run`). 1. Mark the used recordings as imported (`Import::Recording#mark_imported`). diff --git a/test/services/import/broadcast_mapping_test.rb b/test/services/import/broadcast_mapping_test.rb index 7554580..afbe85e 100644 --- a/test/services/import/broadcast_mapping_test.rb +++ b/test/services/import/broadcast_mapping_test.rb @@ -203,6 +203,13 @@ class Import::BroadcastMappingTest < ActiveSupport::TestCase assert mapping.complete? end + test '#complete? with bigger tolerance is true if recordings have bigger gap' do + assign_broadcast + mapping.add_recording_if_overlapping(Import::Recording::File.new('2013-06-12T200000+0200_060.mp3')) + mapping.add_recording_if_overlapping(Import::Recording::File.new('2013-06-12T211400+0200_060.mp3')) + assert mapping.complete?(15.minutes) + end + private def mapping diff --git a/test/services/import/importer_test.rb b/test/services/import/importer_test.rb index e6dfb76..07945cf 100644 --- a/test/services/import/importer_test.rb +++ b/test/services/import/importer_test.rb @@ -12,13 +12,38 @@ class Import::ImporterTest < ActiveSupport::TestCase importer.run end - test 'it does nothing if recordings are not complete' do + test 'it does nothing if recordings are not complete at all' do mapping.add_recording_if_overlapping(Import::Recording::File.new(file('2013-06-19T200000+0200_060.mp3'))) Import::Archiver.expects(:new).never ExceptionNotifier.expects(:notify_exception).never importer.run end + test 'it does nothing if recent recordings have a few minutes gap' do + travel_to(Time.zone.local(2013, 6, 19, 22)) + mapping.add_recording_if_overlapping(Import::Recording::File.new(file('2013-06-19T200000+0200_060.mp3'))) + mapping.add_recording_if_overlapping(Import::Recording::File.new(file('2013-06-19T211200+0200_048.mp3'))) + Import::Archiver.expects(:new).never + ExceptionNotifier.expects(:notify_exception).never + importer.run + end + + test 'imports anyways even if older recordings have a few minutes gap' do + f1 = touch('2013-06-19T200000+0200_060.mp3') + f2 = touch('2013-06-19T211200+0200_048.mp3') + mapping.add_recording_if_overlapping(Import::Recording::File.new(f1)) + mapping.add_recording_if_overlapping(Import::Recording::File.new(f2)) + AudioProcessor::Ffmpeg.any_instance.expects(:duration).returns(60 * 60) + AudioProcessor::Ffmpeg.any_instance.expects(:duration).returns(48 * 60) + Import::Recording::Composer.any_instance.expects(:compose).returns(File.new(f1)) + AudioProcessor::Ffmpeg.any_instance.expects(:transcode).times(3) + assert_difference('Broadcast.count', 1) do + assert_difference('AudioFile.count', 3) do + importer.run + end + end + end + test 'it marks recordings as imported and aborts if broadcast is already imported' do f = touch('2013-06-19T200000+0200_120.mp3') mapping.add_recording_if_overlapping(Import::Recording::File.new(f)) diff --git a/test/services/import/recording/composer_test.rb b/test/services/import/recording/composer_test.rb index a9339f0..8fa6742 100644 --- a/test/services/import/recording/composer_test.rb +++ b/test/services/import/recording/composer_test.rb @@ -330,6 +330,22 @@ class Import::Recording::ComposerTest < ActiveSupport::TestCase composer.compose end + test 'returns merged recordings with gaps' do + composer = build_composer('2013-06-12T200000+0200_045.mp3', + '2013-06-12T205500+0200_005.mp3', + '2013-06-12T210000+0200_060.mp3') + + expect_concat(2) + mock_audio_format('mp3', 320) + expect_no_trim(file(0)) + expect_no_trim(file(1)) + expect_no_trim(file(2)) + mock_duration(file(0), 45) + mock_duration(file(1), 5) + mock_duration(file(2), 60) + composer.compose + end + private def build_composer(*recordings)