From 8f23726918fd8ded18ce9e55e6199df87abcc7cf Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Thu, 20 Jun 2019 10:52:36 +0200 Subject: Fix converted media being saved with original extension and mime type (#11130) --- lib/paperclip/type_corrector.rb | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 lib/paperclip/type_corrector.rb (limited to 'lib/paperclip') diff --git a/lib/paperclip/type_corrector.rb b/lib/paperclip/type_corrector.rb new file mode 100644 index 000000000..0b0c10a56 --- /dev/null +++ b/lib/paperclip/type_corrector.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +require 'mime/types/columnar' + +module Paperclip + class TypeCorrector < Paperclip::Processor + def make + target_extension = options[:format] + extension = File.extname(attachment.instance.file_file_name) + + return @file unless options[:style] == :original && target_extension && extension != target_extension + + attachment.instance.file_content_type = options[:content_type] || attachment.instance.file_content_type + attachment.instance.file_file_name = File.basename(attachment.instance.file_file_name, '.*') + '.' + target_extension + + @file + end + end +end -- cgit From ca22a22d7f4db867ad0045a7978e3d8dcd251a69 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Thu, 3 Oct 2019 01:09:12 +0200 Subject: Fix performance of GIF re-encoding (#12057) * Change animated GIF detection to not shell out to ImageMagick Signed-off-by: Eugen Rochko * Change video encoding parameters to limit to 10800 video frames Signed-off-by: Eugen Rochko * Limit GIF image size further Signed-off-by: Eugen Rochko * Always strip metadata from video files * Fix code style issues --- app/models/concerns/attachmentable.rb | 4 +- app/models/media_attachment.rb | 33 ++++++++--- lib/paperclip/gif_transcoder.rb | 101 +++++++++++++++++++++++++++++++++- lib/paperclip/video_transcoder.rb | 2 + 4 files changed, 128 insertions(+), 12 deletions(-) (limited to 'lib/paperclip') diff --git a/app/models/concerns/attachmentable.rb b/app/models/concerns/attachmentable.rb index 246c2c27c..3bbc6453c 100644 --- 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 c4932f2ef..630dab55a 100644 --- 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 cbab6fd99..64f12f963 100644 --- 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 c3504c17c..66f7feda5 100644 --- 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 -- cgit From 6be16d02cb3f03d21226f9b6664d33e3e8650043 Mon Sep 17 00:00:00 2001 From: Yamagishi Kazutoshi Date: Tue, 3 Dec 2019 02:25:43 +0900 Subject: Update ESLint and RuboCop in Code Climate (#12534) --- .codeclimate.yml | 4 ++-- .rubocop.yml | 6 ++++++ app/lib/activitypub/adapter.rb | 1 + app/lib/language_detector.rb | 2 +- app/models/media_attachment.rb | 2 +- lib/paperclip/lazy_thumbnail.rb | 4 ++-- 6 files changed, 13 insertions(+), 6 deletions(-) (limited to 'lib/paperclip') diff --git a/.codeclimate.yml b/.codeclimate.yml index 571507a54..9817d7f1c 100644 --- a/.codeclimate.yml +++ b/.codeclimate.yml @@ -27,10 +27,10 @@ plugins: enabled: true eslint: enabled: true - channel: eslint-5 + channel: eslint-6 rubocop: enabled: true - channel: rubocop-0-71 + channel: rubocop-0-76 sass-lint: enabled: true exclude_patterns: diff --git a/.rubocop.yml b/.rubocop.yml index 8bd4c867f..9a68becbb 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -71,6 +71,9 @@ Naming/MemoizedInstanceVariableName: Rails: Enabled: true +Rails/EnumHash: + Enabled: false + Rails/HasAndBelongsToMany: Enabled: false @@ -102,6 +105,9 @@ Style/Documentation: Style/DoubleNegation: Enabled: true +Style/FormatStringToken: + Enabled: false + Style/FrozenStringLiteralComment: Enabled: true diff --git a/app/lib/activitypub/adapter.rb b/app/lib/activitypub/adapter.rb index 2a8f72333..78138fb73 100644 --- a/app/lib/activitypub/adapter.rb +++ b/app/lib/activitypub/adapter.rb @@ -35,6 +35,7 @@ class ActivityPub::Adapter < ActiveModelSerializers::Adapter::Base def serializable_hash(options = nil) named_contexts = {} context_extensions = {} + options = serialization_options(options) serialized_hash = serializer.serializable_hash(options.merge(named_contexts: named_contexts, context_extensions: context_extensions)) serialized_hash = serialized_hash.select { |k, _| options[:fields].include?(k) } if options[:fields] diff --git a/app/lib/language_detector.rb b/app/lib/language_detector.rb index 6f9511a54..302072bcc 100644 --- a/app/lib/language_detector.rb +++ b/app/lib/language_detector.rb @@ -44,7 +44,7 @@ class LanguageDetector words = text.scan(RELIABLE_CHARACTERS_RE) if words.present? - words.reduce(0) { |acc, elem| acc + elem.size }.to_f / text.size.to_f > 0.3 + words.reduce(0) { |acc, elem| acc + elem.size }.to_f / text.size > 0.3 else false end diff --git a/app/models/media_attachment.rb b/app/models/media_attachment.rb index 1f9d92b22..5d5034a52 100644 --- a/app/models/media_attachment.rb +++ b/app/models/media_attachment.rb @@ -287,7 +287,7 @@ class MediaAttachment < ApplicationRecord width: width, height: height, size: "#{width}x#{height}", - aspect: width.to_f / height.to_f, + aspect: width.to_f / height, } end diff --git a/lib/paperclip/lazy_thumbnail.rb b/lib/paperclip/lazy_thumbnail.rb index 542c17fb2..10b14860c 100644 --- a/lib/paperclip/lazy_thumbnail.rb +++ b/lib/paperclip/lazy_thumbnail.rb @@ -9,8 +9,8 @@ module Paperclip min_side = [@current_geometry.width, @current_geometry.height].min.to_i options[:geometry] = "#{min_side}x#{min_side}#" if @target_geometry.square? && min_side < @target_geometry.width elsif options[:pixels] - width = Math.sqrt(options[:pixels] * (@current_geometry.width.to_f / @current_geometry.height.to_f)).round.to_i - height = Math.sqrt(options[:pixels] * (@current_geometry.height.to_f / @current_geometry.width.to_f)).round.to_i + width = Math.sqrt(options[:pixels] * (@current_geometry.width.to_f / @current_geometry.height)).round.to_i + height = Math.sqrt(options[:pixels] * (@current_geometry.height.to_f / @current_geometry.width)).round.to_i options[:geometry] = "#{width}x#{height}>" end -- cgit