Re-work TimeFX cancel/abort

When processing multiple regions, apply results at the end, so that
when the action is canceled, no changes are applied.
Furthermore, do not commit an undo-command if time-stretch is a no-op.

gtk2_ardour/editor_timefx.cc

@@ -33,6 +33,7 @@
 #include "ardour/midi_stretch.h"
 #include "ardour/pitch.h"
 #include "ardour/region.h"
+#include "ardour/region_factory.h"
 #include "ardour/session.h"
 #include "ardour/stretch.h"
 
@@ -41,6 +42,7 @@
 #include "audio_region_view.h"
 #include "audio_time_axis.h"
 #include "editor.h"
+#include "editor_regions.h"
 #include "region_selection.h"
 #include "time_fx_dialog.h"
 
@@ -63,7 +65,6 @@ Editor::time_stretch (RegionSelection& regions, float fraction)
 {
 	RegionList audio;
 	RegionList midi;
-	int aret;
 
 	begin_reversible_command (_("stretch/shrink"));
 
@@ -75,8 +76,9 @@ Editor::time_stretch (RegionSelection& regions, float fraction)
 		}
 	}
 
-	if ((aret = time_fx (audio, fraction, false)) != 0) {
+	int aret = time_fx (audio, fraction, false);
-		commit_reversible_command ();
+	if (aret < 0) {
+		abort_reversible_command ();
 		return aret;
 	}
 
@@ -110,10 +112,18 @@ Editor::time_stretch (RegionSelection& regions, float fraction)
 	}
 
 	for (set<boost::shared_ptr<Playlist> >::iterator p = midi_playlists_affected.begin(); p != midi_playlists_affected.end(); ++p) {
-		_session->add_command (new StatefulDiffCommand (*p));
+		PBD::StatefulDiffCommand* cmd = new StatefulDiffCommand (*p);
+		_session->add_command (cmd);
+		if (!cmd->empty ()) {
+			++aret;
+		}
 	}
 
-	commit_reversible_command ();
+	if (aret > 0) {
+		commit_reversible_command ();
+	} else {
+		abort_reversible_command ();
+	}
 
 	return 0;
 }
@@ -131,22 +141,25 @@ Editor::pitch_shift (RegionSelection& regions, float fraction)
 
 	int ret = time_fx (rl, fraction, true);
 
-	if (ret == 0) {
+	if (ret > 0) {
 		commit_reversible_command ();
 	} else {
 		abort_reversible_command ();
 	}
 
-	return ret;
+	return ret < 0 ? -1 : 0;
 }
 
 /** @param val Percentage to time stretch by; ignored if pitch-shifting.
  *  @param pitching true to pitch shift, false to time stretch.
- *  @return -1 in case of error, 1 if operation was cancelled by the user, 0 if everything went ok */
+ *  @return -1 in case of error, otherwise number of regions processed */
 int
 Editor::time_fx (RegionList& regions, float val, bool pitching)
 {
+	delete current_timefx;
+
 	if (regions.empty()) {
+		current_timefx = 0;
 		return 0;
 	}
 
@@ -154,7 +167,6 @@ Editor::time_fx (RegionList& regions, float val, bool pitching)
 	const samplecnt_t newlen = (samplecnt_t) (regions.front()->length() * val);
 	const samplecnt_t pos = regions.front()->position ();
 
-	delete current_timefx;
 	current_timefx = new TimeFXDialog (*this, pitching, oldlen, newlen, pos);
 	current_timefx->regions = regions;
 
@@ -163,7 +175,7 @@ Editor::time_fx (RegionList& regions, float val, bool pitching)
 		break;
 	default:
 		current_timefx->hide ();
-		return 1;
+		return -1;
 	}
 
 	current_timefx->status = 0;
@@ -317,39 +329,43 @@ Editor::time_fx (RegionList& regions, float val, bool pitching)
 	pthread_join (current_timefx->request.thread, 0);
 
 	current_timefx->hide ();
+
+	if (current_timefx->status < 0) {
+		/* processing was cancelled, some regions may have
+		 * been created and removed via RegionFactory::map_remove()
+		 * The region-list does not update itself when a region is removed.
+		 */
+		_regions->redisplay ();
+	}
 	return current_timefx->status;
 }
 
 void
 Editor::do_timefx ()
 {
-	boost::shared_ptr<Playlist> playlist;
+	typedef std::map<boost::shared_ptr<Region>, boost::shared_ptr<Region> > ResultMap;
-	boost::shared_ptr<Region>   new_region;
+	ResultMap results;
-	set<boost::shared_ptr<Playlist> > playlists_affected;
 
 	uint32_t const N = current_timefx->regions.size ();
 
-	for (RegionList::iterator i = current_timefx->regions.begin(); i != current_timefx->regions.end(); ++i) {
+	for (RegionList::const_iterator i = current_timefx->regions.begin(); i != current_timefx->regions.end(); ++i) {
 		boost::shared_ptr<Playlist> playlist = (*i)->playlist();
-
 		if (playlist) {
 			playlist->clear_changes ();
 		}
 	}
 
-	for (RegionList::iterator i = current_timefx->regions.begin(); i != current_timefx->regions.end(); ++i) {
+	for (RegionList::const_iterator i = current_timefx->regions.begin(); i != current_timefx->regions.end(); ++i) {
 
 		boost::shared_ptr<AudioRegion> region = boost::dynamic_pointer_cast<AudioRegion> (*i);
+		boost::shared_ptr<Playlist> playlist;
 
 		if (!region || (playlist = region->playlist()) == 0) {
 			continue;
 		}
 
 		if (current_timefx->request.cancel) {
-			/* we were cancelled */
+			break;
-			/* XXX what to do about playlists already affected ? */
-			current_timefx->status = 1;
-			return;
 		}
 
 		Filter* fx;
@@ -367,28 +383,43 @@ Editor::do_timefx ()
 		current_timefx->descend (1.0 / N);
 
 		if (fx->run (region, current_timefx)) {
-			current_timefx->status = -1;
+			current_timefx->request.cancel = true;
-			current_timefx->request.done = true;
 			delete fx;
-			return;
+			break;
 		}
 
 		if (!fx->results.empty()) {
-			new_region = fx->results.front();
+			results[region] = fx->results.front();
-
-			playlist->replace_region (region, new_region, region->position());
-			playlists_affected.insert (playlist);
 		}
 
 		current_timefx->ascend ();
 		delete fx;
 	}
 
-	for (set<boost::shared_ptr<Playlist> >::iterator p = playlists_affected.begin(); p != playlists_affected.end(); ++p) {
+	pthread_setcancelstate (PTHREAD_CANCEL_DISABLE, NULL);
-		_session->add_command (new StatefulDiffCommand (*p));
+	if (current_timefx->request.cancel) {
-	}
+		current_timefx->status = -1;
+		for (ResultMap::const_iterator i = results.begin(); i != results.end(); ++i) {
+			boost::weak_ptr<Region> w = i->second;
+			RegionFactory::map_remove (w);
+		}
+	} else {
+		current_timefx->status = 0;
+		for (ResultMap::const_iterator i = results.begin(); i != results.end(); ++i) {
+			boost::shared_ptr<Region> region = i->first;
+			boost::shared_ptr<Region> new_region = i->second;
+			boost::shared_ptr<Playlist> playlist = region->playlist();
+			playlist->replace_region (region, new_region, region->position());
+
+			PBD::StatefulDiffCommand* cmd = new StatefulDiffCommand (playlist);
+			_session->add_command (cmd);
+			if (!cmd->empty ()) {
+				++current_timefx->status;
+			}
+		}
+	}
+	pthread_setcancelstate (PTHREAD_CANCEL_ENABLE, NULL);
 
-	current_timefx->status = 0;
 	current_timefx->request.done = true;
 }
 

gtk2_ardour/time_fx_dialog.cc

@@ -246,7 +246,6 @@ TimeFXDialog::timer_update ()
 void
 TimeFXDialog::cancel_in_progress ()
 {
-	status = -2;
 	request.cancel = true;
 	first_cancel.disconnect();
 }
@@ -254,7 +253,6 @@ TimeFXDialog::cancel_in_progress ()
 gint
 TimeFXDialog::delete_in_progress (GdkEventAny*)
 {
-	status = -2;
 	request.cancel = true;
 	first_delete.disconnect();
 	return TRUE;