From f89139bcff80d7fb8a6e1a3be7850d7f389b5c4b Mon Sep 17 00:00:00 2001 From: Mason Ballengee Date: Wed, 13 Mar 2024 16:31:11 -0400 Subject: [PATCH] Remove captions from HLS manifest --- app/controllers/master_files_controller.rb | 14 +----- .../master_files/caption_manifest.m3u8.erb | 23 ---------- app/views/master_files/hls_manifest.m3u8.erb | 17 +------ .../modules/player/_video_js_element.html.erb | 5 +-- config/application.rb | 1 - config/routes.rb | 1 - .../master_files_controller_spec.rb | 45 ------------------- spec/routing/master_files_routing_spec.rb | 3 -- 8 files changed, 4 insertions(+), 105 deletions(-) delete mode 100644 app/views/master_files/caption_manifest.m3u8.erb diff --git a/app/controllers/master_files_controller.rb b/app/controllers/master_files_controller.rb index a26932ea7b..31f2011ffe 100644 --- a/app/controllers/master_files_controller.rb +++ b/app/controllers/master_files_controller.rb @@ -21,8 +21,8 @@ class MasterFilesController < ApplicationController include NoidValidator before_action :authenticate_user!, :only => [:create] - before_action :set_masterfile_proxy, except: [:create, :oembed, :attach_structure, :attach_captions, :delete_structure, :delete_captions, :destroy, :update, :set_structure] - before_action :set_masterfile, only: [:attach_structure, :attach_captions, :delete_structure, :delete_captions, :destroy, :update, :set_structure] + before_action :set_masterfile_proxy, except: [:create, :oembed, :attach_structure, :delete_structure, :destroy, :update, :set_structure] + before_action :set_masterfile, only: [:attach_structure, :delete_structure, :destroy, :update, :set_structure] before_action :ensure_readable_filedata, :only => [:create] skip_before_action :verify_authenticity_token, only: [:set_structure, :delete_structure] @@ -280,16 +280,6 @@ def hls_manifest end end - def caption_manifest - return head :unauthorized if cannot?(:read, @master_file) - caption_id = params[:c_id] - @caption_url = if caption_id == 'master_file_caption' - captions_master_file_path - else - captions_master_file_supplemental_file_path(master_file_id: @master_file.id, id: caption_id) - end - end - def structure authorize! :read, @master_file, message: "You do not have sufficient privileges" render json: @master_file.structuralMetadata.as_json diff --git a/app/views/master_files/caption_manifest.m3u8.erb b/app/views/master_files/caption_manifest.m3u8.erb deleted file mode 100644 index c2101f5e9b..0000000000 --- a/app/views/master_files/caption_manifest.m3u8.erb +++ /dev/null @@ -1,23 +0,0 @@ -<%# -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 --- -%> -#EXTM3U -#EXT-X-TARGETDURATION:<%= @master_file.duration %> -#EXT-X-VERSION:3 -#EXT-X-MEDIA-SEQUENCE:0 -#EXT-X-PLAYLIST-TYPE:VOD -#EXTINF:<%= @master_file.duration %> -<%= @caption_url %> -#EXT-X-ENDLIST diff --git a/app/views/master_files/hls_manifest.m3u8.erb b/app/views/master_files/hls_manifest.m3u8.erb index 5a6d528854..7b2db72b83 100644 --- a/app/views/master_files/hls_manifest.m3u8.erb +++ b/app/views/master_files/hls_manifest.m3u8.erb @@ -14,22 +14,7 @@ Unless required by applicable law or agreed to in writing, software distributed --- END LICENSE_HEADER BLOCK --- %> #EXTM3U -<% if @master_file.has_captions? %> -<% captions_list = @master_file.supplemental_file_captions %> -<% captions_list.append(@master_file.captions) if @master_file.captions&.content %> -<% captions_list.each_with_index do |caption, index| %> -<% label = caption.is_a?(SupplementalFile) ? caption.label : 'English' %> -<% language = caption.is_a?(SupplementalFile) ? caption.language : 'en' %> -<% id = caption.is_a?(SupplementalFile) ? caption.id : 'master_file_caption' %> -#EXT-X-MEDIA:TYPE=SUBTITLES,GROUP-ID="subs",LANGUAGE="<%= language %>",NAME="<%= label %>",DEFAULT=<%= index == 0 ? "YES" : "NO" %>,AUTOSELECT=YES,URI="<%= caption_manifest_master_file_path(c_id: id) %>" -<% end %> -<% @hls_streams.each do |hls| %> -#EXT-X-STREAM-INF:BANDWIDTH=<%= hls[:bitrate] %>,SUBTITLES="subs" -<%= hls[:url] %> -<% end %> -<% else %> <% @hls_streams.each do |hls| %> #EXT-X-STREAM-INF:BANDWIDTH=<%= hls[:bitrate] %> <%= hls[:url] %> -<% end %> -<% end %> +<% end %> \ No newline at end of file diff --git a/app/views/modules/player/_video_js_element.html.erb b/app/views/modules/player/_video_js_element.html.erb index 24c8afe746..38e869a58b 100644 --- a/app/views/modules/player/_video_js_element.html.erb +++ b/app/views/modules/player/_video_js_element.html.erb @@ -64,10 +64,7 @@ Unless required by applicable law or agreed to in writing, software distributed <% section_info[:stream_hls].each do |hls| %> <% end %> - <%# Captions are contained in the HLS manifest and so we do not need to manually provide them to VideoJS here %> - <%# TODO: Reenable if/when we remove captions from HLS %> - <% skip_captions = true %> - <% if section_info[:caption_paths].present? && !skip_captions %> + <% if section_info[:caption_paths].present? %> <% section_info[:caption_paths].each do |c| %> label="<%= c[:label] %>" <% end %> srclang="<%= c[:language] %>" kind="subtitles" type="<%= c[:mime_type] %>" src="<%= c[:path] %>"> <% end %> diff --git a/config/application.rb b/config/application.rb index 032119dc35..18a8f037b7 100644 --- a/config/application.rb +++ b/config/application.rb @@ -48,7 +48,6 @@ class Application < Rails::Application resource '/master_files/*/structure.json', headers: :any, methods: [:get, :post, :delete] resource '/master_files/*/waveform.json', headers: :any, methods: [:get] resource '/master_files/*/*.m3u8', headers: :any, credentials: true, methods: [:get, :head] - resource '/master_files/*/caption_manifest/*', headers: :any, methods: [:get] resource '/master_files/*/captions', headers: :any, methods: [:get] resource '/master_files/*/supplemental_files/*', headers: :any, methods: [:get] resource '/playlists/*/manifest.json', headers: :any, credentials: true, methods: [:get] diff --git a/config/routes.rb b/config/routes.rb index 923fcf5bc6..3f803d67d1 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -155,7 +155,6 @@ get :captions get :waveform match ':quality.m3u8', to: 'master_files#hls_manifest', via: [:get], as: :hls_manifest - get 'caption_manifest/:c_id', to: 'master_files#caption_manifest', as: :caption_manifest get 'structure', to: 'master_files#structure', constraints: { format: 'json' } post 'structure', to: 'master_files#set_structure', constraints: { format: 'json' } delete 'structure', to: 'master_files#delete_structure', constraints: { format: 'json' } diff --git a/spec/controllers/master_files_controller_spec.rb b/spec/controllers/master_files_controller_spec.rb index fbe2f17e94..0f2fa35d41 100644 --- a/spec/controllers/master_files_controller_spec.rb +++ b/spec/controllers/master_files_controller_spec.rb @@ -647,51 +647,6 @@ class << file end end - describe '#caption_manifest' do - let(:media_object) { FactoryBot.create(:published_media_object) } - let(:master_file) { FactoryBot.create(:master_file, :with_captions, media_object: media_object) } - let(:public_media_object) { FactoryBot.create(:published_media_object, visibility: 'public') } - let(:public_master_file) { FactoryBot.create(:master_file, :with_captions, media_object: public_media_object) } - - context 'master file has been deleted' do - before do - master_file.destroy - end - - it 'returns unauthorized (401)' do - expect(get('caption_manifest', params: { id: master_file.id, c_id: 1 }, xhr: true)).to have_http_status(:unauthorized) - end - end - - it 'returns unauthorized (401) if cannot read the master file' do - expect(get('caption_manifest', params: { id: master_file.id, c_id: 1 }, xhr: true)).to have_http_status(:unauthorized) - end - - it 'returns the caption manifest' do - login_as :administrator - expect(get('caption_manifest', params: { id: master_file.id, c_id: 1 }, xhr: true)).to have_http_status(:ok) - expect(response.content_type).to eq 'application/x-mpegURL; charset=utf-8' - end - - it 'returns a manifest if public' do - expect(get('caption_manifest', params: { id: public_master_file.id, c_id: 1 }, xhr: true)).to have_http_status(:ok) - expect(response.content_type).to eq 'application/x-mpegURL; charset=utf-8' - expect(get('caption_manifest', params: { id: public_master_file.id, c_id: 'master_file_caption' }, xhr: true)).to have_http_status(:ok) - expect(response.content_type).to eq 'application/x-mpegURL; charset=utf-8' - end - - context 'read from solr' do - it 'should not read from fedora' do - public_master_file - perform_enqueued_jobs(only: MediaObjectIndexingJob) - WebMock.reset_executed_requests! - login_as :administrator - get('caption_manifest', params: { id: public_master_file.id, c_id: 1 }, xhr: true) - expect(a_request(:any, /#{ActiveFedora.fedora.base_uri}/)).not_to have_been_made - end - end - end - describe '#move' do let(:master_file) { FactoryBot.create(:master_file, :with_media_object) } let(:target_media_object) { FactoryBot.create(:media_object) } diff --git a/spec/routing/master_files_routing_spec.rb b/spec/routing/master_files_routing_spec.rb index 8fb7bf4c94..8be9b032c2 100644 --- a/spec/routing/master_files_routing_spec.rb +++ b/spec/routing/master_files_routing_spec.rb @@ -19,8 +19,5 @@ it "routes to #move" do expect(:post => "/master_files/abc1234/move").to route_to("master_files#move", id: 'abc1234') end - it "routes to #caption_manifest" do - expect(:get => "/master_files/abc1234/caption_manifest/def5678").to route_to("master_files#caption_manifest", id: 'abc1234', c_id: 'def5678') - end end end