From c36dfbedb7e6adb321c0c599b9d2def0bc32dde1 Mon Sep 17 00:00:00 2001 From: Robin Gareus Date: Tue, 26 Feb 2019 03:01:53 +0100 Subject: [PATCH] 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 | 93 +++++++++++++++++++++++------------ gtk2_ardour/time_fx_dialog.cc | 2 - 2 files changed, 62 insertions(+), 33 deletions(-) diff --git a/gtk2_ardour/editor_timefx.cc b/gtk2_ardour/editor_timefx.cc index 1f1cd605a9..311c33c3d0 100644 --- a/gtk2_ardour/editor_timefx.cc +++ b/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) { - commit_reversible_command (); + int aret = time_fx (audio, fraction, false); + if (aret < 0) { + abort_reversible_command (); return aret; } @@ -110,10 +112,18 @@ Editor::time_stretch (RegionSelection& regions, float fraction) } for (set >::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; - boost::shared_ptr new_region; - set > playlists_affected; + typedef std::map, boost::shared_ptr > ResultMap; + ResultMap results; 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 = (*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 region = boost::dynamic_pointer_cast (*i); + boost::shared_ptr playlist; if (!region || (playlist = region->playlist()) == 0) { continue; } if (current_timefx->request.cancel) { - /* we were cancelled */ - /* XXX what to do about playlists already affected ? */ - current_timefx->status = 1; - return; + break; } 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.done = true; + current_timefx->request.cancel = true; delete fx; - return; + break; } if (!fx->results.empty()) { - new_region = fx->results.front(); - - playlist->replace_region (region, new_region, region->position()); - playlists_affected.insert (playlist); + results[region] = fx->results.front(); } current_timefx->ascend (); delete fx; } - for (set >::iterator p = playlists_affected.begin(); p != playlists_affected.end(); ++p) { - _session->add_command (new StatefulDiffCommand (*p)); - } + pthread_setcancelstate (PTHREAD_CANCEL_DISABLE, NULL); + if (current_timefx->request.cancel) { + current_timefx->status = -1; + for (ResultMap::const_iterator i = results.begin(); i != results.end(); ++i) { + boost::weak_ptr 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 = i->first; + boost::shared_ptr new_region = i->second; + boost::shared_ptr 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; } diff --git a/gtk2_ardour/time_fx_dialog.cc b/gtk2_ardour/time_fx_dialog.cc index c51bdcd5e2..8ca0119030 100644 --- a/gtk2_ardour/time_fx_dialog.cc +++ b/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;