From b59f90e634be97963e85b76524f6d563f5c636a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Dr=C3=B6ge?= Date: Sun, 3 Apr 2022 11:15:19 +0300 Subject: [PATCH] Don't use unnecessary RefCell wrappers for FnMut callbacks They don't add any safety as this is via unsafe code anyway and are not needed to get mutable references in this context anyway, while adding a bit of runtime overhead. --- gstreamer-app/src/app_sink.rs | 79 ++++++++----------- gstreamer-app/src/app_src.rs | 40 +++++----- .../src/rtsp_session_pool.rs | 9 +-- gstreamer/src/bus.rs | 22 +++--- gstreamer/src/pad.rs | 9 +-- 5 files changed, 68 insertions(+), 91 deletions(-) diff --git a/gstreamer-app/src/app_sink.rs b/gstreamer-app/src/app_sink.rs index b84285990..1d00f6495 100644 --- a/gstreamer-app/src/app_sink.rs +++ b/gstreamer-app/src/app_sink.rs @@ -4,7 +4,6 @@ use crate::AppSink; use glib::ffi::gpointer; use glib::prelude::*; use glib::translate::*; -use std::cell::RefCell; use std::mem; use std::panic; use std::ptr; @@ -22,18 +21,14 @@ use { #[allow(clippy::type_complexity)] pub struct AppSinkCallbacks { - eos: Option>>, + eos: Option>, new_preroll: Option< - RefCell< - Box Result + Send + 'static>, - >, + Box Result + Send + 'static>, >, new_sample: Option< - RefCell< - Box Result + Send + 'static>, - >, + Box Result + Send + 'static>, >, - new_event: Option bool + Send + 'static>>>, + new_event: Option bool + Send + 'static>>, panicked: AtomicBool, callbacks: ffi::GstAppSinkCallbacks, } @@ -56,24 +51,20 @@ impl AppSinkCallbacks { #[allow(clippy::type_complexity)] #[must_use = "The builder must be built to be used"] pub struct AppSinkCallbacksBuilder { - eos: Option>>, + eos: Option>, new_preroll: Option< - RefCell< - Box Result + Send + 'static>, - >, + Box Result + Send + 'static>, >, new_sample: Option< - RefCell< - Box Result + Send + 'static>, - >, + Box Result + Send + 'static>, >, - new_event: Option bool + Send + 'static>>>, + new_event: Option bool + Send + 'static>>, } impl AppSinkCallbacksBuilder { pub fn eos(self, eos: F) -> Self { Self { - eos: Some(RefCell::new(Box::new(eos))), + eos: Some(Box::new(eos)), ..self } } @@ -85,7 +76,7 @@ impl AppSinkCallbacksBuilder { new_preroll: F, ) -> Self { Self { - new_preroll: Some(RefCell::new(Box::new(new_preroll))), + new_preroll: Some(Box::new(new_preroll)), ..self } } @@ -97,7 +88,7 @@ impl AppSinkCallbacksBuilder { new_sample: F, ) -> Self { Self { - new_sample: Some(RefCell::new(Box::new(new_sample))), + new_sample: Some(Box::new(new_sample)), ..self } } @@ -106,7 +97,7 @@ impl AppSinkCallbacksBuilder { #[cfg_attr(feature = "dox", doc(cfg(feature = "v1_20")))] pub fn new_event bool + Send + 'static>(self, new_event: F) -> Self { Self { - new_event: Some(RefCell::new(Box::new(new_event))), + new_event: Some(Box::new(new_event)), ..self } } @@ -159,21 +150,21 @@ fn post_panic_error_message(element: &AppSink, err: &dyn std::any::Any) { } unsafe extern "C" fn trampoline_eos(appsink: *mut ffi::GstAppSink, callbacks: gpointer) { - let callbacks = &*(callbacks as *const AppSinkCallbacks); + let callbacks = callbacks as *mut AppSinkCallbacks; let element: Borrowed = from_glib_borrow(appsink); - if callbacks.panicked.load(Ordering::Relaxed) { + if (*callbacks).panicked.load(Ordering::Relaxed) { let element: Borrowed = from_glib_borrow(appsink); gst::element_error!(element, gst::LibraryError::Failed, ["Panicked"]); return; } - if let Some(ref eos) = callbacks.eos { - let result = panic::catch_unwind(panic::AssertUnwindSafe(|| (*eos.borrow_mut())(&element))); + if let Some(ref mut eos) = (*callbacks).eos { + let result = panic::catch_unwind(panic::AssertUnwindSafe(|| eos(&element))); match result { Ok(result) => result, Err(err) => { - callbacks.panicked.store(true, Ordering::Relaxed); + (*callbacks).panicked.store(true, Ordering::Relaxed); post_panic_error_message(&element, &err); } } @@ -184,23 +175,21 @@ unsafe extern "C" fn trampoline_new_preroll( appsink: *mut ffi::GstAppSink, callbacks: gpointer, ) -> gst::ffi::GstFlowReturn { - let callbacks = &*(callbacks as *const AppSinkCallbacks); + let callbacks = callbacks as *mut AppSinkCallbacks; let element: Borrowed = from_glib_borrow(appsink); - if callbacks.panicked.load(Ordering::Relaxed) { + if (*callbacks).panicked.load(Ordering::Relaxed) { let element: Borrowed = from_glib_borrow(appsink); gst::element_error!(element, gst::LibraryError::Failed, ["Panicked"]); return gst::FlowReturn::Error.into_glib(); } - let ret = if let Some(ref new_preroll) = callbacks.new_preroll { - let result = panic::catch_unwind(panic::AssertUnwindSafe(|| { - (*new_preroll.borrow_mut())(&element).into() - })); + let ret = if let Some(ref mut new_preroll) = (*callbacks).new_preroll { + let result = panic::catch_unwind(panic::AssertUnwindSafe(|| new_preroll(&element).into())); match result { Ok(result) => result, Err(err) => { - callbacks.panicked.store(true, Ordering::Relaxed); + (*callbacks).panicked.store(true, Ordering::Relaxed); post_panic_error_message(&element, &err); gst::FlowReturn::Error @@ -217,23 +206,21 @@ unsafe extern "C" fn trampoline_new_sample( appsink: *mut ffi::GstAppSink, callbacks: gpointer, ) -> gst::ffi::GstFlowReturn { - let callbacks = &*(callbacks as *const AppSinkCallbacks); + let callbacks = callbacks as *mut AppSinkCallbacks; let element: Borrowed = from_glib_borrow(appsink); - if callbacks.panicked.load(Ordering::Relaxed) { + if (*callbacks).panicked.load(Ordering::Relaxed) { let element: Borrowed = from_glib_borrow(appsink); gst::element_error!(element, gst::LibraryError::Failed, ["Panicked"]); return gst::FlowReturn::Error.into_glib(); } - let ret = if let Some(ref new_sample) = callbacks.new_sample { - let result = panic::catch_unwind(panic::AssertUnwindSafe(|| { - (*new_sample.borrow_mut())(&element).into() - })); + let ret = if let Some(ref mut new_sample) = (*callbacks).new_sample { + let result = panic::catch_unwind(panic::AssertUnwindSafe(|| new_sample(&element).into())); match result { Ok(result) => result, Err(err) => { - callbacks.panicked.store(true, Ordering::Relaxed); + (*callbacks).panicked.store(true, Ordering::Relaxed); post_panic_error_message(&element, &err); gst::FlowReturn::Error @@ -250,23 +237,21 @@ unsafe extern "C" fn trampoline_new_event( appsink: *mut ffi::GstAppSink, callbacks: gpointer, ) -> glib::ffi::gboolean { - let callbacks = &*(callbacks as *const AppSinkCallbacks); + let callbacks = callbacks as *mut AppSinkCallbacks; let element: Borrowed = from_glib_borrow(appsink); - if callbacks.panicked.load(Ordering::Relaxed) { + if (*callbacks).panicked.load(Ordering::Relaxed) { let element: Borrowed = from_glib_borrow(appsink); gst::element_error!(element, gst::LibraryError::Failed, ["Panicked"]); return false.into_glib(); } - let ret = if let Some(ref new_event) = callbacks.new_event { - let result = panic::catch_unwind(panic::AssertUnwindSafe(|| { - (*new_event.borrow_mut())(&element) - })); + let ret = if let Some(ref mut new_event) = (*callbacks).new_event { + let result = panic::catch_unwind(panic::AssertUnwindSafe(|| new_event(&element))); match result { Ok(result) => result, Err(err) => { - callbacks.panicked.store(true, Ordering::Relaxed); + (*callbacks).panicked.store(true, Ordering::Relaxed); post_panic_error_message(&element, &err); false diff --git a/gstreamer-app/src/app_src.rs b/gstreamer-app/src/app_src.rs index b9e2c99ae..563aa344b 100644 --- a/gstreamer-app/src/app_src.rs +++ b/gstreamer-app/src/app_src.rs @@ -6,7 +6,6 @@ use glib::prelude::*; use glib::translate::*; use crate::AppSrc; -use std::cell::RefCell; use std::mem; use std::panic; use std::pin::Pin; @@ -17,7 +16,7 @@ use std::task::{Context, Poll, Waker}; #[allow(clippy::type_complexity)] pub struct AppSrcCallbacks { - need_data: Option>>, + need_data: Option>, enough_data: Option>, seek_data: Option bool + Send + Sync + 'static>>, panicked: AtomicBool, @@ -42,7 +41,7 @@ impl AppSrcCallbacks { #[allow(clippy::type_complexity)] #[must_use = "The builder must be built to be used"] pub struct AppSrcCallbacksBuilder { - need_data: Option>>, + need_data: Option>, enough_data: Option>, seek_data: Option bool + Send + Sync + 'static>>, } @@ -50,7 +49,7 @@ pub struct AppSrcCallbacksBuilder { impl AppSrcCallbacksBuilder { pub fn need_data(self, need_data: F) -> Self { Self { - need_data: Some(RefCell::new(Box::new(need_data))), + need_data: Some(Box::new(need_data)), ..self } } @@ -126,23 +125,21 @@ unsafe extern "C" fn trampoline_need_data( length: u32, callbacks: gpointer, ) { - let callbacks = &*(callbacks as *const AppSrcCallbacks); + let callbacks = callbacks as *mut AppSrcCallbacks; let element: Borrowed = from_glib_borrow(appsrc); - if callbacks.panicked.load(Ordering::Relaxed) { + if (*callbacks).panicked.load(Ordering::Relaxed) { let element: Borrowed = from_glib_borrow(appsrc); gst::element_error!(element, gst::LibraryError::Failed, ["Panicked"]); return; } - if let Some(ref need_data) = callbacks.need_data { - let result = panic::catch_unwind(panic::AssertUnwindSafe(|| { - (*need_data.borrow_mut())(&element, length) - })); + if let Some(ref mut need_data) = (*callbacks).need_data { + let result = panic::catch_unwind(panic::AssertUnwindSafe(|| need_data(&element, length))); match result { Ok(result) => result, Err(err) => { - callbacks.panicked.store(true, Ordering::Relaxed); + (*callbacks).panicked.store(true, Ordering::Relaxed); post_panic_error_message(&element, &err); } } @@ -150,21 +147,21 @@ unsafe extern "C" fn trampoline_need_data( } unsafe extern "C" fn trampoline_enough_data(appsrc: *mut ffi::GstAppSrc, callbacks: gpointer) { - let callbacks = &*(callbacks as *const AppSrcCallbacks); + let callbacks = callbacks as *const AppSrcCallbacks; let element: Borrowed = from_glib_borrow(appsrc); - if callbacks.panicked.load(Ordering::Relaxed) { + if (*callbacks).panicked.load(Ordering::Relaxed) { let element: Borrowed = from_glib_borrow(appsrc); gst::element_error!(element, gst::LibraryError::Failed, ["Panicked"]); return; } - if let Some(ref enough_data) = callbacks.enough_data { - let result = panic::catch_unwind(panic::AssertUnwindSafe(|| (*enough_data)(&element))); + if let Some(ref enough_data) = (*callbacks).enough_data { + let result = panic::catch_unwind(panic::AssertUnwindSafe(|| enough_data(&element))); match result { Ok(result) => result, Err(err) => { - callbacks.panicked.store(true, Ordering::Relaxed); + (*callbacks).panicked.store(true, Ordering::Relaxed); post_panic_error_message(&element, &err); } } @@ -176,22 +173,21 @@ unsafe extern "C" fn trampoline_seek_data( offset: u64, callbacks: gpointer, ) -> gboolean { - let callbacks = &*(callbacks as *const AppSrcCallbacks); + let callbacks = callbacks as *const AppSrcCallbacks; let element: Borrowed = from_glib_borrow(appsrc); - if callbacks.panicked.load(Ordering::Relaxed) { + if (*callbacks).panicked.load(Ordering::Relaxed) { let element: Borrowed = from_glib_borrow(appsrc); gst::element_error!(element, gst::LibraryError::Failed, ["Panicked"]); return false.into_glib(); } - let ret = if let Some(ref seek_data) = callbacks.seek_data { - let result = - panic::catch_unwind(panic::AssertUnwindSafe(|| (*seek_data)(&element, offset))); + let ret = if let Some(ref seek_data) = (*callbacks).seek_data { + let result = panic::catch_unwind(panic::AssertUnwindSafe(|| seek_data(&element, offset))); match result { Ok(result) => result, Err(err) => { - callbacks.panicked.store(true, Ordering::Relaxed); + (*callbacks).panicked.store(true, Ordering::Relaxed); post_panic_error_message(&element, &err); false diff --git a/gstreamer-rtsp-server/src/rtsp_session_pool.rs b/gstreamer-rtsp-server/src/rtsp_session_pool.rs index 403853d56..579a9dc62 100644 --- a/gstreamer-rtsp-server/src/rtsp_session_pool.rs +++ b/gstreamer-rtsp-server/src/rtsp_session_pool.rs @@ -5,15 +5,14 @@ use glib::ffi::{gboolean, gpointer}; use glib::prelude::*; use glib::source::{Continue, Priority}; use glib::translate::*; -use std::cell::RefCell; use std::mem::transmute; unsafe extern "C" fn trampoline_watch Continue + Send + 'static>( pool: *mut ffi::GstRTSPSessionPool, func: gpointer, ) -> gboolean { - let func: &RefCell = &*(func as *const RefCell); - (*func.borrow_mut())(&from_glib_borrow(pool)).into_glib() + let func: &mut F = &mut *(func as *mut F); + func(&from_glib_borrow(pool)).into_glib() } unsafe extern "C" fn destroy_closure_watch< @@ -21,12 +20,12 @@ unsafe extern "C" fn destroy_closure_watch< >( ptr: gpointer, ) { - Box::>::from_raw(ptr as *mut _); + Box::::from_raw(ptr as *mut _); } fn into_raw_watch Continue + Send + 'static>(func: F) -> gpointer { #[allow(clippy::type_complexity)] - let func: Box> = Box::new(RefCell::new(func)); + let func: Box = Box::new(func); Box::into_raw(func) as gpointer } diff --git a/gstreamer/src/bus.rs b/gstreamer/src/bus.rs index 4ad3ec83b..9e24e58bf 100644 --- a/gstreamer/src/bus.rs +++ b/gstreamer/src/bus.rs @@ -7,7 +7,6 @@ use glib::ffi::{gboolean, gpointer}; use glib::prelude::*; use glib::source::{Continue, Priority, SourceId}; use glib::translate::*; -use std::cell::RefCell; use std::future; use std::mem::transmute; use std::pin::Pin; @@ -23,8 +22,8 @@ unsafe extern "C" fn trampoline_watch Continue + Sen msg: *mut ffi::GstMessage, func: gpointer, ) -> gboolean { - let func: &RefCell = &*(func as *const RefCell); - (*func.borrow_mut())(&from_glib_borrow(bus), &Message::from_glib_borrow(msg)).into_glib() + let func: &mut F = &mut *(func as *mut F); + func(&from_glib_borrow(bus), &Message::from_glib_borrow(msg)).into_glib() } unsafe extern "C" fn destroy_closure_watch< @@ -32,12 +31,12 @@ unsafe extern "C" fn destroy_closure_watch< >( ptr: gpointer, ) { - Box::>::from_raw(ptr as *mut _); + Box::::from_raw(ptr as *mut _); } fn into_raw_watch Continue + Send + 'static>(func: F) -> gpointer { #[allow(clippy::type_complexity)] - let func: Box> = Box::new(RefCell::new(func)); + let func: Box = Box::new(func); Box::into_raw(func) as gpointer } @@ -46,22 +45,21 @@ unsafe extern "C" fn trampoline_watch_local Continue msg: *mut ffi::GstMessage, func: gpointer, ) -> gboolean { - let func: &glib::thread_guard::ThreadGuard> = - &*(func as *const glib::thread_guard::ThreadGuard>); - (*func.get_ref().borrow_mut())(&from_glib_borrow(bus), &Message::from_glib_borrow(msg)) - .into_glib() + let func: &mut glib::thread_guard::ThreadGuard = + &mut *(func as *mut glib::thread_guard::ThreadGuard); + (func.get_mut())(&from_glib_borrow(bus), &Message::from_glib_borrow(msg)).into_glib() } unsafe extern "C" fn destroy_closure_watch_local Continue + 'static>( ptr: gpointer, ) { - Box::>>::from_raw(ptr as *mut _); + Box::>::from_raw(ptr as *mut _); } fn into_raw_watch_local Continue + 'static>(func: F) -> gpointer { #[allow(clippy::type_complexity)] - let func: Box>> = - Box::new(glib::thread_guard::ThreadGuard::new(RefCell::new(func))); + let func: Box> = + Box::new(glib::thread_guard::ThreadGuard::new(func)); Box::into_raw(func) as gpointer } diff --git a/gstreamer/src/pad.rs b/gstreamer/src/pad.rs index a87538eaa..661e93613 100644 --- a/gstreamer/src/pad.rs +++ b/gstreamer/src/pad.rs @@ -19,7 +19,6 @@ use crate::QueryRef; use crate::StaticPadTemplate; use crate::{SpecificFormattedValue, SpecificFormattedValueIntrinsic}; -use std::cell::RefCell; use std::mem; use std::num::NonZeroU64; use std::ops::ControlFlow; @@ -1597,18 +1596,18 @@ unsafe extern "C" fn destroy_closure(ptr: gpointer) { } unsafe extern "C" fn trampoline_pad_task(func: gpointer) { - let func: &RefCell = &*(func as *const RefCell); - (*func.borrow_mut())() + let func: &mut F = &mut *(func as *mut F); + func() } fn into_raw_pad_task(func: F) -> gpointer { #[allow(clippy::type_complexity)] - let func: Box> = Box::new(RefCell::new(func)); + let func: Box = Box::new(func); Box::into_raw(func) as gpointer } unsafe extern "C" fn destroy_closure_pad_task(ptr: gpointer) { - Box::>::from_raw(ptr as *mut _); + Box::::from_raw(ptr as *mut _); } impl Pad {