From 3e2eb6e65271e34f3e362108e50a266b7c3f0298 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Dr=C3=B6ge?= Date: Mon, 16 Jan 2023 10:17:29 +0200 Subject: [PATCH] gstreamer: Reduce code bloat in panic handling Part-of: --- gstreamer-app/src/app_sink.rs | 43 +++++++++++++---------- gstreamer-app/src/app_src.rs | 35 ++++++++++--------- gstreamer/src/pad.rs | 25 +++---------- gstreamer/src/subclass/element.rs | 1 + gstreamer/src/subclass/error.rs | 58 +++++++++++++++++++++++-------- gstreamer/src/subclass/mod.rs | 2 +- gstreamer/src/subclass/plugin.rs | 17 +++++---- 7 files changed, 100 insertions(+), 81 deletions(-) diff --git a/gstreamer-app/src/app_sink.rs b/gstreamer-app/src/app_sink.rs index bedb43772..b1c352da9 100644 --- a/gstreamer-app/src/app_sink.rs +++ b/gstreamer-app/src/app_sink.rs @@ -135,24 +135,13 @@ impl AppSinkCallbacksBuilder { } } -fn post_panic_error_message(element: &AppSink, err: &dyn std::any::Any) { - skip_assert_initialized!(); - if let Some(cause) = err.downcast_ref::<&str>() { - gst::element_error!(element, gst::LibraryError::Failed, ["Panicked: {}", cause]); - } else if let Some(cause) = err.downcast_ref::() { - gst::element_error!(element, gst::LibraryError::Failed, ["Panicked: {}", cause]); - } else { - gst::element_error!(element, gst::LibraryError::Failed, ["Panicked"]); - } -} - unsafe extern "C" fn trampoline_eos(appsink: *mut ffi::GstAppSink, callbacks: gpointer) { let callbacks = callbacks as *mut AppSinkCallbacks; let element: Borrowed = from_glib_borrow(appsink); if (*callbacks).panicked.load(Ordering::Relaxed) { let element: Borrowed = from_glib_borrow(appsink); - gst::element_error!(element, gst::LibraryError::Failed, ["Panicked"]); + gst::subclass::post_panic_error_message(element.upcast_ref(), element.upcast_ref(), None); return; } @@ -162,7 +151,11 @@ unsafe extern "C" fn trampoline_eos(appsink: *mut ffi::GstAppSink, callbacks: gp Ok(result) => result, Err(err) => { (*callbacks).panicked.store(true, Ordering::Relaxed); - post_panic_error_message(&element, &err); + gst::subclass::post_panic_error_message( + element.upcast_ref(), + element.upcast_ref(), + Some(err), + ); } } } @@ -177,7 +170,7 @@ unsafe extern "C" fn trampoline_new_preroll( if (*callbacks).panicked.load(Ordering::Relaxed) { let element: Borrowed = from_glib_borrow(appsink); - gst::element_error!(element, gst::LibraryError::Failed, ["Panicked"]); + gst::subclass::post_panic_error_message(element.upcast_ref(), element.upcast_ref(), None); return gst::FlowReturn::Error.into_glib(); } @@ -187,7 +180,11 @@ unsafe extern "C" fn trampoline_new_preroll( Ok(result) => result, Err(err) => { (*callbacks).panicked.store(true, Ordering::Relaxed); - post_panic_error_message(&element, &err); + gst::subclass::post_panic_error_message( + element.upcast_ref(), + element.upcast_ref(), + Some(err), + ); gst::FlowReturn::Error } @@ -208,7 +205,7 @@ unsafe extern "C" fn trampoline_new_sample( if (*callbacks).panicked.load(Ordering::Relaxed) { let element: Borrowed = from_glib_borrow(appsink); - gst::element_error!(element, gst::LibraryError::Failed, ["Panicked"]); + gst::subclass::post_panic_error_message(element.upcast_ref(), element.upcast_ref(), None); return gst::FlowReturn::Error.into_glib(); } @@ -218,7 +215,11 @@ unsafe extern "C" fn trampoline_new_sample( Ok(result) => result, Err(err) => { (*callbacks).panicked.store(true, Ordering::Relaxed); - post_panic_error_message(&element, &err); + gst::subclass::post_panic_error_message( + element.upcast_ref(), + element.upcast_ref(), + Some(err), + ); gst::FlowReturn::Error } @@ -239,7 +240,7 @@ unsafe extern "C" fn trampoline_new_event( if (*callbacks).panicked.load(Ordering::Relaxed) { let element: Borrowed = from_glib_borrow(appsink); - gst::element_error!(element, gst::LibraryError::Failed, ["Panicked"]); + gst::subclass::post_panic_error_message(element.upcast_ref(), element.upcast_ref(), None); return false.into_glib(); } @@ -249,7 +250,11 @@ unsafe extern "C" fn trampoline_new_event( Ok(result) => result, Err(err) => { (*callbacks).panicked.store(true, Ordering::Relaxed); - post_panic_error_message(&element, &err); + gst::subclass::post_panic_error_message( + element.upcast_ref(), + element.upcast_ref(), + Some(err), + ); false } diff --git a/gstreamer-app/src/app_src.rs b/gstreamer-app/src/app_src.rs index 224bb5d64..9560275ea 100644 --- a/gstreamer-app/src/app_src.rs +++ b/gstreamer-app/src/app_src.rs @@ -115,17 +115,6 @@ impl AppSrcCallbacksBuilder { } } -fn post_panic_error_message(element: &AppSrc, err: &dyn std::any::Any) { - skip_assert_initialized!(); - if let Some(cause) = err.downcast_ref::<&str>() { - gst::element_error!(element, gst::LibraryError::Failed, ["Panicked: {}", cause]); - } else if let Some(cause) = err.downcast_ref::() { - gst::element_error!(element, gst::LibraryError::Failed, ["Panicked: {}", cause]); - } else { - gst::element_error!(element, gst::LibraryError::Failed, ["Panicked"]); - } -} - unsafe extern "C" fn trampoline_need_data( appsrc: *mut ffi::GstAppSrc, length: u32, @@ -136,7 +125,7 @@ unsafe extern "C" fn trampoline_need_data( if (*callbacks).panicked.load(Ordering::Relaxed) { let element: Borrowed = from_glib_borrow(appsrc); - gst::element_error!(element, gst::LibraryError::Failed, ["Panicked"]); + gst::subclass::post_panic_error_message(element.upcast_ref(), element.upcast_ref(), None); return; } @@ -146,7 +135,11 @@ unsafe extern "C" fn trampoline_need_data( Ok(result) => result, Err(err) => { (*callbacks).panicked.store(true, Ordering::Relaxed); - post_panic_error_message(&element, &err); + gst::subclass::post_panic_error_message( + element.upcast_ref(), + element.upcast_ref(), + Some(err), + ); } } } @@ -158,7 +151,7 @@ unsafe extern "C" fn trampoline_enough_data(appsrc: *mut ffi::GstAppSrc, callbac if (*callbacks).panicked.load(Ordering::Relaxed) { let element: Borrowed = from_glib_borrow(appsrc); - gst::element_error!(element, gst::LibraryError::Failed, ["Panicked"]); + gst::subclass::post_panic_error_message(element.upcast_ref(), element.upcast_ref(), None); return; } @@ -168,7 +161,11 @@ unsafe extern "C" fn trampoline_enough_data(appsrc: *mut ffi::GstAppSrc, callbac Ok(result) => result, Err(err) => { (*callbacks).panicked.store(true, Ordering::Relaxed); - post_panic_error_message(&element, &err); + gst::subclass::post_panic_error_message( + element.upcast_ref(), + element.upcast_ref(), + Some(err), + ); } } } @@ -184,7 +181,7 @@ unsafe extern "C" fn trampoline_seek_data( if (*callbacks).panicked.load(Ordering::Relaxed) { let element: Borrowed = from_glib_borrow(appsrc); - gst::element_error!(element, gst::LibraryError::Failed, ["Panicked"]); + gst::subclass::post_panic_error_message(element.upcast_ref(), element.upcast_ref(), None); return false.into_glib(); } @@ -194,7 +191,11 @@ unsafe extern "C" fn trampoline_seek_data( Ok(result) => result, Err(err) => { (*callbacks).panicked.store(true, Ordering::Relaxed); - post_panic_error_message(&element, &err); + gst::subclass::post_panic_error_message( + element.upcast_ref(), + element.upcast_ref(), + Some(err), + ); false } diff --git a/gstreamer/src/pad.rs b/gstreamer/src/pad.rs index 6fabb7af9..f460668f9 100644 --- a/gstreamer/src/pad.rs +++ b/gstreamer/src/pad.rs @@ -667,26 +667,11 @@ impl> PadExtManual for O { None => panic::resume_unwind(err), }; - let maybe_couldnt_stop = if pad.pause_task().is_err() { - ", could not stop task" - } else { - "" - }; - let cause = if let Some(cause) = err.downcast_ref::<&str>() { - cause - } else if let Some(cause) = err.downcast_ref::() { - cause - } else { - "Panicked" - }; - let _ = element.post_message( - crate::message::Error::builder( - crate::LibraryError::Failed, - &format!("Panicked: {}{}", cause, maybe_couldnt_stop), - ) - .src(&*pad) - .build(), - ); + if pad.pause_task().is_err() { + crate::error!(crate::CAT_RUST, "could not stop pad task on panic"); + } + + crate::subclass::post_panic_error_message(&element, pad.upcast_ref(), Some(err)); } } diff --git a/gstreamer/src/subclass/element.rs b/gstreamer/src/subclass/element.rs index 5c9864acb..7a1b489a4 100644 --- a/gstreamer/src/subclass/element.rs +++ b/gstreamer/src/subclass/element.rs @@ -319,6 +319,7 @@ impl ElementImplExt for T { } } + #[inline(never)] fn panicked(&self) -> &atomic::AtomicBool { self.instance_data::(crate::Element::static_type()) .expect("instance not initialized correctly") diff --git a/gstreamer/src/subclass/error.rs b/gstreamer/src/subclass/error.rs index 99414538f..e72b94a8a 100644 --- a/gstreamer/src/subclass/error.rs +++ b/gstreamer/src/subclass/error.rs @@ -2,33 +2,61 @@ use thiserror::Error; -use crate::{ErrorMessage, FlowReturn}; +use crate::{prelude::ElementExt, ErrorMessage, FlowReturn}; + +#[doc(hidden)] +#[inline(never)] +pub fn post_panic_error_message( + element: &crate::Element, + src: &crate::Object, + panic: Option>, +) { + let cause = panic.as_ref().and_then(|err| { + err.downcast_ref::<&str>() + .copied() + .or_else(|| err.downcast_ref::().map(|s| s.as_str())) + }); + + let msg = if let Some(cause) = cause { + crate::message::Error::builder(crate::LibraryError::Failed, &format!("Panicked: {}", cause)) + .src(src) + .build() + } else { + crate::message::Error::builder(crate::LibraryError::Failed, "Panicked") + .src(src) + .build() + }; + + let _ = element.post_message(msg); +} #[macro_export] macro_rules! panic_to_error( ($imp:expr, $ret:expr, $code:block) => {{ - use std::panic::{self, AssertUnwindSafe}; - use std::sync::atomic::Ordering; - #[allow(clippy::unused_unit)] { - if $imp.panicked().load(Ordering::Relaxed) { - $imp.post_error_message($crate::error_msg!($crate::LibraryError::Failed, ["Panicked"])); + let panicked = $imp.panicked(); + let element = $crate::glib::subclass::types::ObjectSubclassExt::obj($imp); + let element = unsafe { $crate::glib::Cast::unsafe_cast_ref::<$crate::Element>(element.as_ref()) }; + if panicked.load(std::sync::atomic::Ordering::Relaxed) { + $crate::subclass::post_panic_error_message( + element, + $crate::glib::Cast::upcast_ref::<$crate::Object>(element), + None, + ); $ret } else { - let result = panic::catch_unwind(AssertUnwindSafe(|| $code)); + let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| $code)); match result { Ok(result) => result, Err(err) => { - $imp.panicked().store(true, Ordering::Relaxed); - if let Some(cause) = err.downcast_ref::<&str>() { - $imp.post_error_message($crate::error_msg!($crate::LibraryError::Failed, ["Panicked: {}", cause])); - } else if let Some(cause) = err.downcast_ref::() { - $imp.post_error_message($crate::error_msg!($crate::LibraryError::Failed, ["Panicked: {}", cause])); - } else { - $imp.post_error_message($crate::error_msg!($crate::LibraryError::Failed, ["Panicked"])); - } + panicked.store(true, std::sync::atomic::Ordering::Relaxed); + $crate::subclass::post_panic_error_message( + element, + $crate::glib::Cast::upcast_ref::<$crate::Object>(element), + Some(err), + ); $ret } } diff --git a/gstreamer/src/subclass/mod.rs b/gstreamer/src/subclass/mod.rs index da5d398cc..d4f8de985 100644 --- a/gstreamer/src/subclass/mod.rs +++ b/gstreamer/src/subclass/mod.rs @@ -34,7 +34,7 @@ mod uri_handler; pub use self::{ device_provider::DeviceProviderMetadata, element::ElementMetadata, - error::FlowError, + error::{post_panic_error_message, FlowError}, plugin::{MAJOR_VERSION, MINOR_VERSION}, task_pool::TaskPoolFunction, }; diff --git a/gstreamer/src/subclass/plugin.rs b/gstreamer/src/subclass/plugin.rs index 07faf9d4b..fcb5c2519 100644 --- a/gstreamer/src/subclass/plugin.rs +++ b/gstreamer/src/subclass/plugin.rs @@ -100,24 +100,23 @@ macro_rules! plugin_define( #[allow(clippy::missing_safety_doc)] unsafe extern "C" fn plugin_init_trampoline(plugin: *mut $crate::ffi::GstPlugin) -> $crate::glib::ffi::gboolean { - use std::panic::{self, AssertUnwindSafe}; + let panic_result = std::panic::catch_unwind( + std::panic::AssertUnwindSafe(|| super::$plugin_init(&$crate::glib::translate::from_glib_borrow(plugin))) + ); - let panic_result = panic::catch_unwind(AssertUnwindSafe(|| super::$plugin_init(&$crate::glib::translate::from_glib_borrow(plugin)))); match panic_result { Ok(register_result) => match register_result { Ok(_) => $crate::glib::ffi::GTRUE, Err(err) => { - let cat = $crate::DebugCategory::get("GST_PLUGIN_LOADING").unwrap(); - $crate::error!(cat, "Failed to register plugin: {}", err); + $crate::error!($crate::CAT_PLUGIN_LOADING, "Failed to register plugin: {}", err); $crate::glib::ffi::GFALSE } } Err(err) => { - let cat = $crate::DebugCategory::get("GST_PLUGIN_LOADING").unwrap(); - if let Some(cause) = err.downcast_ref::<&str>() { - $crate::error!(cat, "Failed to initialize plugin due to panic: {}", cause); - } else if let Some(cause) = err.downcast_ref::() { - $crate::error!(cat, "Failed to initialize plugin due to panic: {}", cause); + let cause = err.downcast_ref::<&str>().copied() + .or_else(|| err.downcast_ref::().map(|s| s.as_str())); + if let Some(cause) = cause { + $crate::error!($crate::PLUGIN_LOADING, "Failed to initialize plugin due to panic: {}", cause); } else { $crate::error!(cat, "Failed to initialize plugin due to panic"); }