From 7cb5473ba63ab97fed387e70e61268e4813069b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Laignel?= Date: Fri, 28 Apr 2023 21:35:15 +0200 Subject: [PATCH] gst: Pad move name definition to builders Also apply consistent naming for builder/non-builder constructors. See discussion in: https://gitlab.freedesktop.org/gstreamer/gstreamer-rs/-/issues/448#note_1799092 Part-of: --- gstreamer/src/ghost_pad.rs | 190 +++++++++++++++++++---- gstreamer/src/pad.rs | 123 ++++++++++----- gstreamer/src/subclass/element.rs | 6 +- gstreamer/src/subclass/pad.rs | 2 +- tutorials/src/bin/playback-tutorial-7.rs | 5 +- 5 files changed, 252 insertions(+), 74 deletions(-) diff --git a/gstreamer/src/ghost_pad.rs b/gstreamer/src/ghost_pad.rs index 36237fb09..6d1a4e204 100644 --- a/gstreamer/src/ghost_pad.rs +++ b/gstreamer/src/ghost_pad.rs @@ -53,64 +53,94 @@ impl GhostPad { } } + // rustdoc-stripper-ignore-next + /// Creates a new [`GhostPad`] object with a default name. + /// + /// Use [`GhostPad::builder()`] to get a [`PadBuilder`] and then define a specific name. #[doc(alias = "gst_ghost_pad_new_no_target")] - pub fn new(name: Option<&str>, direction: crate::PadDirection) -> Self { + pub fn new(direction: crate::PadDirection) -> Self { skip_assert_initialized!(); - Self::builder(name, direction).build() + Self::builder(direction).build() } #[doc(alias = "gst_ghost_pad_new_no_target")] - pub fn builder(name: Option<&str>, direction: crate::PadDirection) -> PadBuilder { + pub fn builder(direction: crate::PadDirection) -> PadBuilder { skip_assert_initialized!(); - PadBuilder::new(name, direction) + PadBuilder::new(direction) + } + + // rustdoc-stripper-ignore-next + /// Creates a new [`GhostPad`] object from the [`StaticPadTemplate`](crate::StaticPadTemplate) with a default name. + /// + /// Use [`GhostPad::builder_from_static_template()`] to get a [`PadBuilder`] and then define a specific name. + #[doc(alias = "gst_ghost_pad_new_no_target_from_static_template")] + pub fn from_static_template(templ: &StaticPadTemplate) -> Self { + skip_assert_initialized!(); + Self::builder_from_static_template(templ).build() } #[doc(alias = "gst_ghost_pad_new_no_target_from_static_template")] - pub fn from_static_template(templ: &StaticPadTemplate, name: Option<&str>) -> Self { + pub fn builder_from_static_template(templ: &StaticPadTemplate) -> PadBuilder { skip_assert_initialized!(); - Self::builder_with_static_template(templ, name).build() + PadBuilder::from_static_template(templ) } - #[doc(alias = "gst_ghost_pad_new_no_target_from_static_template")] - pub fn builder_with_static_template( - templ: &StaticPadTemplate, - name: Option<&str>, - ) -> PadBuilder { + // rustdoc-stripper-ignore-next + /// Creates a new [`GhostPad`] object from the [`PadTemplate`](crate::PadTemplate) with a default name. + /// + /// Use [`GhostPad::builder_from_template()`] to get a [`PadBuilder`] and then define a specific name. + #[doc(alias = "gst_ghost_pad_new_no_target_from_template")] + pub fn from_template(templ: &crate::PadTemplate) -> Self { skip_assert_initialized!(); - PadBuilder::from_static_template(templ, name) + Self::builder_from_template(templ).build() } #[doc(alias = "gst_ghost_pad_new_no_target_from_template")] - pub fn from_template(templ: &crate::PadTemplate, name: Option<&str>) -> Self { + pub fn builder_from_template(templ: &crate::PadTemplate) -> PadBuilder { skip_assert_initialized!(); - Self::builder_with_template(templ, name).build() - } - - #[doc(alias = "gst_ghost_pad_new_no_target_from_template")] - pub fn builder_with_template( - templ: &crate::PadTemplate, - name: Option<&str>, - ) -> PadBuilder { - skip_assert_initialized!(); - PadBuilder::from_template(templ, name) + PadBuilder::from_template(templ) } + // rustdoc-stripper-ignore-next + /// Creates a new [`GhostPad`] object from the specified `target` `Pad` and a default name. + /// + /// Use [`GhostPad::builder_with_target()`] to get a [`PadBuilder`] and then define a specific name. #[doc(alias = "gst_ghost_pad_new")] - pub fn with_target>( - name: Option<&str>, - target: &P, - ) -> Result { + pub fn with_target>(target: &P) -> Result { skip_assert_initialized!(); - Self::builder(name, target.direction()).build_with_target(target) + Ok(Self::builder_with_target(target)?.build()) } + #[doc(alias = "gst_ghost_pad_new_no_target_from_template")] + pub fn builder_with_target>( + target: &P, + ) -> Result, glib::BoolError> { + skip_assert_initialized!(); + Self::builder(target.direction()).with_target(target) + } + + // rustdoc-stripper-ignore-next + /// Creates a new [`GhostPad`] object from the [`PadTemplate`](crate::PadTemplate) + /// with the specified `target` `Pad` and a default name. + /// + /// Returns `Err(_)` if the `PadTemplate` and the `target` directions differ. + /// + /// Use [`GhostPad::builder_from_template_with_target()`] to get a [`PadBuilder`] and then define a specific name. #[doc(alias = "gst_ghost_pad_new_from_template")] pub fn from_template_with_target>( templ: &crate::PadTemplate, - name: Option<&str>, target: &P, ) -> Result { skip_assert_initialized!(); + Ok(Self::builder_from_template_with_target(templ, target)?.build()) + } + + #[doc(alias = "gst_ghost_pad_new_from_template")] + pub fn builder_from_template_with_target>( + templ: &crate::PadTemplate, + target: &P, + ) -> Result, glib::BoolError> { + skip_assert_initialized!(); if target.direction() != templ.direction() { return Err(glib::bool_error!( @@ -118,7 +148,7 @@ impl GhostPad { )); } - Self::builder_with_template(templ, name).build_with_target(target) + Self::builder_from_template(templ).with_target(target) } } @@ -380,11 +410,107 @@ impl + IsA> PadBuilder { self } - pub fn build_with_target>(self, target: &P) -> Result { + pub fn with_target>(self, target: &P) -> Result { assert_eq!(self.0.direction(), target.direction()); self.0.set_target(Some(target))?; - Ok(self.0) + Ok(self) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn from_template() { + crate::init().unwrap(); + + let caps = crate::Caps::new_any(); + let templ = crate::PadTemplate::new( + "sink", + crate::PadDirection::Sink, + crate::PadPresence::Always, + &caps, + ) + .unwrap(); + + let ghost_pad = GhostPad::from_template(&templ); + assert!(ghost_pad.name().starts_with("ghostpad")); + + let ghost_pad = GhostPad::builder_from_template(&templ).build(); + assert!(ghost_pad.name().starts_with("ghostpad")); + + let ghost_pad = GhostPad::builder_from_template(&templ).name("sink").build(); + assert_eq!(ghost_pad.name(), "sink"); + } + + #[test] + fn with_target() { + crate::init().unwrap(); + + let caps = crate::Caps::new_any(); + let templ = crate::PadTemplate::new( + "sink", + crate::PadDirection::Sink, + crate::PadPresence::Always, + &caps, + ) + .unwrap(); + + let target = crate::Pad::from_template(&templ); + let ghost_pad = GhostPad::with_target(&target).unwrap(); + assert!(ghost_pad.name().starts_with("ghostpad")); + + let target = crate::Pad::from_template(&templ); + let ghost_pad = GhostPad::builder_with_target(&target).unwrap().build(); + assert!(ghost_pad.name().starts_with("ghostpad")); + + let target = crate::Pad::from_template(&templ); + let ghost_pad = GhostPad::builder_with_target(&target) + .unwrap() + .name("sink") + .build(); + assert_eq!(ghost_pad.name(), "sink"); + } + + #[test] + fn from_template_with_target() { + crate::init().unwrap(); + + let caps = crate::Caps::new_any(); + let templ = crate::PadTemplate::new( + "sink", + crate::PadDirection::Sink, + crate::PadPresence::Always, + &caps, + ) + .unwrap(); + + let ghost_templ = crate::PadTemplate::new( + "sink", + crate::PadDirection::Sink, + crate::PadPresence::Request, + &caps, + ) + .unwrap(); + + let target = crate::Pad::from_template(&templ); + let ghost_pad = GhostPad::from_template_with_target(&ghost_templ, &target).unwrap(); + assert!(ghost_pad.name().starts_with("ghostpad")); + + let target = crate::Pad::from_template(&templ); + let ghost_pad = GhostPad::builder_from_template_with_target(&ghost_templ, &target) + .unwrap() + .build(); + assert!(ghost_pad.name().starts_with("ghostpad")); + + let target = crate::Pad::from_template(&templ); + let ghost_pad = GhostPad::builder_from_template_with_target(&ghost_templ, &target) + .unwrap() + .name("sink") + .build(); + assert_eq!(ghost_pad.name(), "sink"); } } diff --git a/gstreamer/src/pad.rs b/gstreamer/src/pad.rs index 3f889b105..8ab8d1ab5 100644 --- a/gstreamer/src/pad.rs +++ b/gstreamer/src/pad.rs @@ -1518,46 +1518,52 @@ unsafe extern "C" fn destroy_closure(ptr: gpointer) { } impl Pad { + // rustdoc-stripper-ignore-next + /// Creates a new [`Pad`] object with a default name. + /// + /// Use [`Pad::builder()`] to get a [`PadBuilder`] and then define a specific name. #[doc(alias = "gst_pad_new")] - pub fn new(name: Option<&str>, direction: crate::PadDirection) -> Self { + pub fn new(direction: crate::PadDirection) -> Self { skip_assert_initialized!(); - Self::builder(name, direction).build() + Self::builder(direction).build() } #[doc(alias = "gst_pad_new")] - pub fn builder(name: Option<&str>, direction: crate::PadDirection) -> PadBuilder { + pub fn builder(direction: crate::PadDirection) -> PadBuilder { skip_assert_initialized!(); - PadBuilder::new(name, direction) + PadBuilder::new(direction) + } + + // rustdoc-stripper-ignore-next + /// Creates a new [`Pad`] object from the [`StaticPadTemplate`](crate::StaticPadTemplate) with a default name. + /// + /// Use [`Pad::builder_from_static_template()`] to get a [`PadBuilder`] and then define a specific name. + #[doc(alias = "gst_pad_new_from_static_template")] + pub fn from_static_template(templ: &StaticPadTemplate) -> Self { + skip_assert_initialized!(); + Self::builder_from_static_template(templ).build() } #[doc(alias = "gst_pad_new_from_static_template")] - pub fn from_static_template(templ: &StaticPadTemplate, name: Option<&str>) -> Self { + pub fn builder_from_static_template(templ: &StaticPadTemplate) -> PadBuilder { skip_assert_initialized!(); - Self::builder_with_static_template(templ, name).build() + PadBuilder::from_static_template(templ) } - #[doc(alias = "gst_pad_new_from_static_template")] - pub fn builder_with_static_template( - templ: &StaticPadTemplate, - name: Option<&str>, - ) -> PadBuilder { + // rustdoc-stripper-ignore-next + /// Creates a new [`Pad`] object from the [`PadTemplate`](crate::PadTemplate) with a default name. + /// + /// Use [`Pad::builder_from_template()`] to get a [`PadBuilder`] and then define a specific name. + #[doc(alias = "gst_pad_new_from_template")] + pub fn from_template(templ: &crate::PadTemplate) -> Self { skip_assert_initialized!(); - PadBuilder::from_static_template(templ, name) + Self::builder_from_template(templ).build() } #[doc(alias = "gst_pad_new_from_template")] - pub fn from_template(templ: &crate::PadTemplate, name: Option<&str>) -> Self { + pub fn builder_from_template(templ: &crate::PadTemplate) -> PadBuilder { skip_assert_initialized!(); - Self::builder_with_template(templ, name).build() - } - - #[doc(alias = "gst_pad_new_from_template")] - pub fn builder_with_template( - templ: &crate::PadTemplate, - name: Option<&str>, - ) -> PadBuilder { - skip_assert_initialized!(); - PadBuilder::from_template(templ, name) + PadBuilder::from_template(templ) } #[doc(alias = "gst_pad_query_default")] @@ -1611,11 +1617,10 @@ impl Pad { pub struct PadBuilder(pub(crate) T); impl + IsA + glib::object::IsClass> PadBuilder { - pub fn new(name: Option<&str>, direction: crate::PadDirection) -> Self { + pub fn new(direction: crate::PadDirection) -> Self { assert_initialized_main_thread!(); let pad = glib::Object::builder::() - .property("name", name) .property("direction", direction) .build(); @@ -1631,14 +1636,14 @@ impl + IsA + glib::object::IsClass> PadBuilder { PadBuilder(pad) } - pub fn from_static_template(templ: &StaticPadTemplate, name: Option<&str>) -> Self { + pub fn from_static_template(templ: &StaticPadTemplate) -> Self { skip_assert_initialized!(); let templ = templ.get(); - Self::from_template(&templ, name) + Self::from_template(&templ) } - pub fn from_template(templ: &crate::PadTemplate, name: Option<&str>) -> Self { + pub fn from_template(templ: &crate::PadTemplate) -> Self { assert_initialized_main_thread!(); let mut type_ = T::static_type(); @@ -1661,7 +1666,6 @@ impl + IsA + glib::object::IsClass> PadBuilder { } let mut properties = [ - ("name", name.map(glib::GString::from).into()), ("direction", templ.direction().into()), ("template", templ.into()), ]; @@ -1681,6 +1685,12 @@ impl + IsA + glib::object::IsClass> PadBuilder { PadBuilder(pad) } + pub fn name(self, name: impl Into) -> Self { + self.0.set_property("name", name.into()); + + self + } + #[doc(alias = "gst_pad_set_activate_function")] pub fn activate_function(self, func: F) -> Self where @@ -1867,7 +1877,8 @@ mod tests { let events_clone = events.clone(); let buffers = Arc::new(Mutex::new(Vec::new())); let buffers_clone = buffers.clone(); - let pad = crate::Pad::builder(Some("sink"), crate::PadDirection::Sink) + let pad = crate::Pad::builder(crate::PadDirection::Sink) + .name("sink") .event_function(move |_, _, event| { let mut events = events_clone.lock().unwrap(); events.push(event); @@ -1910,7 +1921,8 @@ mod tests { fn test_getrange_function() { crate::init().unwrap(); - let pad = crate::Pad::builder(Some("src"), crate::PadDirection::Src) + let pad = crate::Pad::builder(crate::PadDirection::Src) + .name("src") .activate_function(|pad, _parent| { pad.activate_mode(crate::PadMode::Pull, true) .map_err(|err| err.into()) @@ -1936,7 +1948,8 @@ mod tests { pad.set_active(false).unwrap(); drop(pad); - let pad = crate::Pad::builder(Some("src"), crate::PadDirection::Src) + let pad = crate::Pad::builder(crate::PadDirection::Src) + .name("src") .activate_function(|pad, _parent| { pad.activate_mode(crate::PadMode::Pull, true) .map_err(|err| err.into()) @@ -1969,7 +1982,9 @@ mod tests { fn test_task() { crate::init().unwrap(); - let pad = crate::Pad::new(Some("sink"), crate::PadDirection::Sink); + let pad = crate::Pad::builder(crate::PadDirection::Sink) + .name("sink") + .build(); let (sender, receiver) = channel(); let mut i = 0; @@ -1990,8 +2005,11 @@ mod tests { fn test_remove_probe_from_probe() { crate::init().unwrap(); - let src_pad = crate::Pad::new(Some("src"), crate::PadDirection::Src); - let sink_pad = crate::Pad::builder(Some("sink"), crate::PadDirection::Sink) + let src_pad = crate::Pad::builder(crate::PadDirection::Src) + .name("src") + .build(); + let sink_pad = crate::Pad::builder(crate::PadDirection::Sink) + .name("sink") .chain_function(|_pad, _parent, _buffer| Ok(crate::FlowSuccess::Ok)) .build(); @@ -2027,7 +2045,9 @@ mod tests { crate::init().unwrap(); let (major, minor, micro, _) = crate::version(); - let pad = crate::Pad::new(Some("src"), crate::PadDirection::Src); + let pad = crate::Pad::builder(crate::PadDirection::Src) + .name("src") + .build(); let events = Arc::new(Mutex::new(Vec::new())); let buffers = Arc::new(Mutex::new(Vec::new())); @@ -2123,7 +2143,9 @@ mod tests { fn test_sticky_events() { crate::init().unwrap(); - let pad = crate::Pad::builder(Some("sink"), crate::PadDirection::Sink).build(); + let pad = crate::Pad::builder(crate::PadDirection::Sink) + .name("sink") + .build(); pad.set_active(true).unwrap(); // Send some sticky events @@ -2149,7 +2171,9 @@ mod tests { fn test_sticky_events_foreach() { crate::init().unwrap(); - let pad = crate::Pad::builder(Some("sink"), crate::PadDirection::Sink).build(); + let pad = crate::Pad::builder(crate::PadDirection::Sink) + .name("sink") + .build(); pad.set_active(true).unwrap(); // Send some sticky events @@ -2248,4 +2272,27 @@ mod tests { sticky_events5[1].as_ref() as *const _ ); } + + #[test] + fn from_template() { + crate::init().unwrap(); + + let caps = crate::Caps::new_any(); + let templ = crate::PadTemplate::new( + "sink", + crate::PadDirection::Sink, + crate::PadPresence::Always, + &caps, + ) + .unwrap(); + + let pad = Pad::from_template(&templ); + assert!(pad.name().starts_with("pad")); + + let pad = Pad::builder_from_template(&templ).build(); + assert!(pad.name().starts_with("pad")); + + let pad = Pad::builder_from_template(&templ).name("sink").build(); + assert_eq!(pad.name(), "sink"); + } } diff --git a/gstreamer/src/subclass/element.rs b/gstreamer/src/subclass/element.rs index c3b7aa96e..91b8b8de9 100644 --- a/gstreamer/src/subclass/element.rs +++ b/gstreamer/src/subclass/element.rs @@ -584,7 +584,8 @@ mod tests { fn with_class(klass: &Self::Class) -> Self { let templ = klass.pad_template("sink").unwrap(); - let sinkpad = crate::Pad::builder_with_template(&templ, Some("sink")) + let sinkpad = crate::Pad::builder_from_template(&templ) + .name("sink") .chain_function(|pad, parent, buffer| { TestElement::catch_panic_pad_function( parent, @@ -609,7 +610,8 @@ mod tests { .build(); let templ = klass.pad_template("src").unwrap(); - let srcpad = crate::Pad::builder_with_template(&templ, Some("src")) + let srcpad = crate::Pad::builder_from_template(&templ) + .name("src") .event_function(|pad, parent, event| { TestElement::catch_panic_pad_function( parent, diff --git a/gstreamer/src/subclass/pad.rs b/gstreamer/src/subclass/pad.rs index 53a28bc0a..6bfbd9eeb 100644 --- a/gstreamer/src/subclass/pad.rs +++ b/gstreamer/src/subclass/pad.rs @@ -141,7 +141,7 @@ mod tests { assert_eq!(pad.name(), "test"); - let otherpad = Pad::new(Some("other-test"), PadDirection::Sink); + let otherpad = Pad::builder(PadDirection::Sink).name("other-test").build(); pad.link(&otherpad).unwrap(); pad.unlink(&otherpad).unwrap(); diff --git a/tutorials/src/bin/playback-tutorial-7.rs b/tutorials/src/bin/playback-tutorial-7.rs index 86dd0c40f..4a6ba992e 100644 --- a/tutorials/src/bin/playback-tutorial-7.rs +++ b/tutorials/src/bin/playback-tutorial-7.rs @@ -34,7 +34,10 @@ fn tutorial_main() -> Result<(), Error> { let pad = equalizer .static_pad("sink") .expect("Failed to get a static pad from equalizer."); - let ghost_pad = gst::GhostPad::with_target(Some("sink"), &pad).unwrap(); + let ghost_pad = gst::GhostPad::builder_with_target(&pad) + .unwrap() + .name("sink") + .build(); ghost_pad.set_active(true)?; bin.add_pad(&ghost_pad)?;