Skip to content

Commit

Permalink
Merge pull request #5982 from avalonmediasystem/remove_mediainfo
Browse files Browse the repository at this point in the history
Remove mediainfo gem, replace with ffprobe call
  • Loading branch information
masaball authored Aug 29, 2024
2 parents fd5f15b + a7deb92 commit c3dce9d
Show file tree
Hide file tree
Showing 12 changed files with 262 additions and 39 deletions.
3 changes: 1 addition & 2 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ gem 'active_annotations', '~> 0.4'
gem 'activerecord-session_store', '>= 2.0.0'
gem 'acts_as_list'
gem 'api-pagination'
gem 'avalon-about', git: 'https://github.com/avalonmediasystem/avalon-about.git', tag: 'avalon-r7.7'
gem 'avalon-about', git: 'https://github.com/avalonmediasystem/avalon-about.git', branch: 'main'
#gem 'bootstrap-sass', '< 3.4.1' # Pin to less than 3.4.1 due to change in behavior with popovers
gem 'bootstrap-toggle-rails'
gem 'bootstrap_form'
Expand Down Expand Up @@ -78,7 +78,6 @@ gem 'active_encode', '>= 1.2.2'
gem 'audio_waveform-ruby', '~> 1.0.7', require: 'audio_waveform'
gem 'browse-everything', git: "https://github.com/avalonmediasystem/browse-everything.git", branch: 'v1.2-avalon'
gem 'fastimage'
gem 'mediainfo', git: "https://github.com/avalonmediasystem/mediainfo.git", tag: 'v0.7.1-avalon'
gem 'rest-client', '~> 2.0'
gem 'roo'
gem 'wavefile', '~> 1.0.1'
Expand Down
13 changes: 2 additions & 11 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,11 @@ GIT

GIT
remote: https://github.com/avalonmediasystem/avalon-about.git
revision: cc8d816b9751d6a04750399d9ef828a33e7ac7eb
tag: avalon-r7.7
revision: f3106d139d9092ffb0e9ca468fac9e85188ad339
branch: main
specs:
avalon-about (0.1.0)
about_page
mediainfo

GIT
remote: https://github.com/avalonmediasystem/avalon-workflow.git
Expand All @@ -38,13 +37,6 @@ GIT
signet (~> 0.8)
typhoeus

GIT
remote: https://github.com/avalonmediasystem/mediainfo.git
revision: bea9479d33328c6b483ee19c008730f939d98266
tag: v0.7.1-avalon
specs:
mediainfo (0.7.1)

GIT
remote: https://github.com/avalonmediasystem/om.git
revision: ffde890b4187a7d8be41ae387264cd6eb20b6ba8
Expand Down Expand Up @@ -1050,7 +1042,6 @@ DEPENDENCIES
lograge
mail (> 2.8.0.1)
marc
mediainfo!
mysql2
net-ldap
net-smtp
Expand Down
5 changes: 3 additions & 2 deletions app/helpers/application_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,9 @@ def user_key
current_user.user_key if current_user
end

# the mediainfo gem returns duration as milliseconds
# see attr_reader.rb line 48 in the mediainfo source
# We are converting FFprobe's duration output to milliseconds for
# uniformity with existing metadata and consequently leaving these
# conversion methods in place for now.
def milliseconds_to_formatted_time(milliseconds, include_fractions = true)
total_seconds = milliseconds / 1000
hours = total_seconds / (60 * 60)
Expand Down
21 changes: 9 additions & 12 deletions app/models/master_file.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
require 'fileutils'
require 'hooks'
require 'open-uri'
require 'avalon/ffprobe'
require 'avalon/file_resolver'
require 'avalon/m3u8_reader'

Expand Down Expand Up @@ -571,10 +572,6 @@ def delete

protected

def mediainfo
Mediainfo.new(FileLocator.new(file_location).location, headers: @auth_header)
end

def find_frame_source(options={})
options[:offset] ||= 2000

Expand Down Expand Up @@ -729,33 +726,33 @@ def saveDerivativesHash(derivative_hash)
end

def reloadTechnicalMetadata!
#Reset mediainfo
@mediainfo = mediainfo
# Reset ffprobe
@ffprobe = Avalon::FFprobe.new(FileLocator.new(file_location).location)

# Formats like MP4 can be caught as both audio and video
# so the case statement flows in the preferred order
self.file_format = if @mediainfo.video?
self.file_format = if @ffprobe.video?
'Moving image'
elsif @mediainfo.audio?
elsif @ffprobe.audio?
'Sound'
else
'Unknown'
end

self.duration = begin
@mediainfo.duration.to_s
@ffprobe.duration.to_s
rescue
nil
end

unless @mediainfo.video.streams.empty?
display_aspect_ratio_s = @mediainfo.video.streams.first.display_aspect_ratio
unless !@ffprobe.video?
display_aspect_ratio_s = @ffprobe.display_aspect_ratio
if ':'.in? display_aspect_ratio_s
self.display_aspect_ratio = display_aspect_ratio_s.split(/:/).collect(&:to_f).reduce(:/).to_s
else
self.display_aspect_ratio = display_aspect_ratio_s
end
self.original_frame_size = @mediainfo.video.streams.first.frame_size
self.original_frame_size = @ffprobe.original_frame_size
end
end

Expand Down
2 changes: 1 addition & 1 deletion config/initializers/about_page.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
config.fedora = AboutPage::Fedora.new(ActiveFedora.fedora.connection)
config.solr = AboutPage::Solr.new(ActiveFedora.solr.conn, :numDocs => 1)
config.database = Avalon::About::Database.new(User)
config.mediainfo = Avalon::About::MediaInfo.new(:version => '>=0.7.59')
config.mediainfo = Settings&.mediainfo&.path ? Avalon::About::MediaInfo.new(path: Settings.mediainfo.path) : Avalon::About::MediaInfo.new()
config.streaming_server = Avalon::About::HLSServer.new(Settings.streaming.http_base)
config.sidekiq = Avalon::About::Sidekiq.new(numProcesses: 1)
config.redis = Avalon::About::Redis.new(Redis.new(Rails.application.config.cache_store[1]))
Expand Down
6 changes: 1 addition & 5 deletions config/initializers/ac_mediainfo.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,2 @@
require 'mediainfo'

Mediainfo.path = Settings.mediainfo.path if Settings&.mediainfo&.path

# Set up for active_encode, need to happen before active_encode initializer
ENV["MEDIAINFO_PATH"] ||= Mediainfo.path
ENV["MEDIAINFO_PATH"] ||= Settings.mediainfo.path if Settings&.mediainfo&.path
2 changes: 2 additions & 0 deletions config/settings.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ mediainfo:
path: '/usr/bin/mediainfo --urlencode' # Fixes mediainfo v20.03 bug with S3 presigned URL
ffmpeg:
path: '/usr/local/bin/ffmpeg'
ffprobe:
path: '/usr/bin/ffprobe'
email:
mailer: :smtp
config:
Expand Down
60 changes: 60 additions & 0 deletions lib/avalon/ffprobe.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
# Copyright 2011-2024, The Trustees of Indiana University and Northwestern
# University. Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
#
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software distributed
# under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR
# CONDITIONS OF ANY KIND, either express or implied. See the License for the
# specific language governing permissions and limitations under the License.
# --- END LICENSE_HEADER BLOCK ---

module Avalon
class FFprobe
# media_path should be the output of `FileLocator.new(file_location).location`
def initialize(media_path)
ffprobe = Settings&.ffprobe&.path || 'ffprobe'
raw_output = `#{ffprobe} -i "#{media_path}" -v quiet -show_format -show_streams -count_packets -of json`
# $? is a variable for the exit status of the last executed process.
# Success == 0, any other value means the command failed in some way.
unless $?.exitstatus == 0
@json_output = {}
Rails.logger.error "File processing failed. Please ensure that FFprobe is installed and that the correct path is configured."
return
end
@json_output = JSON.parse(raw_output).deep_symbolize_keys
@video_stream = @json_output[:streams].select { |stream| stream[:codec_type] == 'video' }.first
end

def video?
# ffprobe treats plain text files as ANSI or ASCII art. This sets the codec type to video
# but leaves display aspect ratio `nil`. If display_aspect_ratio is nil, return false.
# ffprobe treats image files as a single frame video. This sets the codec type to video
# but the packet/frame count will equal 1. If packet count equals 1, return false.
return true if @video_stream && @video_stream[:display_aspect_ratio] && @video_stream[:nb_read_packets].to_i > 1

false
end

def audio?
@json_output[:streams]&.any? { |stream| stream[:codec_type] == 'audio' }
end

def duration
return unless video? || audio?
# ffprobe return duration as seconds. Existing Avalon logic expects milliseconds.
(@json_output[:format][:duration].to_f * 1000).to_i
end

def display_aspect_ratio
@video_stream[:display_aspect_ratio] if video?
end

def original_frame_size
"#{@video_stream[:width]}x#{@video_stream[:height]}" if video?
end
end
end
4 changes: 2 additions & 2 deletions lib/tasks/avalon_tools.rake
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@

namespace :avalon do
namespace :tools do
ffmpeg_path = Settings.ffmpeg.path || "/usr/bin/ffmpeg"
mediainfo_path = Settings.mediainfo.path || "/usr/bin/mediainfo"
ffmpeg_path = Settings&.ffmpeg&.path || "/usr/bin/ffmpeg"
mediainfo_path = Settings&.mediainfo&.path || "/usr/bin/mediainfo"
DEFAULT_TOOLS = [
{ name: "ffmpeg", path: ffmpeg_path, version_params: "-version", version_string: ">= 4", version_trim_pre: "ffmpeg version ", version_trim_last_char: "-" },
{ name: "mediainfo", path: mediainfo_path, version_string: "> 18", version_line: 1, version_trim_pre: "MediaInfoLib - v" },
Expand Down
14 changes: 10 additions & 4 deletions spec/controllers/master_files_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,11 @@

describe "#create" do
let(:media_object) { FactoryBot.create(:media_object) }
# TODO: fill in the lets below with a legitimate values from mediainfo
# let(:mediainfo_video) { }
# let(:mediainfo_audio) { }

before do
# login_user media_object.collection.managers.first
login_as :administrator
disableCanCan!
# allow_any_instance_of(MasterFile).to receive(:mediainfo).and_return(mediainfo_output)
end

context "must provide a container id" do
Expand Down Expand Up @@ -125,9 +121,12 @@
request.env["HTTP_REFERER"] = "/"

file = fixture_file_upload('/public-domain-book.txt', 'application/json')
image = fixture_file_upload('/collection_poster.jpg', 'image/jpeg')

expect { post :create, params: { Filedata: [file], original: 'any', container_id: media_object.id } }.not_to change { MasterFile.count }
expect(flash[:error]).not_to be_nil

expect { post :create, params: { Filedata: [image], original: 'any', container_id: media_object.id } }.not_to change { MasterFile.count }
expect(flash[:error]).not_to be_nil
end

Expand All @@ -140,6 +139,13 @@

expect(flash[:error]).to be_nil
end

it "rejects when ffprobe is misconfigured" do
allow(Settings.ffprobe).to receive(:path).and_return('misconfigured/path')
file = fixture_file_upload('/jazz-performance.mp3', 'audio/mp3')
expect { post :create, params: { Filedata: [file], original: 'any', container_id: media_object.id } }.not_to change { MasterFile.count }
expect(flash[:error]).not_to be_nil
end
end

# rubocop:disable RSpec/ExampleLength
Expand Down
Binary file added spec/fixtures/videoshort_no_audio.mp4
Binary file not shown.
Loading

0 comments on commit c3dce9d

Please sign in to comment.