-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
allow user to specify frame_index for generating thumbnails from videos or pdfs #2155
Changes from 7 commits
3f942df
ea5c17f
5950bb0
5d8dd45
d47fcff
8b11576
a6ac8c1
d8ebb4e
5520565
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,11 +3,16 @@ module Paperclip | |
class Thumbnail < Processor | ||
|
||
attr_accessor :current_geometry, :target_geometry, :format, :whiny, :convert_options, | ||
:source_file_options, :animated, :auto_orient | ||
:source_file_options, :animated, :auto_orient, :frame_index | ||
|
||
#List of multi frame formats to check against the source file type | ||
#this is not an exhaustive list, should be updated to include more formats | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing space after #. |
||
MULTI_FRAME_FORMATS = %w(mkv avi mp4 mov mpg mpeg gif) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Freeze mutable objects assigned to constants. |
||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Trailing whitespace detected. |
||
# List of formats that we need to preserve animation | ||
ANIMATED_FORMATS = %w(gif) | ||
|
||
|
||
# Creates a Thumbnail object set to work on the +file+ given. It | ||
# will attempt to transform the image into one defined by +target_geometry+ | ||
# which is a "WxH"-style string. +format+ will be inferred from the +file+ | ||
|
@@ -25,6 +30,7 @@ class Thumbnail < Processor | |
# +whiny+ - whether to raise an error when processing fails. Defaults to true | ||
# +format+ - the desired filename extension | ||
# +animated+ - whether to merge all the layers in the image. Defaults to true | ||
# +frame_index+ - the frame index of the source file to render as the thumbnail | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Line is too long. [85/80] |
||
def initialize(file, options = {}, attachment = nil) | ||
super | ||
|
||
|
@@ -36,17 +42,21 @@ def initialize(file, options = {}, attachment = nil) | |
@convert_options = options[:convert_options] | ||
@whiny = options.fetch(:whiny, true) | ||
@format = options[:format] | ||
@frame_index = options.fetch(:frame_index, 0) | ||
@animated = options.fetch(:animated, true) | ||
@auto_orient = options.fetch(:auto_orient, true) | ||
if @auto_orient && @current_geometry.respond_to?(:auto_orient) | ||
@current_geometry.auto_orient | ||
end | ||
|
||
@source_file_options = @source_file_options.split(/\s+/) if @source_file_options.respond_to?(:split) | ||
@convert_options = @convert_options.split(/\s+/) if @convert_options.respond_to?(:split) | ||
|
||
@current_format = File.extname(@file.path) | ||
@basename = File.basename(@file.path, @current_format) | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Trailing whitespace detected. |
||
if !multi_frame_format? | ||
@frame_index = 0 | ||
end | ||
end | ||
|
||
# Returns true if the +target_geometry+ is meant to crop. | ||
|
@@ -75,8 +85,12 @@ def make | |
parameters << ":dest" | ||
|
||
parameters = parameters.flatten.compact.join(" ").strip.squeeze(" ") | ||
|
||
success = convert(parameters, :source => "#{File.expand_path(src.path)}#{'[0]' unless animated?}", :dest => File.expand_path(dst.path)) | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Trailing whitespace detected. |
||
desired_frame = animated? ? "" : "[#{@frame_index.to_s}]" | ||
success = convert(parameters, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Useless assignment to variable - |
||
:source => "#{File.expand_path(src.path)}#{desired_frame}", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Line is too long. [85/80] |
||
:dest => File.expand_path(dst.path), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use the new Ruby 1.9 hash syntax. |
||
) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Align There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @tute do you what they want here for alignment? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Align |
||
rescue Cocaine::ExitStatusError => e | ||
raise Paperclip::Error, "There was an error processing the thumbnail for #{@basename}" if @whiny | ||
rescue Cocaine::CommandNotFoundError => e | ||
|
@@ -101,6 +115,13 @@ def transformation_command | |
|
||
protected | ||
|
||
# Return true if the source file format is animated | ||
def multi_frame_format? | ||
#removing the leading . from the extension | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing space after #. |
||
ext = @current_format.to_s[1..@current_format.length] | ||
MULTI_FRAME_FORMATS.include?(ext) | ||
end | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Trailing whitespace detected. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Trailing whitespace detected. |
||
# Return true if the format is animated | ||
def animated? | ||
@animated && (ANIMATED_FORMATS.include?(@format.to_s) || @format.blank?) && identified_as_animated? | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -480,6 +480,31 @@ def to_s | |
assert_equal "50x50", `#{cmd}`.chomp | ||
end | ||
end | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Trailing whitespace detected. |
||
context "with a specified frame_index" do | ||
before do | ||
@thumb = Paperclip::Thumbnail.new(@file, geometry: "50x50", frame_index: 5, format: :jpg) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Line is too long. [97/80] |
||
end | ||
|
||
it "creates the single frame thumbnail with the specified frame index when sent #make" do | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Line is too long. [95/80] |
||
dst = @thumb.make | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Useless assignment to variable - |
||
assert_equal @thumb.frame_index, 5 | ||
end | ||
end | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Trailing whitespace detected. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Trailing whitespace detected. |
||
context "with a specified frame_index out of bounds" do | ||
before do | ||
@thumb = Paperclip::Thumbnail.new(@file, geometry: "50x50", frame_index: 20, format: :jpg) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Line is too long. [98/80] |
||
end | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Trailing whitespace detected. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Trailing whitespace detected. |
||
it "errors when trying to create the thumbnail" do | ||
assert_raises(Paperclip::Error) do | ||
silence_stream(STDERR) do | ||
@thumb.make | ||
end | ||
end | ||
end | ||
end | ||
end | ||
|
||
context "with a really long file name" do | ||
|
@@ -497,4 +522,5 @@ def to_s | |
expect { thumb.make }.to_not raise_error | ||
end | ||
end | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Extra empty line detected at block body end. |
||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing space after #.