diff --git a/app/jobs/process_upload_media_asset_job.rb b/app/jobs/process_upload_media_asset_job.rb index db5f2f1ed..3f0d1c173 100644 --- a/app/jobs/process_upload_media_asset_job.rb +++ b/app/jobs/process_upload_media_asset_job.rb @@ -5,5 +5,9 @@ class ProcessUploadMediaAssetJob < ApplicationJob def perform(upload_media_asset) upload_media_asset.process_upload! + rescue Exception => e + # This should never happen. It will only happen if `process_upload!` raises an unexpected exception inside its own exception handler. + upload_media_asset.update!(status: :failed, error: e.message) + raise end end diff --git a/app/logical/iqdb_client.rb b/app/logical/iqdb_client.rb index c0e0c7933..e17adc780 100644 --- a/app/logical/iqdb_client.rb +++ b/app/logical/iqdb_client.rb @@ -98,7 +98,7 @@ class IqdbClient # @param file [File] the image to search def query_file(file, limit: 20) media_file = MediaFile.open(file) - preview = media_file.preview(Danbooru.config.small_image_width, Danbooru.config.small_image_width) + preview = media_file.preview!(Danbooru.config.small_image_width, Danbooru.config.small_image_width) file = HTTP::FormData::File.new(preview) request(:post, "query", form: { file: file }, params: { limit: limit }) end diff --git a/app/logical/media_file.rb b/app/logical/media_file.rb index 3206e1b33..ebb145378 100644 --- a/app/logical/media_file.rb +++ b/app/logical/media_file.rb @@ -200,17 +200,23 @@ class MediaFile false end - # Return a preview of the file, sized to fit within the given width and - # height (preserving the aspect ratio). + # Return a preview of the file, sized to fit within the given width and height (preserving the aspect ratio). # # @param width [Integer] the max width of the image # @param height [Integer] the max height of the image # @param options [Hash] extra options when generating the preview # @return [MediaFile, nil] a preview file, or nil if we can't generate a preview for this file type (e.g. Flash files) def preview(width, height, **options) + preview!(width, height, **options) + rescue nil end + # Like `preview`, but raises an exception if generating the preview fails for any reason. + def preview!(width, height, **options) + raise NotImplementedError + end + # Return a set of AI-inferred tags for this image. Performs an API call to # the Autotagger service. The Autotagger service must be running, otherwise # it will return an empty list of tags. diff --git a/app/logical/media_file/image.rb b/app/logical/media_file/image.rb index 0b83c8473..8da9c066e 100644 --- a/app/logical/media_file/image.rb +++ b/app/logical/media_file/image.rb @@ -5,6 +5,8 @@ # @see https://github.com/libvips/ruby-vips # @see https://libvips.github.io/libvips/API/current class MediaFile::Image < MediaFile + delegate :thumbnail_image, to: :image + def dimensions image.size rescue Vips::Error @@ -61,21 +63,21 @@ class MediaFile::Image < MediaFile image.interpretation end - def resize(max_width, max_height, format: :jpeg, quality: 85, **options) + def resize!(max_width, max_height, format: :jpeg, quality: 85, **options) # @see https://www.libvips.org/API/current/Using-vipsthumbnail.md.html # @see https://www.libvips.org/API/current/libvips-resample.html#vips-thumbnail if colorspace.in?(%i[srgb rgb16]) - resized_image = preview_frame.image.thumbnail_image(max_width, height: max_height, import_profile: "srgb", export_profile: "srgb", **options) + resized_image = thumbnail_image(max_width, height: max_height, import_profile: "srgb", export_profile: "srgb", **options) elsif colorspace == :cmyk # Leave CMYK as CMYK for better color accuracy than sRGB. - resized_image = preview_frame.image.thumbnail_image(max_width, height: max_height, import_profile: "cmyk", export_profile: "cmyk", intent: :relative, **options) + resized_image = thumbnail_image(max_width, height: max_height, import_profile: "cmyk", export_profile: "cmyk", intent: :relative, **options) elsif colorspace.in?(%i[b-w grey16]) && has_embedded_profile? # Convert greyscale to sRGB so that the color profile is properly applied before we strip it. - resized_image = preview_frame.image.thumbnail_image(max_width, height: max_height, export_profile: "srgb", **options) + resized_image = thumbnail_image(max_width, height: max_height, export_profile: "srgb", **options) elsif colorspace.in?(%i[b-w grey16]) # Otherwise, leave greyscale without a profile as greyscale because # converting it to sRGB would change it from 1 channel to 3 channels. - resized_image = preview_frame.image.thumbnail_image(max_width, height: max_height, **options) + resized_image = thumbnail_image(max_width, height: max_height, **options) else raise NotImplementedError end @@ -102,9 +104,9 @@ class MediaFile::Image < MediaFile MediaFile::Image.new(output_file) end - def preview(max_width, max_height, **options) + def preview!(max_width, max_height, **options) w, h = MediaFile.scale_dimensions(width, height, max_width, max_height) - resize(w, h, size: :force, **options) + preview_frame.resize!(w, h, size: :force, **options) end def preview_frame diff --git a/app/logical/media_file/ugoira.rb b/app/logical/media_file/ugoira.rb index 2f9015083..3c7f1e925 100644 --- a/app/logical/media_file/ugoira.rb +++ b/app/logical/media_file/ugoira.rb @@ -30,8 +30,8 @@ class MediaFile::Ugoira < MediaFile preview_frame.dimensions end - def preview(width, height, **options) - preview_frame.preview(width, height, **options) + def preview!(width, height, **options) + preview_frame.preview!(width, height, **options) end def duration diff --git a/app/logical/media_file/video.rb b/app/logical/media_file/video.rb index 197f01b3d..badab7be6 100644 --- a/app/logical/media_file/video.rb +++ b/app/logical/media_file/video.rb @@ -11,8 +11,8 @@ class MediaFile::Video < MediaFile [video.width, video.height] end - def preview(max_width, max_height, **options) - preview_frame.preview(max_width, max_height, **options) + def preview!(max_width, max_height, **options) + preview_frame.preview!(max_width, max_height, **options) end def is_supported? @@ -26,6 +26,11 @@ class MediaFile::Video < MediaFile end end + # True if decoding the video fails. + def is_corrupt? + video.playback_info.blank? + end + private def video diff --git a/app/models/media_asset.rb b/app/models/media_asset.rb index cd28760e3..84b73184a 100644 --- a/app/models/media_asset.rb +++ b/app/models/media_asset.rb @@ -86,17 +86,17 @@ class MediaAsset < ApplicationRecord def convert_file(media_file) case type in :preview - media_file.preview(width, height, format: :jpeg, quality: 85) + media_file.preview!(width, height, format: :jpeg, quality: 85) in :"180x180" - media_file.preview(width, height, format: :jpeg, quality: 85) + media_file.preview!(width, height, format: :jpeg, quality: 85) in :"360x360" - media_file.preview(width, height, format: :jpeg, quality: 85) + media_file.preview!(width, height, format: :jpeg, quality: 85) in :"720x720" - media_file.preview(width, height, format: :webp, quality: 75) + media_file.preview!(width, height, format: :webp, quality: 75) in :sample if media_asset.is_ugoira? media_file.convert in :sample | :full if media_asset.is_static_image? - media_file.preview(width, height, format: :jpeg, quality: 85) + media_file.preview!(width, height, format: :jpeg, quality: 85) in :original media_file end @@ -235,7 +235,7 @@ class MediaAsset < ApplicationRecord # XXX should do this in parallel with thumbnail generation. # XXX shouldn't generate thumbnail twice (very slow for ugoira) - media_asset.update!(ai_tags: media_file.preview(360, 360).ai_tags) + media_asset.update!(ai_tags: media_file.preview!(360, 360).ai_tags) media_asset.update!(media_metadata: MediaMetadata.new(file: media_file)) media_asset.distribute_files!(media_file) diff --git a/app/models/upload_media_asset.rb b/app/models/upload_media_asset.rb index 9c3d9809d..a2e6dfb91 100644 --- a/app/models/upload_media_asset.rb +++ b/app/models/upload_media_asset.rb @@ -86,9 +86,10 @@ class UploadMediaAsset < ApplicationRecord Source::Extractor.find(source_url, page_url) end + # Calls `process_upload!` def async_process_upload! if file.present? - process_upload! + ProcessUploadMediaAssetJob.perform_now(self) else ProcessUploadMediaAssetJob.perform_later(self) end diff --git a/config/docker/build-base-image.sh b/config/docker/build-base-image.sh index a14a09fbd..9d83ecf13 100755 --- a/config/docker/build-base-image.sh +++ b/config/docker/build-base-image.sh @@ -14,7 +14,7 @@ COMMON_BUILD_DEPS=" curl ca-certificates build-essential pkg-config git " RUBY_BUILD_DEPS="libssl-dev zlib1g-dev libgmp-dev" -FFMPEG_BUILD_DEPS="libvpx-dev nasm" +FFMPEG_BUILD_DEPS="libvpx-dev libdav1d-dev nasm" MOZJPEG_BUILD_DEPS="cmake nasm libpng-dev zlib1g-dev" VIPS_BUILD_DEPS=" libfftw3-dev libwebp-dev liborc-dev liblcms2-dev libpng-dev @@ -24,7 +24,7 @@ EXIFTOOL_RUNTIME_DEPS="perl perl-modules libarchive-zip-perl" DANBOORU_RUNTIME_DEPS=" ca-certificates mkvtoolnix rclone libpq5 openssl libgmpxx4ldbl zlib1g libfftw3-3 libwebp7 libwebpmux3 libwebpdemux2 liborc-0.4.0 liblcms2-2 - libpng16-16 libexpat1 libglib2.0 libgif7 libexif12 libheif1 libvpx7 + libpng16-16 libexpat1 libglib2.0 libgif7 libexif12 libheif1 libvpx7 libdav1d6 libseccomp2 libseccomp-dev libjemalloc2 " COMMON_RUNTIME_DEPS=" @@ -77,7 +77,7 @@ install_ffmpeg() { curl -L "$FFMPEG_URL" | tar -C /usr/local/src -xzvf - cd /usr/local/src/FFmpeg-n${FFMPEG_VERSION} - ./configure --disable-ffplay --disable-network --disable-doc --enable-libvpx + ./configure --disable-ffplay --disable-network --disable-doc --enable-libvpx --enable-libdav1d make -j "$(nproc)" cp ffmpeg ffprobe /usr/local/bin diff --git a/db/migrate/20221027000931_remove_error_index_on_upload_media_assets.rb b/db/migrate/20221027000931_remove_error_index_on_upload_media_assets.rb new file mode 100644 index 000000000..2fefd8233 --- /dev/null +++ b/db/migrate/20221027000931_remove_error_index_on_upload_media_assets.rb @@ -0,0 +1,5 @@ +class RemoveErrorIndexOnUploadMediaAssets < ActiveRecord::Migration[7.0] + def change + remove_index :upload_media_assets, :error, where: "error IS NOT NULL" + end +end diff --git a/db/structure.sql b/db/structure.sql index 0f8aa3851..634de42cc 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -5500,13 +5500,6 @@ CREATE INDEX index_upgrade_codes_on_status ON public.upgrade_codes USING btree ( CREATE INDEX index_upgrade_codes_on_user_upgrade_id ON public.upgrade_codes USING btree (user_upgrade_id) WHERE (user_upgrade_id IS NOT NULL); --- --- Name: index_upload_media_assets_on_error; Type: INDEX; Schema: public; Owner: - --- - -CREATE INDEX index_upload_media_assets_on_error ON public.upload_media_assets USING btree (error) WHERE (error IS NOT NULL); - - -- -- Name: index_upload_media_assets_on_media_asset_id; Type: INDEX; Schema: public; Owner: - -- @@ -6907,6 +6900,7 @@ INSERT INTO "schema_migrations" (version) VALUES ('20221003080342'), ('20221010035855'), ('20221026084655'), -('20221026084656'); +('20221026084656'), +('20221027000931'); diff --git a/test/files/mp4/test-corrupt.mp4 b/test/files/mp4/test-corrupt.mp4 new file mode 100644 index 000000000..3062e18c1 Binary files /dev/null and b/test/files/mp4/test-corrupt.mp4 differ diff --git a/test/functional/uploads_controller_test.rb b/test/functional/uploads_controller_test.rb index fa1d2d5e8..82aa5efd5 100644 --- a/test/functional/uploads_controller_test.rb +++ b/test/functional/uploads_controller_test.rb @@ -179,7 +179,7 @@ class UploadsControllerTest < ActionDispatch::IntegrationTest end end - context "for a corrupted image" do + context "for a corrupted file" do should "fail for a corrupted jpeg" do create_upload!("test/files/test-corrupt.jpg", user: @user) assert_match("corrupt", Upload.last.error) @@ -195,6 +195,11 @@ class UploadsControllerTest < ActionDispatch::IntegrationTest create_upload!("test/files/test-corrupt.png", user: @user) assert_match("corrupt", Upload.last.error) end + + should "fail for a corrupted mp4" do + create_upload!("test/files/mp4/test-corrupt.mp4", user: @user) + assert_match("corrupt", Upload.last.error) + end end context "for an unsupported WebP file" do @@ -330,6 +335,7 @@ class UploadsControllerTest < ActionDispatch::IntegrationTest should_upload_successfully("test/files/test-static-32x32.gif") should_upload_successfully("test/files/test-animated-86x52.gif") should_upload_successfully("test/files/test-300x300.mp4") + should_upload_successfully("test/files/test-audio.mp4") should_upload_successfully("test/files/webm/test-512x512.webm") should_upload_successfully("test/files/test-audio.m4v") # should_upload_successfully("test/files/compressed.swf") diff --git a/test/unit/media_file_test.rb b/test/unit/media_file_test.rb index f68c2b3de..2e2e2c604 100644 --- a/test/unit/media_file_test.rb +++ b/test/unit/media_file_test.rb @@ -202,15 +202,23 @@ class MediaFileTest < ActiveSupport::TestCase should "determine the duration of the video" do file = MediaFile.open("test/files/test-audio.mp4") + assert_equal(false, file.is_corrupt?) assert_equal(1.002667, file.duration) assert_equal(10/1.002667, file.frame_rate) assert_equal(10, file.frame_count) file = MediaFile.open("test/files/test-300x300.mp4") + assert_equal(false, file.is_corrupt?) assert_equal(5.7, file.duration) assert_equal(1.75, file.frame_rate.round(2)) assert_equal(10, file.frame_count) end + + should "detect corrupt videos" do + file = MediaFile.open("test/files/mp4/test-corrupt.mp4") + + assert_equal(true, file.is_corrupt?) + end end context "for a webm file" do