ImageMagickCommandFactory: only extract first layer for PDFs

Should fix black images; see samvera/hydra-derivatives#110 and samvera/hydra-derivatives#120

Also add more class attributes to allow setting the options.

.rubocop_todo.yml

@@ -1,17 +1,16 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config`
-# on 2017-04-11 15:07:00 -0500 using RuboCop version 0.47.1.
+# on 2017-12-11 14:20:46 -0800 using RuboCop version 0.47.1.
 # The point is for the user to remove these configuration records
 # one by one as the offenses are removed from the code base.
 # Note that changes in the inspected code, or installation of new
 # versions of RuboCop, may require this file to be generated again.
 
-# Offense count: 9
+# Offense count: 2
 # Configuration parameters: AllowSafeAssignment.
 Lint/AssignmentInCondition:
   Exclude:
     - 'app/models/riiif/file.rb'
-    - 'app/services/riiif/option_decoder.rb'
     - 'app/resolvers/riiif/http_file_resolver.rb'
 
 # Offense count: 1
@@ -24,29 +23,25 @@ Lint/UselessAssignment:
   Exclude:
     - 'app/models/riiif/file.rb'
 
-# Offense count: 5
+# Offense count: 12
 Metrics/AbcSize:
-  Max: 29
+  Max: 27
 
 # Offense count: 1
 Metrics/CyclomaticComplexity:
-  Max: 8
+  Max: 7
 
-# Offense count: 96
+# Offense count: 122
 # Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns.
 # URISchemes: http, https
 Metrics/LineLength:
-  Max: 117
+  Max: 112
 
-# Offense count: 6
+# Offense count: 9
 # Configuration parameters: CountComments.
 Metrics/MethodLength:
   Max: 18
 
-Metrics/ParameterLists:
-  Exclude:
-    - 'app/services/riiif/imagemagick_command_factory.rb'
-
 # Offense count: 1
 # Configuration parameters: EnforcedStyle, SupportedStyles.
 # SupportedStyles: nested, compact
@@ -66,8 +61,15 @@ Style/Documentation:
     - 'app/resolvers/riiif/akubra_system_file_resolver.rb'
     - 'app/resolvers/riiif/file_system_file_resolver.rb'
     - 'app/resolvers/riiif/http_file_resolver.rb'
+    - 'app/services/riiif/image_magick_info_extractor.rb'
     - 'app/services/riiif/nil_authorization_service.rb'
     - 'lib/riiif.rb'
     - 'lib/riiif/engine.rb'
     - 'lib/riiif/rails/routes.rb'
     - 'lib/riiif/routes.rb'
+
+# Offense count: 3
+# Cop supports --auto-correct.
+Style/RedundantSelf:
+  Exclude:
+    - 'app/services/riiif/imagemagick_command_factory.rb'

app/controllers/riiif/images_controller.rb

@@ -34,7 +34,7 @@ def info
       image = model.new(image_id)
       if authorization_service.can?(:info, image)
         image_info = image.info
-        return render json: { error: 'no info' }, status: :not_found unless image_info.valid?
+        return render json: { error: 'no info' }, status: :not_found if image_info.to_h.empty?
         headers['Access-Control-Allow-Origin'] = '*'
         # Set a Cache-Control header
         expires_in cache_expires, public: public_cache?

app/models/riiif/image.rb

@@ -51,7 +51,10 @@ def render(args)
     def info
       @info ||= begin
                   result = info_service.call(id, file)
-                  ImageInformation.new(width: result[:width], height: result[:height])
+                  ImageInformation.new(
+                    width: result[:width],
+                    height: result[:height]
+                  ).to_h.merge(format: result[:format])
                 end
     end
 

app/services/riiif/crop.rb

@@ -5,7 +5,7 @@ class Crop
     # @param image_info []
     def initialize(region, image_info)
       @region = region
-      @image_info = image_info
+      @image_info = image_info.to_h
     end
 
     attr_reader :image_info, :region
@@ -48,36 +48,36 @@ def to_kakadu
     private
 
       def imagemagick_percent
-        offset_x = (image_info.width * percentage_to_fraction(region.x_pct)).round
-        offset_y = (image_info.height * percentage_to_fraction(region.y_pct)).round
+        offset_x = (image_info[:width] * percentage_to_fraction(region.x_pct)).round
+        offset_y = (image_info[:height] * percentage_to_fraction(region.y_pct)).round
         "#{region.width_pct}%x#{region.height_pct}+#{offset_x}+#{offset_y}"
       end
 
       def kakadu_percent
-        offset_x = (image_info.width * percentage_to_fraction(region.x_pct)).round
-        offset_y = (image_info.height * percentage_to_fraction(region.y_pct)).round
+        offset_x = (image_info[:width] * percentage_to_fraction(region.x_pct)).round
+        offset_y = (image_info[:height] * percentage_to_fraction(region.y_pct)).round
         "\{#{decimal_offset_y(offset_y)},#{decimal_offset_x(offset_x)}\}," \
         "\{#{percentage_to_fraction(region.height_pct)},#{percentage_to_fraction(region.width_pct)}\}"
       end
 
       def kakadu_square
-        min, max = [image_info.width, image_info.height].minmax
+        min, max = [image_info[:width], image_info[:height]].minmax
         offset = (max - min) / 2
-        if image_info.height >= image_info.width
+        if image_info[:height] >= image_info[:width]
           # Portrait
           "\{#{decimal_height(offset)},0\}," \
-          "\{#{decimal_height(image_info.height)},#{decimal_width(image_info.height)}\}"
+          "\{#{decimal_height(image_info[:height])},#{decimal_width(image_info[:height])}\}"
         else
           # Landscape
           "\{0,#{decimal_width(offset)}\}," \
-          "\{#{decimal_height(image_info.width)},#{decimal_width(image_info.width)}\}"
+          "\{#{decimal_height(image_info[:width])},#{decimal_width(image_info[:width])}\}"
         end
       end
 
       def imagemagick_square
-        min, max = [image_info.width, image_info.height].minmax
+        min, max = [image_info[:width], image_info[:height]].minmax
         offset = (max - min) / 2
-        if image_info.height >= image_info.width
+        if image_info[:height] >= image_info[:width]
           "#{min}x#{min}+0+#{offset}"
         else
           "#{min}x#{min}+#{offset}+0"
@@ -86,34 +86,34 @@ def imagemagick_square
 
       # @return [Integer] the height in pixels
       def height
-        image_info.height
+        image_info[:height]
       end
 
       # @return [Integer] the width in pixels
       def width
-        image_info.width
+        image_info[:width]
       end
 
       # @return [Float] the fractional height with respect to the original size
       def decimal_height(n = height)
-        n.to_f / image_info.height
+        n.to_f / image_info[:height]
       end
 
       # @return [Float] the fractional width with respect to the original size
       def decimal_width(n = width)
-        n.to_f / image_info.width
+        n.to_f / image_info[:width]
       end
 
       def decimal_offset_x(offset_x)
-        offset_x.to_f / image_info.width
+        offset_x.to_f / image_info[:width]
       end
 
       def decimal_offset_y(offset_y)
-        offset_y.to_f / image_info.height
+        offset_y.to_f / image_info[:height]
       end
 
       def maintain_aspect_ratio?
-        (height / width) == (image_info.height / image_info.width)
+        (height / width) == (image_info[:height] / image_info[:width])
       end
 
       def percentage_to_fraction(n)

app/services/riiif/image_magick_info_extractor.rb

@@ -1,5 +1,4 @@
 module Riiif
-  # Get height and width information using imagemagick to interrogate the file
   class ImageMagickInfoExtractor
     # perhaps you want to use GraphicsMagick instead, set to "gm identify"
     class_attribute :external_command
@@ -10,8 +9,16 @@ def initialize(path)
     end
 
     def extract
-      height, width = Riiif::CommandRunner.execute("#{external_command} -format %hx%w #{@path}[0]").split('x')
-      { height: Integer(height), width: Integer(width) }
+      height, width, format = Riiif::CommandRunner.execute(
+        "#{external_command} -format '%h %w %m' #{@path}[0]"
+      ).split(' ')
+
+      {
+        height: Integer(height),
+        width: Integer(width),
+        aspect: Rational(width, height),
+        format: format
+      }
     end
   end
 end

app/services/riiif/imagemagick_command_factory.rb

@@ -3,31 +3,46 @@
 module Riiif
   # Builds a command to run a transformation using Imagemagick
   class ImagemagickCommandFactory
+    class_attribute :external_command,
+                    :compression,
+                    :sampling_factor,
+                    :strip_metadata,
+                    :layer
+
     # perhaps you want to use GraphicsMagick instead, set to "gm convert"
-    class_attribute :external_command
     self.external_command = 'convert'
+    # the compression level to use (set 0 for no compression)
+    self.compression = 85
+    # the chroma sample factor (set 0 for no compression)
+    self.sampling_factor = '4:2:0'
+    # do we want to strip EXIF tags?
+    self.strip_metadata = true
 
     # A helper method to instantiate and invoke build
     # @param [String] path the location of the file
     # @param info [ImageInformation] information about the source
     # @param [Transformation] transformation
-    # @param [Integer] compression (85) the compression level to use (set 0 for no compression)
-    # @param [String] sampling_factor ("4:2:0") the chroma sample factor (set 0 for no compression)
-    # @param [Boolean] strip_metadata (true) do we want to strip EXIF tags?
-    def initialize(path, info, transformation, compression: 85, sampling_factor: '4:2:0', strip_metadata: true)
+    def initialize(path, info, transformation)
       @path = path
       @info = info
       @transformation = transformation
-      @compression = compression
-      @sampling_factor = sampling_factor
-      @strip_metadata = strip_metadata
     end
-
-    attr_reader :path, :info, :transformation, :compression, :sampling_factor, :strip_metadata
+    attr_reader :path, :info, :transformation
 
     # @return [String] a command for running imagemagick to produce the requested output
     def command
-      [external_command, crop, size, rotation, colorspace, quality, sampling, metadata, input, output].join
+      [
+        external_command,
+        crop,
+        size,
+        rotation,
+        colorspace,
+        quality,
+        sampling,
+        metadata,
+        input,
+        output
+      ].join
     end
 
     def reduction_factor
@@ -37,15 +52,19 @@ def reduction_factor
     private
 
       def use_compression?
-        compression > 0 && jpeg?
+        self.compression > 0 && jpeg?
       end
 
       def jpeg?
         transformation.format == 'jpg'.freeze
       end
 
+      def layer_spec
+        '[0]' if info[:format] =~ /pdf/i
+      end
+
       def input
-        " #{path}"
+        " #{path}#{layer_spec}"
       end
 
       # pipe the output to STDOUT
@@ -69,15 +88,15 @@ def rotation
       end
 
       def quality
-        " -quality #{compression}" if use_compression?
+        " -quality #{self.compression}" if use_compression?
       end
 
       def metadata
         ' -strip' if strip_metadata
       end
 
       def sampling
-        " -sampling-factor #{sampling_factor}" if jpeg?
+        " -sampling-factor #{self.sampling_factor}" if jpeg?
       end
 
       def colorspace

app/services/riiif/resize.rb

@@ -5,7 +5,7 @@ class Resize
     # @param image_info []
     def initialize(size, image_info)
       @size = size
-      @image_info = image_info
+      @image_info = image_info.to_h
     end
 
     attr_reader :image_info, :size
@@ -36,11 +36,11 @@ def height
       when IIIF::Image::Size::Absolute
         size.height
       when IIIF::Image::Size::Percent
-        image_info.height * Integer(size.percentage).to_f / 100
+        image_info[:height] * Integer(size.percentage).to_f / 100
       when IIIF::Image::Size::Width
-        size.height_for_aspect_ratio(image_info.aspect)
+        size.height_for_aspect_ratio(image_info[:aspect])
       else
-        image_info.height
+        image_info[:height]
       end
     end
 
@@ -50,11 +50,11 @@ def width
       when IIIF::Image::Size::Absolute
         size.width
       when IIIF::Image::Size::Percent
-        image_info.width * Integer(size.percentage).to_f / 100
+        image_info[:width] * Integer(size.percentage).to_f / 100
       when IIIF::Image::Size::Height
-        size.width_for_aspect_ratio(image_info.aspect)
+        size.width_for_aspect_ratio(image_info[:aspect])
       else
-        image_info.width
+        image_info[:width]
       end
     end
 
@@ -65,7 +65,7 @@ def reduce?
         false
       when IIIF::Image::Size::Absolute
         aspect_ratio = width.to_f / height
-        in_delta?(image_info.aspect, aspect_ratio, 0.001)
+        in_delta?(image_info[:aspect], aspect_ratio, 0.001)
       else
         true
       end
@@ -89,8 +89,8 @@ def reduce(factor)
     # @return [Integer] the reduction factor for this operation
     def reduction_factor(max_factor = 5)
       return nil unless reduce?
-      scale = [width.to_f / image_info.width,
-               height.to_f / image_info.height].min
+      scale = [width.to_f / image_info[:width],
+               height.to_f / image_info[:height]].min
       factor = 0
       raise "I don't know how to scale to #{scale}" if scale > 1
       next_pct = 0.5

app/transformers/riiif/kakadu_transformer.rb

@@ -27,7 +27,7 @@ def post_process(intermediate_file, reduction_factor)
                        else
                          without_crop
                        end
-      Riiif::File.new(intermediate_file).extract(transformation, image_info)
+      Riiif::File.new(intermediate_file).extract(transformation, image_info.to_h)
     end
 
     private
@@ -47,7 +47,7 @@ def without_crop
       # @param [Integer] factor the scale for the new transformation
       # @return [Transformation] a new transformation, scaled by factor
       def reduce(transformation, factor)
-        resize = Resize.new(transformation.size, image_info)
+        resize = Resize.new(transformation.size, image_info.to_h)
         IIIF::Image::Transformation.new(region: transformation.region.dup,
                                         size: resize.reduce(factor),
                                         quality: transformation.quality,