Skip to content

Commit 413d022

Browse files
committed
Refactor error handling and memory management in Node and Plugin modules
- Updated error handling in `GetFrameError` to ensure proper message consumption. - Changed `Node` struct to implement `Drop` for automatic memory management. - Modified cloning behavior in `Node` to correctly increment reference counts. - Updated `free` method in `Node` to use `ManuallyDrop` for safety. - Adjusted return types in `get_creation_function_arguments` to use `MapRef`. - Refactored `Plugin` struct to use `MapRef` for input and output maps in public functions. - Improved documentation and added `#[must_use]` annotations for better API usability. - Cleaned up imports and ensured consistent use of `unsafe` blocks.
1 parent b11388e commit 413d022

23 files changed

Lines changed: 505 additions & 351 deletions

File tree

example/src/lib.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use rustsynth::{
22
core::CoreRef,
33
filter::{Filter, FilterDependency, FilterMode, RequestPattern},
44
frame::{Frame, FrameContext},
5-
map::Map,
5+
map::MapRef,
66
node::Node,
77
vapoursynth_plugin,
88
};
@@ -30,7 +30,7 @@ mod plugin {
3030
const RETURNTYPE: &'static str = "clip:vnode;";
3131
const MODE: FilterMode = FilterMode::Parallel;
3232

33-
fn from_args(args: &Map<'core>, _core: &CoreRef<'core>) -> Result<Self, String> {
33+
fn from_args(args: &MapRef<'core>, _core: &CoreRef<'core>) -> Result<Self, String> {
3434
let input_node = args.get_node("clip")?;
3535
Ok(Self { input_node })
3636
}

rspipe/src/progress.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ impl ProgressTracker {
5454
}
5555
}
5656

57-
pub fn finish(&mut self) {
57+
pub fn finish(&self) {
5858
let elapsed = self.start_time.elapsed().as_secs_f64();
5959
let fps = self.total_frames as f64 / elapsed;
6060

rustsynth-derive/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ fn generate_vs_filter(
171171
let api = &*vsapi;
172172
std::panic::catch_unwind(|| {
173173
let core_ref = rustsynth::core::CoreRef::from_ptr(core);
174-
let in_map = rustsynth::map::Map::from_ptr(in_);
174+
let in_map = rustsynth::map::MapRef::from_ptr(in_);
175175
// Create filter instance from arguments
176176
match <#struct_type>::from_args(&in_map, &core_ref) {
177177
Ok(filter_data) => {

rustsynth/src/api.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,7 @@ impl API {
7373

7474
let handle = if handle.is_null() {
7575
// Attempt retrieving it otherwise.
76-
let handle =
77-
unsafe { ffi::getVapourSynthAPI(ffi::VAPOURSYNTH_API_VERSION) }.cast_mut();
76+
let handle = unsafe { ffi::getVapourSynthAPI(ffi::VAPOURSYNTH_API_VERSION) }.cast_mut();
7877

7978
if !handle.is_null() {
8079
// If we successfully retrieved the API, cache it.
@@ -594,6 +593,10 @@ impl API {
594593
self.handle.as_ref().addFunctionRef.unwrap()(func)
595594
}
596595

596+
pub(crate) unsafe fn clone_frame(&self, frame: *const ffi::VSFrame) -> *const ffi::VSFrame {
597+
self.handle.as_ref().addFrameRef.unwrap()(frame)
598+
}
599+
597600
pub(crate) unsafe fn create_func(
598601
&self,
599602
func: Option<

rustsynth/src/core.rs

Lines changed: 51 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ impl<'core> CoreRef<'core> {
7474
/// let core = CoreRef::new(CoreCreationFlags::ENABLE_GRAPH_INSPECTION | CoreCreationFlags::DISABLE_AUTO_LOADING);
7575
/// ```
7676
#[inline]
77-
#[must_use]
77+
#[must_use]
7878
pub fn new(flags: CoreCreationFlags) -> Self {
7979
let api = API::get().unwrap();
8080
unsafe {
@@ -96,7 +96,7 @@ impl<'core> CoreRef<'core> {
9696

9797
/// Returns the underlying pointer.
9898
#[inline]
99-
#[must_use]
99+
#[must_use]
100100
pub const fn as_ptr(&self) -> *mut ffi::VSCore {
101101
self.handle.as_ptr()
102102
}
@@ -106,7 +106,7 @@ impl<'core> CoreRef<'core> {
106106
/// # Panics
107107
///
108108
/// Will panic if core configuration is not valid
109-
#[must_use]
109+
#[must_use]
110110
pub fn info(&self) -> CoreInfo {
111111
let core_info = unsafe { API::get_cached().get_core_info(self.as_ptr()) };
112112
let version_string = unsafe { CStr::from_ptr(core_info.versionString).to_str().unwrap() };
@@ -127,7 +127,7 @@ impl<'core> CoreRef<'core> {
127127
/// Returns an instance of [Some]<[Plugin]> if there exists a plugin loaded associated with the namespace
128128
///
129129
/// [None] if no plugin is found
130-
#[must_use]
130+
#[must_use]
131131
pub fn plugin_by_namespace(&self, namespace: &str) -> Option<Plugin<'core>> {
132132
let namespace = CString::new(namespace).unwrap();
133133
unsafe { API::get_cached() }.plugin_by_namespace(namespace.as_ptr(), self)
@@ -136,42 +136,42 @@ impl<'core> CoreRef<'core> {
136136
/// Returns an instance of [Some]<[Plugin]> if there exists a plugin loaded associated with the id
137137
///
138138
/// [None] if no plugin is found
139-
#[must_use]
139+
#[must_use]
140140
pub fn plugin_by_id(&self, id: &str) -> Option<Plugin<'_>> {
141141
let id = CString::new(id).unwrap();
142142
unsafe { API::get_cached() }.plugin_by_id(id.as_ptr(), self)
143143
}
144144

145-
#[must_use]
145+
#[must_use]
146146
pub fn std(&self) -> Option<Plugin<'_>> {
147147
unsafe {
148148
API::get_cached().plugin_by_id(ffi::VSH_STD_PLUGIN_ID.as_ptr().cast::<i8>(), self)
149149
}
150150
}
151151

152-
#[must_use]
152+
#[must_use]
153153
pub fn resize(&self) -> Option<Plugin<'_>> {
154154
unsafe {
155155
API::get_cached().plugin_by_id(ffi::VSH_RESIZE_PLUGIN_ID.as_ptr().cast::<i8>(), self)
156156
}
157157
}
158158

159-
#[must_use]
159+
#[must_use]
160160
pub fn text(&self) -> Option<Plugin<'_>> {
161161
unsafe {
162162
API::get_cached().plugin_by_id(ffi::VSH_TEXT_PLUGIN_ID.as_ptr().cast::<i8>(), self)
163163
}
164164
}
165165

166166
/// Returns a iterator over the loaded plugins
167-
#[must_use]
167+
#[must_use]
168168
pub fn plugins(&self) -> Plugins<'_> {
169169
unsafe { API::get_cached() }.plugins(self)
170170
}
171171

172172
/// Sets the number of threads used for processing. Pass 0 to automatically detect. Returns the number of threads that will be used for processing.
173173
#[inline]
174-
#[must_use]
174+
#[must_use]
175175
pub fn set_thread_count(&self, count: usize) -> i32 {
176176
unsafe { API::get_cached().set_thread_count(self.as_ptr(), count as i32) }
177177
}
@@ -186,20 +186,20 @@ impl<'core> CoreRef<'core> {
186186
}
187187

188188
/// Sets the maximum size of the framebuffer cache. Returns the new maximum size.
189-
#[must_use]
189+
#[must_use]
190190
pub fn set_max_cache_size(&self, size: i64) -> i64 {
191191
unsafe { API::get_cached().set_max_cache_size(self.as_ptr(), size) }
192192
}
193193

194194
/// The format identifier: one of [`crate::format::PresetVideoFormat`] or a value gotten from [`VideoFormat::query_format_id`].
195-
#[must_use]
195+
#[must_use]
196196
pub fn get_video_format_by_id(&self, id: u32) -> Option<VideoFormat> {
197197
let format = unsafe { API::get_cached().get_video_format_by_id(id, self.as_ptr()) };
198198
format.map(|f| unsafe { VideoFormat::from_ptr(f) })
199199
}
200200

201201
/// Duplicates the frame (not just the reference). As the frame buffer is shared in a copy-on-write fashion, the frame content is not really duplicated until a write operation occurs. This is transparent for the user.
202-
#[must_use]
202+
#[must_use]
203203
pub fn copy_frame(&'_ self, frame: &Frame) -> Frame<'_> {
204204
let new_frame = unsafe { API::get_cached().copy_frame(frame, self.as_ptr()) };
205205
unsafe { Frame::from_ptr(new_frame) }
@@ -233,15 +233,15 @@ impl<'core> CoreRef<'core> {
233233
}
234234

235235
/// Send a message through `VapourSynth`'s logging framework
236-
pub fn log_mesage(&self, msg_type: MessageType, msg: &str) {
236+
pub fn log_message(&self, msg_type: MessageType, msg: &str) {
237237
let cstr = CString::new(msg).unwrap();
238238
unsafe {
239239
API::get_cached().log_message(msg_type.into(), cstr.as_ptr(), self.handle.as_ptr());
240240
}
241241
}
242242

243243
/// Create a video filter using the Filter trait
244-
pub fn create_video_filter<F>(&self, filter: &F) -> CoreResult<Map<'_>>
244+
pub fn create_video_filter<F>(&self, filter: F) -> CoreResult<Map<'_>>
245245
where
246246
F: Filter<'core>,
247247
{
@@ -251,8 +251,10 @@ impl<'core> CoreRef<'core> {
251251
let dependencies = filter.get_dependencies();
252252

253253
// Convert dependencies to FFI format
254-
let deps_ffi: Vec<ffi::VSFilterDependency> =
255-
dependencies.iter().map(super::filter::FilterDependency::as_ffi).collect();
254+
let deps_ffi: Vec<ffi::VSFilterDependency> = dependencies
255+
.iter()
256+
.map(super::filter::FilterDependency::as_ffi)
257+
.collect();
256258

257259
// Box the filter instance for storage
258260
let filter_box = Box::new(filter);
@@ -268,7 +270,7 @@ impl<'core> CoreRef<'core> {
268270
&video_info.as_ffi(),
269271
Some(filter_get_frame::<F>),
270272
Some(filter_free::<F>),
271-
std::ptr::from_ref(&F::MODE.as_ffi()) as i32,
273+
F::MODE.into(),
272274
deps_ffi.as_ptr(),
273275
deps_ffi.len() as i32,
274276
instance_data,
@@ -280,7 +282,7 @@ impl<'core> CoreRef<'core> {
280282
}
281283

282284
/// Create a video filter using the Filter trait (returns node directly)
283-
pub fn create_video_filter2<F>(&self, filter: &F) -> CoreResult<crate::node::Node<'core>>
285+
pub fn create_video_filter2<F>(&self, filter: F) -> CoreResult<crate::node::Node<'core>>
284286
where
285287
F: Filter<'core>,
286288
{
@@ -289,10 +291,12 @@ impl<'core> CoreRef<'core> {
289291
let dependencies = filter.get_dependencies();
290292

291293
// Convert dependencies to FFI format
292-
let deps_ffi: Vec<ffi::VSFilterDependency> =
293-
dependencies.iter().map(super::filter::FilterDependency::as_ffi).collect();
294+
let deps_ffi: Vec<ffi::VSFilterDependency> = dependencies
295+
.iter()
296+
.map(super::filter::FilterDependency::as_ffi)
297+
.collect();
294298

295-
// Box the filter instance for storage
299+
// Box the filter instance for storage (takes ownership, no double boxing)
296300
let filter_box = Box::new(filter);
297301
let instance_data = Box::into_raw(filter_box).cast::<std::ffi::c_void>();
298302

@@ -305,7 +309,7 @@ impl<'core> CoreRef<'core> {
305309
&video_info.as_ffi(),
306310
Some(filter_get_frame::<F>),
307311
Some(filter_free::<F>),
308-
std::ptr::from_ref(&F::MODE.as_ffi()) as i32,
312+
F::MODE.as_ffi() as i32,
309313
deps_ffi.as_ptr(),
310314
deps_ffi.len() as i32,
311315
instance_data,
@@ -314,14 +318,16 @@ impl<'core> CoreRef<'core> {
314318
};
315319

316320
if node_ptr.is_null() {
321+
// Free the boxed filter to prevent memory leak
322+
unsafe { drop(Box::from_raw(instance_data.cast::<F>())) };
317323
return Err(CoreError::VideoFilterCreationFailed);
318324
}
319325

320326
Ok(unsafe { crate::node::Node::from_ptr(node_ptr) })
321327
}
322328

323329
/// Create a audio filter using the Filter trait
324-
pub fn create_audio_filter<F>(&self, filter: &F) -> CoreResult<Map<'_>>
330+
pub fn create_audio_filter<F>(&self, filter: F) -> CoreResult<Map<'_>>
325331
where
326332
F: Filter<'core>,
327333
{
@@ -331,10 +337,12 @@ impl<'core> CoreRef<'core> {
331337
let dependencies = filter.get_dependencies();
332338

333339
// Convert dependencies to FFI format
334-
let deps_ffi: Vec<ffi::VSFilterDependency> =
335-
dependencies.iter().map(super::filter::FilterDependency::as_ffi).collect();
340+
let deps_ffi: Vec<ffi::VSFilterDependency> = dependencies
341+
.iter()
342+
.map(super::filter::FilterDependency::as_ffi)
343+
.collect();
336344

337-
// Box the filter instance for storage
345+
// Box the filter instance for storage (takes ownership, no double boxing)
338346
let filter_box = Box::new(filter);
339347
let instance_data = Box::into_raw(filter_box).cast::<std::ffi::c_void>();
340348

@@ -348,7 +356,7 @@ impl<'core> CoreRef<'core> {
348356
&audio_info.as_ffi(),
349357
Some(filter_get_frame::<F>),
350358
Some(filter_free::<F>),
351-
std::ptr::from_ref(&F::MODE.as_ffi()) as i32,
359+
F::MODE.as_ffi() as i32,
352360
deps_ffi.as_ptr(),
353361
deps_ffi.len() as i32,
354362
instance_data,
@@ -360,7 +368,7 @@ impl<'core> CoreRef<'core> {
360368
}
361369

362370
/// Create an audio filter using the Filter trait (returns node directly)
363-
pub fn create_audio_filter2<F>(&self, filter: &F) -> CoreResult<Node<'core>>
371+
pub fn create_audio_filter2<F>(&self, filter: F) -> CoreResult<Node<'core>>
364372
where
365373
F: Filter<'core>,
366374
{
@@ -369,10 +377,12 @@ impl<'core> CoreRef<'core> {
369377
let dependencies = filter.get_dependencies();
370378

371379
// Convert dependencies to FFI format
372-
let deps_ffi: Vec<ffi::VSFilterDependency> =
373-
dependencies.iter().map(super::filter::FilterDependency::as_ffi).collect();
380+
let deps_ffi: Vec<ffi::VSFilterDependency> = dependencies
381+
.iter()
382+
.map(super::filter::FilterDependency::as_ffi)
383+
.collect();
374384

375-
// Box the filter instance for storage
385+
// Box the filter instance for storage (takes ownership, no double boxing)
376386
let filter_box = Box::new(filter);
377387
let instance_data = Box::into_raw(filter_box).cast::<std::ffi::c_void>();
378388

@@ -385,7 +395,7 @@ impl<'core> CoreRef<'core> {
385395
std::ptr::from_ref(&audio_info.as_ffi()),
386396
Some(filter_get_frame::<F>),
387397
Some(filter_free::<F>),
388-
std::ptr::from_ref(&F::MODE.as_ffi()) as i32,
398+
F::MODE.as_ffi() as i32,
389399
deps_ffi.as_ptr(),
390400
deps_ffi.len() as i32,
391401
instance_data,
@@ -394,6 +404,8 @@ impl<'core> CoreRef<'core> {
394404
};
395405

396406
if node_ptr.is_null() {
407+
// Free the boxed filter to prevent memory leak
408+
unsafe { drop(Box::from_raw(instance_data.cast::<F>())) };
397409
return Err(CoreError::AudioFilterCreationFailed);
398410
}
399411

@@ -497,7 +509,7 @@ impl CoreRef<'_> {
497509
}
498510

499511
/// Returns true if node timing is enabled.
500-
#[must_use]
512+
#[must_use]
501513
pub fn get_node_timing(&self) -> bool {
502514
(unsafe { API::get_cached().get_core_node_timing(self.as_ptr()) } > 0)
503515
}
@@ -577,36 +589,36 @@ pub struct CoreBuilder {
577589

578590
impl<'core> CoreBuilder {
579591
/// Creates a new `CoreBuilder` with default flags.
580-
#[must_use]
592+
#[must_use]
581593
pub const fn new() -> Self {
582594
Self {
583595
flags: CoreCreationFlags::NONE,
584596
}
585597
}
586598

587599
/// Enables graph inspection API functions.
588-
#[must_use]
600+
#[must_use]
589601
pub fn with_graph_inspection(mut self) -> Self {
590602
self.flags |= CoreCreationFlags::ENABLE_GRAPH_INSPECTION;
591603
self
592604
}
593605

594606
/// Disables autoloading of user plugins.
595-
#[must_use]
607+
#[must_use]
596608
pub fn disable_auto_loading(mut self) -> Self {
597609
self.flags |= CoreCreationFlags::DISABLE_AUTO_LOADING;
598610
self
599611
}
600612

601613
/// Disables unloading of plugin libraries when the core is destroyed.
602-
#[must_use]
614+
#[must_use]
603615
pub fn disable_library_unloading(mut self) -> Self {
604616
self.flags |= CoreCreationFlags::DISABLE_LIBRARY_UNLOADING;
605617
self
606618
}
607619

608620
/// Builds and returns a [`CoreRef`].
609-
#[must_use]
621+
#[must_use]
610622
pub fn build(self) -> CoreRef<'core> {
611623
CoreRef::new(self.flags)
612624
}

0 commit comments

Comments
 (0)