Optimizes the screenshot option by migrating seek_time to an input op…

…tion (#162)

* Optimizes the screenshot option by migrating seek_time to an input option

* Attempts to simplify the new method

lib/ffmpeg/transcoder.rb

@@ -17,15 +17,8 @@ def initialize(input, output_file, options = EncodingOptions.new, transcoder_opt
       end
       @output_file = output_file
 
-      if options.is_a?(Array) || options.is_a?(EncodingOptions)
-        @raw_options = options
-      elsif options.is_a?(Hash)
-        @raw_options = EncodingOptions.new(options)
-      else
-        raise ArgumentError, "Unknown options format '#{options.class}', should be either EncodingOptions, Hash or Array."
-      end
+      @raw_options, @transcoder_options = optimize_screenshot_parameters(options, transcoder_options)
 
-      @transcoder_options = transcoder_options
       @errors = []
 
       apply_transcoder_options
@@ -145,5 +138,51 @@ def fix_encoding(output)
     rescue ArgumentError
       output.force_encoding("ISO-8859-1")
     end
+
+    def optimize_screenshot_parameters(options, transcoder_options)
+      # Moves any screenshot seek_time to an 'ss' input_option
+      raw_options, input_seek_time = screenshot_seek_time(options)
+      screenshot_to_transcoder_options(input_seek_time, transcoder_options)
+
+      return raw_options, transcoder_options
+    end
+
+    def screenshot_seek_time(options)
+      # Returns any seek_time for the screenshot and removes it from the options
+      # such that the seek time can be moved to an input option for improved FFMPEG performance
+      if options.is_a?(Array)
+        seek_time_idx = options.find_index('-seek_time') unless options.find_index('-screenshot').nil?
+        unless seek_time_idx.nil?
+          options.delete_at(seek_time_idx) # delete 'seek_time'
+          input_seek_time = options.delete_at(seek_time_idx).to_s # fetch the seek value
+        end
+        result = options, input_seek_time
+      elsif options.is_a?(Hash)
+        raw_options = EncodingOptions.new(options)
+        input_seek_time = raw_options.delete(:seek_time).to_s unless raw_options[:screenshot].nil?
+        result = raw_options, input_seek_time
+      else
+        raise ArgumentError, "Unknown options format '#{options.class}', should be either EncodingOptions, Hash or Array."
+      end
+      result
+    end
+
+    def screenshot_to_transcoder_options(seek_time, transcoder_options)
+      return if seek_time.to_s == ''
+
+      input_options = transcoder_options[:input_options] || []
+      # remove ss from input options because we're overriding from seek_time
+      if input_options.is_a?(Array)
+        fi = input_options.find_index('-ss')
+        if fi.nil?
+          input_options.concat(['-ss', seek_time])
+        else
+          input_options[fi + 1] = seek_time
+        end
+      else
+        input_options[:ss] = seek_time
+      end
+      transcoder_options[:input_options] = input_options
+    end
   end
 end

spec/ffmpeg/transcoder_spec.rb

@@ -345,19 +345,19 @@ module FFMPEG
       end
 
       context 'with input_options' do
-        let(:input_options) { nil }
-        let(:transcoder) { Transcoder.new(movie, 'tmp.mp4', {}, input_options: option) }
+        let(:transcoder) { Transcoder.new(movie, 'tmp.mp4', {}, input_options: input_options) }
+        let(:expected_result) { "-y -ss 30 -i #{transcoder.input}" }
 
-        context 'input_options is an array' do
-          let(:option) { %w(-framerate 1/5 -re) }
+        context 'as an array' do
+          let(:input_options) { %w(-framerate 1/5 -re) }
 
           it 'should add the input_options before the input' do
             expect(transcoder.command.join(' ')).to include("-framerate 1/5 -re -i #{transcoder.input}")
           end
         end
 
-        context 'input_options is a hash' do
-          let(:option) { {framerate: '1/5'} }
+        context 'as a hash' do
+          let(:input_options) { {framerate: '1/5'} }
 
           it 'should add the input_options before the input' do
             expect(transcoder.command.join(' ')).to include("-framerate 1/5 -i #{transcoder.input}")
@@ -366,7 +366,7 @@ module FFMPEG
           context 'to create a slideshow' do
             let(:file_spec) { "#{fixture_path}/images/img_%03d.jpeg"}
             let(:output) { "#{tmp_path}/slideshow.mp4"}
-            let(:transcoder) { Transcoder.new(movie, output, {}, input: file_spec, input_options: option) }
+            let(:transcoder) { Transcoder.new(movie, output, {}, input: file_spec, input_options: input_options) }
 
             it 'should add the input_options before the input' do
               expect(transcoder.command.join(' ')).to include("-framerate 1/5 -i #{file_spec}")
@@ -399,6 +399,51 @@ module FFMPEG
             end
           end
         end
+
+        context 'not specified but with screenshot transcoder_options' do
+          let(:input_options) { nil }
+          let(:transcoder) { Transcoder.new(movie, 'tmp.mp4', transcoder_options, input_options: input_options) }
+
+          let(:transcoder_options) { { screenshot: true, seek_time: 30 } }
+          it 'should add the input_options before the input' do
+            expect(transcoder.command.join(' ')).to include(expected_result)
+          end
+        end
+
+        context 'specified without seek_time but with other transcoder_options' do
+          let(:input_options) { %w(-re) }
+          let(:transcoder_options) { { screenshot: true } }
+          let(:transcoder) { Transcoder.new(movie, 'tmp.mp4', transcoder_options, input_options: input_options) }
+
+          it 'should add the input_options before the input' do
+            expect(transcoder.command.join(' ')).to_not include('-ss')
+          end
+
+          it 'should add the input_options before the input' do
+            expect(transcoder.command.join(' ')).to include("-y -re -i #{transcoder.input}")
+          end
+        end
+
+        context 'specified seek_time and options with -ss prefers seek_time value' do
+          let(:transcoder_options) { { screenshot: true, seek_time: 30 } }
+          let(:transcoder) { Transcoder.new(movie, 'tmp.mp4', transcoder_options, input_options: input_options) }
+
+          context 'as a Hash' do
+            let(:input_options) { { ss: 20 } }
+
+            it 'should add the input_options before the input' do
+              expect(transcoder.command.join(' ')).to include(expected_result)
+            end
+
+          end
+
+          context 'as an Array' do
+            let(:input_options) { %w(-ss 20) }
+            it 'should add the input_options before the input' do
+              expect(transcoder.command.join(' ')).to include(expected_result)
+            end
+          end
+        end
       end
     end
   end