diff --git a/app/models/media_attachment.rb b/app/models/media_attachment.rb index 6708cd7793..76815dede5 100644 --- a/app/models/media_attachment.rb +++ b/app/models/media_attachment.rb @@ -58,6 +58,7 @@ class MediaAttachment < ApplicationRecord ).freeze IMAGE_MIME_TYPES = %w(image/jpeg image/png image/gif image/heic image/heif image/webp image/avif).freeze + IMAGE_ANIMATED_MIME_TYPES = %w(image/png image/gif).freeze IMAGE_CONVERTIBLE_MIME_TYPES = %w(image/heic image/heif image/avif).freeze VIDEO_MIME_TYPES = %w(video/webm video/mp4 video/quicktime video/ogg).freeze VIDEO_CONVERTIBLE_MIME_TYPES = %w(video/webm video/quicktime).freeze @@ -102,7 +103,7 @@ class MediaAttachment < ApplicationRecord 'preset' => 'veryfast', 'movflags' => 'faststart', # Move metadata to start of file so playback can begin before download finishes 'pix_fmt' => 'yuv420p', # Ensure color space for cross-browser compatibility - 'vf' => 'crop=floor(iw/2)*2:floor(ih/2)*2', # h264 requires width and height to be even. Crop instead of scale to avoid blurring + 'filter_complex' => 'drawbox=t=fill:c=white[bg];[bg][0]overlay,crop=trunc(iw/2)*2:trunc(ih/2)*2', # Remove transparency. h264 requires width and height to be even; crop instead of scale to avoid blurring 'c:v' => 'h264', 'c:a' => 'aac', 'b:a' => '192k', @@ -296,7 +297,7 @@ class MediaAttachment < ApplicationRecord private def file_styles(attachment) - if attachment.instance.file_content_type == 'image/gif' || VIDEO_CONVERTIBLE_MIME_TYPES.include?(attachment.instance.file_content_type) + if attachment.instance.animated_image? || VIDEO_CONVERTIBLE_MIME_TYPES.include?(attachment.instance.file_content_type) VIDEO_CONVERTED_STYLES elsif IMAGE_CONVERTIBLE_MIME_TYPES.include?(attachment.instance.file_content_type) IMAGE_CONVERTED_STYLES @@ -310,8 +311,8 @@ class MediaAttachment < ApplicationRecord end def file_processors(instance) - if instance.file_content_type == 'image/gif' - [:gif_transcoder, :blurhash_transcoder] + if instance.animated_image? + [:gifv_transcoder, :blurhash_transcoder] elsif VIDEO_MIME_TYPES.include?(instance.file_content_type) [:transcoder, :blurhash_transcoder, :type_corrector] elsif AUDIO_MIME_TYPES.include?(instance.file_content_type) @@ -322,6 +323,17 @@ class MediaAttachment < ApplicationRecord end end + def animated_image? + if processing_complete? + gifv? + elsif IMAGE_ANIMATED_MIME_TYPES.include?(file_content_type) + @animated_image = FastImage.animated?(file.queued_for_write[:original].path) unless defined?(@animated_image) + @animated_image + else + false + end + end + private def set_unknown_type diff --git a/config/application.rb b/config/application.rb index e4e9680e66..59afafd5be 100644 --- a/config/application.rb +++ b/config/application.rb @@ -28,7 +28,7 @@ require_relative '../lib/redis/namespace_extensions' require_relative '../lib/paperclip/url_generator_extensions' require_relative '../lib/paperclip/attachment_extensions' -require_relative '../lib/paperclip/gif_transcoder' +require_relative '../lib/paperclip/gifv_transcoder' require_relative '../lib/paperclip/media_type_spoof_detector_extensions' require_relative '../lib/paperclip/transcoder' require_relative '../lib/paperclip/type_corrector' diff --git a/lib/paperclip/blurhash_transcoder.rb b/lib/paperclip/blurhash_transcoder.rb index b4ff4a12a0..fe58d1bce1 100644 --- a/lib/paperclip/blurhash_transcoder.rb +++ b/lib/paperclip/blurhash_transcoder.rb @@ -23,7 +23,7 @@ module Paperclip image = Vips::Image.thumbnail(@file.path, 100) [image.width, image.height, image.colourspace(:srgb).extract_band(0, n: 3).to_a.flatten] else - pixels = convert(':source -depth 8 RGB:-', source: File.expand_path(@file.path)).unpack('C*') + pixels = convert(':source -flatten -depth 8 -compress none RGB:-', source: File.expand_path(@file.path)).unpack('C*') geometry = options.fetch(:file_geometry_parser).from_file(@file) [geometry.width, geometry.height, pixels] end diff --git a/lib/paperclip/gif_transcoder.rb b/lib/paperclip/gif_transcoder.rb deleted file mode 100644 index 32bdb8a863..0000000000 --- a/lib/paperclip/gif_transcoder.rb +++ /dev/null @@ -1,126 +0,0 @@ -# frozen_string_literal: true - -class GifReader - attr_reader :animated - - EXTENSION_LABELS = [0xf9, 0x01, 0xff].freeze - GIF_HEADERS = %w(GIF87a GIF89a).freeze - - class GifReaderException < StandardError; 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).unpack1('C') >= 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).unpack1('C')) - 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 videos - - class GifTranscoder < Paperclip::Processor - def make - return File.open(@file.path) unless needs_convert? - - final_file = Paperclip::Transcoder.make(file, options, attachment) - - if options[:style] == :original - attachment.instance.file_file_name = "#{File.basename(attachment.instance.file_file_name, '.*')}.mp4" - attachment.instance.file_content_type = 'video/mp4' - attachment.instance.type = MediaAttachment.types[:gifv] - end - - final_file - end - - private - - def needs_convert? - GifReader.animated?(file.path) - end - end -end diff --git a/lib/paperclip/gifv_transcoder.rb b/lib/paperclip/gifv_transcoder.rb new file mode 100644 index 0000000000..4bb27e3b75 --- /dev/null +++ b/lib/paperclip/gifv_transcoder.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +module Paperclip + # This transcoder is only to be used for the MediaAttachment model + # to convert animated GIFs and PNGs to videos + + class GifvTranscoder < Paperclip::Processor + def make + final_file = Paperclip::Transcoder.make(file, options, attachment) + + if options[:style] == :original + attachment.instance.file_file_name = "#{File.basename(attachment.instance.file_file_name, '.*')}.mp4" + attachment.instance.file_content_type = 'video/mp4' + attachment.instance.type = MediaAttachment.types[:gifv] + end + + final_file + end + end +end diff --git a/spec/fixtures/files/600x400-animated.gif b/spec/fixtures/files/600x400-animated.gif new file mode 100644 index 0000000000..1a773c0af1 Binary files /dev/null and b/spec/fixtures/files/600x400-animated.gif differ diff --git a/spec/fixtures/files/600x400-animated.png b/spec/fixtures/files/600x400-animated.png new file mode 100644 index 0000000000..6430fceabb Binary files /dev/null and b/spec/fixtures/files/600x400-animated.png differ diff --git a/spec/fixtures/files/600x400.gif b/spec/fixtures/files/600x400.gif new file mode 100644 index 0000000000..bab39cb6f7 Binary files /dev/null and b/spec/fixtures/files/600x400.gif differ diff --git a/spec/fixtures/files/attachment.gif b/spec/fixtures/files/attachment.gif deleted file mode 100644 index 89dd73ad3e..0000000000 Binary files a/spec/fixtures/files/attachment.gif and /dev/null differ diff --git a/spec/models/media_attachment_spec.rb b/spec/models/media_attachment_spec.rb index 5f91ae0967..cce4b30479 100644 --- a/spec/models/media_attachment_spec.rb +++ b/spec/models/media_attachment_spec.rb @@ -90,7 +90,7 @@ RSpec.describe MediaAttachment, :attachment_processing do media.destroy end - it 'saves media attachment with correct file and size metadata' do + it 'saves metadata and generates styles' do expect(media) .to be_persisted .and be_processing_complete @@ -98,18 +98,28 @@ RSpec.describe MediaAttachment, :attachment_processing do file: be_present, type: eq('image'), file_content_type: eq(content_type), - file_file_name: end_with(extension) + file_file_name: end_with(extension), + blurhash: have_attributes(size: eq(36)) ) - # Rack::Mime (used by PublicFileServerMiddleware) recognizes file extension - expect(Rack::Mime.mime_type(extension, nil)).to eq content_type - # Strip original file name expect(media.file_file_name) .to_not start_with '600x400' + # Generate styles + expect(FastImage.size(media.file.path(:original))) + .to eq [600, 400] + expect(FastImage.size(media.file.path(:small))) + .to eq [588, 392] + + # Use extension recognized by Rack::Mime (used by PublicFileServerMiddleware) + expect(media.file.path(:original)) + .to end_with(extension) + expect(media.file.path(:small)) + .to end_with(extension) + # Set meta for original and thumbnail - expect(media.file.meta.deep_symbolize_keys) + expect(media_metadata) .to include( original: include( width: eq(600), @@ -122,6 +132,60 @@ RSpec.describe MediaAttachment, :attachment_processing do aspect: eq(1.5) ) ) + + # Rack::Mime (used by PublicFileServerMiddleware) recognizes file extension + expect(Rack::Mime.mime_type(extension, nil)).to eq content_type + end + end + + shared_examples 'animated 600x400 image' do + after do + media.destroy + end + + it 'saves metadata and generates styles' do + expect(media) + .to be_persisted + .and be_processing_complete + .and have_attributes( + file: be_present, + type: eq('gifv'), + file_content_type: eq('video/mp4'), + file_file_name: end_with('.mp4'), + blurhash: have_attributes(size: eq(36)) + ) + + # Strip original file name + expect(media.file_file_name) + .to_not start_with '600x400' + + # Transcode to MP4 + expect(media.file.path(:original)) + .to end_with('.mp4') + + # Generate static thumbnail + expect(FastImage.size(media.file.path(:small))) + .to eq [600, 400] + expect(FastImage.animated?(media.file.path(:small))) + .to be false + expect(media.file.path(:small)) + .to end_with('.png') + + # Set meta for styles + expect(media_metadata) + .to include( + original: include( + width: eq(600), + height: eq(400), + duration: eq(3), + frame_rate: '1/1' + ), + small: include( + width: eq(600), + height: eq(400), + aspect: eq(1.5) + ) + ) end end @@ -137,10 +201,10 @@ RSpec.describe MediaAttachment, :attachment_processing do it_behaves_like 'static 600x400 image', 'image/png', '.png' end - describe 'monochrome jpg' do - let(:media) { Fabricate(:media_attachment, file: attachment_fixture('monochrome.png')) } + describe 'gif' do + let(:media) { Fabricate(:media_attachment, file: attachment_fixture('600x400.gif')) } - it_behaves_like 'static 600x400 image', 'image/png', '.png' + it_behaves_like 'static 600x400 image', 'image/gif', '.gif' end describe 'webp' do @@ -161,6 +225,12 @@ RSpec.describe MediaAttachment, :attachment_processing do it_behaves_like 'static 600x400 image', 'image/jpeg', '.jpeg' end + describe 'monochrome jpg' do + let(:media) { Fabricate(:media_attachment, file: attachment_fixture('monochrome.png')) } + + it_behaves_like 'static 600x400 image', 'image/png', '.png' + end + describe 'base64-encoded image' do let(:base64_attachment) { "data:image/jpeg;base64,#{Base64.encode64(attachment_fixture('600x400.jpeg').read)}" } let(:media) { Fabricate(:media_attachment, file: base64_attachment) } @@ -169,51 +239,15 @@ RSpec.describe MediaAttachment, :attachment_processing do end describe 'animated gif' do - let(:media) { Fabricate(:media_attachment, file: attachment_fixture('avatar.gif')) } + let(:media) { Fabricate(:media_attachment, file: attachment_fixture('600x400-animated.gif')) } - it 'sets correct file metadata' do - expect(media) - .to have_attributes( - type: eq('gifv'), - file_content_type: eq('video/mp4') - ) - expect(media_metadata) - .to include( - original: include( - width: eq(128), - height: eq(128) - ) - ) - end + it_behaves_like 'animated 600x400 image' end - describe 'static gif' do - fixtures = [ - { filename: 'attachment.gif', width: 600, height: 400, aspect: 1.5 }, - { filename: 'mini-static.gif', width: 32, height: 32, aspect: 1.0 }, - ] + describe 'animated png' do + let(:media) { Fabricate(:media_attachment, file: attachment_fixture('600x400-animated.png')) } - fixtures.each do |fixture| - context fixture[:filename] do - let(:media) { Fabricate(:media_attachment, file: attachment_fixture(fixture[:filename])) } - - it 'sets correct file metadata' do - expect(media) - .to have_attributes( - type: eq('image'), - file_content_type: eq('image/gif') - ) - expect(media_metadata) - .to include( - original: include( - width: eq(fixture[:width]), - height: eq(fixture[:height]), - aspect: eq(fixture[:aspect]) - ) - ) - end - end - end + it_behaves_like 'animated 600x400 image' end describe 'ogg with cover art' do diff --git a/spec/requests/api/v1/media_spec.rb b/spec/requests/api/v1/media_spec.rb index d7d0b92f11..3340e26b98 100644 --- a/spec/requests/api/v1/media_spec.rb +++ b/spec/requests/api/v1/media_spec.rb @@ -137,7 +137,7 @@ RSpec.describe 'Media' do end context 'with image/gif', :attachment_processing do - let(:params) { { file: fixture_file_upload('attachment.gif', 'image/gif') } } + let(:params) { { file: fixture_file_upload('600x400.gif', 'image/gif') } } it_behaves_like 'a successful media upload', 'image' end