Fix performance of GIF re-encoding (#12057)

* Change animated GIF detection to not shell out to ImageMagick

Signed-off-by: Eugen Rochko <eugen@zeonfederated.com>
* Change video encoding parameters to limit to 10800 video frames

Signed-off-by: Eugen Rochko <eugen@zeonfederated.com>
* Limit GIF image size further

Signed-off-by: Eugen Rochko <eugen@zeonfederated.com>
* Always strip metadata from video files

* Fix code style issues

diff --git a/app/models/concerns/attachmentable.rb b/app/models/concerns/attachmentable.rb
index 246c2c27cde2c8492192d90ec67f1fad7a2fb02e..3bbc6453c7f5575937352fb4c5b0f224a6d3dfbf 100644 (file)
--- a/app/models/concerns/attachmentable.rb
+++ b/app/models/concerns/attachmentable.rb
@@ -6,6 +6,7 @@ module Attachmentable
   extend ActiveSupport::Concern
 
   MAX_MATRIX_LIMIT = 16_777_216 # 4096x4096px or approx. 16MB
+  GIF_MATRIX_LIMIT = 921_600    # 1280x720px
 
   included do
     before_post_process :set_file_extensions
@@ -42,8 +43,9 @@ module Attachmentable
       next if attachment.blank? || !/image.*/.match?(attachment.content_type) || attachment.queued_for_write[:original].blank?
 
       width, height = FastImage.size(attachment.queued_for_write[:original].path)
+      matrix_limit  = attachment.content_type == 'image/gif' ? GIF_MATRIX_LIMIT : MAX_MATRIX_LIMIT
 
-      raise Mastodon::DimensionsValidationError, "#{width}x#{height} images are not supported, must be below #{MAX_MATRIX_LIMIT} sqpx" if width.present? && height.present? && (width * height >= MAX_MATRIX_LIMIT)
+      raise Mastodon::DimensionsValidationError, "#{width}x#{height} images are not supported" if width.present? && height.present? && (width * height > matrix_limit)
     end
   end
 

diff --git a/app/models/media_attachment.rb b/app/models/media_attachment.rb
index c4932f2ef5923e91368acb7517137ce9de56d119..630dab55ae9ab2b4ef416d73400056b13218c742 100644 (file)
--- a/app/models/media_attachment.rb
+++ b/app/models/media_attachment.rb
@@ -65,6 +65,17 @@ class MediaAttachment < ApplicationRecord
       file_geometry_parser: FastGeometryParser,
       blurhash: BLURHASH_OPTIONS,
     },
+
+    original: {
+      keep_same_format: true,
+      convert_options: {
+        output: {
+          'map_metadata' => '-1',
+          'c:v' => 'copy',
+          'c:a' => 'copy',
+        },
+      },
+    },
   }.freeze
 
   AUDIO_STYLES = {
@@ -86,14 +97,15 @@ class MediaAttachment < ApplicationRecord
       output: {
         'loglevel' => 'fatal',
         'movflags' => 'faststart',
-        'pix_fmt'  => 'yuv420p',
-        'vf'       => 'scale=\'trunc(iw/2)*2:trunc(ih/2)*2\'',
-        'vsync'    => 'cfr',
-        'c:v'      => 'h264',
-        'b:v'      => '500K',
-        'maxrate'  => '1300K',
-        'bufsize'  => '1300K',
-        'crf'      => 18,
+        'pix_fmt' => 'yuv420p',
+        'vf' => 'scale=\'trunc(iw/2)*2:trunc(ih/2)*2\'',
+        'vsync' => 'cfr',
+        'c:v' => 'h264',
+        'maxrate' => '1300K',
+        'bufsize' => '1300K',
+        'frames:v' => 60 * 60 * 3,
+        'crf' => 18,
+        'map_metadata' => '-1',
       },
     },
   }.freeze
@@ -103,7 +115,7 @@ class MediaAttachment < ApplicationRecord
     original: VIDEO_FORMAT,
   }.freeze
 
-  IMAGE_LIMIT = 8.megabytes
+  IMAGE_LIMIT = 10.megabytes
   VIDEO_LIMIT = 40.megabytes
 
   belongs_to :account,          inverse_of: :media_attachments, optional: true
@@ -244,7 +256,9 @@ class MediaAttachment < ApplicationRecord
 
   def set_meta
     meta = populate_meta
+
     return if meta == {}
+
     file.instance_write :meta, meta
   end
 
@@ -287,6 +301,7 @@ class MediaAttachment < ApplicationRecord
 
   def reset_parent_cache
     return if status_id.nil?
+
     Rails.cache.delete("statuses/#{status_id}")
   end
 end

diff --git a/lib/paperclip/gif_transcoder.rb b/lib/paperclip/gif_transcoder.rb
index cbab6fd99a29e7ac7f95a966c0614ee5f27904b9..64f12f9637e57fd63953aa2aa2e6293dd9a40eee 100644 (file)
--- a/lib/paperclip/gif_transcoder.rb
+++ b/lib/paperclip/gif_transcoder.rb
@@ -1,5 +1,103 @@
 # frozen_string_literal: true
 
+class GifReader
+  attr_reader :animated
+
+  EXTENSION_LABELS = [0xf9, 0x01, 0xff].freeze
+  GIF_HEADERS      = %w(GIF87a GIF89a).freeze
+
+  class GifReaderException; end
+
+  class UnknownImageType < GifReaderException; end
+
+  class CannotParseImage < GifReaderException; end
+
+  def self.animated?(path)
+    new(path).animated
+  rescue GifReaderException
+    false
+  end
+
+  def initialize(path, max_frames = 2)
+    @path      = path
+    @nb_frames = 0
+
+    File.open(path, 'rb') do |s|
+      raise UnknownImageType unless GIF_HEADERS.include?(s.read(6))
+
+      # Skip to "packed byte"
+      s.seek(4, IO::SEEK_CUR)
+
+      # "Packed byte" gives us the size of the GIF color table
+      packed_byte, = s.read(1).unpack('C')
+
+      # Skip background color and aspect ratio
+      s.seek(2, IO::SEEK_CUR)
+
+      if packed_byte & 0x80 != 0
+        # GIF uses a global color table, skip it
+        s.seek(3 * (1 << ((packed_byte & 0x07) + 1)), IO::SEEK_CUR)
+      end
+
+      # Now read data
+      while @nb_frames < max_frames
+        separator = s.read(1)
+
+        case separator
+        when ',' # Image block
+          @nb_frames += 1
+
+          # Skip to "packed byte"
+          s.seek(8, IO::SEEK_CUR)
+          packed_byte, = s.read(1).unpack('C')
+
+          if packed_byte & 0x80 != 0
+            # Image uses a local color table, skip it
+            s.seek(3 * (1 << ((packed_byte & 0x07) + 1)), IO::SEEK_CUR)
+          end
+
+          # Skip lzw min code size
+          raise InvalidValue unless s.read(1).unpack('C')[0] >= 2
+
+          # Skip image data sub-blocks
+          skip_sub_blocks!(s)
+        when '!' # Extension block
+          skip_extension_block!(s)
+        when ';' # Trailer
+          break
+        else
+          raise CannotParseImage
+        end
+      end
+    end
+
+    @animated = @nb_frames > 1
+  end
+
+  private
+
+  def skip_extension_block!(file)
+    if EXTENSION_LABELS.include?(file.read(1).unpack('C')[0])
+      block_size, = file.read(1).unpack('C')
+      file.seek(block_size, IO::SEEK_CUR)
+    end
+
+    # Read until extension block end marker
+    skip_sub_blocks!(file)
+  end
+
+  # Skip sub-blocks up until block end marker
+  def skip_sub_blocks!(file)
+    loop do
+      size, = file.read(1).unpack('C')
+
+      break if size.zero?
+
+      file.seek(size, IO::SEEK_CUR)
+    end
+  end
+end
+
 module Paperclip
   # This transcoder is only to be used for the MediaAttachment model
   # to convert animated gifs to webm
@@ -19,8 +117,7 @@ module Paperclip
     private
 
     def needs_convert?
-      num_frames = identify('-format %n :file', file: file.path).to_i
-      options[:style] == :original && num_frames > 1
+      options[:style] == :original && GifReader.animated?(file.path)
     end
   end
 end

diff --git a/lib/paperclip/video_transcoder.rb b/lib/paperclip/video_transcoder.rb
index c3504c17c4fbef2c959d2e86d924b67994530fed..66f7feda5603201043366962b72f8cbf3ff7a3e3 100644 (file)
--- a/lib/paperclip/video_transcoder.rb
+++ b/lib/paperclip/video_transcoder.rb
@@ -6,7 +6,9 @@ module Paperclip
   class VideoTranscoder < Paperclip::Processor
     def make
       meta = ::Av.cli.identify(@file.path)
+
       attachment.instance.type = MediaAttachment.types[:gifv] unless meta[:audio_encode]
+      options[:format] = File.extname(attachment.instance.file_file_name)[1..-1] if options[:keep_same_format]
 
       Paperclip::Transcoder.make(file, options, attachment)
     end