gst: anticipate GQuark to GstIdStr lifetime changes

GStreamer fixes a memory leak due to GQuarks by switching to GstIdStr.
The consequence is that strings previously backed by a GQuark returned by a
function will now get their lifetime bound to that of its owner, while the
GQuark version ensured static lifetime.

Because some functions return a string with the assumption that they are static
and because we can't alter the API for existing versions of the bindings, this
MR temporarily forces affected strings as GQuarks, thus gaining static lifetime
regardless of the GStreamer version actually being used.

For newer versions of the bindings, the API will be fixed and GQuarks will be
removed in favor a leakless solution.

See: https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/7432
Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer-rs/-/merge_requests/1525>
This commit is contained in:
François Laignel 2024-09-11 11:11:28 +02:00
parent cf9151a9b3
commit 08723d37a8
2 changed files with 44 additions and 2 deletions

View file

@ -400,6 +400,8 @@ impl CapsFeaturesRef {
return None; return None;
} }
// Safety: we can return a GStr based on the feature here because
// the lifetime of the returned value is constrained by &self.
Some(glib::GStr::from_ptr(feature)) Some(glib::GStr::from_ptr(feature))
} }
} }
@ -572,6 +574,8 @@ impl<'a> Iterator for Iter<'a> {
self.idx += 1; self.idx += 1;
// Safety: we can return a GStr based on the feature here because the lifetime
// of the returned Item is constrained by the underlying CapsFeatureRef.
Some(glib::GStr::from_ptr(feature)) Some(glib::GStr::from_ptr(feature))
} }
} }
@ -598,6 +602,9 @@ impl<'a> Iterator for Iter<'a> {
let feature = let feature =
ffi::gst_caps_features_get_nth(self.caps_features.as_ptr(), end as u32); ffi::gst_caps_features_get_nth(self.caps_features.as_ptr(), end as u32);
debug_assert!(!feature.is_null()); debug_assert!(!feature.is_null());
// Safety: we can return a GStr based on the feature here because the lifetime
// of the returned Item is constrained by the underlying CapsFeatureRef.
Some(glib::GStr::from_ptr(feature)) Some(glib::GStr::from_ptr(feature))
} }
} }
@ -613,6 +620,9 @@ impl<'a> Iterator for Iter<'a> {
self.n_features as u32 - 1, self.n_features as u32 - 1,
); );
debug_assert!(!feature.is_null()); debug_assert!(!feature.is_null());
// Safety: we can return a GStr based on the feature here because the lifetime
// of the returned Item is constrained by the underlying CapsFeatureRef.
Some(glib::GStr::from_ptr(feature)) Some(glib::GStr::from_ptr(feature))
} }
} }
@ -632,6 +642,8 @@ impl<'a> DoubleEndedIterator for Iter<'a> {
ffi::gst_caps_features_get_nth(self.caps_features.as_ptr(), self.n_features as u32); ffi::gst_caps_features_get_nth(self.caps_features.as_ptr(), self.n_features as u32);
debug_assert!(!feature.is_null()); debug_assert!(!feature.is_null());
// Safety: we can return a GStr based on the feature here because the lifetime
// of the returned Item is constrained by the underlying CapsFeatureRef.
Some(glib::GStr::from_ptr(feature)) Some(glib::GStr::from_ptr(feature))
} }
} }
@ -650,6 +662,8 @@ impl<'a> DoubleEndedIterator for Iter<'a> {
); );
debug_assert!(!feature.is_null()); debug_assert!(!feature.is_null());
// Safety: we can return a GStr based on the feature here because the lifetime
// of the returned Item is constrained by the underlying CapsFeatureRef.
Some(glib::GStr::from_ptr(feature)) Some(glib::GStr::from_ptr(feature))
} }
} }

View file

@ -537,7 +537,11 @@ impl StructureRef {
#[doc(alias = "get_name")] #[doc(alias = "get_name")]
#[doc(alias = "gst_structure_get_name")] #[doc(alias = "gst_structure_get_name")]
pub fn name<'a>(&self) -> &'a glib::GStr { pub fn name<'a>(&self) -> &'a glib::GStr {
unsafe { glib::GStr::from_ptr(ffi::gst_structure_get_name(&self.0)) } unsafe {
let name = ffi::gst_structure_get_name(&self.0);
// Ensure the name is static whatever the GStreamer version being used.
glib::GStr::from_ptr(glib::ffi::g_intern_string(name))
}
} }
#[doc(alias = "gst_structure_get_name_id")] #[doc(alias = "gst_structure_get_name_id")]
@ -637,7 +641,8 @@ impl StructureRef {
let field_name = ffi::gst_structure_nth_field_name(&self.0, idx); let field_name = ffi::gst_structure_nth_field_name(&self.0, idx);
debug_assert!(!field_name.is_null()); debug_assert!(!field_name.is_null());
Some(glib::GStr::from_ptr(field_name)) // Ensure the name is static whatever the GStreamer version being used.
Some(glib::GStr::from_ptr(glib::ffi::g_intern_string(field_name)))
} }
} }
@ -963,6 +968,8 @@ impl<'a> Iterator for FieldIterator<'a> {
return None; return None;
} }
// Safety: nth_field_name() ensures static lifetime for the returned string,
// whatever the GStreamer version being used.
let field_name = self.structure.nth_field_name(self.idx as u32).unwrap(); let field_name = self.structure.nth_field_name(self.idx as u32).unwrap();
self.idx += 1; self.idx += 1;
@ -983,6 +990,8 @@ impl<'a> DoubleEndedIterator for FieldIterator<'a> {
} }
self.n_fields -= 1; self.n_fields -= 1;
// Safety: nth_field_name() ensures static lifetime for the returned string,
// whatever the GStreamer version being used.
Some(self.structure.nth_field_name(self.n_fields as u32).unwrap()) Some(self.structure.nth_field_name(self.n_fields as u32).unwrap())
} }
} }
@ -993,6 +1002,8 @@ impl<'a> std::iter::FusedIterator for FieldIterator<'a> {}
#[derive(Debug)] #[derive(Debug)]
pub struct Iter<'a> { pub struct Iter<'a> {
// Safety: FieldIterator ensures static lifetime for the returned Item,
// whatever the GStreamer version being used.
iter: FieldIterator<'a>, iter: FieldIterator<'a>,
} }
@ -1262,4 +1273,21 @@ mod tests {
assert_eq!(format!("{s:?}"), "Structure(test { f1: (gchararray) \"abc\", f2: (gchararray) \"bcd\", f3: (gint) 123, f4: Structure(nested { badger: (gboolean) TRUE }), f5: Array([(gchararray) \"a\", (gchararray) \"b\", (gchararray) \"c\"]), f6: List([(gchararray) \"d\", (gchararray) \"e\", (gchararray) \"f\"]) })"); assert_eq!(format!("{s:?}"), "Structure(test { f1: (gchararray) \"abc\", f2: (gchararray) \"bcd\", f3: (gint) 123, f4: Structure(nested { badger: (gboolean) TRUE }), f5: Array([(gchararray) \"a\", (gchararray) \"b\", (gchararray) \"c\"]), f6: List([(gchararray) \"d\", (gchararray) \"e\", (gchararray) \"f\"]) })");
} }
#[test]
fn test_name_and_field_name_lt() {
crate::init().unwrap();
let (name, field_name) = {
let s = Structure::builder("name")
.field("field0", "val0")
.field("field1", "val1")
.build();
(s.name(), s.nth_field_name(0).unwrap())
};
assert_eq!(name, "name");
assert_eq!(field_name, "field0");
}
} }