From 0c0d671922ab58c39f5918fb44e3b8fc573a2699 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Dr=C3=B6ge?= Date: Sun, 21 Jun 2020 19:43:21 +0300 Subject: [PATCH] gstreamer/pad: Don't provide constructors anymore but instead a builder This handles safely setting the pad functions during construction and also has special support for ghost pads. Fixes https://gitlab.freedesktop.org/gstreamer/gstreamer-rs/-/issues/247 --- Gir_Gst.toml | 18 +- gstreamer/src/auto/ghost_pad.rs | 35 --- gstreamer/src/auto/pad.rs | 22 -- gstreamer/src/auto/versions.txt | 2 +- gstreamer/src/ghost_pad.rs | 34 --- gstreamer/src/lib.rs | 1 + gstreamer/src/pad.rs | 423 ++++++++++++++++++++++++++---- gstreamer/src/subclass/element.rs | 83 +++--- 8 files changed, 423 insertions(+), 195 deletions(-) diff --git a/Gir_Gst.toml b/Gir_Gst.toml index e53f4b5db..85829cc2b 100644 --- a/Gir_Gst.toml +++ b/Gir_Gst.toml @@ -827,9 +827,19 @@ manual_traits = ["PadExtManual"] # Use Result ignore = true + [[object.function]] + name = "new" + # Builder + ignore = true + + [[object.function]] + name = "new_from_template" + # Builder + ignore = true + [[object.function]] name = "new_from_static_template" - # Correct mutability + # Builder ignore = true [[object.function]] @@ -929,13 +939,11 @@ status = "generate" [[object.function]] name = "new_no_target" - [object.function.return] - nullable_return_is_error = "Failed to create GhostPad" + ignore = true [[object.function]] name = "new_no_target_from_template" - [object.function.return] - nullable_return_is_error = "Failed to create GhostPad" + ignore = true [[object.function]] name = "construct" diff --git a/gstreamer/src/auto/ghost_pad.rs b/gstreamer/src/auto/ghost_pad.rs index e27a6aab5..2650fee93 100644 --- a/gstreamer/src/auto/ghost_pad.rs +++ b/gstreamer/src/auto/ghost_pad.rs @@ -3,14 +3,11 @@ // DO NOT EDIT use glib; -use glib::object::Cast; use glib::object::IsA; use glib::translate::*; use gst_sys; use Object; use Pad; -use PadDirection; -use PadTemplate; use ProxyPad; glib_wrapper! { @@ -21,38 +18,6 @@ glib_wrapper! { } } -impl GhostPad { - pub fn new_no_target( - name: Option<&str>, - dir: PadDirection, - ) -> Result { - assert_initialized_main_thread!(); - unsafe { - Option::::from_glib_none(gst_sys::gst_ghost_pad_new_no_target( - name.to_glib_none().0, - dir.to_glib(), - )) - .map(|o| o.unsafe_cast()) - .ok_or_else(|| glib_bool_error!("Failed to create GhostPad")) - } - } - - pub fn new_no_target_from_template( - name: Option<&str>, - templ: &PadTemplate, - ) -> Result { - skip_assert_initialized!(); - unsafe { - Option::::from_glib_none(gst_sys::gst_ghost_pad_new_no_target_from_template( - name.to_glib_none().0, - templ.to_glib_none().0, - )) - .map(|o| o.unsafe_cast()) - .ok_or_else(|| glib_bool_error!("Failed to create GhostPad")) - } - } -} - unsafe impl Send for GhostPad {} unsafe impl Sync for GhostPad {} diff --git a/gstreamer/src/auto/pad.rs b/gstreamer/src/auto/pad.rs index d6a2e0d9e..6a8857c15 100644 --- a/gstreamer/src/auto/pad.rs +++ b/gstreamer/src/auto/pad.rs @@ -39,28 +39,6 @@ glib_wrapper! { } } -impl Pad { - pub fn new(name: Option<&str>, direction: PadDirection) -> Pad { - assert_initialized_main_thread!(); - unsafe { - from_glib_none(gst_sys::gst_pad_new( - name.to_glib_none().0, - direction.to_glib(), - )) - } - } - - pub fn from_template(templ: &PadTemplate, name: Option<&str>) -> Pad { - skip_assert_initialized!(); - unsafe { - from_glib_none(gst_sys::gst_pad_new_from_template( - templ.to_glib_none().0, - name.to_glib_none().0, - )) - } - } -} - unsafe impl Send for Pad {} unsafe impl Sync for Pad {} diff --git a/gstreamer/src/auto/versions.txt b/gstreamer/src/auto/versions.txt index 64bce6d59..ee445cf85 100644 --- a/gstreamer/src/auto/versions.txt +++ b/gstreamer/src/auto/versions.txt @@ -1,2 +1,2 @@ Generated by gir (https://github.com/gtk-rs/gir @ 5a5b8f5) -from gir-files (https://github.com/gtk-rs/gir-files @ 2bd82b67) +from gir-files (https://github.com/gtk-rs/gir-files @ 7e318657) diff --git a/gstreamer/src/ghost_pad.rs b/gstreamer/src/ghost_pad.rs index 8f5d809b6..03df5f204 100644 --- a/gstreamer/src/ghost_pad.rs +++ b/gstreamer/src/ghost_pad.rs @@ -6,48 +6,14 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use glib::object::Cast; use glib::object::IsA; use glib::translate::*; use gst_sys; use GhostPad; use Object; -use Pad; use PadMode; -use PadTemplate; impl GhostPad { - pub fn new>(name: Option<&str>, target: &Q) -> Result { - skip_assert_initialized!(); - let name = name.to_glib_none(); - unsafe { - Option::::from_glib_none(gst_sys::gst_ghost_pad_new( - name.0, - target.as_ref().to_glib_none().0, - )) - .map(|o| Cast::unsafe_cast(o)) - .ok_or_else(|| glib_bool_error!("Failed to create GhostPad")) - } - } - - pub fn from_template>( - name: Option<&str>, - target: &Q, - templ: &PadTemplate, - ) -> Result { - skip_assert_initialized!(); - let name = name.to_glib_none(); - unsafe { - Option::::from_glib_none(gst_sys::gst_ghost_pad_new_from_template( - name.0, - target.as_ref().to_glib_none().0, - templ.to_glib_none().0, - )) - .map(|o| Cast::unsafe_cast(o)) - .ok_or_else(|| glib_bool_error!("Failed to create GhostPad")) - } - } - pub fn activate_mode_default, Q: IsA>( pad: &P, parent: Option<&Q>, diff --git a/gstreamer/src/lib.rs b/gstreamer/src/lib.rs index d28e77668..cbeec163d 100644 --- a/gstreamer/src/lib.rs +++ b/gstreamer/src/lib.rs @@ -204,6 +204,7 @@ mod gobject; mod iterator; mod object; mod pad; +pub use pad::PadBuilder; mod parse_context; mod proxy_pad; mod tag_setter; diff --git a/gstreamer/src/pad.rs b/gstreamer/src/pad.rs index f7103fc7a..53ff16bd2 100644 --- a/gstreamer/src/pad.rs +++ b/gstreamer/src/pad.rs @@ -38,8 +38,8 @@ use std::ptr; use glib; use glib::object::{Cast, IsA}; use glib::translate::{ - from_glib, from_glib_borrow, from_glib_full, from_glib_none, mut_override, FromGlib, - FromGlibPtrBorrow, ToGlib, ToGlibPtr, + from_glib, from_glib_borrow, from_glib_full, from_glib_none, FromGlib, FromGlibPtrBorrow, + ToGlib, ToGlibPtr, }; use glib::StaticType; use glib_sys; @@ -49,18 +49,6 @@ use libc; use gst_sys; -impl Pad { - pub fn from_static_template(templ: &StaticPadTemplate, name: Option<&str>) -> Pad { - assert_initialized_main_thread!(); - unsafe { - from_glib_none(gst_sys::gst_pad_new_from_static_template( - mut_override(templ.to_glib_none().0), - name.to_glib_none().0, - )) - } - } -} - #[derive(Debug, PartialEq, Eq)] pub struct PadProbeId(NonZeroU64); @@ -1557,6 +1545,330 @@ unsafe extern "C" fn destroy_closure_pad_task(ptr: gpointer) { Box::>::from_raw(ptr as *mut _); } +impl Pad { + pub fn new(name: Option<&str>, direction: ::PadDirection) -> Self { + skip_assert_initialized!(); + Self::builder(name, direction).build() + } + + pub fn builder(name: Option<&str>, direction: ::PadDirection) -> PadBuilder { + skip_assert_initialized!(); + PadBuilder::new(name, direction) + } + + pub fn from_static_template(templ: &StaticPadTemplate, name: Option<&str>) -> Self { + skip_assert_initialized!(); + Self::builder_from_static_template(templ, name).build() + } + + pub fn builder_from_static_template( + templ: &StaticPadTemplate, + name: Option<&str>, + ) -> PadBuilder { + skip_assert_initialized!(); + PadBuilder::from_static_template(templ, name) + } + + pub fn from_template(templ: &::PadTemplate, name: Option<&str>) -> Self { + skip_assert_initialized!(); + Self::builder_from_template(templ, name).build() + } + + pub fn builder_from_template(templ: &::PadTemplate, name: Option<&str>) -> PadBuilder { + skip_assert_initialized!(); + PadBuilder::from_template(templ, name) + } +} + +impl ::GhostPad { + pub fn new(name: Option<&str>, direction: ::PadDirection) -> Self { + skip_assert_initialized!(); + Self::builder(name, direction).build() + } + + pub fn builder(name: Option<&str>, direction: ::PadDirection) -> PadBuilder { + skip_assert_initialized!(); + PadBuilder::new(name, direction) + } + + pub fn from_static_template(templ: &StaticPadTemplate, name: Option<&str>) -> Self { + skip_assert_initialized!(); + Self::builder_from_static_template(templ, name).build() + } + + pub fn builder_from_static_template( + templ: &StaticPadTemplate, + name: Option<&str>, + ) -> PadBuilder { + skip_assert_initialized!(); + PadBuilder::from_static_template(templ, name) + } + + pub fn from_template(templ: &::PadTemplate, name: Option<&str>) -> Self { + skip_assert_initialized!(); + Self::builder_from_template(templ, name).build() + } + + pub fn builder_from_template(templ: &::PadTemplate, name: Option<&str>) -> PadBuilder { + skip_assert_initialized!(); + PadBuilder::from_template(templ, name) + } +} + +pub struct PadBuilder(T); + +impl + IsA> PadBuilder { + pub fn new(name: Option<&str>, direction: ::PadDirection) -> Self { + assert_initialized_main_thread!(); + + let pad = glib::Object::new( + T::static_type(), + &[("name", &name), ("direction", &direction)], + ) + .expect("Failed to create pad") + .downcast::() + .unwrap(); + + // Ghost pads are a bit special + if let Some(pad) = pad.dynamic_cast_ref::<::GhostPad>() { + unsafe { + let res = gst_sys::gst_ghost_pad_construct(pad.to_glib_none().0); + // This can't really fail... + assert_ne!(res, glib_sys::GFALSE, "Failed to construct ghost pad"); + } + } + + PadBuilder(pad) + } + + pub fn from_static_template(templ: &StaticPadTemplate, name: Option<&str>) -> Self { + assert_initialized_main_thread!(); + + let templ = templ.get(); + Self::from_template(&templ, name) + } + + pub fn from_template(templ: &::PadTemplate, name: Option<&str>) -> Self { + assert_initialized_main_thread!(); + + use glib::ObjectExt; + + let mut type_ = T::static_type(); + + // Since 1.14 templates can keep a pad GType with them, so we need to do some + // additional checks here now + if templ.has_property("gtype", Some(glib::Type::static_type())) { + let gtype = templ + .get_property("gtype") + .unwrap() + .get_some::() + .unwrap(); + + if gtype == glib::Type::Unit { + // Nothing to be done, we can create any kind of pad + } else if gtype.is_a(&type_) { + // We were asked to create a parent type of the template type, e.g. a gst::Pad for + // a template that wants a gst_base::AggregatorPad. Not a problem: update the type + type_ = gtype; + } else { + // Otherwise the requested type must be a subclass of the template pad type + assert!(type_.is_a(>ype)); + } + } + + let pad = glib::Object::new( + type_, + &[ + ("name", &name), + ("direction", &templ.get_property_direction()), + ("template", templ), + ], + ) + .expect("Failed to create pad") + .downcast::() + .unwrap(); + + // Ghost pads are a bit special + if let Some(pad) = pad.dynamic_cast_ref::<::GhostPad>() { + unsafe { + let res = gst_sys::gst_ghost_pad_construct(pad.to_glib_none().0); + // This can't really fail... + assert_ne!(res, glib_sys::GFALSE, "Failed to construct ghost pad"); + } + } + + PadBuilder(pad) + } + + pub fn activate_function(self, func: F) -> Self + where + F: Fn(&T, Option<&::Object>) -> Result<(), LoggableError> + Send + Sync + 'static, + { + unsafe { + self.0.set_activate_function(func); + } + + self + } + + pub fn activatemode_function(self, func: F) -> Self + where + F: Fn(&T, Option<&::Object>, ::PadMode, bool) -> Result<(), LoggableError> + + Send + + Sync + + 'static, + { + unsafe { + self.0.set_activatemode_function(func); + } + + self + } + + pub fn chain_function(self, func: F) -> Self + where + F: Fn(&T, Option<&::Object>, ::Buffer) -> Result + + Send + + Sync + + 'static, + { + unsafe { + self.0.set_chain_function(func); + } + + self + } + + pub fn chain_list_function(self, func: F) -> Self + where + F: Fn(&T, Option<&::Object>, ::BufferList) -> Result + + Send + + Sync + + 'static, + { + unsafe { + self.0.set_chain_list_function(func); + } + + self + } + + pub fn event_function(self, func: F) -> Self + where + F: Fn(&T, Option<&::Object>, ::Event) -> bool + Send + Sync + 'static, + { + unsafe { + self.0.set_event_function(func); + } + + self + } + + pub fn event_full_function(self, func: F) -> Self + where + F: Fn(&T, Option<&::Object>, ::Event) -> Result + + Send + + Sync + + 'static, + { + unsafe { + self.0.set_event_full_function(func); + } + + self + } + + pub fn getrange_function(self, func: F) -> Self + where + F: Fn( + &T, + Option<&::Object>, + u64, + Option<&mut ::BufferRef>, + u32, + ) -> Result + + Send + + Sync + + 'static, + { + unsafe { + self.0.set_getrange_function(func); + } + + self + } + + pub fn iterate_internal_links_function(self, func: F) -> Self + where + F: Fn(&T, Option<&::Object>) -> ::Iterator + Send + Sync + 'static, + { + unsafe { + self.0.set_iterate_internal_links_function(func); + } + + self + } + + pub fn link_function(self, func: F) -> Self + where + F: Fn(&T, Option<&::Object>, &Pad) -> Result<::PadLinkSuccess, ::PadLinkError> + + Send + + Sync + + 'static, + { + unsafe { + self.0.set_link_function(func); + } + + self + } + + pub fn query_function(self, func: F) -> Self + where + F: Fn(&T, Option<&::Object>, &mut ::QueryRef) -> bool + Send + Sync + 'static, + { + unsafe { + self.0.set_query_function(func); + } + + self + } + + pub fn unlink_function(self, func: F) -> Self + where + F: Fn(&T, Option<&::Object>) + Send + Sync + 'static, + { + unsafe { + self.0.set_unlink_function(func); + } + + self + } + + pub fn flags(self, flags: PadFlags) -> Self { + self.0.set_pad_flags(flags); + + self + } + + pub fn build(self) -> T { + self.0 + } +} + +impl + IsA> PadBuilder { + pub fn build_with_target>(self, target: &P) -> Result { + use GhostPadExt; + use PadExt; + + assert_eq!(self.0.get_direction(), target.get_direction()); + + self.0.set_target(Some(target))?; + + Ok(self.0) + } +} + #[cfg(test)] mod tests { use super::*; @@ -1568,25 +1880,24 @@ mod tests { fn test_event_chain_functions() { ::init().unwrap(); - let pad = ::Pad::new(Some("sink"), ::PadDirection::Sink); - let events = Arc::new(Mutex::new(Vec::new())); let events_clone = events.clone(); - pad.set_event_function(move |_, _, event| { - let mut events = events_clone.lock().unwrap(); - events.push(event); - - true - }); - let buffers = Arc::new(Mutex::new(Vec::new())); let buffers_clone = buffers.clone(); - pad.set_chain_function(move |_, _, buffer| { - let mut buffers = buffers_clone.lock().unwrap(); - buffers.push(buffer); + let pad = ::Pad::builder(Some("sink"), ::PadDirection::Sink) + .event_function(move |_, _, event| { + let mut events = events_clone.lock().unwrap(); + events.push(event); - Ok(FlowSuccess::Ok) - }); + true + }) + .chain_function(move |_, _, buffer| { + let mut buffers = buffers_clone.lock().unwrap(); + buffers.push(buffer); + + Ok(FlowSuccess::Ok) + }) + .build(); pad.set_active(true).unwrap(); @@ -1616,17 +1927,18 @@ mod tests { fn test_getrange_function() { ::init().unwrap(); - let pad = ::Pad::new(Some("src"), ::PadDirection::Src); - pad.set_activate_function(|pad, _parent| { - pad.activate_mode(::PadMode::Pull, true) - .map_err(|err| err.into()) - }); - pad.set_getrange_function(|_pad, _parent, offset, _buffer, size| { - assert_eq!(offset, 0); - assert_eq!(size, 5); - let buffer = ::Buffer::from_slice(b"abcde"); - Ok(PadGetRangeSuccess::NewBuffer(buffer)) - }); + let pad = ::Pad::builder(Some("src"), ::PadDirection::Src) + .activate_function(|pad, _parent| { + pad.activate_mode(::PadMode::Pull, true) + .map_err(|err| err.into()) + }) + .getrange_function(|_pad, _parent, offset, _buffer, size| { + assert_eq!(offset, 0); + assert_eq!(size, 5); + let buffer = ::Buffer::from_slice(b"abcde"); + Ok(PadGetRangeSuccess::NewBuffer(buffer)) + }) + .build(); pad.set_active(true).unwrap(); let buffer = pad.get_range(0, 5).unwrap(); @@ -1641,22 +1953,23 @@ mod tests { pad.set_active(false).unwrap(); drop(pad); - let pad = ::Pad::new(Some("src"), ::PadDirection::Src); - pad.set_activate_function(|pad, _parent| { - pad.activate_mode(::PadMode::Pull, true) - .map_err(|err| err.into()) - }); - pad.set_getrange_function(|_pad, _parent, offset, buffer, size| { - assert_eq!(offset, 0); - assert_eq!(size, 5); - if let Some(buffer) = buffer { - buffer.copy_from_slice(0, b"fghij").unwrap(); - Ok(PadGetRangeSuccess::FilledBuffer) - } else { - let buffer = ::Buffer::from_slice(b"abcde"); - Ok(PadGetRangeSuccess::NewBuffer(buffer)) - } - }); + let pad = ::Pad::builder(Some("src"), ::PadDirection::Src) + .activate_function(|pad, _parent| { + pad.activate_mode(::PadMode::Pull, true) + .map_err(|err| err.into()) + }) + .getrange_function(|_pad, _parent, offset, buffer, size| { + assert_eq!(offset, 0); + assert_eq!(size, 5); + if let Some(buffer) = buffer { + buffer.copy_from_slice(0, b"fghij").unwrap(); + Ok(PadGetRangeSuccess::FilledBuffer) + } else { + let buffer = ::Buffer::from_slice(b"abcde"); + Ok(PadGetRangeSuccess::NewBuffer(buffer)) + } + }) + .build(); pad.set_active(true).unwrap(); let buffer = pad.get_range(0, 5).unwrap(); diff --git a/gstreamer/src/subclass/element.rs b/gstreamer/src/subclass/element.rs index 2caf0c12e..0d0610fd6 100644 --- a/gstreamer/src/subclass/element.rs +++ b/gstreamer/src/subclass/element.rs @@ -533,45 +533,6 @@ mod tests { } impl TestElement { - fn set_pad_functions(sinkpad: &::Pad, srcpad: &::Pad) { - sinkpad.set_chain_function(|pad, parent, buffer| { - TestElement::catch_panic_pad_function( - parent, - || Err(::FlowError::Error), - |identity, element| identity.sink_chain(pad, element, buffer), - ) - }); - sinkpad.set_event_function(|pad, parent, event| { - TestElement::catch_panic_pad_function( - parent, - || false, - |identity, element| identity.sink_event(pad, element, event), - ) - }); - sinkpad.set_query_function(|pad, parent, query| { - TestElement::catch_panic_pad_function( - parent, - || false, - |identity, element| identity.sink_query(pad, element, query), - ) - }); - - srcpad.set_event_function(|pad, parent, event| { - TestElement::catch_panic_pad_function( - parent, - || false, - |identity, element| identity.src_event(pad, element, event), - ) - }); - srcpad.set_query_function(|pad, parent, query| { - TestElement::catch_panic_pad_function( - parent, - || false, - |identity, element| identity.src_query(pad, element, query), - ) - }); - } - fn sink_chain( &self, _pad: &::Pad, @@ -609,11 +570,47 @@ mod tests { fn with_class(klass: &subclass::simple::ClassStruct) -> Self { let templ = klass.get_pad_template("sink").unwrap(); - let sinkpad = ::Pad::from_template(&templ, Some("sink")); - let templ = klass.get_pad_template("src").unwrap(); - let srcpad = ::Pad::from_template(&templ, Some("src")); + let sinkpad = ::Pad::builder_from_template(&templ, Some("sink")) + .chain_function(|pad, parent, buffer| { + TestElement::catch_panic_pad_function( + parent, + || Err(::FlowError::Error), + |identity, element| identity.sink_chain(pad, element, buffer), + ) + }) + .event_function(|pad, parent, event| { + TestElement::catch_panic_pad_function( + parent, + || false, + |identity, element| identity.sink_event(pad, element, event), + ) + }) + .query_function(|pad, parent, query| { + TestElement::catch_panic_pad_function( + parent, + || false, + |identity, element| identity.sink_query(pad, element, query), + ) + }) + .build(); - TestElement::set_pad_functions(&sinkpad, &srcpad); + let templ = klass.get_pad_template("src").unwrap(); + let srcpad = ::Pad::builder_from_template(&templ, Some("src")) + .event_function(|pad, parent, event| { + TestElement::catch_panic_pad_function( + parent, + || false, + |identity, element| identity.src_event(pad, element, event), + ) + }) + .query_function(|pad, parent, query| { + TestElement::catch_panic_pad_function( + parent, + || false, + |identity, element| identity.src_query(pad, element, query), + ) + }) + .build(); Self { n_buffers: atomic::AtomicU32::new(0),