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.
This commit is contained in:
Sebastian Dröge 2022-04-03 11:15:19 +03:00
parent 8908d4bc98
commit 73ab9054c4
5 changed files with 68 additions and 91 deletions

View file

@ -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<RefCell<Box<dyn FnMut(&AppSink) + Send + 'static>>>,
eos: Option<Box<dyn FnMut(&AppSink) + Send + 'static>>,
new_preroll: Option<
RefCell<
Box<dyn FnMut(&AppSink) -> Result<gst::FlowSuccess, gst::FlowError> + Send + 'static>,
>,
Box<dyn FnMut(&AppSink) -> Result<gst::FlowSuccess, gst::FlowError> + Send + 'static>,
>,
new_sample: Option<
RefCell<
Box<dyn FnMut(&AppSink) -> Result<gst::FlowSuccess, gst::FlowError> + Send + 'static>,
>,
Box<dyn FnMut(&AppSink) -> Result<gst::FlowSuccess, gst::FlowError> + Send + 'static>,
>,
new_event: Option<RefCell<Box<dyn FnMut(&AppSink) -> bool + Send + 'static>>>,
new_event: Option<Box<dyn FnMut(&AppSink) -> 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<RefCell<Box<dyn FnMut(&AppSink) + Send + 'static>>>,
eos: Option<Box<dyn FnMut(&AppSink) + Send + 'static>>,
new_preroll: Option<
RefCell<
Box<dyn FnMut(&AppSink) -> Result<gst::FlowSuccess, gst::FlowError> + Send + 'static>,
>,
Box<dyn FnMut(&AppSink) -> Result<gst::FlowSuccess, gst::FlowError> + Send + 'static>,
>,
new_sample: Option<
RefCell<
Box<dyn FnMut(&AppSink) -> Result<gst::FlowSuccess, gst::FlowError> + Send + 'static>,
>,
Box<dyn FnMut(&AppSink) -> Result<gst::FlowSuccess, gst::FlowError> + Send + 'static>,
>,
new_event: Option<RefCell<Box<dyn FnMut(&AppSink) -> bool + Send + 'static>>>,
new_event: Option<Box<dyn FnMut(&AppSink) -> bool + Send + 'static>>,
}
impl AppSinkCallbacksBuilder {
pub fn eos<F: FnMut(&AppSink) + Send + 'static>(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<F: FnMut(&AppSink) -> 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<AppSink> = from_glib_borrow(appsink);
if callbacks.panicked.load(Ordering::Relaxed) {
if (*callbacks).panicked.load(Ordering::Relaxed) {
let element: Borrowed<AppSink> = 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<AppSink> = from_glib_borrow(appsink);
if callbacks.panicked.load(Ordering::Relaxed) {
if (*callbacks).panicked.load(Ordering::Relaxed) {
let element: Borrowed<AppSink> = 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<AppSink> = from_glib_borrow(appsink);
if callbacks.panicked.load(Ordering::Relaxed) {
if (*callbacks).panicked.load(Ordering::Relaxed) {
let element: Borrowed<AppSink> = 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<AppSink> = from_glib_borrow(appsink);
if callbacks.panicked.load(Ordering::Relaxed) {
if (*callbacks).panicked.load(Ordering::Relaxed) {
let element: Borrowed<AppSink> = 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

View file

@ -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<RefCell<Box<dyn FnMut(&AppSrc, u32) + Send + 'static>>>,
need_data: Option<Box<dyn FnMut(&AppSrc, u32) + Send + 'static>>,
enough_data: Option<Box<dyn Fn(&AppSrc) + Send + Sync + 'static>>,
seek_data: Option<Box<dyn Fn(&AppSrc, u64) -> 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<RefCell<Box<dyn FnMut(&AppSrc, u32) + Send + 'static>>>,
need_data: Option<Box<dyn FnMut(&AppSrc, u32) + Send + 'static>>,
enough_data: Option<Box<dyn Fn(&AppSrc) + Send + Sync + 'static>>,
seek_data: Option<Box<dyn Fn(&AppSrc, u64) -> bool + Send + Sync + 'static>>,
}
@ -50,7 +49,7 @@ pub struct AppSrcCallbacksBuilder {
impl AppSrcCallbacksBuilder {
pub fn need_data<F: FnMut(&AppSrc, u32) + Send + 'static>(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<AppSrc> = from_glib_borrow(appsrc);
if callbacks.panicked.load(Ordering::Relaxed) {
if (*callbacks).panicked.load(Ordering::Relaxed) {
let element: Borrowed<AppSrc> = 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<AppSrc> = from_glib_borrow(appsrc);
if callbacks.panicked.load(Ordering::Relaxed) {
if (*callbacks).panicked.load(Ordering::Relaxed) {
let element: Borrowed<AppSrc> = 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<AppSrc> = from_glib_borrow(appsrc);
if callbacks.panicked.load(Ordering::Relaxed) {
if (*callbacks).panicked.load(Ordering::Relaxed) {
let element: Borrowed<AppSrc> = 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

View file

@ -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<F: FnMut(&RTSPSessionPool) -> Continue + Send + 'static>(
pool: *mut ffi::GstRTSPSessionPool,
func: gpointer,
) -> gboolean {
let func: &RefCell<F> = &*(func as *const RefCell<F>);
(*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::<RefCell<F>>::from_raw(ptr as *mut _);
Box::<F>::from_raw(ptr as *mut _);
}
fn into_raw_watch<F: FnMut(&RTSPSessionPool) -> Continue + Send + 'static>(func: F) -> gpointer {
#[allow(clippy::type_complexity)]
let func: Box<RefCell<F>> = Box::new(RefCell::new(func));
let func: Box<F> = Box::new(func);
Box::into_raw(func) as gpointer
}

View file

@ -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<F: FnMut(&Bus, &Message) -> Continue + Sen
msg: *mut ffi::GstMessage,
func: gpointer,
) -> gboolean {
let func: &RefCell<F> = &*(func as *const RefCell<F>);
(*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::<RefCell<F>>::from_raw(ptr as *mut _);
Box::<F>::from_raw(ptr as *mut _);
}
fn into_raw_watch<F: FnMut(&Bus, &Message) -> Continue + Send + 'static>(func: F) -> gpointer {
#[allow(clippy::type_complexity)]
let func: Box<RefCell<F>> = Box::new(RefCell::new(func));
let func: Box<F> = Box::new(func);
Box::into_raw(func) as gpointer
}
@ -46,22 +45,21 @@ unsafe extern "C" fn trampoline_watch_local<F: FnMut(&Bus, &Message) -> Continue
msg: *mut ffi::GstMessage,
func: gpointer,
) -> gboolean {
let func: &glib::thread_guard::ThreadGuard<RefCell<F>> =
&*(func as *const glib::thread_guard::ThreadGuard<RefCell<F>>);
(*func.get_ref().borrow_mut())(&from_glib_borrow(bus), &Message::from_glib_borrow(msg))
.into_glib()
let func: &mut glib::thread_guard::ThreadGuard<F> =
&mut *(func as *mut glib::thread_guard::ThreadGuard<F>);
(func.get_mut())(&from_glib_borrow(bus), &Message::from_glib_borrow(msg)).into_glib()
}
unsafe extern "C" fn destroy_closure_watch_local<F: FnMut(&Bus, &Message) -> Continue + 'static>(
ptr: gpointer,
) {
Box::<glib::thread_guard::ThreadGuard<RefCell<F>>>::from_raw(ptr as *mut _);
Box::<glib::thread_guard::ThreadGuard<F>>::from_raw(ptr as *mut _);
}
fn into_raw_watch_local<F: FnMut(&Bus, &Message) -> Continue + 'static>(func: F) -> gpointer {
#[allow(clippy::type_complexity)]
let func: Box<glib::thread_guard::ThreadGuard<RefCell<F>>> =
Box::new(glib::thread_guard::ThreadGuard::new(RefCell::new(func)));
let func: Box<glib::thread_guard::ThreadGuard<F>> =
Box::new(glib::thread_guard::ThreadGuard::new(func));
Box::into_raw(func) as gpointer
}

View file

@ -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<F>(ptr: gpointer) {
}
unsafe extern "C" fn trampoline_pad_task<F: FnMut() + Send + 'static>(func: gpointer) {
let func: &RefCell<F> = &*(func as *const RefCell<F>);
(*func.borrow_mut())()
let func: &mut F = &mut *(func as *mut F);
func()
}
fn into_raw_pad_task<F: FnMut() + Send + 'static>(func: F) -> gpointer {
#[allow(clippy::type_complexity)]
let func: Box<RefCell<F>> = Box::new(RefCell::new(func));
let func: Box<F> = Box::new(func);
Box::into_raw(func) as gpointer
}
unsafe extern "C" fn destroy_closure_pad_task<F>(ptr: gpointer) {
Box::<RefCell<F>>::from_raw(ptr as *mut _);
Box::<F>::from_raw(ptr as *mut _);
}
impl Pad {