Merge pull request #1 from instructure/2024-06-05

6.0.0

lib/ffmpeg.rb

@@ -10,6 +10,7 @@
 require_relative 'ffmpeg/version'
 require_relative 'ffmpeg/encoding_options'
 require_relative 'ffmpeg/errors'
+require_relative 'ffmpeg/timeout'
 require_relative 'ffmpeg/io'
 require_relative 'ffmpeg/media'
 require_relative 'ffmpeg/stream'

lib/ffmpeg/io.rb

@@ -7,50 +7,80 @@ module FFMPEG
   # The IO class is a simple wrapper around IO objects that adds a timeout
   # to all read operations and fixes encoding issues.
   class IO
-    attr_accessor :timeout
+    attr_accessor :encoding, :timeout
+
+    @encoding = 'UTF-8'
+
+    class << self
+      attr_accessor :encoding
+    end
 
     def self.force_encoding(chunk)
       chunk[/test/]
     rescue ArgumentError
-      chunk.force_encoding('ISO-8859-1')
+      chunk.force_encoding(encoding)
     end
 
     def initialize(target)
       @target = target
     end
 
+    def each(&block)
+      timer = timeout.nil? ? nil : Timeout.start(timeout)
+      buffer = String.new
+
+      until eof?
+        char = getc
+        case char
+        when "\n", "\r"
+          timer&.tick
+          timer&.pause
+          block.call(buffer)
+          timer&.resume
+          buffer = String.new
+        else
+          buffer << char
+        end
+      end
+
+      block.call(buffer) unless buffer.empty?
+    ensure
+      timer&.cancel
+    end
+
     %i[
       getc
       gets
       readchar
       readline
     ].each do |symbol|
       define_method(symbol) do |*args|
-        Timeout.timeout(timeout) do
-          output = @target.send(symbol, *args)
-          self.class.force_encoding(output)
-          output
-        end
+        data = @target.send(symbol, *args)
+        self.class.force_encoding(data) unless data.nil?
+        data
       end
     end
 
     %i[
-      each
       each_char
       each_line
     ].each do |symbol|
-      read = symbol == :each_char ? :getc : :gets
       define_method(symbol) do |*args, &block|
-        until eof?
-          output = send(read, *args)
-          block.call(output)
+        timer = timeout.nil? ? nil : Timeout.start(timeout)
+        @target.send(symbol, *args) do |data|
+          timer&.tick
+          timer&.pause
+          block.call(self.class.force_encoding(data))
+          timer&.resume
         end
+      ensure
+        timer&.cancel
       end
     end
 
     def readlines(*args)
       lines = []
-      lines << gets(*args) until eof?
+      each(*args) { |line| lines << line }
       lines
     end
 

lib/ffmpeg/media.rb

@@ -47,7 +47,7 @@ def initialize(path)
 
         @size = response.content_length
       else
-        raise Errno::ENOENT, "The file at '#{@path}' does not exist" unless File.exist?(path)
+        raise Errno::ENOENT, "The file at '#{@path}' does not exist" unless File.exist?(@path)
 
         @size = File.size(@path)
       end

lib/ffmpeg/timeout.rb

@@ -0,0 +1,53 @@
+# frozen_string_literal: true
+
+require 'timeout'
+
+module FFMPEG
+  # The Timeout class is a simple wrapper around the Timeout module that
+  # provides a more convenient API to handle timeouts in a loop.
+  class Timeout
+    def self.start(duration, message = nil)
+      new(duration, message)
+    end
+
+    def pause
+      @paused = true
+    end
+
+    def resume
+      @paused = false
+    end
+
+    def tick
+      @last_tick = Time.now
+    end
+
+    def cancel
+      return if @wait_thread.nil?
+
+      @wait_thread.kill
+      @wait_thread.join
+    end
+
+    private
+
+    def initialize(duration, message = nil)
+      @duration = duration
+      @message = message
+
+      @last_tick = Time.now
+      @current_thread = Thread.current
+      @wait_thread = Thread.new { loop }
+      @paused = false
+    end
+
+    def loop
+      if !@paused && Time.now - @last_tick >= @duration
+        @current_thread.raise(::Timeout::Error, @message || self.class.name)
+      else
+        sleep 0.1
+        loop
+      end
+    end
+  end
+end

lib/ffmpeg/transcoder.rb

@@ -6,7 +6,8 @@ module FFMPEG
   # The Transcoder class is responsible for transcoding multimedia files.
   # It accepts a Media object or a path to a multimedia file as input.
   class Transcoder
-    attr_reader :args, :output, :errors, :input_path, :output_path
+    attr_reader :args, :input_path, :output_path,
+                :output, :progress, :succeeded
 
     @timeout = 30
 
@@ -20,6 +21,7 @@ def initialize(
       options,
       validate: true,
       preserve_aspect_ratio: true,
+      progress_digits: 2,
       input_options: [],
       filters: []
     )
@@ -34,9 +36,9 @@ def initialize(
       @options = options.is_a?(Hash) ? EncodingOptions.new(options) : options
       @validate = validate
       @preserve_aspect_ratio = preserve_aspect_ratio
+      @progress_digits = progress_digits
       @input_options = input_options
       @filters = filters
-      @errors = []
 
       if @input_options.is_a?(Hash)
         @input_options = @input_options.reduce([]) do |acc, (key, value)|
@@ -66,21 +68,27 @@ def command
 
     def run(&block)
       execute(&block)
-      return nil unless @validate
+      validate_result if @validate
+    end
 
-      validate_output_file(&block)
-      result
+    def finished?
+      !@succeeded.nil?
     end
 
     def succeeded?
-      @errors.empty?
+      return false unless @succeeded
+      return true unless @validate
+
+      result&.valid?
     end
 
     def failed?
       !succeeded?
     end
 
     def result
+      return nil unless @succeeded
+
       @result ||= Media.new(@output_path) if File.exist?(@output_path)
     end
 
@@ -133,25 +141,14 @@ def prepare_seek_time
       end
     end
 
-    def validate_output_file
-      @errors << 'no output file created' unless File.exist?(@output_path)
-      @errors << 'encoded file is invalid' if result.nil? || !result.valid?
+    def validate_result
+      return result if result&.valid?
 
-      if succeeded?
-        yield(1.0) if block_given?
-        FFMPEG.logger.info(self.class) do
-          "Transcoding #{@input_path} to #{@output_path} succeeded\n" \
-            "Command: #{command.join(' ')}\n" \
-            "Output: #{@output}"
-        end
-      else
-        message = "Transcoding #{@input_path} to #{@output_path} failed\n" \
-                  "Command: #{command.join(' ')}\n" \
-                  "Errors: #{@errors.join(', ')}\n " \
-                  "Output: #{@output}\n"
-        FFMPEG.logger.error(self.class) { message }
-        raise Error, message
-      end
+      message = "Transcoding #{@input_path} to #{@output_path} produced invalid media\n" \
+                "Command: #{command.join(' ')}\n" \
+                "Output: #{@output}"
+      FFMPEG.logger.error(self.class) { message }
+      raise Error, message
     end
 
     def execute
@@ -161,6 +158,8 @@ def execute
       end
 
       @output = String.new
+      @progress = 0.0
+      @succeeded = nil
 
       FFMPEG.ffmpeg_popen3(*@args) do |_stdin, stdout, stderr, wait_thr|
         yield(0.0) if block_given?
@@ -174,21 +173,42 @@ def execute
           @output << line
 
           next unless @media
-          next unless block_given?
           next unless line =~ /time=(\d+):(\d+):(\d+.\d+)/ # time=00:02:42.28
 
           time = (::Regexp.last_match(1).to_i * 3600) +
                  (::Regexp.last_match(2).to_i * 60) +
                  ::Regexp.last_match(3).to_f
-          yield(time / @media.duration)
+          progress = (time / @media.duration).round(@progress_digits)
+          next unless progress < 1.0 || progress == @progress
+
+          @progress = progress
+          yield(@progress) if block_given?
         end
 
-        @errors << 'ffmpeg returned non-zero exit code' unless wait_thr.value.success?
-      rescue Timeout::Error
-        message = "Transcoding #{@input_path} to #{@output_path} failed, process hung\n" \
+        if wait_thr.value.success?
+          @succeeded = true
+          @progress = 1.0
+          yield(@progress) if block_given?
+
+          FFMPEG.logger.info(self.class) do
+            "Transcoding #{@input_path} to #{@output_path} succeeded\n" \
+              "Command: #{command.join(' ')}\n" \
+              "Output: #{@output}"
+          end
+        else
+          @succeeded = false
+          message = "Transcoding #{@input_path} to #{@output_path} failed\n" \
+                    "Command: #{command.join(' ')}\n" \
+                    "Output: #{@output}"
+          FFMPEG.logger.error(self.class) { message }
+          raise Error, message
+        end
+      rescue ::Timeout::Error
+        @succeeded = false
+        Process.kill(FFMPEG::SIGKILL, wait_thr.pid)
+        message = "Transcoding #{@input_path} to #{@output_path} timed out\n" \
                   "Command: #{command.join(' ')}\n" \
                   "Output: #{@output}"
-        Process.kill(FFMPEG::SIGKILL, wait_thr.pid)
         FFMPEG.logger.error(self.class) { message }
         raise Error, message
       end

lib/ffmpeg/version.rb

@@ -1,5 +1,5 @@
 # frozen_string_literal: true
 
 module FFMPEG
-  VERSION = '5.0.1'
+  VERSION = '6.0.0'
 end

spec/ffmpeg/media_spec.rb

@@ -58,7 +58,7 @@ module FFMPEG
         let(:stdout) { read_fixture_file("outputs/#{stdout_fixture_file}") }
         let(:stderr) { stderr_fixture_file ? read_fixture_file("outputs/#{stderr_fixture_file}") : '' }
 
-        before { allow(Open3).to receive(:capture3).and_return([stdout, stderr, double(success?: true)]) }
+        before { allow(Open3).to receive(:capture3).and_return([stdout, stderr, double(succeeded: true)]) }
         subject { described_class.new(__FILE__) }
 
         context 'cannot be parsed' do
@@ -105,6 +105,15 @@ module FFMPEG
           it 'should not raise an error' do
             expect { subject }.not_to raise_error
           end
+
+          context 'with IO encoding set to ISO-8859-1' do
+            before { FFMPEG::IO.encoding = 'ISO-8859-1' }
+            after { FFMPEG::IO.encoding = 'UTF-8' }
+
+            it 'should not raise an error' do
+              expect { subject }.not_to raise_error
+            end
+          end
         end
       end
     end