From dcf6d164962da7fe9b0eef28c06c81afac04b9d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Laignel?= Date: Tue, 11 Oct 2022 13:25:53 +0200 Subject: [PATCH] gst/format: new panicking constructors and some Percent fixes Previous proposition for constructing specific formatted values was to use an operation such as `42 * Default::ONE` which, in retrospect, doesn't seem idiomatic. This commit adds `from_u64` and `from_usize` constructors for most formatted values. Having `from_usize` is convenient when dealing with quantities related to containers indices or length. This also fixes the `Percent` from float constructors from which was derived the `ONE` constant as well as previous display implementation. Also removed the `pub` specifier for `Undefined` inner value. It wasn't removed in a previous commit as `Undefined` can use the full range of the inner type. But now, it seems preferable not to expose the inner value for proper encapsulation and so as to reduce the differences with other formatted values (kind of least astonishment principle). --- gstreamer-audio/src/audio_meta.rs | 4 +- gstreamer/src/format/format_serde.rs | 53 ++++++------ gstreamer/src/format/generic.rs | 42 +++++++-- gstreamer/src/format/macros.rs | 2 +- gstreamer/src/format/mod.rs | 35 +++++--- gstreamer/src/format/specific.rs | 123 +++++++++++++++++++++++++-- gstreamer/src/format/undefined.rs | 2 +- 7 files changed, 202 insertions(+), 59 deletions(-) diff --git a/gstreamer-audio/src/audio_meta.rs b/gstreamer-audio/src/audio_meta.rs index 9c6add8c1..825fd99c7 100644 --- a/gstreamer-audio/src/audio_meta.rs +++ b/gstreamer-audio/src/audio_meta.rs @@ -294,8 +294,8 @@ mod tests { let mut buffer = gst::Buffer::with_size(1024).unwrap(); - let start = gst::format::Default::ONE; - let stop = 2 * gst::format::Default::ONE; + let start = gst::format::Default::from_u64(1); + let stop = gst::format::Default::from_u64(2); { let cmeta = AudioClippingMeta::add(buffer.get_mut().unwrap(), start, stop); diff --git a/gstreamer/src/format/format_serde.rs b/gstreamer/src/format/format_serde.rs index d4a23293d..4e6049cff 100644 --- a/gstreamer/src/format/format_serde.rs +++ b/gstreamer/src/format/format_serde.rs @@ -45,14 +45,15 @@ impl_serde!(Percent, u32); impl Serialize for Undefined { fn serialize(&self, serializer: S) -> Result { - self.0.serialize(serializer) + use std::ops::Deref; + self.deref().serialize(serializer) } } impl<'de> Deserialize<'de> for Undefined { fn deserialize>(deserializer: D) -> Result { skip_assert_initialized!(); - i64::deserialize(deserializer).map(Undefined) + i64::deserialize(deserializer).map(Into::into) } } @@ -65,17 +66,15 @@ mod tests { #[test] fn test_serialize() { - crate::init().unwrap(); - let pretty_config = ron::ser::PrettyConfig::new().new_line("".to_string()); - let value = GenericFormattedValue::from(Undefined(42)); + let value = GenericFormattedValue::from(Undefined::from(42)); let res = ron::ser::to_string_pretty(&value, pretty_config.clone()); assert_eq!(Ok("Undefined(42)".to_owned()), res); let res = serde_json::to_string(&value).unwrap(); assert_eq!("{\"Undefined\":42}".to_owned(), res); - let value = GenericFormattedValue::from(42 * Default::ONE); + let value = GenericFormattedValue::from(Default::from_u64(42)); let res = ron::ser::to_string_pretty(&value, pretty_config.clone()); assert_eq!(Ok("Default(Some(42))".to_owned()), res); let res = serde_json::to_string(&value).unwrap(); @@ -87,7 +86,7 @@ mod tests { let res = serde_json::to_string(&value).unwrap(); assert_eq!("{\"Default\":null}".to_owned(), res); - let value = GenericFormattedValue::from(42 * Bytes::ONE); + let value = GenericFormattedValue::from(Bytes::from_usize(42)); let res = ron::ser::to_string_pretty(&value, pretty_config.clone()); assert_eq!(Ok("Bytes(Some(42))".to_owned()), res); let res = serde_json::to_string(&value).unwrap(); @@ -99,25 +98,25 @@ mod tests { let res = serde_json::to_string(&value).unwrap(); assert_eq!("{\"Time\":42123456789}".to_owned(), res); - let value = GenericFormattedValue::from(42 * Buffers::ONE); + let value = GenericFormattedValue::from(Buffers::from_u64(42)); let res = ron::ser::to_string_pretty(&value, pretty_config.clone()); assert_eq!(Ok("Buffers(Some(42))".to_owned()), res); let res = serde_json::to_string(&value).unwrap(); assert_eq!("{\"Buffers\":42}".to_owned(), res); - let percent = Percent::try_from(0.42).unwrap(); + let percent = Percent::from_ratio(0.42); let value = GenericFormattedValue::from(percent); let res = ron::ser::to_string_pretty(&value, pretty_config.clone()); - assert_eq!(Ok("Percent(Some(4200))".to_owned()), res); + assert_eq!(Ok("Percent(Some(420000))".to_owned()), res); let res = serde_json::to_string(&value).unwrap(); - assert_eq!("{\"Percent\":4200}".to_owned(), res); + assert_eq!("{\"Percent\":420000}".to_owned(), res); let other = Other::try_from(42).ok(); - let value = GenericFormattedValue::Other(Format::Percent, other); + let value = GenericFormattedValue::Other(Format::Default, other); let res = ron::ser::to_string_pretty(&value, pretty_config.clone()); - assert_eq!(Ok("Other(Percent, Some(42))".to_owned()), res); + assert_eq!(Ok("Other(Default, Some(42))".to_owned()), res); let res = serde_json::to_string(&value).unwrap(); - assert_eq!("{\"Other\":[\"Percent\",42]}".to_owned(), res); + assert_eq!("{\"Other\":[\"Default\",42]}".to_owned(), res); let value = GenericFormattedValue::new(Format::__Unknown(7), 42); let res = ron::ser::to_string_pretty(&value, pretty_config); @@ -128,31 +127,27 @@ mod tests { #[test] fn test_deserialize() { - crate::init().unwrap(); - let value_ron = "Default(Some(42))"; let value_de: GenericFormattedValue = ron::de::from_str(value_ron).unwrap(); - assert_eq!(value_de, GenericFormattedValue::from(42 * Default::ONE)); + assert_eq!(value_de, GenericFormattedValue::from(Default::from_u64(42))); let value_json = "{\"Default\":42}"; let value_de: GenericFormattedValue = serde_json::from_str(value_json).unwrap(); - assert_eq!(value_de, GenericFormattedValue::from(42 * Default::ONE)); + assert_eq!(value_de, GenericFormattedValue::from(Default::from_u64(42))); - let value_ron = "Other(Percent, Some(42))"; - let gfv_value = GenericFormattedValue::Other(Format::Percent, Some(42 * Other::ONE)); + let value_ron = "Other(Buffers, Some(42))"; + let gfv_value = GenericFormattedValue::Other(Format::Buffers, Some(42 * Other::ONE)); let value_de: GenericFormattedValue = ron::de::from_str(value_ron).unwrap(); assert_eq!(value_de, gfv_value); - let value_json = "{\"Other\":[\"Percent\",42]}"; + let value_json = "{\"Other\":[\"Buffers\",42]}"; let value_de: GenericFormattedValue = serde_json::from_str(value_json).unwrap(); assert_eq!(value_de, gfv_value); } #[test] fn test_serde_roundtrip() { - crate::init().unwrap(); - macro_rules! test_roundrip( ($value:expr) => { let value_ser = ron::ser::to_string(&$value).unwrap(); @@ -161,15 +156,15 @@ mod tests { } ); - test_roundrip!(GenericFormattedValue::Undefined(Undefined(42))); - test_roundrip!(GenericFormattedValue::from(42 * Default::ONE)); - test_roundrip!(GenericFormattedValue::from(42 * Bytes::ONE)); + test_roundrip!(GenericFormattedValue::Undefined(Undefined::from(42))); + test_roundrip!(GenericFormattedValue::from(Default::from_u64(42))); + test_roundrip!(GenericFormattedValue::from(Bytes::from_u64(42))); test_roundrip!(GenericFormattedValue::from(ClockTime::from_nseconds( 42_123_456_789 ))); - test_roundrip!(GenericFormattedValue::from(42 * Buffers::ONE)); - test_roundrip!(GenericFormattedValue::from(42 * Percent::ONE)); - let gfv_value = GenericFormattedValue::Other(Format::Percent, Other::try_from(42).ok()); + test_roundrip!(GenericFormattedValue::from(Buffers::from_u64(42))); + test_roundrip!(GenericFormattedValue::from(Percent::from_percent(42))); + let gfv_value = GenericFormattedValue::Other(Format::Default, Other::try_from(42).ok()); test_roundrip!(gfv_value); test_roundrip!(GenericFormattedValue::new(Format::__Unknown(7), 42)); } diff --git a/gstreamer/src/format/generic.rs b/gstreamer/src/format/generic.rs index e69174fa8..e319974a0 100644 --- a/gstreamer/src/format/generic.rs +++ b/gstreamer/src/format/generic.rs @@ -19,6 +19,32 @@ impl Other { pub const MAX: Self = Self(u64::MAX - 1); } +impl Other { + // rustdoc-stripper-ignore-next + /// Builds a new `Other` value with the provided quantity. + /// + /// # Panics + /// + /// Panics if the provided quantity equals `u64::MAX`, + /// which is reserved for `None` in C. + #[track_caller] + pub fn from_u64(quantity: u64) -> Self { + Other::try_from(quantity).expect("`Other` value out of range") + } + + // rustdoc-stripper-ignore-next + /// Builds a new `Other` value with the provided quantity. + /// + /// # Panics + /// + /// Panics if the provided quantity equals `u64::MAX`, + /// which is reserved for `None` in C. + #[track_caller] + pub fn from_usize(quantity: usize) -> Self { + Other::from_u64(quantity.try_into().unwrap()) + } +} + impl_common_ops_for_newtype_uint!(Other, u64); impl_signed_div_mul!(Other, u64); option_glib_newtype_from_to!(Other, u64::MAX); @@ -82,7 +108,7 @@ impl GenericFormattedValue { pub fn new(format: Format, value: i64) -> Self { skip_assert_initialized!(); match format { - Format::Undefined => Self::Undefined(Undefined(value)), + Format::Undefined => Self::Undefined(value.into()), Format::Default => Self::Default(unsafe { FromGlib::from_glib(value) }), Format::Bytes => Self::Bytes(unsafe { FromGlib::from_glib(value) }), Format::Time => Self::Time(unsafe { FromGlib::from_glib(value) }), @@ -109,7 +135,7 @@ impl GenericFormattedValue { pub fn value(&self) -> i64 { unsafe { match *self { - Self::Undefined(v) => v.0, + Self::Undefined(v) => *v, Self::Default(v) => v.into_raw_value(), Self::Bytes(v) => v.into_raw_value(), Self::Time(v) => v.into_raw_value(), @@ -390,9 +416,9 @@ mod tests { let other_none: Option = Other::try_from(u64::MAX).ok(); assert!(other_none.is_none()); - let other_10 = Other::try_from(10).unwrap(); - let other_20 = Other::try_from(20).unwrap(); - let other_30 = Other::try_from(30).unwrap(); + let other_10 = Other::from_u64(10); + let other_20 = Other::from_usize(20); + let other_30 = Other::from_u64(30); assert_eq!(other_10 + other_20, other_30); assert_eq!(other_30 - other_20, other_10); @@ -409,7 +435,7 @@ mod tests { GenericFormattedValue::new(Format::__Unknown(128), 42); assert_eq!( gen_other_42, - GenericFormattedValue::Other(Format::__Unknown(128), Some(Other(42))) + GenericFormattedValue::Other(Format::__Unknown(128), Other::try_from(42).ok()) ); assert_eq!(gen_other_42.format(), Format::__Unknown(128)); assert_eq!(gen_other_42.value(), 42); @@ -438,7 +464,7 @@ mod tests { p_gen_other_42, GenericSignedFormattedValue::Other( Format::__Unknown(128), - Some(Signed::Positive(Other(42))), + Some(Signed::Positive(Other::from_u64(42))), ), ); @@ -447,7 +473,7 @@ mod tests { n_gen_other_42, GenericSignedFormattedValue::Other( Format::__Unknown(128), - Some(Signed::Negative(Other(42))), + Some(Signed::Negative(Other::from_u64(42))), ), ); } diff --git a/gstreamer/src/format/macros.rs b/gstreamer/src/format/macros.rs index 24efc1701..ecb88a429 100644 --- a/gstreamer/src/format/macros.rs +++ b/gstreamer/src/format/macros.rs @@ -1605,7 +1605,7 @@ macro_rules! glib_newtype_display { } }; - ($typ:ty, $displayable_option_name:ident, Format::$format:ident$(,)?) => { + ($typ:ty, $displayable_option_name:ident, Format::$format:ident) => { glib_newtype_display!($typ, Format::$format); pub struct $displayable_option_name(Option<$typ>); diff --git a/gstreamer/src/format/mod.rs b/gstreamer/src/format/mod.rs index c167be256..995691420 100644 --- a/gstreamer/src/format/mod.rs +++ b/gstreamer/src/format/mod.rs @@ -73,10 +73,17 @@ //! assert_eq!(Default::try_from(42), Ok(default)); //! assert_eq!(Default::try_from(42).ok(), Some(default)); //! -//! // `ClockTime` provides specific constructors: +//! // `ClockTime` provides specific constructors, +//! // which can panic if the requested value is out of range. //! let time = ClockTime::from_nseconds(45_834_908_569_837); //! let time = ClockTime::from_seconds(20); //! +//! // Other formatted values also come with (panicking) constructors: +//! let buffers_nb = Buffers::from_u64(512); +//! let received = Bytes::from_u64(64); +//! let sample_size = Bytes::from_usize([0u8; 4].len()); +//! let quantity = Default::from_u64(42); +//! //! // This can be convenient: //! assert_eq!( //! 20 * ClockTime::MSECOND, @@ -87,24 +94,32 @@ //! ClockTime::from_nseconds(40_000_000_000), //! ); //! -//! // Specific formatted values provide the `ONE` value: -//! assert_eq!(*(128 * Buffers::ONE), 128); -//! -//! // `ZERO` and `NONE` can also come in handy sometimes: +//! // `ZERO` and `NONE` can come in handy sometimes: //! assert_eq!(*Buffers::ZERO, 0); //! assert!(ClockTime::NONE.is_none()); //! +//! // Specific formatted values provide the `ONE` value: +//! assert_eq!(*(128 * Buffers::ONE), 128); +//! //! // `Bytes` also comes with usual multipliers: //! assert_eq!(*(512 * Bytes::K), 512 * 1024); //! assert_eq!(*(8 * Bytes::M), 8 * 1024 * 1024); //! assert_eq!(*(4 * Bytes::G), 4 * 1024 * 1024 * 1024); //! -//! // `Percent` can be built from a float: -//! let a_quarter = Percent::try_from(0.25).unwrap(); -//! // `Percent` has `SCALE` which represents 100%: -//! assert_eq!(Percent::SCALE / 4, a_quarter); -//! // ... and `ONE` which is 1%: +//! // `Percent` can be built from a floating point ratio: +//! let a_quarter_from_ratio = Percent::from_ratio(0.25); +//! // ... from a percent integer value: +//! let a_quarter = Percent::from_percent(25); +//! assert_eq!(a_quarter, a_quarter_from_ratio); +//! // ... from a part per million integer value: +//! let a_quarter_from_ppm = Percent::from_ppm(25 * 10_000); +//! assert_eq!(a_quarter, a_quarter_from_ppm); +//! // ... `MAX` which represents 100%: +//! assert_eq!(Percent::MAX / 4, a_quarter); +//! // ... `ONE` which is 1%: //! assert_eq!(25 * Percent::ONE, a_quarter); +//! // ... and `SCALE` which is 1% in ppm: +//! assert_eq!(Percent::SCALE, Percent::from_ppm(10_000)); //! ``` //! //! ### Displaying a formatted value diff --git a/gstreamer/src/format/specific.rs b/gstreamer/src/format/specific.rs index 1d0194578..6ebe6ba27 100644 --- a/gstreamer/src/format/specific.rs +++ b/gstreamer/src/format/specific.rs @@ -28,6 +28,32 @@ impl Buffers { pub const MAX: Self = Self(Self::OFFSET_NONE - 1); } +impl Buffers { + // rustdoc-stripper-ignore-next + /// Builds a new `Buffers` formatted value with the provided buffers count. + /// + /// # Panics + /// + /// Panics if the provided count equals `u64::MAX`, + /// which is reserved for `None` in C. + #[track_caller] + pub fn from_u64(buffers: u64) -> Self { + Buffers::try_from(buffers).expect("`Buffers` value out of range") + } + + // rustdoc-stripper-ignore-next + /// Builds a new `Buffers` formatted value with the provided buffers count. + /// + /// # Panics + /// + /// Panics if the provided count equals `u64::MAX`, + /// which is reserved for `None` in C. + #[track_caller] + pub fn from_usize(buffers: usize) -> Self { + Buffers::from_u64(buffers.try_into().unwrap()) + } +} + impl_common_ops_for_newtype_uint!(Buffers, u64); impl_signed_div_mul!(Buffers, u64); impl_format_value_traits!(Buffers, Buffers, Buffers, u64); @@ -49,6 +75,32 @@ impl Bytes { pub const MAX: Self = Self(u64::MAX - 1); } +impl Bytes { + // rustdoc-stripper-ignore-next + /// Builds a new `Bytes` formatted value with the provided bytes count. + /// + /// # Panics + /// + /// Panics if the provided count equals `u64::MAX`, + /// which is reserved for `None` in C. + #[track_caller] + pub fn from_u64(bytes: u64) -> Self { + Bytes::try_from(bytes).expect("`Bytes` value out of range") + } + + // rustdoc-stripper-ignore-next + /// Builds a new `Bytes` formatted value with the provided bytes count. + /// + /// # Panics + /// + /// Panics if the provided count equals `u64::MAX`, + /// which is reserved for `None` in C. + #[track_caller] + pub fn from_usize(bytes: usize) -> Self { + Bytes::from_u64(bytes.try_into().unwrap()) + } +} + impl_common_ops_for_newtype_uint!(Bytes, u64); impl_signed_div_mul!(Bytes, u64); impl_format_value_traits!(Bytes, Bytes, Bytes, u64); @@ -61,6 +113,32 @@ impl Default { pub const MAX: Self = Self(u64::MAX - 1); } +impl Default { + // rustdoc-stripper-ignore-next + /// Builds a new `Default` formatted value with the provided quantity. + /// + /// # Panics + /// + /// Panics if the provided quantity equals `u64::MAX`, + /// which is reserved for `None` in C. + #[track_caller] + pub fn from_u64(quantity: u64) -> Self { + Default::try_from(quantity).expect("`Default` value out of range") + } + + // rustdoc-stripper-ignore-next + /// Builds a new `Default` formatted value with the provided quantity. + /// + /// # Panics + /// + /// Panics if the provided quantity equals `u64::MAX`, + /// which is reserved for `None` in C. + #[track_caller] + pub fn from_usize(quantity: usize) -> Self { + Default::from_u64(quantity.try_into().unwrap()) + } +} + impl_common_ops_for_newtype_uint!(Default, u64); impl_signed_div_mul!(Default, u64); impl_format_value_traits!(Default, Default, Default, u64); @@ -76,13 +154,42 @@ impl Percent { pub const MAX: Self = Self(ffi::GST_FORMAT_PERCENT_MAX as u32); #[doc(alias = "GST_FORMAT_PERCENT_SCALE")] pub const SCALE: Self = Self(ffi::GST_FORMAT_PERCENT_SCALE as u32); + + // rustdoc-stripper-ignore-next + /// Builds a new `Percent` with the provided percent value. + /// + /// # Panics + /// + /// Panics if the provided value is larger than 100. + #[track_caller] + pub fn from_percent(percent: u32) -> Self { + Percent::try_from(*Self::SCALE * percent).expect("`Percent` value out of range") + } + + // rustdoc-stripper-ignore-next + /// Builds a new `Percent` with the provided parts per million value. + /// + /// # Panics + /// + /// Panics if the provided value is larger than [`Self::MAX`]. + #[track_caller] + pub fn from_ppm(ppm: u32) -> Self { + Percent::try_from(ppm).expect("`Percent` value out of range") + } + + // rustdoc-stripper-ignore-next + /// Builds a new `Percent` with the provided ratio. + /// + /// # Panics + /// + /// Panics if the provided radio is out of the range [0.0, 1.0]. + #[track_caller] + pub fn from_ratio(ratio: f32) -> Self { + Percent::try_from(ratio).expect("`Percent` ratio out of range") + } } -impl_common_ops_for_newtype_uint!( - Percent, - u32, - one: ffi::GST_FORMAT_PERCENT_SCALE as u32 / 100, -); +impl_common_ops_for_newtype_uint!(Percent, u32, one: ffi::GST_FORMAT_PERCENT_SCALE as u32); impl_signed_div_mul!(Percent, u32); impl FormattedValue for Option { @@ -217,7 +324,7 @@ impl TryFrom for Percent { Err(TryPercentFromFloatError(())) } else { Ok(Percent( - (v * ffi::GST_FORMAT_PERCENT_SCALE as f64).round() as u32 + (v * ffi::GST_FORMAT_PERCENT_MAX as f64).round() as u32 )) } } @@ -232,7 +339,7 @@ impl TryFrom for Percent { Err(TryPercentFromFloatError(())) } else { Ok(Percent( - (v * ffi::GST_FORMAT_PERCENT_SCALE as f32).round() as u32 + (v * ffi::GST_FORMAT_PERCENT_MAX as f32).round() as u32 )) } } @@ -240,7 +347,7 @@ impl TryFrom for Percent { impl std::fmt::Display for Percent { fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { - std::fmt::Display::fmt(&(self.0 as f32 / (*Percent::ONE) as f32), f)?; + std::fmt::Display::fmt(&(self.0 as f32 / (*Percent::SCALE) as f32), f)?; f.write_str(" %") } } diff --git a/gstreamer/src/format/undefined.rs b/gstreamer/src/format/undefined.rs index 41f9f8a2c..1084c7c36 100644 --- a/gstreamer/src/format/undefined.rs +++ b/gstreamer/src/format/undefined.rs @@ -8,7 +8,7 @@ use super::{FormattedValueError, GenericFormattedValue, Signed}; use crate::Format; #[derive(PartialEq, Eq, PartialOrd, Ord, Hash, Clone, Copy, Debug, Default)] -pub struct Undefined(pub i64); +pub struct Undefined(i64); impl Undefined { pub const ONE: Undefined = Undefined(1);