From d608f1301738869c87e184c19d48ddd1614e9bb4 Mon Sep 17 00:00:00 2001
From: cjcolvar <chris.colvard@gmail.com>
Date: Wed, 27 Mar 2024 17:33:51 -0400
Subject: [PATCH] Set duration of MasterFile from ActiveEncode instead of
 relying on the imprecise mediainfo duration

---
 app/models/master_file.rb       | 10 +++++++---
 spec/factories/encode.rb        | 18 +++++++++++++++++-
 spec/models/master_file_spec.rb | 29 ++++++++++++++---------------
 3 files changed, 38 insertions(+), 19 deletions(-)

diff --git a/app/models/master_file.rb b/app/models/master_file.rb
index 9ffe260089..6ae184a631 100644
--- a/app/models/master_file.rb
+++ b/app/models/master_file.rb
@@ -294,9 +294,13 @@ def update_progress_on_success!(encode)
     #TODO pull this from the encode
     self.date_digitized ||= Time.now.utc.iso8601
 
-    # Set duration after transcode if mediainfo fails to find.
-    # e.x. WebM files missing technical metadata
-    self.duration ||= encode.input.duration
+    # Update the duration detected by ActiveEncode
+    # because it has higher precision than mediainfo
+    # Set for the first time for files without duration
+    # e.g. WebM files missing technical metadata
+    # ActiveEncode returns duration in milliseconds which
+    # is stored as an integer string
+    self.duration = encode.input.duration.to_i.to_s if encode.input.duration.present?
 
     outputs = Array(encode.output).collect do |output|
       {
diff --git a/spec/factories/encode.rb b/spec/factories/encode.rb
index ba2c5d2590..0a4b5770a7 100644
--- a/spec/factories/encode.rb
+++ b/spec/factories/encode.rb
@@ -26,12 +26,14 @@
       state { :running }
       percent_complete { 50.5 }
       current_operations { ['encoding'] }
+      input { FactoryBot.build(:encode_output) }
     end
       
     trait :succeeded do
       state { :completed }
       percent_complete { 100 }
       current_operations { ['DONE'] }
+      input { FactoryBot.build(:encode_output) }
       output { [ FactoryBot.build(:encode_output) ] }
     end
 
@@ -39,15 +41,29 @@
       state { :failed }
       percent_complete { 50.5 }
       current_operations { ['FAILED'] }
+      input { FactoryBot.build(:encode_output) }
       errors { ['Out of disk space.'] }
     end
   end
 
+  factory :encode_input, class: ActiveEncode::Input do
+    id { SecureRandom.uuid }
+    label { 'quality-high' }
+    url { 'file://path/to/output.mp4' }
+    duration { '21575.0' }
+    audio_bitrate { '163842.0' }
+    audio_codec { 'AAC' }
+    video_bitrate { '4000000.0' }
+    video_codec { 'AVC' }
+    width { '1024' }
+    height { '768' }
+  end
+
   factory :encode_output, class: ActiveEncode::Output do
     id { SecureRandom.uuid }
     label { 'quality-high' }
     url { 'file://path/to/output.mp4' }
-    duration { '21575' }
+    duration { '21575.0' }
     audio_bitrate { '163842.0' }
     audio_codec { 'AAC' }
     video_bitrate { '4000000.0' }
diff --git a/spec/models/master_file_spec.rb b/spec/models/master_file_spec.rb
index 488e950a4d..70ffa27e91 100644
--- a/spec/models/master_file_spec.rb
+++ b/spec/models/master_file_spec.rb
@@ -539,21 +539,6 @@
     end
   end
 
-  describe '#update_progress_on_success!' do
-    subject(:master_file) { FactoryBot.create(:master_file) }
-    let(:encode) { double("encode", :output => []) }
-    before do
-      allow(master_file).to receive(:update_ingest_batch).and_return(true)
-    end
-
-    it 'should set the digitized date' do
-      master_file.update_progress_on_success!(encode)
-      master_file.reload
-      expect(master_file.date_digitized).to_not be_empty
-    end
-
-  end
-
   describe "#structural_metadata_labels" do
     subject(:master_file) { FactoryBot.create(:master_file, :with_structure) }
     it 'should return correct list of labels' do
@@ -816,11 +801,25 @@
     let(:master_file) { FactoryBot.build(:master_file) }
     let(:encode_succeeded) { FactoryBot.build(:encode, :succeeded) }
 
+    before do
+      allow(master_file).to receive(:update_derivatives)
+      allow(master_file).to receive(:run_hook)
+    end
+
     it 'calls update_derivatives' do
       expect(master_file).to receive(:update_derivatives).with(array_including(hash_including(label: 'quality-high')))
       expect(master_file).to receive(:run_hook).with(:after_transcoding)
       master_file.update_progress_on_success!(encode_succeeded)
     end
+
+    it 'updates duration' do
+      expect { master_file.update_progress_on_success!(encode_succeeded) }.to change { master_file.duration }.from("200000").to("21575")
+    end
+
+    it 'should set the digitized date' do
+      master_file.update_progress_on_success!(encode_succeeded)
+      expect(master_file.date_digitized).to_not be_empty
+    end
   end
 
   describe 'update_derivatives' do