From 82fdf45fe5dd74a6dfd962715dd611e03571f896 Mon Sep 17 00:00:00 2001 From: Breno Gazzola Date: Sun, 20 Jun 2021 16:51:51 -0300 Subject: [PATCH] Add support to libvips in the image analyzer --- activestorage/CHANGELOG.md | 4 ++ activestorage/lib/active_storage/analyzer.rb | 2 +- .../active_storage/analyzer/image_analyzer.rb | 32 +--------- .../analyzer/image_analyzer/image_magick.rb | 36 +++++++++++ .../analyzer/image_analyzer/vips.rb | 46 ++++++++++++++ activestorage/lib/active_storage/engine.rb | 4 +- .../image_analyzer/image_magick_test.rb | 60 +++++++++++++++++++ .../test/analyzer/image_analyzer/vips_test.rb | 60 +++++++++++++++++++ .../test/analyzer/image_analyzer_test.rb | 40 ------------- activestorage/test/models/variant_test.rb | 2 +- guides/source/configuring.md | 4 +- 11 files changed, 215 insertions(+), 75 deletions(-) create mode 100644 activestorage/lib/active_storage/analyzer/image_analyzer/image_magick.rb create mode 100644 activestorage/lib/active_storage/analyzer/image_analyzer/vips.rb create mode 100644 activestorage/test/analyzer/image_analyzer/image_magick_test.rb create mode 100644 activestorage/test/analyzer/image_analyzer/vips_test.rb delete mode 100644 activestorage/test/analyzer/image_analyzer_test.rb diff --git a/activestorage/CHANGELOG.md b/activestorage/CHANGELOG.md index b3771fdbe5..2c599d8318 100644 --- a/activestorage/CHANGELOG.md +++ b/activestorage/CHANGELOG.md @@ -1,3 +1,7 @@ +* Use libvips instead of ImageMagick to analyze images when `active_storage.variant_processor = vips` + + *Breno Gazzola* + * Add metadata value for presence of video channel in video blobs The `metadata` attribute of video blobs has a new boolean key named `video` that is set to diff --git a/activestorage/lib/active_storage/analyzer.rb b/activestorage/lib/active_storage/analyzer.rb index 32edc48a15..9e29e7716e 100644 --- a/activestorage/lib/active_storage/analyzer.rb +++ b/activestorage/lib/active_storage/analyzer.rb @@ -2,7 +2,7 @@ module ActiveStorage # This is an abstract base class for analyzers, which extract metadata from blobs. See - # ActiveStorage::Analyzer::ImageAnalyzer for an example of a concrete subclass. + # ActiveStorage::Analyzer::VideoAnalyzer for an example of a concrete subclass. class Analyzer attr_reader :blob diff --git a/activestorage/lib/active_storage/analyzer/image_analyzer.rb b/activestorage/lib/active_storage/analyzer/image_analyzer.rb index bd1bef3076..94deb8fae6 100644 --- a/activestorage/lib/active_storage/analyzer/image_analyzer.rb +++ b/activestorage/lib/active_storage/analyzer/image_analyzer.rb @@ -1,17 +1,14 @@ # frozen_string_literal: true module ActiveStorage - # Extracts width and height in pixels from an image blob. + # This is an abstract base class for image analyzers, which extract width and height from an image blob. # # If the image contains EXIF data indicating its angle is 90 or 270 degrees, its width and height are swapped for convenience. # # Example: # - # ActiveStorage::Analyzer::ImageAnalyzer.new(blob).metadata + # ActiveStorage::Analyzer::ImageAnalyzer::ImageMagick.new(blob).metadata # # => { width: 4104, height: 2736 } - # - # This analyzer relies on the third-party {MiniMagick}[https://github.com/minimagick/minimagick] gem. MiniMagick requires - # the {ImageMagick}[http://www.imagemagick.org] system library. class Analyzer::ImageAnalyzer < Analyzer def self.accept?(blob) blob.image? @@ -26,30 +23,5 @@ module ActiveStorage end end end - - private - def read_image - download_blob_to_tempfile do |file| - require "mini_magick" - image = MiniMagick::Image.new(file.path) - - if image.valid? - yield image - else - logger.info "Skipping image analysis because ImageMagick doesn't support the file" - {} - end - end - rescue LoadError - logger.info "Skipping image analysis because the mini_magick gem isn't installed" - {} - rescue MiniMagick::Error => error - logger.error "Skipping image analysis due to an ImageMagick error: #{error.message}" - {} - end - - def rotated_image?(image) - %w[ RightTop LeftBottom ].include?(image["%[orientation]"]) - end end end diff --git a/activestorage/lib/active_storage/analyzer/image_analyzer/image_magick.rb b/activestorage/lib/active_storage/analyzer/image_analyzer/image_magick.rb new file mode 100644 index 0000000000..5704245e59 --- /dev/null +++ b/activestorage/lib/active_storage/analyzer/image_analyzer/image_magick.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +module ActiveStorage + # This analyzer relies on the third-party {MiniMagick}[https://github.com/minimagick/minimagick] gem. MiniMagick requires + # the {ImageMagick}[http://www.imagemagick.org] system library. + class Analyzer::ImageAnalyzer::ImageMagick < Analyzer::ImageAnalyzer + def self.accept?(blob) + super && ActiveStorage.variant_processor == :mini_magick + end + + private + def read_image + download_blob_to_tempfile do |file| + require "mini_magick" + image = MiniMagick::Image.new(file.path) + + if image.valid? + yield image + else + logger.info "Skipping image analysis because ImageMagick doesn't support the file" + {} + end + end + rescue LoadError + logger.info "Skipping image analysis because the mini_magick gem isn't installed" + {} + rescue MiniMagick::Error => error + logger.error "Skipping image analysis due to an ImageMagick error: #{error.message}" + {} + end + + def rotated_image?(image) + %w[ RightTop LeftBottom TopRight BottomLeft ].include?(image["%[orientation]"]) + end + end +end diff --git a/activestorage/lib/active_storage/analyzer/image_analyzer/vips.rb b/activestorage/lib/active_storage/analyzer/image_analyzer/vips.rb new file mode 100644 index 0000000000..e846e114eb --- /dev/null +++ b/activestorage/lib/active_storage/analyzer/image_analyzer/vips.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +module ActiveStorage + # This analyzer relies on the third-party {ruby-vips}[https://github.com/libvips/ruby-vips] gem. Ruby-vips requires + # the {libvips}[https://libvips.github.io/libvips/] system library. + class Analyzer::ImageAnalyzer::Vips < Analyzer::ImageAnalyzer + def self.accept?(blob) + super && ActiveStorage.variant_processor == :vips + end + + private + def read_image + download_blob_to_tempfile do |file| + require "ruby-vips" + image = ::Vips::Image.new_from_file(file.path, access: :sequential) + + if valid_image?(image) + yield image + else + logger.info "Skipping image analysis because Vips doesn't support the file" + {} + end + end + rescue LoadError + logger.info "Skipping image analysis because the ruby-vips gem isn't installed" + {} + rescue ::Vips::Error => error + logger.error "Skipping image analysis due to an Vips error: #{error.message}" + {} + end + + ROTATIONS = /Right-top|Left-bottom|Top-right|Bottom-left/ + def rotated_image?(image) + ROTATIONS === image.get("exif-ifd0-Orientation") + rescue ::Vips::Error + false + end + + def valid_image?(image) + image.avg + true + rescue ::Vips::Error + false + end + end +end diff --git a/activestorage/lib/active_storage/engine.rb b/activestorage/lib/active_storage/engine.rb index 632a6589e3..b3c6aa21ec 100644 --- a/activestorage/lib/active_storage/engine.rb +++ b/activestorage/lib/active_storage/engine.rb @@ -12,6 +12,8 @@ require "active_storage/previewer/mupdf_previewer" require "active_storage/previewer/video_previewer" require "active_storage/analyzer/image_analyzer" +require "active_storage/analyzer/image_analyzer/image_magick" +require "active_storage/analyzer/image_analyzer/vips" require "active_storage/analyzer/video_analyzer" require "active_storage/analyzer/audio_analyzer" @@ -25,7 +27,7 @@ module ActiveStorage config.active_storage = ActiveSupport::OrderedOptions.new config.active_storage.previewers = [ ActiveStorage::Previewer::PopplerPDFPreviewer, ActiveStorage::Previewer::MuPDFPreviewer, ActiveStorage::Previewer::VideoPreviewer ] - config.active_storage.analyzers = [ ActiveStorage::Analyzer::ImageAnalyzer, ActiveStorage::Analyzer::VideoAnalyzer, ActiveStorage::Analyzer::AudioAnalyzer ] + config.active_storage.analyzers = [ ActiveStorage::Analyzer::ImageAnalyzer::Vips, ActiveStorage::Analyzer::ImageAnalyzer::ImageMagick, ActiveStorage::Analyzer::VideoAnalyzer, ActiveStorage::Analyzer::AudioAnalyzer ] config.active_storage.paths = ActiveSupport::OrderedOptions.new config.active_storage.queues = ActiveSupport::InheritableOptions.new diff --git a/activestorage/test/analyzer/image_analyzer/image_magick_test.rb b/activestorage/test/analyzer/image_analyzer/image_magick_test.rb new file mode 100644 index 0000000000..c067968a9e --- /dev/null +++ b/activestorage/test/analyzer/image_analyzer/image_magick_test.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +require "test_helper" +require "database/setup" + +require "active_storage/analyzer/image_analyzer" + +class ActiveStorage::Analyzer::ImageAnalyzer::ImageMagickTest < ActiveSupport::TestCase + test "analyzing a JPEG image" do + analyze_with_image_magick do + blob = create_file_blob(filename: "racecar.jpg", content_type: "image/jpeg") + metadata = extract_metadata_from(blob) + + assert_equal 4104, metadata[:width] + assert_equal 2736, metadata[:height] + end + end + + test "analyzing a rotated JPEG image" do + analyze_with_image_magick do + blob = create_file_blob(filename: "racecar_rotated.jpg", content_type: "image/jpeg") + metadata = extract_metadata_from(blob) + + assert_equal 2736, metadata[:width] + assert_equal 4104, metadata[:height] + end + end + + test "analyzing an SVG image without an XML declaration" do + analyze_with_image_magick do + blob = create_file_blob(filename: "icon.svg", content_type: "image/svg+xml") + metadata = extract_metadata_from(blob) + + assert_equal 792, metadata[:width] + assert_equal 584, metadata[:height] + end + end + + test "analyzing an unsupported image type" do + analyze_with_image_magick do + blob = create_blob(data: "bad", filename: "bad_file.bad", content_type: "image/bad_type") + metadata = extract_metadata_from(blob) + + assert_nil metadata[:width] + assert_nil metadata[:height] + end + end + + private + def analyze_with_image_magick + previous_processor, ActiveStorage.variant_processor = ActiveStorage.variant_processor, :mini_magick + require "mini_magick" + + yield + rescue LoadError + ENV["CI"] ? raise : skip("Variant processor image_magick is not installed") + ensure + ActiveStorage.variant_processor = previous_processor + end +end diff --git a/activestorage/test/analyzer/image_analyzer/vips_test.rb b/activestorage/test/analyzer/image_analyzer/vips_test.rb new file mode 100644 index 0000000000..a7c3b2c4d8 --- /dev/null +++ b/activestorage/test/analyzer/image_analyzer/vips_test.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +require "test_helper" +require "database/setup" + +require "active_storage/analyzer/image_analyzer" + +class ActiveStorage::Analyzer::ImageAnalyzer::VipsTest < ActiveSupport::TestCase + test "analyzing a JPEG image" do + analyze_with_vips do + blob = create_file_blob(filename: "racecar.jpg", content_type: "image/jpeg") + metadata = extract_metadata_from(blob) + + assert_equal 4104, metadata[:width] + assert_equal 2736, metadata[:height] + end + end + + test "analyzing a rotated JPEG image" do + analyze_with_vips do + blob = create_file_blob(filename: "racecar_rotated.jpg", content_type: "image/jpeg") + metadata = extract_metadata_from(blob) + + assert_equal 2736, metadata[:width] + assert_equal 4104, metadata[:height] + end + end + + test "analyzing an SVG image without an XML declaration" do + analyze_with_vips do + blob = create_file_blob(filename: "icon.svg", content_type: "image/svg+xml") + metadata = extract_metadata_from(blob) + + assert_equal 792, metadata[:width] + assert_equal 584, metadata[:height] + end + end + + test "analyzing an unsupported image type" do + analyze_with_vips do + blob = create_blob(data: "bad", filename: "bad_file.bad", content_type: "image/bad_type") + metadata = extract_metadata_from(blob) + + assert_nil metadata[:width] + assert_nil metadata[:height] + end + end + + private + def analyze_with_vips + previous_processor, ActiveStorage.variant_processor = ActiveStorage.variant_processor, :vips + require "ruby-vips" + + yield + rescue LoadError + ENV["CI"] ? raise : skip("Variant processor vips is not installed") + ensure + ActiveStorage.variant_processor = previous_processor + end +end diff --git a/activestorage/test/analyzer/image_analyzer_test.rb b/activestorage/test/analyzer/image_analyzer_test.rb deleted file mode 100644 index 576674ebb3..0000000000 --- a/activestorage/test/analyzer/image_analyzer_test.rb +++ /dev/null @@ -1,40 +0,0 @@ -# frozen_string_literal: true - -require "test_helper" -require "database/setup" - -require "active_storage/analyzer/image_analyzer" - -class ActiveStorage::Analyzer::ImageAnalyzerTest < ActiveSupport::TestCase - test "analyzing a JPEG image" do - blob = create_file_blob(filename: "racecar.jpg", content_type: "image/jpeg") - metadata = extract_metadata_from(blob) - - assert_equal 4104, metadata[:width] - assert_equal 2736, metadata[:height] - end - - test "analyzing a rotated JPEG image" do - blob = create_file_blob(filename: "racecar_rotated.jpg", content_type: "image/jpeg") - metadata = extract_metadata_from(blob) - - assert_equal 2736, metadata[:width] - assert_equal 4104, metadata[:height] - end - - test "analyzing an SVG image without an XML declaration" do - blob = create_file_blob(filename: "icon.svg", content_type: "image/svg+xml") - metadata = extract_metadata_from(blob) - - assert_equal 792, metadata[:width] - assert_equal 584, metadata[:height] - end - - test "analyzing an unsupported image type" do - blob = create_blob(data: "bad", filename: "bad_file.bad", content_type: "image/bad_type") - metadata = extract_metadata_from(blob) - - assert_nil metadata[:width] - assert_nil metadata[:height] - end -end diff --git a/activestorage/test/models/variant_test.rb b/activestorage/test/models/variant_test.rb index 7e8f792fa1..0b16367218 100644 --- a/activestorage/test/models/variant_test.rb +++ b/activestorage/test/models/variant_test.rb @@ -206,7 +206,7 @@ class ActiveStorage::VariantTest < ActiveSupport::TestCase previous_processor, ActiveStorage.variant_processor = ActiveStorage.variant_processor, processor yield rescue LoadError - skip "Variant processor #{processor.inspect} is not installed" + ENV["CI"] ? raise : skip("Variant processor #{processor.inspect} is not installed") ensure ActiveStorage.variant_processor = previous_processor end diff --git a/guides/source/configuring.md b/guides/source/configuring.md index 7d6edcc010..9d36cee7c8 100644 --- a/guides/source/configuring.md +++ b/guides/source/configuring.md @@ -985,9 +985,9 @@ You can find more detailed configuration options in the `config.active_storage` provides the following configuration options: -* `config.active_storage.variant_processor` accepts a symbol `:mini_magick` or `:vips`, specifying whether variant transformations will be performed with MiniMagick or ruby-vips. The default is `:mini_magick`. +* `config.active_storage.variant_processor` accepts a symbol `:mini_magick` or `:vips`, specifying whether variant transformations and blob analysis will be performed with MiniMagick or ruby-vips. The default is `:mini_magick`. -* `config.active_storage.analyzers` accepts an array of classes indicating the analyzers available for Active Storage blobs. The default is `[ActiveStorage::Analyzer::ImageAnalyzer, ActiveStorage::Analyzer::VideoAnalyzer, ActiveStorage::Analyzer::AudioAnalyzer]`. The image analyzer can extract width and height of an image blob; the video analyzer can extract width, height, duration, angle, aspect ratio and presence/absence of video/audio channels of a video blob; the audio analyzer can extract duration and bit rate of an audio blob. +* `config.active_storage.analyzers` accepts an array of classes indicating the analyzers available for Active Storage blobs. The default is `[ActiveStorage::Analyzer::ImageAnalyzer::Vips, ActiveStorage::Analyzer::ImageAnalyzer::ImageMagick, ActiveStorage::Analyzer::VideoAnalyzer, ActiveStorage::Analyzer::AudioAnalyzer]`. The image analyzers can extract width and height of an image blob; the video analyzer can extract width, height, duration, angle, aspect ratio and presence/absence of video/audio channels of a video blob; the audio analyzer can extract duration and bit rate of an audio blob. * `config.active_storage.previewers` accepts an array of classes indicating the image previewers available in Active Storage blobs. The default is `[ActiveStorage::Previewer::PopplerPDFPreviewer, ActiveStorage::Previewer::MuPDFPreviewer, ActiveStorage::Previewer::VideoPreviewer]`. `PopplerPDFPreviewer` and `MuPDFPreviewer` can generate a thumbnail from the first page of a PDF blob; `VideoPreviewer` from the relevant frame of a video blob.