From 148009ece54d1f2ffa44ddfa79bd956ab006bc0b Mon Sep 17 00:00:00 2001 From: Seth Call Date: Tue, 4 Feb 2014 20:28:00 +0000 Subject: [PATCH] * VRFS-1018 - audiomixer ruby runner generates an mp3 with metadata using ffmpeg --- admin/config/boot.rb | 4 +- db/manifest | 3 +- db/up/audiomixer_mp3.sql | 9 ++ ruby/lib/jam_ruby/models/mix.rb | 52 ++++++--- ruby/lib/jam_ruby/models/recording.rb | 6 +- ruby/lib/jam_ruby/resque/audiomixer.rb | 105 +++++++++++++------ ruby/spec/jam_ruby/models/mix_spec.rb | 16 ++- ruby/spec/jam_ruby/resque/audiomixer_spec.rb | 4 +- ruby/spec/support/utilities.rb | 4 + web/app/assets/javascripts/web/welcome.js | 4 +- web/app/controllers/api_mixes_controller.rb | 10 +- web/config/application.rb | 1 + web/config/boot.rb | 4 +- web/config/routes.rb | 1 - 14 files changed, 152 insertions(+), 71 deletions(-) diff --git a/admin/config/boot.rb b/admin/config/boot.rb index 2b59194e5..56e91c277 100644 --- a/admin/config/boot.rb +++ b/admin/config/boot.rb @@ -12,7 +12,9 @@ module Rails class Server alias :default_options_alias :default_options def default_options - default_options_alias.merge!(:Port => 3333) + default_options_alias.merge!( + :Port => 3333 + ENV['JAM_INSTANCE'].to_i, + :pid => File.expand_path("tmp/pids/server-#{ENV['JAM_INSTANCE'].to_i}.pid")) end end end \ No newline at end of file diff --git a/db/manifest b/db/manifest index 25fd574e1..042e0661c 100755 --- a/db/manifest +++ b/db/manifest @@ -97,4 +97,5 @@ icecast_config_changed.sql invited_users_facebook_support.sql first_recording_at.sql share_token.sql -facebook_signup.sql \ No newline at end of file +facebook_signup.sql +audiomixer_mp3.sql \ No newline at end of file diff --git a/db/up/audiomixer_mp3.sql b/db/up/audiomixer_mp3.sql index e69de29bb..d48cfed01 100644 --- a/db/up/audiomixer_mp3.sql +++ b/db/up/audiomixer_mp3.sql @@ -0,0 +1,9 @@ +-- add idea of a mix having mp3 as well as ogg + +ALTER TABLE mixes RENAME COLUMN md5 TO ogg_md5; +ALTER TABLE mixes RENAME COLUMN length TO ogg_length; +ALTER TABLE mixes RENAME COLUMN url TO ogg_url; + +ALTER TABLE mixes ADD COLUMN mp3_md5 VARCHAR(100); +ALTER TABLE mixes ADD COLUMN mp3_length INTEGER; +ALTER TABLE mixes ADD COLUMN mp3_url VARCHAR(1024); \ No newline at end of file diff --git a/ruby/lib/jam_ruby/models/mix.rb b/ruby/lib/jam_ruby/models/mix.rb index 66a58e1e5..9ec1c1e66 100644 --- a/ruby/lib/jam_ruby/models/mix.rb +++ b/ruby/lib/jam_ruby/models/mix.rb @@ -17,7 +17,8 @@ module JamRuby mix = Mix.new mix.recording = recording mix.save - mix.url = construct_filename(mix.created_at, recording.id, mix.id) + mix.ogg_url = construct_filename(mix.created_at, recording.id, mix.id, type='ogg') + mix.mp3_url = construct_filename(mix.created_at, recording.id, mix.id, type='mp3') if mix.save mix.enqueue end @@ -27,7 +28,7 @@ module JamRuby def enqueue begin - Resque.enqueue(AudioMixer, self.id, self.sign_put) + Resque.enqueue(AudioMixer, self.id, self.sign_put(3600 * 24, 'ogg'), self.sign_put(3600 * 24, 'mp3')) rescue # implies redis is down. we don't update started_at false @@ -51,10 +52,12 @@ module JamRuby save end - def finish(length, md5) + def finish(ogg_length, ogg_md5, mp3_length, mp3_md5) self.completed_at = Time.now - self.length = length - self.md5 = md5 + self.ogg_length = ogg_length + self.ogg_md5 = ogg_md5 + self.mp3_length = mp3_length + self.mp3_md5 = mp3_md5 self.completed = true if save Notification.send_recording_master_mix_complete(recording) @@ -74,41 +77,56 @@ module JamRuby manifest["timeline"] << { "timestamp" => 0, "mix" => mix_params } manifest["output"] = { "codec" => "vorbis" } - manifest["recording_id"] = self.id + manifest["recording_id"] = self.recording.id manifest end - def s3_url - s3_manager.s3_url(url) + def s3_url(type='ogg') + if type == 'ogg' + s3_manager.s3_url(ogg_url) + else + s3_manager.s3_url(mp3_url) + end + + end def is_completed completed end - def sign_url(expiration_time = 120) + def sign_url(expiration_time = 120, type='ogg') # expire link in 1 minute--the expectation is that a client is immediately following this link - s3_manager.sign_url(self.url, {:expires => expiration_time, :response_content_type => 'audio/ogg', :secure => false}) + if type == 'ogg' + s3_manager.sign_url(self.ogg_url, {:expires => expiration_time, :response_content_type => 'audio/ogg', :secure => false}) + else + s3_manager.sign_url(self.mp3_url, {:expires => expiration_time, :response_content_type => 'audio/mp3', :secure => false}) + end end - def sign_put(expiration_time = 3600 * 24) - s3_manager.sign_url(self.url, {:expires => expiration_time, :content_type => 'audio/ogg', :secure => false}, :put) + def sign_put(expiration_time = 3600 * 24, type='ogg') + if type == 'ogg' + s3_manager.sign_url(self.ogg_url, {:expires => expiration_time, :content_type => 'audio/ogg', :secure => false}, :put) + else + s3_manager.sign_url(self.mp3_url, {:expires => expiration_time, :content_type => 'audio/mp3', :secure => false}, :put) + end end private def delete_s3_files - s3_manager.delete(filename) + s3_manager.delete(filename(type='ogg')) + s3_manager.delete(filename(type='mp3')) end - def filename + def filename(type='ogg') # construct a path for s3 - Mix.construct_filename(self.created_at, self.recording.id, self.id) + Mix.construct_filename(self.created_at, self.recording.id, self.id, type) end - def self.construct_filename(created_at, recording_id, id) + def self.construct_filename(created_at, recording_id, id, type='ogg') raise "unknown ID" unless id - "recordings/#{created_at.strftime('%m-%d-%Y')}/#{recording_id}/mix-#{id}.ogg" + "recordings/#{created_at.strftime('%m-%d-%Y')}/#{recording_id}/mix-#{id}.#{type}" end end end diff --git a/ruby/lib/jam_ruby/models/recording.rb b/ruby/lib/jam_ruby/models/recording.rb index 450385067..738b319bb 100644 --- a/ruby/lib/jam_ruby/models/recording.rb +++ b/ruby/lib/jam_ruby/models/recording.rb @@ -214,9 +214,9 @@ module JamRuby :type => "mix", :id => mix.id.to_s, :recording_id => mix.recording_id, - :length => mix.length, - :md5 => mix.md5, - :url => mix.url, + :length => mix.ogg_length, + :md5 => mix.ogg_md5, + :url => mix.ogg_url, :created_at => mix.created_at, :next => mix.id } diff --git a/ruby/lib/jam_ruby/resque/audiomixer.rb b/ruby/lib/jam_ruby/resque/audiomixer.rb index 3424b11d0..dd1a304e0 100644 --- a/ruby/lib/jam_ruby/resque/audiomixer.rb +++ b/ruby/lib/jam_ruby/resque/audiomixer.rb @@ -13,14 +13,16 @@ module JamRuby @@log = Logging.logger[AudioMixer] - attr_accessor :mix_id, :manifest, :manifest_file, :output_filename, :error_out_filename, :postback_output_url, + attr_accessor :mix_id, :manifest, :manifest_file, :output_filename, :error_out_filename, + :postback_ogg_url, :postback_mp3_url, :error_reason, :error_detail - def self.perform(mix_id, postback_output_url) + def self.perform(mix_id, postback_ogg_url, postback_mp3_url) JamWebEventMachine.run_wait_stop do audiomixer = AudioMixer.new() - audiomixer.postback_output_url = postback_output_url + audiomixer.postback_ogg_url = postback_ogg_url + audiomixer.postback_mp3_url = postback_mp3_url audiomixer.mix_id = mix_id audiomixer.run end @@ -123,12 +125,13 @@ module JamRuby # make a suitable location to store the output mix, and pass the chosen filepath into the manifest def prepare_output - @output_filename = Dir::Tmpname.make_tmpname( ["#{Dir.tmpdir}/audiomixer-output-#{@manifest[:recording_id]}", '.ogg'], nil) + @output_ogg_filename = Dir::Tmpname.make_tmpname( ["#{Dir.tmpdir}/audiomixer-output-#{@manifest[:recording_id]}", '.ogg'], nil) + @output_mp3_filename = Dir::Tmpname.make_tmpname( ["#{Dir.tmpdir}/audiomixer-output-#{@manifest[:recording_id]}", '.mp3'], nil) # update manifest so that audiomixer writes here - @manifest[:output][:filename] = @output_filename + @manifest[:output][:filename] = @output_ogg_filename - @@log.debug("output ogg file: #{@output_filename}") + @@log.debug("output ogg file: #{@output_ogg_filename}, output mp3 file: #{@output_mp3_filename}") end # make a suitable location to store an output error file, which will be populated on failure to help diagnose problems. @@ -157,39 +160,62 @@ module JamRuby end def postback - raise "no output file after mix" unless File.exist? @output_filename - @@log.debug("posting mix to #{@postback_output_url}") + @@log.debug("posting ogg mix to #{@postback_ogg_url}") - uri = URI.parse(@postback_output_url) + uri = URI.parse(@postback_ogg_url) http = Net::HTTP.new(uri.host, uri.port) request = Net::HTTP::Put.new(uri.request_uri) response = nil - File.open(@output_filename,"r") do |f| + File.open(@output_ogg_filename,"r") do |f| request.body_stream=f request["Content-Type"] = "audio/ogg" - request.add_field('Content-Length', File.size(@output_filename)) + request.add_field('Content-Length', File.size(@output_ogg_filename)) response = http.request(request) end response_code = response.code.to_i unless response_code >= 200 && response_code <= 299 - @error_reason = "postback-mix-to-s3" - raise "unable to put to url: #{@postback_output_url}, status: #{response.code}, body: #{response.body}" + @error_reason = "postback-ogg-mix-to-s3" + raise "unable to put to url: #{@postback_ogg_url}, status: #{response.code}, body: #{response.body}" + end + + + @@log.debug("posting mp3 mix to #{@postback_mp3_url}") + + uri = URI.parse(@postback_mp3_url) + http = Net::HTTP.new(uri.host, uri.port) + request = Net::HTTP::Put.new(uri.request_uri) + + response = nil + File.open(@output_mp3_filename,"r") do |f| + request.body_stream=f + request["Content-Type"] = "audio/mp3" + request.add_field('Content-Length', File.size(@output_mp3_filename)) + response = http.request(request) + end + + response_code = response.code.to_i + unless response_code >= 200 && response_code <= 299 + @error_reason = "postback-mp3-mix-to-s3" + raise "unable to put to url: #{@postback_mp3_url}, status: #{response.code}, body: #{response.body}" end end def post_success(mix) - raise "no output file after mix" unless File.exist? @output_filename - length = File.size(@output_filename) + ogg_length = File.size(@output_ogg_filename) + ogg_md5 = Digest::MD5.new + File.open(@output_ogg_filename, 'rb').each {|line| ogg_md5.update(line)} - md5 = Digest::MD5.new - File.open(@output_filename, 'rb').each {|line| md5.update(line)} + mp3_length = File.size(@output_mp3_filename) + mp3_md5 = Digest::MD5.new + File.open(@output_mp3_filename, 'rb').each {|line| mp3_md5.update(line)} - mix.finish(length, md5.to_s) + + mix.finish(ogg_length, ogg_md5.to_s, mp3_length, mp3_md5.to_s) end def post_error(mix, e) @@ -233,16 +259,12 @@ module JamRuby execute(@manifest_file) - if $? == 0 - postback - post_success(mix) - @@log.info("audiomixer job successful. mix_id #{mix_id}") - else - parse_error_out - error_msg = "audiomixer job failed status=#{$?} error_reason=#{error_reason} error_detail=#{error_detail}" - @@log.info(error_msg) - raise error_msg - end + postback + + post_success(mix) + + @@log.info("audiomixer job successful. mix_id #{mix_id}") + rescue Exception => e post_error(mix, e) raise @@ -260,20 +282,41 @@ module JamRuby unless File.exist? APP_CONFIG.audiomixer_path @@log.error("unable to find audiomixer") - error_msg = "audiomixer job failed status=#{$?} error_reason=#{error_reason} error_detail=#{error_detail}" + error_msg = "audiomixer job failed status=#{$?} error_reason=#{@error_reason} error_detail=#{@error_detail}" @@log.info(error_msg) @error_reason = "unable-find-appmixer" @error_detail = APP_CONFIG.audiomixer_path raise error_msg end - audiomixer_cmd = "#{APP_CONFIG.audiomixer_path} #{manifest_file}" - @@log.debug("executing #{audiomixer_cmd}") system(audiomixer_cmd) + + unless $? == 0 + parse_error_out + error_msg = "audiomixer job failed status=#{$?} error_reason=#{@error_reason} error_detail=#{@error_detail}" + @@log.info(error_msg) + raise error_msg + end + + raise "no output ogg file after mix" unless File.exist? @output_ogg_filename + + ffmpeg_cmd = "#{APP_CONFIG.ffmpeg_path} -i \"#{@output_ogg_filename}\" -ab 128k -metadata JamRecordingId=#{@manifest[:recording_id]} -metadata JamMixId=#{@mix_id} -metadata JamType=Mix \"#{@output_mp3_filename}\"" + + system(ffmpeg_cmd) + + unless $? == 0 + @error_reason = 'ffmpeg-failed' + @error_detail = $?.to_s + error_msg = "ffmpeg failed status=#{$?} error_reason=#{@error_reason} error_detail=#{@error_detail}" + @@log.info(error_msg) + raise error_msg + end + + raise "no output mp3 file after conversion" unless File.exist? @output_mp3_filename end def symbolize_keys(obj) diff --git a/ruby/spec/jam_ruby/models/mix_spec.rb b/ruby/spec/jam_ruby/models/mix_spec.rb index bb361bc99..359100423 100755 --- a/ruby/spec/jam_ruby/models/mix_spec.rb +++ b/ruby/spec/jam_ruby/models/mix_spec.rb @@ -26,11 +26,19 @@ describe Mix do end it "should record when a mix has finished" do - Mix.find(@mix.id).finish(10000, "md5hash") + Mix.find(@mix.id).finish(10000, "md5hash", 10000, "md5hash") @mix.reload @mix.completed_at.should_not be_nil - @mix.length.should == 10000 - @mix.md5.should == "md5hash" + @mix.ogg_length.should == 10000 + @mix.ogg_md5.should == "md5hash" + end + + it "create a good manifest" do + Mix.find(@mix.id).finish(10000, "md5hash", 10000, "md5hash") + @mix.reload + manifest = @mix.manifest + manifest["recording_id"].should == @recording.id + manifest["files"].length.should == 1 end it "signs url" do @@ -40,7 +48,7 @@ describe Mix do it "mixes are restricted by user" do - @mix.finish(1, "abc") + @mix.finish(1, "abc", 1, "def") @mix.reload @mix.errors.any?.should be_false diff --git a/ruby/spec/jam_ruby/resque/audiomixer_spec.rb b/ruby/spec/jam_ruby/resque/audiomixer_spec.rb index 5b27e35d7..5411acc39 100644 --- a/ruby/spec/jam_ruby/resque/audiomixer_spec.rb +++ b/ruby/spec/jam_ruby/resque/audiomixer_spec.rb @@ -233,8 +233,8 @@ describe AudioMixer do @mix.reload @mix.completed.should be_true - @mix.length.should == 0 - @mix.md5.should == 'd41d8cd98f00b204e9800998ecf8427e' # that's the md5 of a touched file, which is what the stubbed :execute does + @mix.ogg_length.should == 0 + @mix.ogg_md5.should == 'd41d8cd98f00b204e9800998ecf8427e' # that's the md5 of a touched file, which is what the stubbed :execute does end it "bails out with no error if already completed" do diff --git a/ruby/spec/support/utilities.rb b/ruby/spec/support/utilities.rb index 52d00aea6..8886ac7bb 100644 --- a/ruby/spec/support/utilities.rb +++ b/ruby/spec/support/utilities.rb @@ -21,6 +21,10 @@ def app_config ENV['AUDIOMIXER_PATH'] || audiomixer_workspace_path || "/var/lib/audiomixer/audiomixer/audiomixerapp" end + def ffmpeg_path + ENV['FFMPEG_PATH'] || '/usr/local/bin/ffmpeg' + end + def icecast_reload_cmd 'true' # as in, /bin/true end diff --git a/web/app/assets/javascripts/web/welcome.js b/web/app/assets/javascripts/web/welcome.js index 55ed205a1..582e6d220 100644 --- a/web/app/assets/javascripts/web/welcome.js +++ b/web/app/assets/javascripts/web/welcome.js @@ -18,6 +18,8 @@ }); } - initialize() + $(function() { + initialize(); + }) })(window, jQuery); \ No newline at end of file diff --git a/web/app/controllers/api_mixes_controller.rb b/web/app/controllers/api_mixes_controller.rb index 60944c480..26e9ded8d 100644 --- a/web/app/controllers/api_mixes_controller.rb +++ b/web/app/controllers/api_mixes_controller.rb @@ -20,15 +20,7 @@ class ApiMixesController < ApiController render :json => { :message => "next mix could not be found" }, :status => 403 end end - - def finish - begin - @mix.finish - rescue - render :json => { :message => "mix finish failed" }, :status => 403 - end - respond_with responder: ApiResponder, :status => 204 - end + def download @mix = Mix.find(params[:id]) diff --git a/web/config/application.rb b/web/config/application.rb index c913c0a9e..02806981d 100644 --- a/web/config/application.rb +++ b/web/config/application.rb @@ -173,6 +173,7 @@ include JamRuby config.redis_host = "localhost:6379" config.audiomixer_path = "/var/lib/audiomixer/audiomixer/audiomixerapp" + config.ffmpeg_path = ENV['FFMPEG_PATH'] || (File.exist?('/usr/local/bin/ffmpeg') ? '/usr/local/bin/ffmpeg' : '/usr/bin/ffmpeg') # if it looks like linux, use init.d script; otherwise use kill config.icecast_reload_cmd = ENV['ICECAST_RELOAD_CMD'] || (File.exist?('/usr/local/bin/icecast2') ? "bash -l -c #{Shellwords.escape("sudo /etc/init.d/icecast2 reload")}" : "bash -l -c #{Shellwords.escape("kill -1 `ps -f | grep /usr/local/bin/icecast | grep -v grep | awk \'{print $2}\'`")}") diff --git a/web/config/boot.rb b/web/config/boot.rb index e582639f1..fa8246936 100644 --- a/web/config/boot.rb +++ b/web/config/boot.rb @@ -11,7 +11,9 @@ module Rails class Server alias :default_options_alias :default_options def default_options - default_options_alias.merge!(:Port => 3000 + ENV['JAM_INSTANCE'].to_i) + default_options_alias.merge!( + :Port => 3000 + ENV['JAM_INSTANCE'].to_i, + :pid => File.expand_path("tmp/pids/server-#{ENV['JAM_INSTANCE'].to_i}.pid")) end end end \ No newline at end of file diff --git a/web/config/routes.rb b/web/config/routes.rb index 41b3a003c..a0c08691c 100644 --- a/web/config/routes.rb +++ b/web/config/routes.rb @@ -317,7 +317,6 @@ SampleApp::Application.routes.draw do # Mixes match '/mixes/schedule' => 'api_mixes#schedule', :via => :post match '/mixes/next' => 'api_mixes#next', :via => :get - match '/mixes/:id/finish' => 'api_mixes#finish', :via => :put match '/mixes/:id/download' => 'api_mixes#download', :via => :get # version check for JamClient