Still import recordings with gaps of a few minutes

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.

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

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|

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

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`).

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

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))

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)