Skip to content

Commit

Permalink
Fix media processing getting stuck on too much stdin/stderr (mastodon…
Browse files Browse the repository at this point in the history
…#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
  • Loading branch information
Gargron authored and ClearlyClaire committed Jan 28, 2022
1 parent b593a7d commit 4b9a0cf
Show file tree
Hide file tree
Showing 12 changed files with 234 additions and 67 deletions.
2 changes: 0 additions & 2 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ gem 'aws-sdk-s3', '~> 1.85', require: false
gem 'fog-core', '<= 2.1.0'
gem 'fog-openstack', '~> 0.3', require: false
gem 'paperclip', '~> 6.0'
gem 'paperclip-av-transcoder', '~> 0.6'
gem 'streamio-ffmpeg', '~> 3.0'
gem 'blurhash', '~> 0.1'

gem 'active_model_serializers', '~> 0.10'
Expand Down
11 changes: 0 additions & 11 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,6 @@ GEM
ast (2.4.1)
attr_encrypted (3.1.0)
encryptor (~> 3.0.0)
av (0.9.0)
cocaine (~> 0.5.3)
awrence (1.1.1)
aws-eventstream (1.1.0)
aws-partitions (1.397.0)
Expand Down Expand Up @@ -151,8 +149,6 @@ GEM
cld3 (3.3.0)
ffi (>= 1.1.0, < 1.12.0)
climate_control (0.2.0)
cocaine (0.5.8)
climate_control (>= 0.0.3, < 1.0)
coderay (1.1.3)
color_diff (0.1)
concurrent-ruby (1.1.7)
Expand Down Expand Up @@ -394,9 +390,6 @@ GEM
mime-types
mimemagic (~> 0.3.0)
terrapin (~> 0.6.0)
paperclip-av-transcoder (0.6.4)
av (~> 0.9.0)
paperclip (>= 2.5.2)
parallel (1.20.1)
parallel_tests (3.4.0)
parallel
Expand Down Expand Up @@ -609,8 +602,6 @@ GEM
stackprof (0.2.16)
statsd-ruby (1.4.0)
stoplight (2.2.1)
streamio-ffmpeg (3.0.2)
multi_json (~> 1.8)
strong_migrations (0.7.2)
activerecord (>= 5)
temple (0.8.2)
Expand Down Expand Up @@ -755,7 +746,6 @@ DEPENDENCIES
omniauth-saml (~> 1.10)
ox (~> 2.13)
paperclip (~> 6.0)
paperclip-av-transcoder (~> 0.6)
parallel (~> 1.20)
parallel_tests (~> 3.4)
parslet
Expand Down Expand Up @@ -801,7 +791,6 @@ DEPENDENCIES
sprockets-rails (~> 3.2)
stackprof
stoplight (~> 2.2.1)
streamio-ffmpeg (~> 3.0)
strong_migrations (~> 0.7)
thor (~> 1.0)
tty-prompt (~> 0.22)
Expand Down
54 changes: 54 additions & 0 deletions app/lib/video_metadata_extractor.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
# frozen_string_literal: true

class VideoMetadataExtractor
attr_reader :duration, :bitrate, :video_codec, :audio_codec,
:colorspace, :width, :height, :frame_rate

def initialize(path)
@path = path
@metadata = Oj.load(ffmpeg_command_output, mode: :strict, symbol_keys: true)

parse_metadata
rescue Terrapin::ExitStatusError, Oj::ParseError
@invalid = true
rescue Terrapin::CommandNotFoundError
raise Paperclip::Errors::CommandNotFoundError, 'Could not run the `ffprobe` command. Please install ffmpeg.'
end

def valid?
!@invalid
end

private

def ffmpeg_command_output
command = Terrapin::CommandLine.new('ffprobe', '-i :path -print_format :format -show_format -show_streams -show_error -loglevel :loglevel')
command.run(path: @path, format: 'json', loglevel: 'fatal')
end

def parse_metadata
if @metadata.key?(:format)
@duration = @metadata[:format][:duration].to_f
@bitrate = @metadata[:format][:bit_rate].to_i
end

if @metadata.key?(:streams)
video_streams = @metadata[:streams].select { |stream| stream[:codec_type] == 'video' }
audio_streams = @metadata[:streams].select { |stream| stream[:codec_type] == 'audio' }

if (video_stream = video_streams.first)
@video_codec = video_stream[:codec_name]
@colorspace = video_stream[:pix_fmt]
@width = video_stream[:width]
@height = video_stream[:height]
@frame_rate = video_stream[:avg_frame_rate] == '0/0' ? nil : Rational(video_stream[:avg_frame_rate])
end

if (audio_stream = audio_streams.first)
@audio_codec = audio_stream[:codec_name]
end
end

@invalid = true if @metadata.key?(:error)
end
end
4 changes: 2 additions & 2 deletions app/models/media_attachment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ def file_processors(instance)
if instance.file_content_type == 'image/gif'
[:gif_transcoder, :blurhash_transcoder]
elsif VIDEO_MIME_TYPES.include?(instance.file_content_type)
[:video_transcoder, :blurhash_transcoder, :type_corrector]
[:transcoder, :blurhash_transcoder, :type_corrector]
elsif AUDIO_MIME_TYPES.include?(instance.file_content_type)
[:image_extractor, :transcoder, :type_corrector]
else
Expand Down Expand Up @@ -388,7 +388,7 @@ def video_metadata(file)
# paths but ultimately the same file, so it makes sense to memoize the
# result while disregarding the path
def ffmpeg_data(path = nil)
@ffmpeg_data ||= FFMPEG::Movie.new(path)
@ffmpeg_data ||= VideoMetadataExtractor.new(path)
end

def enqueue_processing
Expand Down
4 changes: 2 additions & 2 deletions config/application.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,12 @@
require_relative '../lib/paperclip/url_generator_extensions'
require_relative '../lib/paperclip/attachment_extensions'
require_relative '../lib/paperclip/media_type_spoof_detector_extensions'
require_relative '../lib/paperclip/transcoder_extensions'
require_relative '../lib/paperclip/lazy_thumbnail'
require_relative '../lib/paperclip/gif_transcoder'
require_relative '../lib/paperclip/video_transcoder'
require_relative '../lib/paperclip/transcoder'
require_relative '../lib/paperclip/type_corrector'
require_relative '../lib/paperclip/response_with_limit_adapter'
require_relative '../lib/terrapin/multi_pipe_extensions'
require_relative '../lib/mastodon/snowflake'
require_relative '../lib/mastodon/version'
require_relative '../lib/devise/two_factor_ldap_authenticatable'
Expand Down
4 changes: 4 additions & 0 deletions lib/paperclip/attachment_extensions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion lib/paperclip/gif_transcoder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,8 @@ def skip_sub_blocks!(file)

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?
Expand Down
14 changes: 5 additions & 9 deletions lib/paperclip/image_extractor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,21 +31,17 @@ def make
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
Expand Down
102 changes: 102 additions & 0 deletions lib/paperclip/transcoder.rb
Original file line number Diff line number Diff line change
@@ -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
14 changes: 0 additions & 14 deletions lib/paperclip/transcoder_extensions.rb

This file was deleted.

26 changes: 0 additions & 26 deletions lib/paperclip/video_transcoder.rb

This file was deleted.

Loading

0 comments on commit 4b9a0cf

Please sign in to comment.