Merge pull request #146 from robotmay/auto-rotate

Improvements to image processing.

app/models/metadata.rb

@@ -83,11 +83,6 @@ def set_defaults
     self.creator ||= {}
     self.image ||= {}
   end
-
-  after_commit :enqueue_extraction, on: :create
-  def enqueue_extraction
-    MetadataWorker.perform_async(id)
-  end
 
   def extract_from_photograph
     begin
@@ -106,6 +101,9 @@ def extract_from_photograph
 
       convert_lat_lng
       set_format
+
+      self.processing = false
+      return true
     rescue ArgumentError => ex
       # Prevent UTF-8 bug from stopping photo upload
       self.camera ||= {}
@@ -178,6 +176,23 @@ def square?
     format == 'square'
   end
 
+  def rotate?
+    camera.present? && camera['camera_orientation'].present?
+  end
+
+  def rotate_by
+    if rotate?
+      match = camera['camera_orientation'].match(/Rotate (\d+) (CW|CCW)/)
+
+      if match
+        degrees = match[1]
+        direction = match[2]
+
+        direction == "CCW" ? "-#{degrees}".to_i : degrees.to_i
+      end
+    end
+  end
+
   def can_edit?(key)
     self.respond_to?("#{key}=") && EDITABLE_KEYS.include?(key.to_sym)
   end

app/views/shared/_anon_nav.html.slim

@@ -58,7 +58,7 @@ nav id="layout-nav" class="top-bar"
         active_wrapper: :li
 
       li= link_to t("nav.blog"), blog_path
-      li= mail_to t("nav.support"), ISO[:support_email]
+      li= link_to t("nav.support"), "mailto:#{ISO[:support_email]}"
 
     ul class="right"
       = link_to t("nav.sign_in"), new_user_session_path, active_on: true, \

app/views/shared/_nav.html.slim

@@ -58,7 +58,7 @@ nav id="layout-nav" class="top-bar"
         active_wrapper: :li
 
       li= link_to t("nav.blog"), blog_path
-      li= mail_to t("nav.support"), ISO[:support_email]
+      li= link_to t("nav.support"), "mailto:#{ISO[:support_email]}"
 
     ul class="right"
       - if user_signed_in?

app/workers/photo_expansion_worker.rb

@@ -1,26 +1,84 @@
 class PhotoExpansionWorker
+  class MetadataRecordMissing < StandardError; end
+
   include Sidekiq::Worker
   include Timeout
   sidekiq_options queue: :photos
 
   def perform(photograph_id)
-    timeout(120) do
-      photo = Photograph.find(photograph_id)
-      photo.standard_image = photo.image.thumb("3000x3000>")
-      photo.save!
-
-      if photo.standard_image.width > photo.standard_image.height
-        ImageWorker.perform_async(photo.id, :standard_image, :homepage_image, "2000x", "-quality 80")
-        ImageWorker.perform_async(photo.id, :standard_image, :large_image, "1500x", "-quality 80")
-      elsif photo.standard_image.height > photo.standard_image.width
-        ImageWorker.perform_async(photo.id, :standard_image, :homepage_image, "2000x1400#", "-quality 80")
-        ImageWorker.perform_async(photo.id, :standard_image, :large_image, "x1000", "-quality 80")
-      else
-        ImageWorker.perform_async(photo.id, :standard_image, :homepage_image, "2000x1400#", "-quality 80")
-        ImageWorker.perform_async(photo.id, :standard_image, :large_image, "1500x1500>", "-quality 80")
+    timeout(300) do
+      @photo = Photograph.find(photograph_id)
+
+      # Extract the metadata and save it
+      extract_metadata
+
+      # Generate all the other image sizes
+      generate_images
+
+      @photo.save!
+    end
+  end
+
+  private
+
+  def extract_metadata
+    Benchmark.measure "Extracting metadata" do
+      metadata = @photo.metadata
+
+      # It's possible that Sidekiq could hit this before Postgres catches up
+      raise MetadataRecordMissing if metadata.nil?
+
+      # Extract the metadata first to allow us to work off that data
+      metadata.extract_from_photograph
+      metadata.save!
+    end
+  end
+
+  def generate_standard_image
+    Benchmark.measure "Generating standard image" do
+      # Create a standard image for generating the smaller sizes
+      standard_image = @photo.image.thumb("3000x3000>")
+
+      # Rotate the image if the metadata says so
+      if @photo.metadata.rotate?
+        standard_image = standard_image.process(:rotate, @photo.metadata.rotate_by)
       end
 
-      ImageWorker.perform_async(photo.id, :standard_image, :thumbnail_image, "500x500>", "-quality 70")
+      # Set the standard image and save
+      @photo.standard_image = standard_image
+      @photo.save!
+    end
+  end
+
+  def generate_images
+    # Generate a sensible base size first
+    generate_standard_image
+
+    # Generate other sizes based on dimensions
+    case
+    when @photo.landscape?
+      ImageWorker.perform_async(@photo.id, :standard_image, :homepage_image, "2000x", "-quality 80")
+      ImageWorker.perform_async(@photo.id, :standard_image, :large_image, "1500x", "-quality 80")
+    when @photo.portrait? 
+      ImageWorker.perform_async(@photo.id, :standard_image, :homepage_image, "2000x1400#", "-quality 80")
+      ImageWorker.perform_async(@photo.id, :standard_image, :large_image, "x1000", "-quality 80")
+    else
+      ImageWorker.perform_async(@photo.id, :standard_image, :homepage_image, "2000x1400#", "-quality 80")
+      ImageWorker.perform_async(@photo.id, :standard_image, :large_image, "1500x1500>", "-quality 80")
+    end
+
+    ImageWorker.perform_async(@photo.id, :standard_image, :thumbnail_image, "500x500>", "-quality 70")
+  end
+
+  def generate_image(source, target, size, encode_opts)
+    Benchmark.measure "Generating #{target} image from #{source}" do
+      source = @photo.send(source)
+      if source.present?
+        image = source.thumb(size).encode(:jpg, encode_opts)
+        @photo.send("#{target}=", image)
+      else
+        raise "Source doesn't exist yet"
+      end
     end
   end
 end

config/initializers/dragonfly.rb

@@ -1,7 +1,8 @@
 require 'dragonfly'
 require 'rack/cache'
+require Rails.root.join("lib/extensions/caching_s3_data_store")
 
-datastore = Dragonfly::DataStorage::S3DataStore.new(
+datastore = Dragonfly::DataStorage::CachingS3DataStore.new(
   :region => ENV['S3_REGION'],
   :bucket_name => ENV['S3_BUCKET'],
   :access_key_id => ENV['S3_KEY'],

lib/extensions/caching_s3_data_store.rb

@@ -0,0 +1,21 @@
+module Dragonfly
+  module DataStorage
+    class CachingS3DataStore < S3DataStore
+      def retrieve(uid)
+        cache.fetch(cache_key_for(uid), expires_in: 5.minutes) do
+          super(uid)
+        end
+      end
+
+      private
+
+      def cache
+        @cache ||= Dalli::Client.new
+      end
+
+      def cache_key_for(uid)
+        "dragonfly-#{uid}"
+      end
+    end
+  end
+end

spec/models/metadata_spec.rb

@@ -52,5 +52,47 @@
         })
       end
     end
+
+    describe "#rotate?" do
+      context "rotate" do
+        before { metadata.stub(:camera) { { 'camera_orientation' => 'Rotate 90 CW' } } }
+
+        it "returns true" do
+          metadata.rotate?.should be true
+        end
+      end
+
+      context "don't rotate" do
+        it "returns false" do
+          metadata.rotate?.should be false
+        end
+      end
+    end
+
+    describe "#rotate_by" do
+      context "clockwise" do      
+        before { metadata.stub(:camera) { { 'camera_orientation' => 'Rotate 90 CW' } } }
+
+        it "returns a positive number" do
+          metadata.rotate_by.should eq(90)
+        end
+      end
+
+      context "counter-clockwise" do
+        before { metadata.stub(:camera) { { 'camera_orientation' => 'Rotate 90 CCW' } } }
+
+        it "returns a negative number" do
+          metadata.rotate_by.should eq(-90)
+        end
+      end
+
+      context "not a rotation command" do
+        before { metadata.stub(:camera) { { 'camera_orientation' => 'Horizontal (normal)' } } }
+
+        it "returns nil" do
+          metadata.rotate_by.should be nil
+        end
+      end
+    end
   end
 end

spec/workers/photo_expansion_worker_spec.rb

@@ -0,0 +1,73 @@
+require 'spec_helper'
+
+describe PhotoExpansionWorker do
+  subject { PhotoExpansionWorker.new }
+  let(:photograph) { Photograph.make(metadata: metadata) }
+  let(:metadata) { Metadata.make }
+
+  before { Photograph.stub(:find) { photograph } }
+  before { Metadata.stub(:find) { metadata } }
+  before { photograph.stub(:save!) { true } }
+
+  describe "metadata" do
+    before { metadata.stub(:extract_from_photograph) { true } }
+    before { metadata.stub(:save!) { true } }
+    after { subject.perform(1) }
+
+    it "calls extract_metadata" do
+      subject.should_receive(:extract_metadata)
+    end
+
+    it "calls extract_from_photograph on the metadata" do
+      metadata.should_receive(:extract_from_photograph)
+    end
+
+    it "saves the metadata" do
+      metadata.should_receive(:save!)
+    end
+  end
+
+  describe "image generation" do
+    before { subject.instance_variable_set('@photo', photograph) }
+
+    describe "#generate_standard_image" do
+      let(:image) { double('image').as_null_object }
+      before { photograph.stub_chain(:image, :thumb) { image } }
+      after { subject.send(:generate_standard_image) }
+
+      context "metadata.rotate? is true" do
+        before { metadata.stub(:rotate?) { true } }
+        before { metadata.stub(:rotate_by) { 90 } }
+
+        it "rotates the image" do
+          image.should_receive(:process).with(:rotate, 90)
+        end
+      end
+
+      context "metadata.rotate? is false" do
+        before { metadata.stub(:rotate?) { false } }
+
+        it "doesn't rotate the image" do
+          image.should_not_receive(:process)
+        end
+      end
+
+      it "sets the standard image" do
+        photograph.should_receive(:standard_image=)
+      end
+    end
+
+    describe "#generate_images" do
+      before { subject.stub(:generate_image) { true } }
+      after { subject.send(:generate_images) }
+
+      it "generates the standard image" do
+        subject.should_receive(:generate_standard_image)
+      end
+
+      it "generates 3 smaller images" do
+
+      end
+    end
+  end
+end