Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improvements to image processing. #146

Merged
merged 4 commits into from
Aug 17, 2013
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 20 additions & 5 deletions app/models/metadata.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 ||= {}
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion app/views/shared/_anon_nav.html.slim
Original file line number Diff line number Diff line change
Expand Up @@ -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, \
Expand Down
2 changes: 1 addition & 1 deletion app/views/shared/_nav.html.slim
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand Down
88 changes: 73 additions & 15 deletions app/workers/photo_expansion_worker.rb
Original file line number Diff line number Diff line change
@@ -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
3 changes: 2 additions & 1 deletion config/initializers/dragonfly.rb
Original file line number Diff line number Diff line change
@@ -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'],
Expand Down
21 changes: 21 additions & 0 deletions lib/extensions/caching_s3_data_store.rb
Original file line number Diff line number Diff line change
@@ -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
42 changes: 42 additions & 0 deletions spec/models/metadata_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
73 changes: 73 additions & 0 deletions spec/workers/photo_expansion_worker_spec.rb
Original file line number Diff line number Diff line change
@@ -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