From b060c645968d7003fcd94566777619008dd280ae Mon Sep 17 00:00:00 2001 From: Omar Abdel-Wahab Date: Sun, 3 May 2015 12:00:44 +0200 Subject: [PATCH] Added proper screenshot handling. --- README.md | 33 +++-- .../paperclip_processors/transcoder.rb | 125 +++++++++++------- spec/spec_helper.rb | 10 +- spec/transcoder/transcoder_spec.rb | 54 ++++++-- 4 files changed, 152 insertions(+), 70 deletions(-) diff --git a/README.md b/README.md index eda9480..93af6cb 100644 --- a/README.md +++ b/README.md @@ -38,19 +38,34 @@ In your model: }, :processors => [:transcoder] end -This will produce: +### Chaining -1. A transcoded `:medium` FLV file with the requested dimensions if they will match the aspect ratio of the original file, otherwise, width will be maintained and height will be recalculated to keep the original aspect ration. -2. A screenshot `:thumb` with the requested dimensions regardless of the aspect ratio. +You can chain other `paperclip` processors in the `processors` array. In this +case `paperclip-av-transcoder` will only process files recognized by `av` gem. -You may optionally add `_meta` to your model and it will get populated with information about the processed video. +### Caching attachment information -The geometry parameters are: +You may create a migration to add `_meta` to your model +and it will get populated with information about the processed attachment. -1. '!' - Keep the same aspect of the image/video, but with the passed dimesion. -2. '#' - Pad the image/video. -3. '<' - Enlarge the image/video. -4. '>' - Shrink the image/video. +### Screenshot + +When transcoding from video to image (taking screenshots from a video), you can +specify the time using one of the following methods: + +#### Integer/String + +Will be used as-is. You might need to make sure the number is less than the +video length. + +#### Symbol + +You can provide a method name as a symbol. The method will be passed two +parameters: + + `meta`: `hash` containing video information. + + `options`: `hash` containing the style options. ## Contributing diff --git a/lib/paperclip/paperclip_processors/transcoder.rb b/lib/paperclip/paperclip_processors/transcoder.rb index 01ee8d4..da19100 100755 --- a/lib/paperclip/paperclip_processors/transcoder.rb +++ b/lib/paperclip/paperclip_processors/transcoder.rb @@ -1,8 +1,8 @@ module Paperclip class Transcoder < Processor - attr_accessor :geometry, :format, :whiny, :convert_options + attr_accessor :geometry, :format, :whiny, :convert_options, :parameters # Creates a Video object set to work on the +file+ given. It - # will attempt to transcode the video into one defined by +target_geometry+ + # will attempt to transcode the video into one defined by +geometry+ # which is a "WxH"-style string. +format+ should be specified. # Video transcoding will raise no errors unless # +whiny+ is true (which it is, by default. If +convert_options+ is @@ -11,32 +11,14 @@ def initialize file, options = {}, attachment = nil @file = file @current_format = File.extname(@file.path) @basename = File.basename(@file.path, @current_format) - @cli = ::Av.cli - @meta = ::Av.cli.identify(@file.path) + @attachment = attachment @whiny = options[:whiny].nil? ? true : options[:whiny] - - @convert_options = set_convert_options(options) - @format = options[:format] - - @geometry = options[:geometry] - unless @geometry.nil? - modifier = @geometry[0] - @geometry[0] = '' if ['#', '<', '>'].include? modifier - @width, @height = @geometry.split('x') - @keep_aspect = @width[0] == '!' || @height[0] == '!' - @pad_only = @keep_aspect && modifier == '#' - @enlarge_only = @keep_aspect && modifier == '<' - @shrink_only = @keep_aspect && modifier == '>' - end - - @time = options[:time].nil? ? 3 : options[:time] - @auto_rotate = options[:auto_rotate].nil? ? false : options[:auto_rotate] - @pad_color = options[:pad_color].nil? ? "black" : options[:pad_color] - - @convert_options[:output][:s] = format_geometry(@geometry) if @geometry.present? - - attachment.instance_write(:meta, @meta) if attachment + @cli = ::Av.cli(log: false) + @meta = identify + @options = options + @default_time = 3 + @parameters = {} # will be set later by set_parameters end # Performs the transcoding of the +file+ into a thumbnail/video. Returns the Tempfile @@ -48,24 +30,20 @@ def make dst.binmode if @meta - log "Transocding supported file #{@file.path}" + log "Transcoding supported file #{@file.path}" + set_parameters @cli.add_source(@file.path) @cli.add_destination(dst.path) @cli.reset_input_filters - if output_is_image? - @time = @time.call(@meta, @options) if @time.respond_to?(:call) - @cli.filter_seek @time - end - - if @convert_options.present? - if @convert_options[:input] - @convert_options[:input].each do |h| + if @parameters.present? + if @parameters[:input] + @parameters[:input].each do |h| @cli.add_input_param h end end - if @convert_options[:output] - @convert_options[:output].each do |h| + if @parameters[:output] + @parameters[:output].each do |h| @cli.add_output_param h end end @@ -73,7 +51,7 @@ def make begin @cli.run - log "Successfully transcoded #{@basename} to #{dst}" + log "Transcoding successful" rescue Cocaine::ExitStatusError => e raise Paperclip::Error, "error while transcoding #{@basename}: #{e}" if @whiny end @@ -86,24 +64,75 @@ def make dst end - def log message - Paperclip.log "[transcoder] #{message}" + def set_parameters + format_time + options[:auto_rotate] = options[:auto_rotate] || false + options[:pad_color] = options[:pad_color] || 'black' end - def set_convert_options options - return options[:convert_options] if options[:convert_options].present? - options[:convert_options] = {output: {}} - return options[:convert_options] + def format_geometry + unless @options[:geometry].nil? + modifier = @options[:geometry][-1,1] + geometry = @options[:geometry].gsub(/[#!<>)]/, '') + width, height = geometry.split('x') + keep_aspect = !!modifier == '!' + @pad_only = @keep_aspect && modifier == '#' + @enlarge_only = @keep_aspect && modifier == '<' + @shrink_only = @keep_aspect && modifier == '>' + @parameters[:output][:s] = "#{width}x#{height}" + end end - - def format_geometry geometry - return unless geometry.present? - return geometry.gsub(/[#!<>)]/, '') + + def format_time + calculate_time + if @parameters[:time] && output_is_image? + @cli.filter_seek(@parameters[:time]) + end + end + + def calculate_time + time = @options[:time] + case time.class.to_s + when 'Symbol' + debug "Calling method #{time} to calculate time" + @parameters[:time] = @attachment.instance.send(time, @meta, @options) + debug "Screenshot time calculated by calling method #{time} '#{@parameters[:time]}'" + when 'Fixnum' || 'String' || 'Integer' || 'Float' + debug "Screenshot time provided '#{time}'" + @parameters[:time] = time.to_i + when 'NilClass' + debug "No screenshot time provided, using default '#{@default_time}'" + @parameters[:time] = @default_time + else + log "Unknown time format: `#{time}`, using default '#{@default_time}'" + @parameters[:time] = @default_time + end + log "Screenshot time is: #{@parameters[:time]}" end def output_is_image? !!@format.to_s.match(/jpe?g|png|gif$/) end + + private + + def identify + meta = ::Av.cli.identify(@file.path) + debug "Detected metadata: #{meta.inspect}" + unless @attachment.nil? + log 'Writing _meta' + attachment.instance_write(:meta, meta) + end + meta + end + + def log message + Paperclip.log "[transcoder] #{message}" + end + + def debug message + Paperclip.log "[transcoder] #{message}" if @whiny + end end class Attachment diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 1735ebf..e4950b8 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -45,9 +45,13 @@ class Document < ActiveRecord::Base thumb: { format: 'jpg', time: 0 + }, + thumb_with_time_method: { + format: 'png', + time: :set_screenshot_time } }, - processors: [:transcoder] + processors: [:transcoder] has_attached_file :image, storage: :filesystem, @@ -61,4 +65,8 @@ class Document < ActiveRecord::Base do_not_validate_attachment_file_type :video do_not_validate_attachment_file_type :image + + def set_screenshot_time meta, options + 2 + end end \ No newline at end of file diff --git a/spec/transcoder/transcoder_spec.rb b/spec/transcoder/transcoder_spec.rb index 33ff890..d8d5bdb 100644 --- a/spec/transcoder/transcoder_spec.rb +++ b/spec/transcoder/transcoder_spec.rb @@ -6,21 +6,51 @@ let(:destination) { Pathname.new("#{Dir.tmpdir}/transcoder/") } - describe "supported formats" do - let(:subject) { Paperclip::Transcoder.new(supported) } - let(:document) { Document.create(video: Rack::Test::UploadedFile.new(supported, 'video/mp4')) } - - describe ".transcode" do - it { expect(File.exists?(document.video.path(:small))).to eq true } - it { expect(File.exists?(document.video.path(:thumb))).to eq true } + describe "#format_geometry" do + let(:options) { { geometry: '100x100' } } + let(:subject) { Paperclip::Transcoder.new(supported, options) } + + # it { expect(subject.format_geometry).to be_of_kind Hash } + end + + describe "#calculate_time" do + let(:subject) { Paperclip::Transcoder.new(supported, options) } + before do + subject.calculate_time + end + + describe "defaults to 3" do + let(:options) { {} } + it { expect(subject.parameters[:time]).to eq 3 } + end + describe "accepts time" do + describe "as integer" do + let(:options) { { time: 10 } } + it { expect(subject.parameters[:time]).to eq 10 } + end + describe "as method name" do + let(:options) { { time: -> (meta, options) { 17 } } } + let(:document) { Document.create(video: Rack::Test::UploadedFile.new(supported, 'video/mp4')) } + it { expect(File.exists?(document.video.path(:thumb_with_time_method))).to eq true } + end end end + + describe "transcoding" do + describe "supported formats" do + let(:document) { Document.create(video: Rack::Test::UploadedFile.new(supported, 'video/mp4')) } + + describe ".transcode" do + it { expect(File.exists?(document.video.path(:small))).to eq true } + it { expect(File.exists?(document.video.path(:thumb))).to eq true } + end + end - describe "unsupported formats" do - let(:subject) { Paperclip::Transcoder.new(unsupported) } - let(:document) { Document.create(image: Rack::Test::UploadedFile.new(unsupported, 'image/png')) } - describe ".transcode" do - it { expect(File.exists?(document.image.path(:small))).to eq true } + describe "unsupported formats" do + let(:document) { Document.create(image: Rack::Test::UploadedFile.new(unsupported, 'image/png')) } + describe ".transcode" do + it { expect(File.exists?(document.image.path(:small))).to eq true } + end end end