Merge pull request #1162 from PRX/fix/transcode_vbr

Transcode VBR mp3 files to CBR

app/models/task.rb

@@ -25,6 +25,7 @@ def result
   scope :copy_image, -> { where(type: "Tasks::CopyImageTask") }
   scope :bad_audio_duration, -> { where("result ~ '\"DurationDiscrepancy\":([5-9]\\d[1-9]|[6-9]\\d{2}|[1-9]\d{3})'") }
   scope :bad_audio_bytes, -> { where("result ~ '\"UnidentifiedBytes\":[1-9]'") }
+  scope :bad_audio_vbr, -> { where("result ~ '\"VariableBitrate\":true'") }
 
   def self.callback(msg)
     Task.transaction do
@@ -47,6 +48,10 @@ def job_id
   def source_url
   end
 
+  def bad_audio?
+    bad_audio_duration? || bad_audio_bytes? || bad_audio_vbr?
+  end
+
   def bad_audio_duration?
     porter_callback_inspect.dig(:Audio, :DurationDiscrepancy).to_i > 500
   end
@@ -55,6 +60,10 @@ def bad_audio_bytes?
     porter_callback_inspect.dig(:Audio, :UnidentifiedBytes).to_i > 0
   end
 
+  def bad_audio_vbr?
+    !!porter_callback_inspect.dig(:Audio, :VariableBitrate)
+  end
+
   def start!
     self.status = "started"
     self.options = {

app/models/tasks/copy_media_task.rb

@@ -58,7 +58,7 @@ def update_media_resource
 
     media_resource.save!
 
-    if media_resource.status_complete? && (bad_audio_duration? || bad_audio_bytes?)
+    if media_resource.status_complete? && bad_audio?
       fix_media!
     else
       slice_media!
@@ -69,9 +69,13 @@ def fix_media!
     media_resource.status = "processing"
     media_resource.save!
 
-    # fix media, but set an explicit format for ffmpeg to use if possible
+    # set an explicit format for ffmpeg to use if possible
     fmt = porter_callback_inspect.dig(:Audio, :Format) || porter_callback_inspect.dig(:Video, :Format)
-    Tasks::FixMediaTask.create!(owner: owner, media_format: fmt).start!
+
+    # set an explicit bitrate if vbr
+    bit = next_highest_bitrate if bad_audio_vbr?
+
+    Tasks::FixMediaTask.create!(owner: owner, media_format: fmt, media_bitrate: bit).start!
   end
 
   def slice_media!
@@ -81,6 +85,16 @@ def slice_media!
     end
   end
 
+  def next_highest_bitrate
+    bitrate = porter_callback_inspect.dig(:Audio, :Bitrate).to_i / 1000
+    if bitrate > 0
+      higher_bits = AudioFormatValidator::BIT_RATES.select { |b| b >= bitrate }
+      higher_bits.first || AudioFormatValidator::BIT_RATES.last
+    else
+      128
+    end
+  end
+
   private
 
   def porter_inspect_task

app/models/tasks/fix_media_task.rb

@@ -1,7 +1,7 @@
 class Tasks::FixMediaTask < ::Task
   before_save :update_media_resource, if: ->(t) { t.status_changed? && t.media_resource }
 
-  attr_accessor :media_format
+  attr_accessor :media_format, :media_bitrate
 
   def media_resource
     owner
@@ -27,7 +27,7 @@ def porter_tasks
           }
         },
         FFmpeg: {
-          OutputFileOptions: "-acodec copy"
+          OutputFileOptions: media_bitrate ? "-acodec copy -b:a #{media_bitrate}k" : "-acodec copy"
         }
       }
     ]

test/models/task_test.rb

@@ -83,6 +83,21 @@
     end
   end
 
+  describe "#bad_audio_vbr?" do
+    it "checks for the vbr flag" do
+      refute task.bad_audio_vbr?
+
+      task.result = build(:porter_job_results)
+      refute task.bad_audio_vbr?
+
+      task.result[:JobResult][:TaskResults][1][:Inspection][:Audio][:VariableBitrate] = false
+      refute task.bad_audio_vbr?
+
+      task.result[:JobResult][:TaskResults][1][:Inspection][:Audio][:VariableBitrate] = true
+      assert task.bad_audio_vbr?
+    end
+  end
+
   describe "#start!" do
     it "starts a porter job" do
       task.stub(:source_url, "http://some/file.mp3") do

test/models/tasks/copy_media_task_test.rb

@@ -185,7 +185,7 @@
 
       task.result[:JobResult][:TaskResults][1][:Inspection][:Audio][:DurationDiscrepancy] = 1000
 
-      Task.stub_any_instance(:start!, true) do
+      Task.stub_any_instance(:porter_start!, true) do
         assert_difference("Tasks::FixMediaTask.count", 1) do
           task.update(status: "complete")
 
@@ -194,8 +194,47 @@
 
           # latest task is now to fix
           assert_equal Tasks::FixMediaTask, task.media_resource.task.class
+          assert_equal "mp3", task.media_resource.task.options[:Tasks][0][:Format]
+          assert_equal "-acodec copy", task.media_resource.task.options[:Tasks][0][:FFmpeg][:OutputFileOptions]
         end
       end
     end
+
+    it "runs a FixMediaTask if the audio was vbr" do
+      task.update(status: "created")
+
+      task.result[:JobResult][:TaskResults][1][:Inspection][:Audio][:VariableBitrate] = true
+
+      Task.stub_any_instance(:porter_start!, true) do
+        assert_difference("Tasks::FixMediaTask.count", 1) do
+          task.update(status: "complete")
+
+          # status is still processing
+          assert_equal "processing", task.media_resource.status
+
+          # latest task is now to fix
+          assert_equal Tasks::FixMediaTask, task.media_resource.task.class
+          assert_equal "mp3", task.media_resource.task.options[:Tasks][0][:Format]
+          assert_equal "-acodec copy -b:a 192k", task.media_resource.task.options[:Tasks][0][:FFmpeg][:OutputFileOptions]
+        end
+      end
+    end
+  end
+
+  describe "#next_highest_bitrate" do
+    it "returns the next highest common bitrate" do
+      assert_equal 192, task.next_highest_bitrate
+
+      task.result[:JobResult][:TaskResults][1][:Inspection][:Audio][:Bitrate] = 195678
+      assert_equal 224, task.next_highest_bitrate
+
+      # 320 is the  max
+      task.result[:JobResult][:TaskResults][1][:Inspection][:Audio][:Bitrate] = 999999
+      assert_equal 320, task.next_highest_bitrate
+
+      # 128 is the default
+      task.result[:JobResult][:TaskResults][1][:Inspection][:Audio][:Bitrate] = nil
+      assert_equal 128, task.next_highest_bitrate
+    end
   end
 end