Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove mediainfo gem, replace with ffprobe call #5982

Merged
merged 4 commits into from
Aug 29, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -564,10 +565,6 @@ def stop_processing!

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 @@ -722,33 +719,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
masaball marked this conversation as resolved.
Show resolved Hide resolved
ENV["MEDIAINFO_PATH"] ||= Settings.mediainfo.path if Settings&.mediainfo&.path
50 changes: 50 additions & 0 deletions lib/avalon/ffprobe.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
# 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
masaball marked this conversation as resolved.
Show resolved Hide resolved
# media_path should be the output of `FileLocator.new(file_location).location`
def initialize(media_path)
@json_output = JSON.parse(`ffprobe -i "#{media_path}" -v quiet -show_format -show_streams -count_packets -of json`).deep_symbolize_keys
masaball marked this conversation as resolved.
Show resolved Hide resolved
@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
# 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
7 changes: 3 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 Down