diff options
author | Eugen Rochko <eugen@zeonfederated.com> | 2021-05-05 19:44:01 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-05-05 19:44:01 +0200 |
commit | 036556d3509fac5fa487a0d5ff3cf95767e8d84f (patch) | |
tree | f3435a4f1a5cbb999fde3118e9d17e62a889a59d /lib | |
parent | dfa002932d660656792a78887264dd00820f2dda (diff) |
Fix media processing getting stuck on too much stdin/stderr (#16136)
* Fix media processing getting stuck on too much stdin/stderr See thoughtbot/terrapin#5 * Remove dependency on paperclip-av-transcoder gem * Remove dependency on streamio-ffmpeg gem * Disable stdin on ffmpeg process
Diffstat (limited to 'lib')
-rw-r--r-- | lib/paperclip/attachment_extensions.rb | 4 | ||||
-rw-r--r-- | lib/paperclip/gif_transcoder.rb | 3 | ||||
-rw-r--r-- | lib/paperclip/image_extractor.rb | 14 | ||||
-rw-r--r-- | lib/paperclip/transcoder.rb | 102 | ||||
-rw-r--r-- | lib/paperclip/transcoder_extensions.rb | 14 | ||||
-rw-r--r-- | lib/paperclip/video_transcoder.rb | 26 | ||||
-rw-r--r-- | lib/terrapin/multi_pipe_extensions.rb | 63 |
7 files changed, 176 insertions, 50 deletions
diff --git a/lib/paperclip/attachment_extensions.rb b/lib/paperclip/attachment_extensions.rb index e25a34213..271f8b603 100644 --- a/lib/paperclip/attachment_extensions.rb +++ b/lib/paperclip/attachment_extensions.rb @@ -2,6 +2,10 @@ module Paperclip module AttachmentExtensions + def meta + instance_read(:meta) + end + # We overwrite this method to support delayed processing in # Sidekiq. Since we process the original file to reduce disk # usage, and we still want to generate thumbnails straight diff --git a/lib/paperclip/gif_transcoder.rb b/lib/paperclip/gif_transcoder.rb index 9f3c8e8be..74aa1a0b2 100644 --- a/lib/paperclip/gif_transcoder.rb +++ b/lib/paperclip/gif_transcoder.rb @@ -100,7 +100,8 @@ end module Paperclip # This transcoder is only to be used for the MediaAttachment model - # to convert animated gifs to webm + # to convert animated GIFs to videos + class GifTranscoder < Paperclip::Processor def make return File.open(@file.path) unless needs_convert? diff --git a/lib/paperclip/image_extractor.rb b/lib/paperclip/image_extractor.rb index aab675a06..17fe4326f 100644 --- a/lib/paperclip/image_extractor.rb +++ b/lib/paperclip/image_extractor.rb @@ -31,21 +31,17 @@ module Paperclip private def extract_image_from_file! - ::Av.logger = Paperclip.logger - - cli = ::Av.cli dst = Tempfile.new([File.basename(@file.path, '.*'), '.png']) dst.binmode - cli.add_source(@file.path) - cli.add_destination(dst.path) - cli.add_output_param loglevel: 'fatal' - begin - cli.run - rescue Cocaine::ExitStatusError, ::Av::CommandError + command = Terrapin::CommandLine.new('ffmpeg', '-i :source -loglevel :loglevel -y :destination', logger: Paperclip.logger) + command.run(source: @file.path, destination: dst.path, loglevel: 'fatal') + rescue Terrapin::ExitStatusError dst.close(true) return nil + rescue Terrapin::CommandNotFoundError + raise Paperclip::Errors::CommandNotFoundError, 'Could not run the `ffmpeg` command. Please install ffmpeg.' end dst diff --git a/lib/paperclip/transcoder.rb b/lib/paperclip/transcoder.rb new file mode 100644 index 000000000..e99704086 --- /dev/null +++ b/lib/paperclip/transcoder.rb @@ -0,0 +1,102 @@ +# frozen_string_literal: true + +module Paperclip + # This transcoder is only to be used for the MediaAttachment model + # to check when uploaded videos are actually gifv's + class Transcoder < Paperclip::Processor + def initialize(file, options = {}, attachment = nil) + super + + @current_format = File.extname(@file.path) + @basename = File.basename(@file.path, @current_format) + @format = options[:format] + @time = options[:time] || 3 + @passthrough_options = options[:passthrough_options] + @convert_options = options[:convert_options].dup + end + + def make + metadata = VideoMetadataExtractor.new(@file.path) + + unless metadata.valid? + log("Unsupported file #{@file.path}") + return File.open(@file.path) + end + + update_attachment_type(metadata) + update_options_from_metadata(metadata) + + destination = Tempfile.new([@basename, @format ? ".#{@format}" : '']) + destination.binmode + + @output_options = @convert_options[:output]&.dup || {} + @input_options = @convert_options[:input]&.dup || {} + + case @format.to_s + when /jpg$/, /jpeg$/, /png$/, /gif$/ + @input_options['ss'] = @time + + @output_options['f'] = 'image2' + @output_options['vframes'] = 1 + when 'mp4' + @output_options['acodec'] = 'aac' + @output_options['strict'] = 'experimental' + end + + command_arguments, interpolations = prepare_command(destination) + + begin + command = Terrapin::CommandLine.new('ffmpeg', command_arguments.join(' '), logger: Paperclip.logger) + command.run(interpolations) + rescue Terrapin::ExitStatusError => e + raise Paperclip::Error, "Error while transcoding #{@basename}: #{e}" + rescue Terrapin::CommandNotFoundError + raise Paperclip::Errors::CommandNotFoundError, 'Could not run the `ffmpeg` command. Please install ffmpeg.' + end + + destination + end + + private + + def prepare_command(destination) + command_arguments = ['-nostdin'] + interpolations = {} + interpolation_keys = 0 + + @input_options.each_pair do |key, value| + interpolation_key = interpolation_keys + command_arguments << "-#{key} :#{interpolation_key}" + interpolations[interpolation_key] = value + interpolation_keys += 1 + end + + command_arguments << '-i :source' + interpolations[:source] = @file.path + + @output_options.each_pair do |key, value| + interpolation_key = interpolation_keys + command_arguments << "-#{key} :#{interpolation_key}" + interpolations[interpolation_key] = value + interpolation_keys += 1 + end + + command_arguments << '-y :destination' + interpolations[:destination] = destination.path + + [command_arguments, interpolations] + end + + def update_options_from_metadata(metadata) + return unless @passthrough_options && @passthrough_options[:video_codecs].include?(metadata.video_codec) && @passthrough_options[:audio_codecs].include?(metadata.audio_codec) && @passthrough_options[:colorspaces].include?(metadata.colorspace) + + @format = @passthrough_options[:options][:format] || @format + @time = @passthrough_options[:options][:time] || @time + @convert_options = @passthrough_options[:options][:convert_options].dup + end + + def update_attachment_type(metadata) + @attachment.instance.type = MediaAttachment.types[:gifv] unless metadata.audio_codec + end + end +end diff --git a/lib/paperclip/transcoder_extensions.rb b/lib/paperclip/transcoder_extensions.rb deleted file mode 100644 index c0b2447f3..000000000 --- a/lib/paperclip/transcoder_extensions.rb +++ /dev/null @@ -1,14 +0,0 @@ -# frozen_string_literal: true - -module Paperclip - module TranscoderExtensions - # Prevent the transcoder from modifying our meta hash - def initialize(file, options = {}, attachment = nil) - meta_value = attachment&.instance_read(:meta) - super - attachment&.instance_write(:meta, meta_value) - end - end -end - -Paperclip::Transcoder.prepend(Paperclip::TranscoderExtensions) diff --git a/lib/paperclip/video_transcoder.rb b/lib/paperclip/video_transcoder.rb deleted file mode 100644 index 4d9544231..000000000 --- a/lib/paperclip/video_transcoder.rb +++ /dev/null @@ -1,26 +0,0 @@ -# frozen_string_literal: true - -module Paperclip - # This transcoder is only to be used for the MediaAttachment model - # to check when uploaded videos are actually gifv's - class VideoTranscoder < Paperclip::Processor - def make - movie = FFMPEG::Movie.new(@file.path) - - attachment.instance.type = MediaAttachment.types[:gifv] unless movie.audio_codec - - Paperclip::Transcoder.make(file, actual_options(movie), attachment) - end - - private - - def actual_options(movie) - opts = options[:passthrough_options] - if opts && opts[:video_codecs].include?(movie.video_codec) && opts[:audio_codecs].include?(movie.audio_codec) && opts[:colorspaces].include?(movie.colorspace) - opts[:options] - else - options - end - end - end -end diff --git a/lib/terrapin/multi_pipe_extensions.rb b/lib/terrapin/multi_pipe_extensions.rb new file mode 100644 index 000000000..51d7de37c --- /dev/null +++ b/lib/terrapin/multi_pipe_extensions.rb @@ -0,0 +1,63 @@ +# frozen_string_literal: false +# Fix adapted from https://github.com/thoughtbot/terrapin/pull/5 + +module Terrapin + module MultiPipeExtensions + def read + read_streams(@stdout_in, @stderr_in) + end + + def close_read + begin + @stdout_in.close + rescue IOError + # Do nothing + end + + begin + @stderr_in.close + rescue IOError + # Do nothing + end + end + + def read_streams(output, error) + @stdout_output = '' + @stderr_output = '' + + read_fds = [output, error] + + until read_fds.empty? + to_read, = IO.select(read_fds) + + if to_read.include?(output) + @stdout_output << read_stream(output) + read_fds.delete(output) if output.closed? + end + + if to_read.include?(error) + @stderr_output << read_stream(error) + read_fds.delete(error) if error.closed? + end + end + end + + def read_stream(io) + result = '' + + begin + while (partial_result = io.read_nonblock(8192)) + result << partial_result + end + rescue EOFError, Errno::EPIPE + io.close + rescue Errno::EINTR, Errno::EWOULDBLOCK, Errno::EAGAIN + # Do nothing + end + + result + end + end +end + +Terrapin::CommandLine::MultiPipe.prepend(Terrapin::MultiPipeExtensions) |