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/1519>
This commit is contained in:
François Laignel 2024-09-11 11:11:28 +02:00 committed by GStreamer Marge Bot
parent 96ec17b1e9
commit 88a52d9ea1
2 changed files with 44 additions and 2 deletions

View file

@ -401,6 +401,8 @@ impl CapsFeaturesRef {
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))
}
}
@ -573,6 +575,8 @@ impl<'a> Iterator for Iter<'a> {
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))
}
}
@ -599,6 +603,9 @@ impl<'a> Iterator for Iter<'a> {
let feature =
ffi::gst_caps_features_get_nth(self.caps_features.as_ptr(), end as u32);
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))
}
}
@ -614,6 +621,9 @@ impl<'a> Iterator for Iter<'a> {
self.n_features as u32 - 1,
);
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))
}
}
@ -633,6 +643,8 @@ impl<'a> DoubleEndedIterator for Iter<'a> {
ffi::gst_caps_features_get_nth(self.caps_features.as_ptr(), self.n_features as u32);
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))
}
}
@ -651,6 +663,8 @@ impl<'a> DoubleEndedIterator for Iter<'a> {
);
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))
}
}

View file

@ -637,7 +637,11 @@ impl StructureRef {
#[doc(alias = "get_name")]
#[doc(alias = "gst_structure_get_name")]
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")]
@ -744,7 +748,8 @@ impl StructureRef {
let field_name = ffi::gst_structure_nth_field_name(&self.0, idx as u32);
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)))
}
}
@ -1099,6 +1104,8 @@ impl<'a> Iterator for FieldIterator<'a> {
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).unwrap();
self.idx += 1;
@ -1119,6 +1126,8 @@ impl<'a> DoubleEndedIterator for FieldIterator<'a> {
}
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).unwrap())
}
}
@ -1129,6 +1138,8 @@ impl<'a> std::iter::FusedIterator for FieldIterator<'a> {}
#[derive(Debug)]
pub struct Iter<'a> {
// Safety: FieldIterator ensures static lifetime for the returned Item,
// whatever the GStreamer version being used.
iter: FieldIterator<'a>,
}
@ -1480,4 +1491,21 @@ mod tests {
assert!(!s.has_field("array"));
assert!(!s.has_field("list"));
}
#[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");
}
}