From 8beee7b625c22fd7be4002546756b3af8cf30329 Mon Sep 17 00:00:00 2001 From: Benjamin Klum Date: Tue, 23 May 2023 16:45:05 +0200 Subject: [PATCH] #842 Prevent ReaLearn from replacing itself, avoiding a potential crash --- main/src/domain/backbone_state.rs | 12 +- main/src/domain/instance_state.rs | 31 ++- .../infrastructure/plugin/realearn_plugin.rs | 2 +- pot/src/lib.rs | 176 +++++++++++------- pot/src/preview_recorder.rs | 2 +- 5 files changed, 141 insertions(+), 82 deletions(-) diff --git a/main/src/domain/backbone_state.rs b/main/src/domain/backbone_state.rs index a7b1d57ef..62f9363a7 100644 --- a/main/src/domain/backbone_state.rs +++ b/main/src/domain/backbone_state.rs @@ -6,9 +6,9 @@ use base::{ use crate::domain::{ AdditionalFeedbackEvent, ClipMatrixRef, ControlInput, DeviceControlInput, DeviceFeedbackOutput, FeedbackOutput, InstanceId, InstanceState, InstanceStateChanged, NormalAudioHookTask, - NormalRealTimeTask, QualifiedClipMatrixEvent, RealearnClipMatrix, RealearnSourceState, - RealearnTargetState, ReaperTarget, ReaperTargetType, SafeLua, SharedInstanceState, - WeakInstanceState, + NormalRealTimeTask, ProcessorContext, QualifiedClipMatrixEvent, RealearnClipMatrix, + RealearnSourceState, RealearnTargetState, ReaperTarget, ReaperTargetType, SafeLua, + SharedInstanceState, WeakInstanceState, }; use enum_iterator::IntoEnumIterator; use pot::{PotFavorites, PotFilterExcludes}; @@ -16,7 +16,7 @@ use pot::{PotFavorites, PotFilterExcludes}; use once_cell::sync::Lazy; use playtime_clip_engine::rt::WeakMatrix; use realearn_api::persistence::TargetTouchCause; -use reaper_high::{Reaper, Track}; +use reaper_high::Reaper; use std::cell::{Cell, Ref, RefCell, RefMut}; use std::collections::{HashMap, HashSet}; use std::hash::Hash; @@ -243,19 +243,19 @@ impl BackboneState { pub fn create_instance( &self, id: InstanceId, + processor_context: ProcessorContext, instance_feedback_event_sender: SenderToNormalThread, clip_matrix_event_sender: SenderToNormalThread, audio_hook_task_sender: SenderToRealTimeThread, real_time_processor_sender: SenderToRealTimeThread, - this_track: Option, ) -> SharedInstanceState { let instance_state = InstanceState::new( id, + processor_context, instance_feedback_event_sender, clip_matrix_event_sender, audio_hook_task_sender, real_time_processor_sender, - this_track, ); let shared_instance_state = Rc::new(RefCell::new(instance_state)); self.instance_states diff --git a/main/src/domain/instance_state.rs b/main/src/domain/instance_state.rs index 559c427c2..9a889c8b1 100644 --- a/main/src/domain/instance_state.rs +++ b/main/src/domain/instance_state.rs @@ -4,15 +4,15 @@ use std::rc::{Rc, Weak}; use std::sync::RwLock; use enum_map::EnumMap; -use reaper_high::{Fx, Track}; +use reaper_high::Fx; use rxrust::prelude::*; use crate::base::Prop; use crate::domain::{ AnyThreadBackboneState, BackboneState, Compartment, FxDescriptor, FxInputClipRecordTask, GlobalControlAndFeedbackState, GroupId, HardwareInputClipRecordTask, InstanceId, MappingId, - MappingSnapshotContainer, NormalAudioHookTask, NormalRealTimeTask, QualifiedMappingId, Tag, - TagScope, TrackDescriptor, VirtualMappingSnapshotIdForLoad, + MappingSnapshotContainer, NormalAudioHookTask, NormalRealTimeTask, ProcessorContext, + QualifiedMappingId, Tag, TagScope, TrackDescriptor, VirtualMappingSnapshotIdForLoad, }; use base::{tracing_debug, NamedChannelSender, SenderToNormalThread, SenderToRealTimeThread}; use playtime_clip_engine::base::{ @@ -41,6 +41,7 @@ pub type RealearnClipMatrix = Matrix; #[derive(Debug)] pub struct InstanceState { instance_id: InstanceId, + processor_context: ProcessorContext, /// Owned clip matrix or reference to a clip matrix owned by another instance. /// /// Persistent. @@ -49,7 +50,6 @@ pub struct InstanceState { clip_matrix_event_sender: SenderToNormalThread, audio_hook_task_sender: SenderToRealTimeThread, real_time_processor_sender: SenderToRealTimeThread, - this_track: Option, slot_contents_changed_subject: LocalSubject<'static, (), ()>, /// Which mappings are in which group. /// @@ -206,20 +206,20 @@ pub struct MappingInfo { impl InstanceState { pub(super) fn new( instance_id: InstanceId, + processor_context: ProcessorContext, instance_feedback_event_sender: SenderToNormalThread, clip_matrix_event_sender: SenderToNormalThread, audio_hook_task_sender: SenderToRealTimeThread, real_time_processor_sender: SenderToRealTimeThread, - this_track: Option, ) -> Self { Self { instance_id, + processor_context, clip_matrix_ref: None, instance_feedback_event_sender, clip_matrix_event_sender, audio_hook_task_sender, real_time_processor_sender, - this_track, slot_contents_changed_subject: Default::default(), mappings_by_group: Default::default(), active_mapping_by_group: Default::default(), @@ -287,7 +287,10 @@ impl InstanceState { /// /// Returns an error if the necessary pot database is not available. pub fn pot_unit(&mut self) -> Result { - let integration = RealearnPotIntegration::new(self.instance_feedback_event_sender.clone()); + let integration = RealearnPotIntegration::new( + self.processor_context.containing_fx().clone(), + self.instance_feedback_event_sender.clone(), + ); self.pot_unit.loaded(Box::new(integration)) } @@ -434,7 +437,7 @@ impl InstanceState { self.real_time_processor_sender.clone(), self.clip_matrix_event_sender.clone(), ); - Matrix::new(clip_matrix_handler, self.this_track.clone()) + Matrix::new(clip_matrix_handler, self.processor_context.track().cloned()) } pub(super) fn set_clip_matrix_ref(&mut self, matrix_ref: Option) { @@ -733,12 +736,16 @@ pub enum ClipMatrixRelevance<'a> { } struct RealearnPotIntegration { + containing_fx: Fx, sender: SenderToNormalThread, } impl RealearnPotIntegration { - fn new(sender: SenderToNormalThread) -> Self { - Self { sender } + fn new(containing_fx: Fx, sender: SenderToNormalThread) -> Self { + Self { + containing_fx, + sender, + } } } @@ -781,4 +788,8 @@ impl PotIntegration for RealearnPotIntegration { PotStateChangedEvent::IndexesRebuilt, )); } + + fn protected_fx(&self) -> &Fx { + &self.containing_fx + } } diff --git a/main/src/infrastructure/plugin/realearn_plugin.rs b/main/src/infrastructure/plugin/realearn_plugin.rs index 79d02cd24..34672f82c 100644 --- a/main/src/infrastructure/plugin/realearn_plugin.rs +++ b/main/src/infrastructure/plugin/realearn_plugin.rs @@ -416,11 +416,11 @@ impl RealearnPlugin { SenderToNormalThread::new_unbounded_channel("instance state change events"); let instance_state = BackboneState::get().create_instance( instance_id, + processor_context.clone(), instance_feedback_event_sender, App::get().clip_matrix_event_sender().clone(), App::get().normal_audio_hook_task_sender().clone(), normal_real_time_task_sender.clone(), - processor_context.track().cloned(), ); // Session (application - shared) let session = Session::new( diff --git a/pot/src/lib.rs b/pot/src/lib.rs index 707c43905..a62ae060b 100644 --- a/pot/src/lib.rs +++ b/pot/src/lib.rs @@ -139,6 +139,10 @@ pub trait PotIntegration { fn notify_preset_changed(&self, id: Option); fn notify_filter_changed(&self, kind: PotFilterKind, filter: OptFilter); fn notify_indexes_rebuilt(&self); + /// Returns an FX instance which must not be removed (e.g. in the process of loading a preset). + /// + /// This should be the FX holding the ReaLearn instance which controls the pot. + fn protected_fx(&self) -> &Fx; } #[derive(derivative::Derivative)] @@ -513,6 +517,10 @@ impl RuntimePotUnit { self.preview_volume } + pub fn protected_fx(&self) -> &Fx { + self.integration.protected_fx() + } + pub fn set_preview_volume(&mut self, volume: ReaperVolumeValue) { self.preview_volume = volume; self.sound_player @@ -653,12 +661,13 @@ impl RuntimePotUnit { is_shim_preset: false, }); } - let outcome = load_file_based_preset( - self, + let protected_fx = self.protected_fx().clone(); + let outcome = self.load_file_based_preset( &shim_file_path, build_destination, options, true, + &protected_fx, )?; Ok(self.process_preset_load_outcome(preset, outcome)) } @@ -666,6 +675,47 @@ impl RuntimePotUnit { } } + fn load_file_based_preset( + &mut self, + preset_file: &Path, + build_destination: impl Fn(&mut RuntimePotUnit) -> Result, + options: LoadPresetOptions, + is_shim_preset: bool, + protected_fx: &Fx, + ) -> Result { + let ext = preset_file.extension().and_then(|e| e.to_str()).ok_or( + LoadPresetError::UnsupportedPresetFormat { + file_extension: "".to_string(), + is_shim_preset, + }, + )?; + let outcome = match ext.to_lowercase().as_str() { + _ if is_audio_file_extension(ext) => { + let dest = build_destination(self)?; + load_audio_preset(preset_file, &dest, options, protected_fx)? + } + "nksf" | "nksfx" => { + let dest = build_destination(self)?; + load_nks_preset(preset_file, &dest, options, protected_fx)? + } + "rfxchain" => { + let dest = build_destination(self)?; + load_rfx_chain_preset_using_chunks(preset_file, &dest, options, protected_fx)? + } + "rtracktemplate" => { + let dest = build_destination(self)?; + load_track_template_preset(preset_file, &dest, options, protected_fx)? + } + x => { + return Err(LoadPresetError::UnsupportedPresetFormat { + file_extension: x.to_string(), + is_shim_preset, + }); + } + }; + Ok(outcome) + } + /// Doesn't try to load a shim preset yet. fn load_preset_at_internal( &mut self, @@ -673,21 +723,27 @@ impl RuntimePotUnit { options: LoadPresetOptions, build_destination: &impl Fn(&mut RuntimePotUnit) -> Result, ) -> Result { + let protected_fx = self.protected_fx().clone(); let outcome = match &preset.kind { - PresetKind::FileBased(k) => { - load_file_based_preset(self, &k.path, build_destination, options, false)? - } + PresetKind::FileBased(k) => self.load_file_based_preset( + &k.path, + build_destination, + options, + false, + &protected_fx, + )?, PresetKind::ProjectBased(k) => load_project_based_rfx_chain_preset( self, build_destination, &k.path_to_rpp, k.fx_chain_range.clone(), options, + &protected_fx, )?, PresetKind::Internal(k) => { if let Some(plugin_id) = k.plugin_id { let dest = build_destination(self)?; - load_internal_preset(plugin_id, preset.name(), &dest, options) + load_internal_preset(plugin_id, preset.name(), &dest, options, &protected_fx) .map_err(LoadPresetError::Other)? } else { return Err(LoadPresetError::Other( @@ -697,7 +753,7 @@ impl RuntimePotUnit { } PresetKind::DefaultFactory(plugin_id) => { let dest = build_destination(self)?; - load_default_factory_preset(*plugin_id, &dest, options) + load_default_factory_preset(*plugin_id, &dest, options, &protected_fx) .map_err(LoadPresetError::Other)? } }; @@ -1208,17 +1264,24 @@ fn load_nks_preset( path: &Path, destination: &Destination, options: LoadPresetOptions, + protected_fx: &Fx, ) -> Result> { let nks_file = NksFile::load(path)?; let mut nks_content = nks_file.content()?; - load_preset_single_fx(nks_content.plugin_id, destination, options, |fx| { - fx.set_vst_chunk(nks_content.vst_chunk)?; - resolve_macro_param_ids(&mut nks_content.macro_param_banks, fx); - let outcome = InternalLoadPresetOutcome { - banks: nks_content.macro_param_banks, - }; - Ok(outcome) - }) + load_preset_single_fx( + nks_content.plugin_id, + destination, + options, + protected_fx, + |fx| { + fx.set_vst_chunk(nks_content.vst_chunk)?; + resolve_macro_param_ids(&mut nks_content.macro_param_banks, fx); + let outcome = InternalLoadPresetOutcome { + banks: nks_content.macro_param_banks, + }; + Ok(outcome) + }, + ) } fn resolve_macro_param_ids(banks: &mut [MacroParamBank], fx: &Fx) { @@ -1255,8 +1318,9 @@ fn load_rfx_chain_preset_using_add_fx( path: &Path, destination: &Destination, options: LoadPresetOptions, + protected_fx: &Fx, ) -> Result> { - load_preset_multi_fx(destination, options, true, || { + load_preset_multi_fx(destination, options, true, protected_fx, || { let root_dir = Reaper::get().resource_path().join("FXChains"); let relative_path = path.strip_prefix(&root_dir)?; let relative_path = relative_path.to_string_lossy().to_string(); @@ -1281,18 +1345,20 @@ fn load_rfx_chain_preset_using_chunks( path: &Path, destination: &Destination, options: LoadPresetOptions, + protected_fx: &Fx, ) -> Result> { let rppxml = fs::read_to_string(path).map_err(|_| "couldn't read FX chain template as string")?; - load_rfx_chain_preset_using_chunks_from_string(&rppxml, destination, options) + load_rfx_chain_preset_using_chunks_from_string(&rppxml, destination, options, protected_fx) } fn load_rfx_chain_preset_using_chunks_from_string( rppxml: &str, destination: &Destination, options: LoadPresetOptions, + protected_fx: &Fx, ) -> Result> { - load_preset_multi_fx(destination, options, false, || { + load_preset_multi_fx(destination, options, false, protected_fx, || { // Our chunk stuff assumes we have a chunk without indentation. Time to use proper parsing... let rppxml: String = rppxml.lines().map(|l| l.trim()).join("\n"); let fx_chain_content_chunk = Chunk::new(rppxml); @@ -1312,20 +1378,27 @@ fn load_project_based_rfx_chain_preset( path_to_rpp: &Path, fx_chain_range: Range, options: LoadPresetOptions, + protected_fx: &Fx, ) -> Result> { let project_rppxml = fs::read_to_string(path_to_rpp).map_err(|_| "couldn't read project as string")?; let fx_chain_rppxml = &project_rppxml[fx_chain_range]; let destination = &build_destination(pot_unit)?; - load_rfx_chain_preset_using_chunks_from_string(fx_chain_rppxml, destination, options) + load_rfx_chain_preset_using_chunks_from_string( + fx_chain_rppxml, + destination, + options, + protected_fx, + ) } fn load_track_template_preset( path: &Path, destination: &Destination, options: LoadPresetOptions, + protected_fx: &Fx, ) -> Result> { - load_preset_multi_fx(destination, options, false, || { + load_preset_multi_fx(destination, options, false, protected_fx, || { let rppxml = fs::read_to_string(path).map_err(|_| "couldn't read track template as string")?; // Our chunk stuff assumes we have a chunk without indentation. Time to use proper parsing... @@ -1349,8 +1422,9 @@ fn load_internal_preset( preset_name: &str, destination: &Destination, options: LoadPresetOptions, + protected_fx: &Fx, ) -> Result> { - load_preset_single_fx(plugin_id, destination, options, |fx| { + load_preset_single_fx(plugin_id, destination, options, protected_fx, |fx| { fx.activate_preset_by_name(preset_name)?; Ok(Default::default()) }) @@ -1360,8 +1434,9 @@ fn load_default_factory_preset( plugin_id: PluginId, destination: &Destination, options: LoadPresetOptions, + protected_fx: &Fx, ) -> Result> { - load_preset_single_fx(plugin_id, destination, options, |fx| { + load_preset_single_fx(plugin_id, destination, options, protected_fx, |fx| { fx.activate_preset(FxPresetRef::FactoryPreset)?; Ok(Default::default()) }) @@ -1371,12 +1446,13 @@ fn load_audio_preset( path: &Path, destination: &Destination, options: LoadPresetOptions, + protected_fx: &Fx, ) -> Result> { const RS5K_VST_ID: i32 = 1920167789; let plugin_id = PluginId::Vst2 { vst_magic_number: RS5K_VST_ID, }; - load_preset_single_fx(plugin_id, destination, options, |fx| { + load_preset_single_fx(plugin_id, destination, options, protected_fx, |fx| { // First try it the modern way ... if load_media_in_specific_rs5k_modern(fx, path).is_err() { // ... and if this didn't work, try it the old-school way. @@ -1424,6 +1500,7 @@ fn load_preset_single_fx( plugin_id: PluginId, destination: &Destination, options: LoadPresetOptions, + protected_fx: &Fx, f: impl FnOnce(&Fx) -> Result>, ) -> Result> { let existing_fx = destination.resolve(); @@ -1431,7 +1508,7 @@ fn load_preset_single_fx( .as_ref() .map(|fx| fx.window_is_open()) .unwrap_or(false); - let output = ensure_fx_has_correct_type(plugin_id, destination, existing_fx)?; + let output = ensure_fx_has_correct_type(plugin_id, destination, existing_fx, protected_fx)?; let outcome = f(&output.fx)?; options .window_behavior @@ -1452,6 +1529,7 @@ fn load_preset_multi_fx( destination: &Destination, options: LoadPresetOptions, remove_existing_fx_manually: bool, + protected_fx: &Fx, f: impl FnOnce() -> Result>, ) -> Result> { let mut fx_was_open_before = false; @@ -1464,6 +1542,9 @@ fn load_preset_multi_fx( if fx.window_is_open() { fx_was_open_before = true; } + if &fx == protected_fx { + return Err(CANT_REMOVE_PROTECTED_FX.into()); + } if remove_existing_fx_manually { destination.chain.remove_fx(&fx)?; } @@ -1499,6 +1580,7 @@ fn ensure_fx_has_correct_type( plugin_id: PluginId, destination: &Destination, existing_fx: Option, + protected_fx: &Fx, ) -> Result> { let output = match existing_fx { None => { @@ -1511,12 +1593,16 @@ fn ensure_fx_has_correct_type( Some(fx) => { let fx_info = fx.info()?; if fx_info.id == plugin_id.content_formatted_for_reaper() { + // This is the right plug-in type. Leave as is. FxEnsureOutput { fx, op: FxEnsureOp::Same, } } else { // We don't have the right plug-in type. Remove FX and insert correct one. + if &fx == protected_fx { + return Err(CANT_REMOVE_PROTECTED_FX.into()); + } destination.chain.remove_fx(&fx)?; let fx = insert_fx_by_plugin_id(plugin_id, destination)?; FxEnsureOutput { @@ -1670,46 +1756,6 @@ impl From> for LoadPresetError { impl Error for LoadPresetError {} -fn load_file_based_preset( - pot_unit: &mut RuntimePotUnit, - preset_file: &Path, - build_destination: impl Fn(&mut RuntimePotUnit) -> Result, - options: LoadPresetOptions, - is_shim_preset: bool, -) -> Result { - let ext = preset_file.extension().and_then(|e| e.to_str()).ok_or( - LoadPresetError::UnsupportedPresetFormat { - file_extension: "".to_string(), - is_shim_preset, - }, - )?; - let outcome = match ext.to_lowercase().as_str() { - _ if is_audio_file_extension(ext) => { - let dest = build_destination(pot_unit)?; - load_audio_preset(preset_file, &dest, options)? - } - "nksf" | "nksfx" => { - let dest = build_destination(pot_unit)?; - load_nks_preset(preset_file, &dest, options)? - } - "rfxchain" => { - let dest = build_destination(pot_unit)?; - load_rfx_chain_preset_using_chunks(preset_file, &dest, options)? - } - "rtracktemplate" => { - let dest = build_destination(pot_unit)?; - load_track_template_preset(preset_file, &dest, options)? - } - x => { - return Err(LoadPresetError::UnsupportedPresetFormat { - file_extension: x.to_string(), - is_shim_preset, - }); - } - }; - Ok(outcome) -} - #[derive(Copy, Clone, Eq, PartialEq)] pub enum Debounce { No, @@ -1778,3 +1824,5 @@ pub fn create_plugin_factory_preset( kind: PresetKind::DefaultFactory(plugin.core.id), } } + +const CANT_REMOVE_PROTECTED_FX: &str = "Can't replace ReaLearn itself. Either set \"Load into\" correctly or put ReaLearn on the monitoring FX chain!"; diff --git a/pot/src/preview_recorder.rs b/pot/src/preview_recorder.rs index f019d48d7..65fb394a0 100644 --- a/pot/src/preview_recorder.rs +++ b/pot/src/preview_recorder.rs @@ -91,7 +91,7 @@ pub async fn record_previews( // Load preset let options = LoadPresetOptions { window_behavior: LoadPresetWindowBehavior::AlwaysShow, - ..Default::default() + audio_sample_behavior: Default::default(), }; { let load_result = blocking_lock_arc(&shared_pot_unit, "record_previews pot unit")