From 364343453c616a13ac215ff370fb37762ffe0f0d Mon Sep 17 00:00:00 2001 From: evazion Date: Tue, 19 May 2020 02:18:30 -0500 Subject: [PATCH] uploads: factor out remaining image methods to MediaFile. --- app/logical/media_file.rb | 18 ++++++++++- app/logical/media_file/image.rb | 19 ++++++++++++ app/logical/media_file/ugoira.rb | 1 - app/logical/media_file/video.rb | 10 ++++-- app/logical/upload_service/utils.rb | 45 +++------------------------ app/models/upload.rb | 48 +++++------------------------ test/unit/upload_service_test.rb | 3 -- 7 files changed, 56 insertions(+), 88 deletions(-) diff --git a/app/logical/media_file.rb b/app/logical/media_file.rb index 709b8743d..fe8652f27 100644 --- a/app/logical/media_file.rb +++ b/app/logical/media_file.rb @@ -89,6 +89,22 @@ class MediaFile file_ext == :swf end + def is_corrupt? + false + end + + def is_animated? + is_video? + end + + def has_audio? + false + end + + def duration + 0.0 + end + def preview(width, height, **options) nil end @@ -97,5 +113,5 @@ class MediaFile nil end - memoize :dimensions, :file_ext, :file_size, :md5 + memoize :dimensions, :file_ext, :file_size, :md5, :is_corrupt?, :is_animated? end diff --git a/app/logical/media_file/image.rb b/app/logical/media_file/image.rb index d26ad93f5..7b26b52b5 100644 --- a/app/logical/media_file/image.rb +++ b/app/logical/media_file/image.rb @@ -20,6 +20,17 @@ class MediaFile::Image < MediaFile [0, 0] end + def is_corrupt? + image.stats + false + rescue Vips::Error + true + end + + def is_animated? + is_animated_gif? || is_animated_png? + end + # https://github.com/jcupitt/libvips/wiki/HOWTO----Image-shrinking # http://jcupitt.github.io/libvips/API/current/Using-vipsthumbnail.md.html def preview(width, height) @@ -40,6 +51,14 @@ class MediaFile::Image < MediaFile private + def is_animated_gif? + file_ext == :gif && image.get("n-pages") > 1 + end + + def is_animated_png? + file_ext == :png && APNGInspector.new(file.path).inspect!.animated? + end + def image @image ||= Vips::Image.new_from_file(file.path, fail: true) end diff --git a/app/logical/media_file/ugoira.rb b/app/logical/media_file/ugoira.rb index 1b3791e92..391f14825 100644 --- a/app/logical/media_file/ugoira.rb +++ b/app/logical/media_file/ugoira.rb @@ -1,5 +1,4 @@ class MediaFile::Ugoira < MediaFile - extend Memoist class Error < StandardError; end attr_reader :frame_data diff --git a/app/logical/media_file/video.rb b/app/logical/media_file/video.rb index 396a2e19e..0457604b0 100644 --- a/app/logical/media_file/video.rb +++ b/app/logical/media_file/video.rb @@ -1,10 +1,12 @@ class MediaFile::Video < MediaFile - extend Memoist - def dimensions [video.width, video.height] end + def duration + video.duration + end + def preview(max_width, max_height) preview_frame.preview(max_width, max_height) end @@ -13,6 +15,10 @@ class MediaFile::Video < MediaFile preview_frame.crop(max_width, max_height) end + def has_audio? + video.audio_channels.present? + end + private def video diff --git a/app/logical/upload_service/utils.rb b/app/logical/upload_service/utils.rb index 49d11cc08..94f006497 100644 --- a/app/logical/upload_service/utils.rb +++ b/app/logical/upload_service/utils.rb @@ -2,14 +2,6 @@ class UploadService module Utils module_function - def corrupt?(filename) - image = Vips::Image.new_from_file(filename, fail: true) - image.stats - false - rescue Vips::Error - true - end - def distribute_files(file, record, type, original_post_id: nil) # need to do this for hybrid storage manager post = Post.new @@ -51,7 +43,7 @@ class UploadService upload.image_height = media_file.height upload.validate!(:file) - upload.tag_string = "#{upload.tag_string} #{Utils.automatic_tags(upload, file)}" + upload.tag_string = "#{upload.tag_string} #{Utils.automatic_tags(media_file)}" preview_file, crop_file, sample_file = Utils.generate_resizes(media_file) @@ -67,38 +59,11 @@ class UploadService end end - # these methods are only really used during upload processing even - # though logically they belong on upload. post can rely on the - # automatic tag that's added. - def is_animated_gif?(upload, file) - return false if upload.file_ext != "gif" - - # Check whether the gif has multiple frames by trying to load the second frame. - result = Vips::Image.gifload(file.path, page: 1) rescue $ERROR_INFO - if result.is_a?(Vips::Image) - true - elsif result.is_a?(Vips::Error) && result.message =~ /too few frames in GIF file/ - false - else - raise result - end - end - - def is_animated_png?(upload, file) - upload.file_ext == "png" && APNGInspector.new(file.path).inspect!.animated? - end - - def is_video_with_audio?(upload, file) - return false if !upload.is_video? # avoid ffprobe'ing the file if it's not a video (issue #3826) - video = FFMPEG::Movie.new(file.path) - video.audio_channels.present? - end - - def automatic_tags(upload, file) + def automatic_tags(media_file) tags = [] - tags << "video_with_sound" if is_video_with_audio?(upload, file) - tags << "animated_gif" if is_animated_gif?(upload, file) - tags << "animated_png" if is_animated_png?(upload, file) + tags << "video_with_sound" if media_file.has_audio? + tags << "animated_gif" if media_file.file_ext == :gif && media_file.is_animated? + tags << "animated_png" if media_file.file_ext == :png && media_file.is_animated? tags.join(" ") end diff --git a/app/models/upload.rb b/app/models/upload.rb index e96ab255b..3f6aaa82d 100644 --- a/app/models/upload.rb +++ b/app/models/upload.rb @@ -1,5 +1,3 @@ -require "tmpdir" - class Upload < ApplicationRecord class Error < StandardError; end @@ -19,8 +17,8 @@ class Upload < ApplicationRecord end def validate_integrity(record) - if record.file_ext.in?(["jpg", "gif", "png"]) && UploadService::Utils.corrupt?(record.file.path) - record.errors[:file] << "File is corrupted" + if record.media_file.is_corrupt? + record.errors[:file] << "is corrupted" end end @@ -55,7 +53,7 @@ class Upload < ApplicationRecord end def validate_video_duration(record) - if record.is_video? && record.video.duration > 120 + if record.media_file.is_video? && record.media_file.duration > 120 record.errors[:base] << "video must not be longer than 2 minutes" end end @@ -99,27 +97,11 @@ class Upload < ApplicationRecord end end - module FileMethods + concerning :FileMethods do def media_file @media_file ||= MediaFile.open(file, frame_data: context.to_h.dig("ugoira", "frame_data")) end - def is_image? - %w(jpg gif png).include?(file_ext) - end - - def is_flash? - %w(swf).include?(file_ext) - end - - def is_video? - %w(webm mp4).include?(file_ext) - end - - def is_ugoira? - %w(zip).include?(file_ext) - end - def delete_files # md5 is blank if the upload errored out before downloading the file. if is_completed? || md5.blank? || Upload.where(md5: md5).exists? || Post.where(md5: md5).exists? @@ -136,7 +118,7 @@ class Upload < ApplicationRecord end end - module StatusMethods + concerning :StatusMethods do def is_pending? status == "pending" end @@ -178,7 +160,7 @@ class Upload < ApplicationRecord end end - module SourceMethods + concerning :SourceMethods do def source=(source) source = source.unicode_normalize(:nfc) @@ -196,13 +178,7 @@ class Upload < ApplicationRecord end end - module VideoMethods - def video - @video ||= FFMPEG::Movie.new(file.path) - end - end - - module SearchMethods + concerning :SearchMethods do def search(params) q = super @@ -234,22 +210,12 @@ class Upload < ApplicationRecord end end - include FileMethods - include StatusMethods - include VideoMethods - extend SearchMethods - include SourceMethods - def assign_rating_from_tags if rating = PostQueryBuilder.new(tag_string).find_metatag(:rating) self.rating = rating.downcase.first end end - def presenter - @presenter ||= UploadPresenter.new(self) - end - def upload_as_pending? as_pending.to_s.truthy? end diff --git a/test/unit/upload_service_test.rb b/test/unit/upload_service_test.rb index 7716c873f..71353ac21 100644 --- a/test/unit/upload_service_test.rb +++ b/test/unit/upload_service_test.rb @@ -843,7 +843,6 @@ class UploadServiceTest < ActiveSupport::TestCase upload = upload_from_file(@bad_jpeg_path) assert_match(/corrupt/, upload.status) - assert_equal(true, UploadService::Utils.corrupt?(@bad_jpeg_path)) end should "fail for a corrupted gif" do @@ -851,7 +850,6 @@ class UploadServiceTest < ActiveSupport::TestCase upload = upload_from_file(@bad_gif_path) assert_match(/corrupt/, upload.status) - assert_equal(true, UploadService::Utils.corrupt?(@bad_gif_path)) end # https://schaik.com/pngsuite/pngsuite_xxx_png.html @@ -860,7 +858,6 @@ class UploadServiceTest < ActiveSupport::TestCase upload = upload_from_file(@bad_png_path) assert_match(/corrupt/, upload.status) - assert_equal(true, UploadService::Utils.corrupt?(@bad_png_path)) end end end