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

Fix audio and video playback through valkyrie codepaths #6297

Merged
merged 3 commits into from
Sep 7, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ def send_file_contents_valkyrie(file_set)
response.headers["Accept-Ranges"] = "bytes"
self.status = 200
use = params.fetch(:file, :original_file).to_sym
file_metadata = find_file_metadata(file_set: file_set, use: use)
mime_type = params[:mime_type]
file_metadata = find_file_metadata(file_set: file_set, use: use, mime_type: mime_type)
return unless stale?(last_modified: file_metadata.updated_at, template: false)

file = Hyrax.storage_adapter.find_by(id: file_metadata.file_identifier)
Expand Down Expand Up @@ -60,15 +61,19 @@ def prepare_file_headers_valkyrie(metadata:, file:, inline: false)
self.content_type = metadata.mime_type
end

def find_file_metadata(file_set:, use: :original_file)
use = :thumbnail_file if use == :thumbnail
begin
def find_file_metadata(file_set:, use: :original_file, mime_type: nil)
if mime_type.nil?
use = :thumbnail_file if use == :thumbnail
use = Hyrax::FileMetadata::Use.uri_for(use: use)
rescue ArgumentError
raise Hyrax::ObjectNotFoundError
results = Hyrax.custom_queries.find_many_file_metadata_by_use(resource: file_set, use: use)
else
files = Hyrax.custom_queries.find_files(file_set: file_set)
results = [files.find { |f| f.mime_type == mime_type }]
end
results = Hyrax.custom_queries.find_many_file_metadata_by_use(resource: file_set, use: use)

results.first || raise(Hyrax::ObjectNotFoundError)
rescue ArgumentError
raise(Hyrax::ObjectNotFoundError)
end
end
end
1 change: 1 addition & 0 deletions app/models/hyrax/file_metadata.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ module Use
ORIGINAL_FILE = ::Valkyrie::Vocab::PCDMUse.OriginalFile
EXTRACTED_TEXT = ::Valkyrie::Vocab::PCDMUse.ExtractedText
THUMBNAIL = ::Valkyrie::Vocab::PCDMUse.ThumbnailImage
SERVICE_FILE = ::Valkyrie::Vocab::PCDMUse.ServiceFile

##
# @param use [RDF::URI, Symbol]
Expand Down
8 changes: 4 additions & 4 deletions app/services/hyrax/file_set_derivatives_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -91,15 +91,15 @@ def create_office_document_derivatives(filename)

def create_audio_derivatives(filename)
Hydra::Derivatives::AudioDerivatives.create(filename,
outputs: [{ label: 'mp3', format: 'mp3', url: derivative_url('mp3'), mime_type: 'audio/mpeg' },
{ label: 'ogg', format: 'ogg', url: derivative_url('ogg'), mime_type: 'audio/ogg' }])
outputs: [{ label: 'mp3', format: 'mp3', url: derivative_url('mp3'), mime_type: 'audio/mpeg', container: 'service_file' },
{ label: 'ogg', format: 'ogg', url: derivative_url('ogg'), mime_type: 'audio/ogg', container: 'service_file' }])
end

def create_video_derivatives(filename)
Hydra::Derivatives::VideoDerivatives.create(filename,
outputs: [{ label: :thumbnail, format: 'jpg', url: derivative_url('thumbnail'), mime_type: 'image/jpeg' },
{ label: 'webm', format: 'webm', url: derivative_url('webm'), mime_type: 'video/webm' },
{ label: 'mp4', format: 'mp4', url: derivative_url('mp4'), mime_type: 'video/mp4' }])
{ label: 'webm', format: 'webm', url: derivative_url('webm'), mime_type: 'video/webm', container: 'service_file' },
{ label: 'mp4', format: 'mp4', url: derivative_url('mp4'), mime_type: 'video/mp4', container: 'service_file' }])
end

def create_image_derivatives(filename)
Expand Down
2 changes: 2 additions & 0 deletions app/services/hyrax/valkyrie_upload.rb
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,8 @@ def set_file_use_ids(file_set, file_metadata)
file_set.thumbnail_id = file_metadata.id
when Hyrax::FileMetadata::Use::EXTRACTED_TEXT
file_set.extracted_text_id = file_metadata.id
when Hyrax::FileMetadata::Use::SERVICE_FILE
# do nothing
else
Hyrax.logger.warn "Unknown file use #{file_metadata.type} specified for #{file_metadata.file_identifier}"
end
Expand Down
8 changes: 4 additions & 4 deletions app/views/hyrax/file_sets/media_display/_audio.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
<div>
<h2 class="sr-only"><%= t('hyrax.file_set.show.downloadable_content.heading') %></h2>
<audio controls="controls" class="audiojs" style="width:100%" controlsList="nodownload" preload="auto">
<source src="<%= hyrax.download_path(file_set, file: 'ogg') %>" type="audio/ogg" />
<source src="<%= hyrax.download_path(file_set, file: 'mp3') %>" type="audio/mpeg" />
<source src="<%= hyrax.download_path(file_set, file: 'ogg', mime_type: 'audio/ogg') %>" type="audio/ogg" />
<source src="<%= hyrax.download_path(file_set, file: 'mp3', mime_type: 'audio/mpeg') %>" type="audio/mpeg" />
<%= t('hyrax.file_set.show.downloadable_content.audio_tag_not_supported') %>
</audio>
<%= link_to t('hyrax.file_set.show.downloadable_content.audio_link'),
Expand All @@ -15,8 +15,8 @@
<% else %>
<div>
<audio controls="controls" class="audiojs" style="width:100%" controlsList="nodownload" preload="auto">
<source src="<%= hyrax.download_path(file_set, file: 'ogg') %>" type="audio/ogg" />
<source src="<%= hyrax.download_path(file_set, file: 'mp3') %>" type="audio/mpeg" />
<source src="<%= hyrax.download_path(file_set, file: 'ogg', mime_type: 'audio/ogg') %>" type="audio/ogg" />
<source src="<%= hyrax.download_path(file_set, file: 'mp3', mime_type: 'audio/mpeg') %>" type="audio/mpeg" />
<%= t('hyrax.file_set.show.downloadable_content.audio_tag_not_supported') %>
</audio>
</div>
Expand Down
4 changes: 2 additions & 2 deletions app/views/hyrax/file_sets/media_display/_video.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
<div>
<h2 class="sr-only"><%= t('hyrax.file_set.show.downloadable_content.heading') %></h2>
<video controls="controls" class="video-js vjs-default-skin" style="width:100%" data-setup="{}" controlsList="nodownload" preload="auto">
<source src="<%= hyrax.download_path(file_set, file: 'webm') %>" type="video/webm" />
<source src="<%= hyrax.download_path(file_set, file: 'mp4') %>" type="video/mp4" />
<source src="<%= hyrax.download_path(file_set, file: 'webm', mime_type: 'video/webm') %>" type="video/webm" />
<source src="<%= hyrax.download_path(file_set, file: 'mp4', mime_type: 'video/mp4') %>" type="video/mp4" />
<%= t('hyrax.file_set.show.downloadable_content.video_tag_not_supported') %>
</video>
<%= link_to t('hyrax.file_set.show.downloadable_content.video_link'),
Expand Down
26 changes: 26 additions & 0 deletions spec/controllers/hyrax/downloads_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,32 @@
end
end

context 'with video file' do
let(:service_file_use) { Hyrax::FileMetadata::Use::SERVICE_FILE }
let(:file_path) { fixture_path + '/sample_mpeg4.mp4' }
let(:service_file_metadata) { FactoryBot.valkyrie_create(:hyrax_file_metadata, use: service_file_use, mime_type: 'video/webm', file_identifier: "disk://#{file_path}") }
let(:file_set) do
if Hyrax.config.use_valkyrie?
FactoryBot.valkyrie_create(:hyrax_file_set,
:in_work,
files: [original_file_metadata, service_file_metadata],
edit_users: [user],
visibility_setting: Hydra::AccessControls::AccessRight::VISIBILITY_TEXT_VALUE_AUTHENTICATED)
else
create(:file_with_work, user: user, content: original_file)
end
end
before do
allow(subject).to receive(:authorize!).and_return(true)
allow(subject).to receive(:workflow_restriction?).and_return(false)
end

it 'accepts a mime_type param' do
get :show, params: { id: file_set, file: "webm", mime_type: 'video/webm' }
expect(response.body).to eq IO.binread(file_path)
end
end

context 'when restricted by workflow' do
before do
allow(subject).to receive(:authorize!).and_return(true)
Expand Down
14 changes: 10 additions & 4 deletions spec/services/hyrax/file_set_derivatives_service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,16 @@
FactoryBot.valkyrie_create(:hyrax_file_metadata, :audio_file, file_set_id: SecureRandom.uuid)
end

it "passes a mime-type to the audio derivatives service" do
it "passes a mime-type and container to the audio derivatives service" do
allow(Hydra::Derivatives::AudioDerivatives).to receive(:create)
described_class.new(valid_file_set).create_derivatives('foo')
expect(Hydra::Derivatives::AudioDerivatives).to have_received(:create).with('foo', outputs: contain_exactly(hash_including(mime_type: 'audio/mpeg'), hash_including(mime_type: 'audio/ogg')))
expect(Hydra::Derivatives::AudioDerivatives).to have_received(:create).with(
'foo',
outputs: contain_exactly(
hash_including(mime_type: 'audio/mpeg', container: 'service_file'),
hash_including(mime_type: 'audio/ogg', container: 'service_file')
)
)
end
end

Expand All @@ -47,8 +53,8 @@
expect(Hydra::Derivatives::VideoDerivatives).to have_received(:create).with(
'foo',
outputs: contain_exactly(
hash_including(mime_type: 'video/mp4'),
hash_including(mime_type: 'video/webm'),
hash_including(mime_type: 'video/mp4', container: 'service_file'),
hash_including(mime_type: 'video/webm', container: 'service_file'),
hash_including(mime_type: 'image/jpeg')
)
)
Expand Down
9 changes: 6 additions & 3 deletions spec/services/hyrax/valkyrie_persist_derivatives_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,17 @@

context "when given a mime_type directive" do
let(:directives) do
{ url: "file:///app/samvera/hyrax-webapp/derivatives/#{id}-webm.webm",
mime_type: 'video/webm' }
{ url: "file:///app/samvera/hyrax-webapp/derivatives/#{id}-mp4.mp4",
mime_type: 'video/webm',
container: 'service_file' }
end

it 'adds the mime_type to the file metadata' do
described_class.call(stream, directives)
files = Hyrax.custom_queries.find_files(file_set: file_set)
expect(files.first.mime_type).to eq 'video/webm'
file = files.first
expect(file.mime_type).to eq 'video/webm'
expect(file.pcdm_use).to eq [Hyrax::FileMetadata::Use::SERVICE_FILE]
end
end
end
Expand Down