-
-
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 all 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,8 +3,12 @@ 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 | ||
MULTI_FRAME_FORMATS = %w(mkv avi mp4 mov mpg mpeg gif) | ||
|
||
# List of formats that we need to preserve animation | ||
ANIMATED_FORMATS = %w(gif) | ||
|
||
|
@@ -25,6 +29,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 +41,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 +84,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,9 +114,16 @@ def transformation_command | |
|
||
protected | ||
|
||
# Return true if the source file format is animated | ||
def multi_frame_format? | ||
# removing the leading . from the extension | ||
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? | ||
@animated && (ANIMATED_FORMATS.include?(@format.to_s) || @format.blank?) && identified_as_animated? | ||
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. [105/80] |
||
end | ||
|
||
# Return true if ImageMagick's +identify+ returns an animated format | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -480,6 +480,39 @@ 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, | ||
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 the elements of a hash literal if they span more than one line. |
||
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. Align the elements of a hash literal if they span more than one line. |
||
) | ||
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. Align |
||
end | ||
|
||
it "creates the thumbnail from the frame index when sent #make" do | ||
@thumb.make | ||
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, | ||
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 the elements of a hash literal if they span more than one line. |
||
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. Align the elements of a hash literal if they span more than one line. |
||
) | ||
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. 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 can you help me out with what Hound wants here? I have tried a few different options for aligning the ( ), but it keeps reporting errors |
||
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 | ||
|
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.
Trailing whitespace detected.