From 970401374b249f377085b52898163f21c7875f31 Mon Sep 17 00:00:00 2001 From: Seth Call Date: Mon, 27 Oct 2014 12:42:30 -0500 Subject: [PATCH 1/8] * VRFS-2399 - fixed recording sync track issue --- ruby/lib/jam_ruby/models/recording.rb | 2 ++ ruby/lib/jam_ruby/resque/audiomixer.rb | 4 ++- web/app/assets/javascripts/sessionModel.js | 32 ++++++++++++---------- 3 files changed, 22 insertions(+), 16 deletions(-) diff --git a/ruby/lib/jam_ruby/models/recording.rb b/ruby/lib/jam_ruby/models/recording.rb index 8ebe32436..a20acd869 100644 --- a/ruby/lib/jam_ruby/models/recording.rb +++ b/ruby/lib/jam_ruby/models/recording.rb @@ -235,6 +235,8 @@ module JamRuby keep(user) end + puts "claimed_recording.save #{claimed_recording.errors.inspect}" + claimed_recording end diff --git a/ruby/lib/jam_ruby/resque/audiomixer.rb b/ruby/lib/jam_ruby/resque/audiomixer.rb index 1b9065d3d..d81fd0bd9 100644 --- a/ruby/lib/jam_ruby/resque/audiomixer.rb +++ b/ruby/lib/jam_ruby/resque/audiomixer.rb @@ -31,7 +31,9 @@ module JamRuby def self.queue_jobs_needing_retry Mix.find_each(:conditions => "completed = FALSE AND (should_retry = TRUE OR (started_at IS NOT NULL AND NOW() - started_at > '1 hour'::INTERVAL))", :batch_size => 100) do |mix| - mix.enqueue + mix.enq only some of the tracks for client could be located + ueue + end end end diff --git a/web/app/assets/javascripts/sessionModel.js b/web/app/assets/javascripts/sessionModel.js index 82a8724b0..7f7f69d2d 100644 --- a/web/app/assets/javascripts/sessionModel.js +++ b/web/app/assets/javascripts/sessionModel.js @@ -538,26 +538,28 @@ if(inSession() && text == "RebuildAudioIoControl") { - if(sessionPageEnterDeferred) { - // this means we are still waiting for the BACKEND_MIXER_CHANGE that indicates we have user tracks built-out/ready - - // we will get at least one BACKEND_MIXER_CHANGE that corresponds to the backend doing a 'audio pause', which won't matter much - // so we need to check that we actaully have userTracks before considering ourselves done - var inputTracks = context.JK.TrackHelpers.getUserTracks(context.jamClient); - if(inputTracks.length > 0) { - logger.debug("obtained tracks at start of session") - sessionPageEnterDeferred.resolve(inputTracks); - sessionPageEnterDeferred = null; - } - - return; - } - // the backend will send these events rapid-fire back to back. // the server can still perform correctly, but it is nicer to wait 100 ms to let them all fall through if(backendMixerAlertThrottleTimer) {clearTimeout(backendMixerAlertThrottleTimer);} backendMixerAlertThrottleTimer = setTimeout(function() { + + if(sessionPageEnterDeferred) { + // this means we are still waiting for the BACKEND_MIXER_CHANGE that indicates we have user tracks built-out/ready + + // we will get at least one BACKEND_MIXER_CHANGE that corresponds to the backend doing a 'audio pause', which won't matter much + // so we need to check that we actaully have userTracks before considering ourselves done + var inputTracks = context.JK.TrackHelpers.getUserTracks(context.jamClient); + if(inputTracks.length > 0) { + logger.debug("obtained tracks at start of session") + sessionPageEnterDeferred.resolve(inputTracks); + sessionPageEnterDeferred = null; + } + + return; + } + + // this is a local change to our tracks. we need to tell the server about our updated track information var inputTracks = context.JK.TrackHelpers.getUserTracks(context.jamClient); From a12e6335eb6aff3327464b32a0a12f77ec4fb1e0 Mon Sep 17 00:00:00 2001 From: Seth Call Date: Mon, 27 Oct 2014 13:26:04 -0500 Subject: [PATCH 2/8] * VRFS-2403 and VRFS-2403 - deal with leaving a session while joining, and with sync track info while joining --- web/app/assets/javascripts/sessionModel.js | 88 ++++++++++++++-------- 1 file changed, 57 insertions(+), 31 deletions(-) diff --git a/web/app/assets/javascripts/sessionModel.js b/web/app/assets/javascripts/sessionModel.js index 7f7f69d2d..9bfb3409d 100644 --- a/web/app/assets/javascripts/sessionModel.js +++ b/web/app/assets/javascripts/sessionModel.js @@ -31,6 +31,7 @@ var sessionPageEnterDeferred = null; var sessionPageEnterTimeout = null; var startTime = null; + var joinDeferred = null; server.registerOnSocketClosed(onWebsocketDisconnected); @@ -123,10 +124,18 @@ */ function joinSession(sessionId) { logger.debug("SessionModel.joinSession(" + sessionId + ")"); - var deferred = joinSessionRest(sessionId); + joinDeferred = joinSessionRest(sessionId); + + joinDeferred + .done(function(response){ + + if(!inSession()) { + // the user has left the session before they got joined. We need to issue a leave again to the server to make sure they are out + logger.debug("user left before fully joined to session. telling server again that they have left") + leaveSessionRest(response.id); + return; + } - deferred - .done(function(response){ logger.debug("calling jamClient.JoinSession"); // on temporary disconnect scenarios, a user may already be in a session when they enter this path // so we avoid double counting @@ -153,7 +162,7 @@ updateCurrentSession(null); }); - return deferred; + return joinDeferred; } function performLeaveSession(deferred) { @@ -251,13 +260,15 @@ } // universal place to clean up, reset items - function sessionEnded() { + // fullyJoined means the user stayed in the screen until they had a successful rest.joinSession + // you might not get a fully joined if you join/leave really quickly before the REST API completes + function sessionEnded(fullyJoined) { //cleanup server.unregisterMessageCallback(context.JK.MessageType.SESSION_JOIN, trackChanges); server.unregisterMessageCallback(context.JK.MessageType.SESSION_DEPART, trackChanges); server.unregisterMessageCallback(context.JK.MessageType.TRACKS_CHANGED, trackChanges); - server.registerMessageCallback(context.JK.MessageType.HEARTBEAT_ACK, trackChanges); + server.unregisterMessageCallback(context.JK.MessageType.HEARTBEAT_ACK, trackChanges); if(sessionPageEnterDeferred != null) { sessionPageEnterDeferred.reject('session_over'); @@ -265,7 +276,10 @@ } userTracks = null; startTime = null; - $(document).trigger(EVENTS.SESSION_ENDED, {session: {id: currentSessionId}}); + joinDeferred = null; + if(fullyJoined) { + $(document).trigger(EVENTS.SESSION_ENDED, {session: {id: currentSessionId}}); + } currentSessionId = null; } @@ -280,8 +294,8 @@ currentSession = sessionData; // the 'beforeUpdate != null' makes sure we only do a clean up one time internally - if(sessionData == null && beforeUpdate != null) { - sessionEnded(); + if(sessionData == null) { + sessionEnded(beforeUpdate != null); } } @@ -559,32 +573,44 @@ return; } - - // this is a local change to our tracks. we need to tell the server about our updated track information - var inputTracks = context.JK.TrackHelpers.getUserTracks(context.jamClient); + // wait until we are fully in session before trying to sync tracks to server + if(joinDeferred) { + joinDeferred.done(function() { - // create a trackSync request based on backend data - var syncTrackRequest = {}; - syncTrackRequest.client_id = app.clientId; - syncTrackRequest.tracks = inputTracks; - syncTrackRequest.id = id(); - - rest.putTrackSyncChange(syncTrackRequest) - .done(function() { - }) - .fail(function(jqXHR) { - if(jqXHR.status != 404) { - app.notify({ - "title": "Can't Sync Local Tracks", - "text": "The client is unable to sync local track information with the server. You should rejoin the session to ensure a good experience.", - "icon_url": "/assets/content/icon_alert_big.png" - }); - } - else { - logger.debug("Unable to sync local tracks because session is gone.") + // double check that we are in session, since a bunch could have happened since then + if(!inSession()) { + logger.debug("dropping queued up sync tracks because no longer in session"); + return; } + // this is a local change to our tracks. we need to tell the server about our updated track information + var inputTracks = context.JK.TrackHelpers.getUserTracks(context.jamClient); + + // create a trackSync request based on backend data + var syncTrackRequest = {}; + syncTrackRequest.client_id = app.clientId; + syncTrackRequest.tracks = inputTracks; + syncTrackRequest.id = id(); + + rest.putTrackSyncChange(syncTrackRequest) + .done(function() { + }) + .fail(function(jqXHR) { + if(jqXHR.status != 404) { + app.notify({ + "title": "Can't Sync Local Tracks", + "text": "The client is unable to sync local track information with the server. You should rejoin the session to ensure a good experience.", + "icon_url": "/assets/content/icon_alert_big.png" + }); + } + else { + logger.debug("Unable to sync local tracks because session is gone.") + } + + }) }) + } + }, 100); } else if(inSession() && (text == 'RebuildMediaControl' || text == 'RebuildRemoteUserControl')) { From bbff3ccab54b7aa8959f43b4f87486880f17af3b Mon Sep 17 00:00:00 2001 From: Seth Call Date: Mon, 27 Oct 2014 13:50:44 -0500 Subject: [PATCH 3/8] * remove debug print --- ruby/lib/jam_ruby/models/recording.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/ruby/lib/jam_ruby/models/recording.rb b/ruby/lib/jam_ruby/models/recording.rb index a20acd869..8ebe32436 100644 --- a/ruby/lib/jam_ruby/models/recording.rb +++ b/ruby/lib/jam_ruby/models/recording.rb @@ -235,8 +235,6 @@ module JamRuby keep(user) end - puts "claimed_recording.save #{claimed_recording.errors.inspect}" - claimed_recording end From 5f87fdab00abafdf1762aa835457a36a5313a876 Mon Sep 17 00:00:00 2001 From: Seth Call Date: Mon, 27 Oct 2014 13:51:29 -0500 Subject: [PATCH 4/8] * bah adding junk to AudioMixer file --- ruby/lib/jam_ruby/resque/audiomixer.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/ruby/lib/jam_ruby/resque/audiomixer.rb b/ruby/lib/jam_ruby/resque/audiomixer.rb index d81fd0bd9..1b9065d3d 100644 --- a/ruby/lib/jam_ruby/resque/audiomixer.rb +++ b/ruby/lib/jam_ruby/resque/audiomixer.rb @@ -31,9 +31,7 @@ module JamRuby def self.queue_jobs_needing_retry Mix.find_each(:conditions => "completed = FALSE AND (should_retry = TRUE OR (started_at IS NOT NULL AND NOW() - started_at > '1 hour'::INTERVAL))", :batch_size => 100) do |mix| - mix.enq only some of the tracks for client could be located - ueue - end + mix.enqueue end end From fb36b8e38008a8786ab60324e090f0f9ce123efd Mon Sep 17 00:00:00 2001 From: Seth Call Date: Mon, 27 Oct 2014 17:14:40 -0500 Subject: [PATCH 5/8] * VRFS-2408 - remove dup tracks from interfrace --- db/manifest | 1 + db/up/user_syncs_fix_dup_tracks_2408.sql | 32 ++++++++++++++ ruby/spec/jam_ruby/models/user_sync_spec.rb | 48 +++++++++++++++++++++ 3 files changed, 81 insertions(+) create mode 100644 db/up/user_syncs_fix_dup_tracks_2408.sql diff --git a/db/manifest b/db/manifest index 6342f4c83..c8652bebf 100755 --- a/db/manifest +++ b/db/manifest @@ -224,3 +224,4 @@ emails_from_update2.sql add_youtube_flag_to_claimed_recordings.sql add_session_create_type.sql user_syncs_and_quick_mix.sql +user_syncs_fix_dup_tracks_2408.sql diff --git a/db/up/user_syncs_fix_dup_tracks_2408.sql b/db/up/user_syncs_fix_dup_tracks_2408.sql new file mode 100644 index 000000000..2d98e8940 --- /dev/null +++ b/db/up/user_syncs_fix_dup_tracks_2408.sql @@ -0,0 +1,32 @@ +DROP VIEW user_syncs; + +CREATE VIEW user_syncs AS + SELECT DISTINCT b.id AS recorded_track_id, + CAST(NULL as BIGINT) AS mix_id, + CAST(NULL as BIGINT) as quick_mix_id, + b.id AS unified_id, + a.user_id AS user_id, + b.fully_uploaded, + recordings.created_at AS created_at, + recordings.id AS recording_id + FROM recorded_tracks a INNER JOIN recordings ON a.recording_id = recordings.id AND duration IS NOT NULL AND all_discarded = FALSE INNER JOIN recorded_tracks b ON a.recording_id = b.recording_id + UNION ALL + SELECT CAST(NULL as BIGINT) AS recorded_track_id, + mixes.id AS mix_id, + CAST(NULL as BIGINT) AS quick_mix_id, + mixes.id AS unified_id, + claimed_recordings.user_id AS user_id, + NULL as fully_uploaded, + recordings.created_at AS created_at, + recordings.id AS recording_id + FROM mixes INNER JOIN recordings ON mixes.recording_id = recordings.id INNER JOIN claimed_recordings ON recordings.id = claimed_recordings.recording_id WHERE claimed_recordings.discarded = FALSE + UNION ALL + SELECT CAST(NULL as BIGINT) AS recorded_track_id, + CAST(NULL as BIGINT) AS mix_id, + quick_mixes.id AS quick_mix_id, + quick_mixes.id AS unified_id, + quick_mixes.user_id, + quick_mixes.fully_uploaded, + recordings.created_at AS created_at, + recordings.id AS recording_id + FROM quick_mixes INNER JOIN recordings ON quick_mixes.recording_id = recordings.id AND duration IS NOT NULL AND all_discarded = FALSE; diff --git a/ruby/spec/jam_ruby/models/user_sync_spec.rb b/ruby/spec/jam_ruby/models/user_sync_spec.rb index 4781200b5..349cf706e 100644 --- a/ruby/spec/jam_ruby/models/user_sync_spec.rb +++ b/ruby/spec/jam_ruby/models/user_sync_spec.rb @@ -167,6 +167,54 @@ describe UserSync do user_syncs.length.should == 0 end end + + describe "one recording with multi-track users" do + let!(:recording1) { + recording = FactoryGirl.create(:recording, owner: user1, band: nil, duration:1) + recording.recorded_tracks << FactoryGirl.create(:recorded_track, recording: recording, user: recording.owner) + recording.recorded_tracks << FactoryGirl.create(:recorded_track, recording: recording, user: recording.owner) + recording.recorded_tracks << FactoryGirl.create(:recorded_track, recording: recording, user: user2) + recording.recorded_tracks << FactoryGirl.create(:recorded_track, recording: recording, user: user2) + recording.save! + recording.reload + recording + } + + let(:sorted_tracks) { + Array.new(recording1.recorded_tracks).sort! {|a, b| + if a.created_at == b.created_at + a.id <=> b.id + else + a.created_at <=> b.created_at + end + } + } + + + it "one user decides to keep the recording" do + claimed_recording = FactoryGirl.create(:claimed_recording, user: user1, recording: recording1, discarded:false) + claimed_recording.recording.should == recording1 + + data = UserSync.index({user_id: user1.id}) + data[:next].should be_nil + user_syncs = data[:query] + user_syncs.length.should == 4 + user_syncs[0].recorded_track.should == sorted_tracks[0] + user_syncs[1].recorded_track.should == sorted_tracks[1] + user_syncs[2].recorded_track.should == sorted_tracks[2] + user_syncs[3].recorded_track.should == sorted_tracks[3] + + data = UserSync.index({user_id: user2.id}) + data[:next].should be_nil + user_syncs = data[:query] + user_syncs.length.should == 4 + user_syncs[0].recorded_track.should == sorted_tracks[0] + user_syncs[1].recorded_track.should == sorted_tracks[1] + user_syncs[2].recorded_track.should == sorted_tracks[2] + user_syncs[3].recorded_track.should == sorted_tracks[3] + end + end + end describe "pagination" do From e26f870e18fe834d52fb2482397c0c46537db493 Mon Sep 17 00:00:00 2001 From: Seth Call Date: Mon, 27 Oct 2014 17:21:52 -0500 Subject: [PATCH 6/8] * VRFS-2406 prevent export while in session --- web/app/assets/javascripts/recording_utils.js.coffee | 2 +- web/app/assets/javascripts/sync_viewer.js.coffee | 6 +++++- web/app/views/clients/_help.html.erb | 2 +- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/web/app/assets/javascripts/recording_utils.js.coffee b/web/app/assets/javascripts/recording_utils.js.coffee index 89c556a7c..3a6dbd002 100644 --- a/web/app/assets/javascripts/recording_utils.js.coffee +++ b/web/app/assets/javascripts/recording_utils.js.coffee @@ -34,7 +34,7 @@ class RecordingUtils mixStateDefinition: (mixState) => definition = switch mixState - when 'still-uploading' then "STILL UPLOADING means the you or others in the recording have not uploaded enough information yet to make a mix." + when 'still-uploading' then "STILL UPLOADING means you or others in the recording have not uploaded enough information yet to make a mix." when 'stream-mix' then "STREAM MIX means a user's real-time mix is available to listen to." when 'discarded' then "DISCARDED means you chose to not keep this recording when the recording was over." when 'mixed' then "MIXED means all tracks have been uploaded, and a final mix has been made." diff --git a/web/app/assets/javascripts/sync_viewer.js.coffee b/web/app/assets/javascripts/sync_viewer.js.coffee index 31f371a01..02fd55c1d 100644 --- a/web/app/assets/javascripts/sync_viewer.js.coffee +++ b/web/app/assets/javascripts/sync_viewer.js.coffee @@ -401,7 +401,7 @@ context.JK.SyncViewer = class SyncViewer summary = "Both you and the JamKazam server have the high-quality version of this track. Once all the other tracks for this recording are also synchronized, then the final mix can be made." clientStateDefinition = switch clientState - when @clientStates.too_many_downloads then "This track has been downloaded a unusually large number of times. No more downloads are allowed." + when @clientStates.too_many_downloads then "This track has been downloaded an unusually large number of times. No more downloads are allowed." when @clientStates.hq then "HIGHEST QUALITY means you have the original version of this track, as recorded by the user that made it." when @clientStates.sq then "STREAM QUALITY means you have the version of the track that you received over the internet in real-time." when @clientStates.missing then "MISSING means you do not have this track anymore." @@ -529,6 +529,10 @@ context.JK.SyncViewer = class SyncViewer exportRecording: (e) => $export = $(e.target) + if context.JK.CurrentSessionModel and context.JK.CurrentSessionModel.inSession() + context.JK.confirmBubble($export, 'sync-viewer-paused', {}, {offsetParent: $export.closest('.dialog')}) + return + recordingId = $export.closest('.details').attr('data-recording-id') if !recordingId? or recordingId == "" throw "exportRecording can't find data-recording-id" diff --git a/web/app/views/clients/_help.html.erb b/web/app/views/clients/_help.html.erb index 767a09794..405384d99 100644 --- a/web/app/views/clients/_help.html.erb +++ b/web/app/views/clients/_help.html.erb @@ -129,7 +129,7 @@ From 004674c562c1699ddb11941a07f05dcf1f9cc0ad Mon Sep 17 00:00:00 2001 From: Seth Call Date: Mon, 27 Oct 2014 17:49:14 -0500 Subject: [PATCH 7/8] * VRFS-2405 - cleanup files after successful audiomix --- ruby/lib/jam_ruby/resque/audiomixer.rb | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/ruby/lib/jam_ruby/resque/audiomixer.rb b/ruby/lib/jam_ruby/resque/audiomixer.rb index 1b9065d3d..58a023222 100644 --- a/ruby/lib/jam_ruby/resque/audiomixer.rb +++ b/ruby/lib/jam_ruby/resque/audiomixer.rb @@ -206,6 +206,13 @@ module JamRuby end + def cleanup_files() + File.delete(@output_ogg_filename) if File.exists(@output_ogg_filename) + File.delete(@output_mp3_filename) if File.exists(@output_mp3_filename) + File.delete(@manifest_file) if File.exists(@manifest_file) + File.delete(@error_out_filename) if File.exists(@error_out_filename) + end + def post_success(mix) ogg_length = File.size(@output_ogg_filename) @@ -220,6 +227,7 @@ module JamRuby mix.finish(ogg_length, ogg_md5.to_s, mp3_length, mp3_md5.to_s) end + def post_error(mix, e) begin @@ -265,6 +273,9 @@ module JamRuby post_success(mix) + # only cleanup files if we manage to get this far + cleanup_files + @@log.info("audiomixer job successful. mix_id #{mix_id}") rescue Exception => e From 53706aec71440cd0729fcd4edd56ed0b0ae5a277 Mon Sep 17 00:00:00 2001 From: Seth Call Date: Mon, 27 Oct 2014 17:52:56 -0500 Subject: [PATCH 8/8] * VRFS-2405 - cleanup more files in audiomixer, and input/output files in quickmixer --- ruby/lib/jam_ruby/resque/audiomixer.rb | 5 +++++ ruby/lib/jam_ruby/resque/quick_mixer.rb | 7 +++++++ 2 files changed, 12 insertions(+) diff --git a/ruby/lib/jam_ruby/resque/audiomixer.rb b/ruby/lib/jam_ruby/resque/audiomixer.rb index 58a023222..66973b12a 100644 --- a/ruby/lib/jam_ruby/resque/audiomixer.rb +++ b/ruby/lib/jam_ruby/resque/audiomixer.rb @@ -211,6 +211,11 @@ module JamRuby File.delete(@output_mp3_filename) if File.exists(@output_mp3_filename) File.delete(@manifest_file) if File.exists(@manifest_file) File.delete(@error_out_filename) if File.exists(@error_out_filename) + + @manifest[:files].each do |file| + filename = file[:filename] + File.delete(filename) if File.exists(filename) + end end def post_success(mix) diff --git a/ruby/lib/jam_ruby/resque/quick_mixer.rb b/ruby/lib/jam_ruby/resque/quick_mixer.rb index 4f949c6d3..f9b9d80bb 100644 --- a/ruby/lib/jam_ruby/resque/quick_mixer.rb +++ b/ruby/lib/jam_ruby/resque/quick_mixer.rb @@ -63,6 +63,8 @@ module JamRuby post_success(@quick_mix) + cleanup_files + @@log.info("audiomixer job successful. mix_id #{quick_mix_id}") rescue Exception => e @@ -121,6 +123,11 @@ module JamRuby end end + def cleanup_files() + File.delete(@input_ogg_filename) if File.exists(@input_ogg_filename) + File.delete(@output_mp3_filename) if File.exists(@output_mp3_filename) + end + def fetch_audio_files @input_ogg_filename = Dir::Tmpname.make_tmpname( ["#{Dir.tmpdir}/quick_mixer_#{@quick_mix.id}}", '.ogg'], nil)