Merge pull request #5735 from avalonmediasystem/remove_hls_captions

Remove captions from HLS manifest

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

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 %>

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| %>
         <source src="<%= hls[:url] %>" type="application/x-mpegURL" data-quality="<%= hls[:quality] %>" label="<%= hls[:quality] %>"/>
       <% 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| %>
           <track <% if c[:label] %>label="<%= c[:label] %>" <% end %> srclang="<%= c[:language] %>" kind="subtitles" type="<%= c[:mime_type] %>" src="<%= c[:path] %>"></track>
         <% end %>

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]

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' }

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) }

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