From 051df59cd1cf66c5b1dabe33b5e0dc78f779c6eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Laignel?= Date: Mon, 4 Oct 2021 20:37:26 +0200 Subject: [PATCH] clock_time, format: fix checked, saturating, wrapping ops These operations were implemented using the integer implementations. These types' MAX values are different from the integers so they must use specific implementations. Also add the overflowing variants. --- gstreamer/src/clock_time.rs | 196 ++++++++++++++++++++---------------- gstreamer/src/format.rs | 17 +++- 2 files changed, 124 insertions(+), 89 deletions(-) diff --git a/gstreamer/src/clock_time.rs b/gstreamer/src/clock_time.rs index dab740212..17fcaef82 100644 --- a/gstreamer/src/clock_time.rs +++ b/gstreamer/src/clock_time.rs @@ -3,9 +3,10 @@ use glib::translate::*; use glib::StaticType; use num_integer::div_rem; +use std::convert::{From, TryFrom}; use std::io::{self, prelude::*}; use std::time::Duration; -use std::{cmp, convert, fmt, str}; +use std::{cmp, fmt, str}; #[derive(PartialEq, Eq, PartialOrd, Ord, Hash, Clone, Copy, Default)] pub struct ClockTime(pub(crate) u64); @@ -15,6 +16,7 @@ impl ClockTime { pub const MSECOND: ClockTime = ClockTime(1_000_000); pub const USECOND: ClockTime = ClockTime(1_000); pub const NSECOND: ClockTime = ClockTime(1); + pub const MAX: ClockTime = ClockTime(ffi::GST_CLOCK_TIME_NONE - 1); pub const fn hours(self) -> u64 { self.0 / Self::SECOND.0 / 60 / 60 @@ -176,7 +178,7 @@ impl fmt::Display for DurationError { impl std::error::Error for DurationError {} -impl convert::TryFrom for ClockTime { +impl TryFrom for ClockTime { type Error = DurationError; fn try_from(d: Duration) -> Result { @@ -193,7 +195,7 @@ impl convert::TryFrom for ClockTime { } } -impl convert::From for Duration { +impl From for Duration { fn from(t: ClockTime) -> Self { skip_assert_initialized!(); @@ -213,25 +215,41 @@ macro_rules! impl_common_ops_for_newtype_u64( #[must_use = "this returns the result of the operation, without modifying the original"] #[inline] - // FIXME Can't use `map` in a `const fn` as of rustc 1.53.0-beta.2 - #[allow(clippy::manual_map)] pub const fn checked_add(self, rhs: Self) -> Option { match self.0.checked_add(rhs.0) { - Some(res) => Some(Self(res)), - None => None, + Some(res) if res <= Self::MAX.0 => Some(Self(res)), + _ => None, } } #[must_use = "this returns the result of the operation, without modifying the original"] #[inline] pub const fn saturating_add(self, rhs: Self) -> Self { - Self(self.0.saturating_add(rhs.0)) + let res = self.0.saturating_add(rhs.0); + if res <= Self::MAX.0 { + Self(res) + } else { + Self::MAX + } } #[must_use = "this returns the result of the operation, without modifying the original"] #[inline] - pub const fn wrapping_add(self, rhs: Self) -> Self { - Self(self.0.wrapping_add(rhs.0)) + pub fn overflowing_add(self, rhs: Self) -> (Self, bool) { + let self_u128 = self.0 as u128; + let rhs_128 = rhs.0 as u128; + let res_u128 = self_u128 + rhs_128; + if res_u128 <= Self::MAX.0 as u128 { + (TryFrom::try_from(res_u128 as u64).unwrap(), false) + } else { + (TryFrom::try_from((res_u128 - Self::MAX.0 as u128 - 1) as u64).unwrap(), true) + } + } + + #[must_use = "this returns the result of the operation, without modifying the original"] + #[inline] + pub fn wrapping_add(self, rhs: Self) -> Self { + self.overflowing_add(rhs).0 } #[must_use = "this returns the result of the operation, without modifying the original"] @@ -251,10 +269,20 @@ macro_rules! impl_common_ops_for_newtype_u64( Self(self.0.saturating_sub(rhs.0)) } + #[must_use = "this returns the result of the operation, without modifying the original"] + #[inline] + pub const fn overflowing_sub(self, rhs: Self) -> (Self, bool) { + if self.0 >= rhs.0 { + (Self(self.0 - rhs.0), false) + } else { + (Self(Self::MAX.0 - rhs.0 + self.0 + 1), true) + } + } + #[must_use = "this returns the result of the operation, without modifying the original"] #[inline] pub const fn wrapping_sub(self, rhs: Self) -> Self { - Self(self.0.wrapping_sub(rhs.0)) + self.overflowing_sub(rhs).0 } } }; @@ -466,119 +494,111 @@ impl std::iter::Sum for ClockTime { mod tests { use super::*; + const CT_1: ClockTime = ClockTime::from_nseconds(1); + const CT_2: ClockTime = ClockTime::from_nseconds(2); + const CT_3: ClockTime = ClockTime::from_nseconds(3); + const CT_10: ClockTime = ClockTime::from_nseconds(10); + const CT_20: ClockTime = ClockTime::from_nseconds(20); + const CT_30: ClockTime = ClockTime::from_nseconds(30); + #[test] fn opt_time_clock() { - let ct_1 = ClockTime(1); - let opt_ct_none: Option = None; - - assert_eq!(ct_1.into_glib(), 1); - assert_eq!(Some(ct_1).into_glib(), 1); - assert_eq!(opt_ct_none.into_glib(), ffi::GST_CLOCK_TIME_NONE); + assert_eq!(CT_1.into_glib(), 1); + assert_eq!(Some(CT_1).into_glib(), 1); + assert_eq!(ClockTime::NONE.into_glib(), ffi::GST_CLOCK_TIME_NONE); let ct_1_from: ClockTime = unsafe { try_from_glib(1u64) }.unwrap(); - assert_eq!(ct_1_from, ct_1); + assert_eq!(ct_1_from, CT_1); let opt_ct_some: Option = unsafe { from_glib(1u64) }; - assert_eq!(opt_ct_some, Some(ct_1)); + assert_eq!(opt_ct_some, Some(CT_1)); - let opt_ct_none: Option = unsafe { from_glib(ffi::GST_CLOCK_TIME_NONE) }; - assert_eq!(opt_ct_none, None); + let ct_none: Option = unsafe { from_glib(ffi::GST_CLOCK_TIME_NONE) }; + assert_eq!(ct_none, None); } #[test] #[allow(clippy::eq_op, clippy::op_ref)] fn ops() { - let ct_10 = 10 * ClockTime::MSECOND; - let ct_20 = 20 * ClockTime::MSECOND; - let ct_30 = 30 * ClockTime::MSECOND; - - assert_eq!(ct_10 + ct_20, ct_30); - assert_eq!(ct_10 + &ct_20, ct_30); - assert_eq!(&ct_10 + &ct_20, ct_30); - assert_eq!(ct_30 - ct_20, ct_10); - assert_eq!(ct_30 - ct_30, ClockTime::ZERO); - assert_eq!(ct_10 * 3, ct_30); - assert_eq!(3 * ct_10, ct_30); - assert_eq!(3 * &ct_10, ct_30); - assert_eq!(ct_30.nseconds(), 30_000_000); + assert_eq!(CT_10 + CT_20, CT_30); + assert_eq!(CT_10 + &CT_20, CT_30); + assert_eq!(&CT_10 + &CT_20, CT_30); + assert_eq!(CT_30 - CT_20, CT_10); + assert_eq!(CT_30 - CT_30, ClockTime::ZERO); + assert_eq!(CT_10 * 3, CT_30); + assert_eq!(3 * CT_10, CT_30); + assert_eq!(3 * &CT_10, CT_30); + assert_eq!(CT_30.nseconds(), 30); } #[test] fn checked_ops() { - let ct_1 = ClockTime::from_nseconds(1); - let ct_2 = ClockTime::from_nseconds(2); + assert_eq!(CT_1.checked_add(CT_1), Some(CT_2)); + assert_eq!(CT_1.checked_add(CT_1), Some(CT_2)); + assert!(ClockTime::MAX.checked_add(CT_1).is_none()); - let ct_max = ClockTime::from_nseconds(std::u64::MAX); + assert_eq!(CT_2.checked_sub(CT_1), Some(CT_1)); + assert_eq!(CT_2.checked_sub(CT_1), Some(CT_1)); + assert!(CT_1.checked_sub(CT_2).is_none()); + } - assert_eq!(ct_1.checked_add(ct_1), Some(ct_2)); - assert_eq!(ct_1.checked_add(ct_1), Some(ct_2)); - assert!(ct_max.checked_add(ct_1).is_none()); + #[test] + fn overflowing_ops() { + assert_eq!(CT_1.overflowing_add(CT_2), (CT_3, false)); + assert_eq!(CT_1.overflowing_add(CT_2), (CT_3, false)); + assert_eq!( + ClockTime::MAX.overflowing_add(CT_1), + (ClockTime::ZERO, true) + ); - assert_eq!(ct_2.checked_sub(ct_1), Some(ct_1)); - assert_eq!(ct_2.checked_sub(ct_1), Some(ct_1)); - assert!(ct_1.checked_sub(ct_2).is_none()); + assert_eq!(CT_3.overflowing_sub(CT_2), (CT_1, false)); + assert_eq!(CT_3.overflowing_sub(CT_2), (CT_1, false)); + assert_eq!(CT_1.overflowing_sub(CT_2), (ClockTime::MAX, true)); } #[test] fn saturating_ops() { - let ct_1 = ClockTime::from_nseconds(1); - let ct_2 = ClockTime::from_nseconds(2); - let ct_3 = ClockTime::from_nseconds(3); + assert_eq!(CT_1.saturating_add(CT_2), CT_3); + assert_eq!(CT_1.saturating_add(CT_2), CT_3); + assert_eq!(ClockTime::MAX.saturating_add(CT_1), ClockTime::MAX); - let ct_max = ClockTime::from_nseconds(std::u64::MAX); - - assert_eq!(ct_1.saturating_add(ct_2), ct_3); - assert_eq!(ct_1.saturating_add(ct_2), ct_3); - assert_eq!(ct_max.saturating_add(ct_1), ct_max); - - assert_eq!(ct_3.saturating_sub(ct_2), ct_1); - assert_eq!(ct_3.saturating_sub(ct_2), ct_1); - assert!(ct_1.saturating_sub(ct_2).is_zero()); + assert_eq!(CT_3.saturating_sub(CT_2), CT_1); + assert_eq!(CT_3.saturating_sub(CT_2), CT_1); + assert!(CT_1.saturating_sub(CT_2).is_zero()); } #[test] fn wrapping_ops() { - let ct_1 = ClockTime::NSECOND; - let ct_2 = 2 * ClockTime::NSECOND; - let ct_3 = 3 * ClockTime::NSECOND; + assert_eq!(CT_1.wrapping_add(CT_2), CT_3); + assert_eq!(CT_1.wrapping_add(CT_2), CT_3); + assert_eq!(ClockTime::MAX.wrapping_add(CT_1), ClockTime::ZERO); - let ct_max = ClockTime::from_nseconds(std::u64::MAX); - - assert_eq!(ct_1.wrapping_add(ct_2), ct_3); - assert_eq!(ct_1.wrapping_add(ct_2), ct_3); - assert_eq!(ct_max.wrapping_add(ct_1), ClockTime::ZERO); - - assert_eq!(ct_3.wrapping_sub(ct_2), ct_1); - assert_eq!(ct_3.wrapping_sub(ct_2), ct_1); - assert_eq!(ct_1.wrapping_sub(ct_2), ct_max); + assert_eq!(CT_3.wrapping_sub(CT_2), CT_1); + assert_eq!(CT_3.wrapping_sub(CT_2), CT_1); + assert_eq!(CT_1.wrapping_sub(CT_2), ClockTime::MAX); } #[test] fn comp() { - let ct_0 = ClockTime::ZERO; - let ct_2 = 2 * ClockTime::NSECOND; - let ct_3 = 3 * ClockTime::NSECOND; - let opt_ct_none: Option = None; + assert!(ClockTime::ZERO < CT_2); + assert!(Some(ClockTime::ZERO) < Some(CT_2)); + assert!(CT_2 < CT_3); + assert!(Some(CT_2) < Some(CT_3)); + assert!(ClockTime::ZERO < CT_3); + assert!(Some(ClockTime::ZERO) < Some(CT_3)); - assert!(ct_0 < ct_2); - assert!(Some(ct_0) < Some(ct_2)); - assert!(ct_2 < ct_3); - assert!(Some(ct_2) < Some(ct_3)); - assert!(ct_0 < ct_3); - assert!(Some(ct_0) < Some(ct_3)); + assert!(CT_3 > CT_2); + assert!(Some(CT_3) > Some(CT_2)); + assert!(CT_2 > ClockTime::ZERO); + assert!(Some(CT_2) > Some(ClockTime::ZERO)); + assert!(CT_3 > ClockTime::ZERO); + assert!(Some(CT_3) > Some(ClockTime::ZERO)); - assert!(ct_3 > ct_2); - assert!(Some(ct_3) > Some(ct_2)); - assert!(ct_2 > ct_0); - assert!(Some(ct_2) > Some(ct_0)); - assert!(ct_3 > ct_0); - assert!(Some(ct_3) > Some(ct_0)); - - assert!(!(opt_ct_none < None)); - assert!(!(opt_ct_none > None)); + assert!(!(ClockTime::NONE < None)); + assert!(!(ClockTime::NONE > None)); // This doesn't work due to the `PartialOrd` impl on `Option` - //assert_eq!(Some(ct_0) > opt_ct_none, false); - assert!(!(Some(ct_0) < opt_ct_none)); + //assert_eq!(Some(ClockTime::ZERO) > ClockTime::ZERO, false); + assert!(!(Some(ClockTime::ZERO) < ClockTime::NONE)); } #[test] diff --git a/gstreamer/src/format.rs b/gstreamer/src/format.rs index 96521a692..5d2a19bad 100644 --- a/gstreamer/src/format.rs +++ b/gstreamer/src/format.rs @@ -27,9 +27,15 @@ pub struct Undefined(pub i64); #[derive(PartialEq, Eq, PartialOrd, Ord, Hash, Clone, Copy, Debug, Default)] pub struct Default(pub u64); +impl Default { + pub const MAX: Self = Self(u64::MAX - 1); +} #[derive(PartialEq, Eq, PartialOrd, Ord, Hash, Clone, Copy, Debug, Default)] pub struct Bytes(pub u64); +impl Bytes { + pub const MAX: Self = Self(u64::MAX - 1); +} pub type Time = ClockTime; @@ -37,12 +43,13 @@ pub type Time = ClockTime; pub struct Buffers(pub u64); impl Buffers { pub const OFFSET_NONE: u64 = ffi::GST_BUFFER_OFFSET_NONE; + pub const MAX: Self = Self(Self::OFFSET_NONE - 1); } #[derive(PartialEq, Eq, PartialOrd, Ord, Hash, Clone, Copy, Debug, Default)] pub struct Percent(pub u32); impl Percent { - pub const MAX: u32 = ffi::GST_FORMAT_PERCENT_MAX as u32; + pub const MAX: Self = Self(ffi::GST_FORMAT_PERCENT_MAX as u32); pub const SCALE: u32 = ffi::GST_FORMAT_PERCENT_SCALE as u32; } @@ -544,6 +551,14 @@ impl From for GenericFormattedValue { GenericFormattedValue::Percent(Some(v)) } } +impl TryFrom for Percent { + type Error = GlibNoneError; + + fn try_from(v: u64) -> Result { + skip_assert_initialized!(); + unsafe { Self::try_from_glib(v as i64) } + } +} impl TryFromGlib for Percent { type Error = GlibNoneError;