video/timecode: Use mem::ManuallyDrop to prevent a double unref of the daily jam

As ffi::GstVideoTimeCode implements Copy, assignments don't move it out
of the passed in value but just copy it. This doesn't increase the
reference count of the daily jam, still runs the Drop impl of the passed
in value to decrease the daily jam and then causes a second unref of it
later when the returned value is dropped.

Fixes https://gitlab.freedesktop.org/gstreamer/gstreamer-rs/-/issues/310
This commit is contained in:
Sebastian Dröge 2021-02-03 22:38:02 +02:00
parent 32a96dd72c
commit e1a964fb00

View file

@ -134,6 +134,10 @@ impl TryFrom<VideoTimeCode> for ValidVideoTimeCode {
fn try_from(v: VideoTimeCode) -> Result<ValidVideoTimeCode, VideoTimeCode> { fn try_from(v: VideoTimeCode) -> Result<ValidVideoTimeCode, VideoTimeCode> {
skip_assert_initialized!(); skip_assert_initialized!();
if v.is_valid() { if v.is_valid() {
// Use ManuallyDrop here to prevent the Drop impl of VideoTimeCode
// from running as we don't move v.0 out here but copy it.
// GstVideoTimeCode implements Copy.
let v = mem::ManuallyDrop::new(v);
Ok(ValidVideoTimeCode(v.0)) Ok(ValidVideoTimeCode(v.0))
} else { } else {
Err(v) Err(v)
@ -504,6 +508,10 @@ impl Ord for ValidVideoTimeCode {
impl From<ValidVideoTimeCode> for VideoTimeCode { impl From<ValidVideoTimeCode> for VideoTimeCode {
fn from(v: ValidVideoTimeCode) -> VideoTimeCode { fn from(v: ValidVideoTimeCode) -> VideoTimeCode {
skip_assert_initialized!(); skip_assert_initialized!();
// Use ManuallyDrop here to prevent the Drop impl of VideoTimeCode
// from running as we don't move v.0 out here but copy it.
// GstVideoTimeCode implements Copy.
let v = mem::ManuallyDrop::new(v);
VideoTimeCode(v.0) VideoTimeCode(v.0)
} }
} }
@ -538,10 +546,11 @@ impl VideoTimeCodeMeta {
#![allow(clippy::cast_ptr_alignment)] #![allow(clippy::cast_ptr_alignment)]
unsafe { unsafe {
ffi::gst_video_time_code_clear(&mut self.0.tc); ffi::gst_video_time_code_clear(&mut self.0.tc);
// Use ManuallyDrop here to prevent the Drop impl of VideoTimeCode
// from running as we don't move tc.0 out here but copy it.
// GstVideoTimeCode implements Copy.
let tc = mem::ManuallyDrop::new(tc);
self.0.tc = tc.0; self.0.tc = tc.0;
if !self.0.tc.config.latest_daily_jam.is_null() {
glib::ffi::g_date_time_ref(self.0.tc.config.latest_daily_jam);
}
} }
} }
} }
@ -561,3 +570,55 @@ impl fmt::Debug for VideoTimeCodeMeta {
.finish() .finish()
} }
} }
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn test_add_get_set_meta() {
gst::init().unwrap();
let mut buffer = gst::Buffer::new();
{
let datetime =
glib::DateTime::new_utc(2021, 02, 04, 10, 53, 17.0).expect("can't create datetime");
let time_code = crate::VideoTimeCode::from_date_time(
gst::Fraction::new(30, 1),
&datetime,
crate::VideoTimeCodeFlags::empty(),
0,
)
.expect("can't create timecode");
drop(datetime);
let mut meta = crate::VideoTimeCodeMeta::add(
buffer.get_mut().unwrap(),
&time_code.try_into().expect("invalid timecode"),
);
let datetime =
glib::DateTime::new_utc(2021, 02, 04, 10, 53, 17.0).expect("can't create datetime");
let mut time_code_2 = crate::ValidVideoTimeCode::try_from(
crate::VideoTimeCode::from_date_time(
gst::Fraction::new(30, 1),
&datetime,
crate::VideoTimeCodeFlags::empty(),
0,
)
.expect("can't create timecode"),
)
.expect("invalid timecode");
assert_eq!(meta.get_tc(), time_code_2);
time_code_2.increment_frame();
assert_eq!(meta.get_tc().get_frames() + 1, time_code_2.get_frames());
meta.set_tc(time_code_2.clone());
assert_eq!(meta.get_tc(), time_code_2);
}
}
}