gst/Signed: use a new trait for into_{signed,positive,negative}

The functions `into_{signed,positive,negative}` used to be implemented
on the `FormattedValue` trait for convenience. This was wrong for the
following reasons:

- They aren't specific to `FormattedValue`s: they can also be
  implemented for regular unsigned integers such as `u64`, `usize` or
  `u32`.
- They were implemented for `format::Undefined` and all variants of
  `GenericFormattedValue`, some of which are already signed.

This commit introduces the new trait `UnsignedIntoSigned`, which makes
it possible to fix both of the above problems.

Users can build a `Signed` from an `Undefined`, an `i64`, `isize` or
`i32` thanks to the `From` trait implementations.

gstreamer/src/clock_time.rs

@@ -435,8 +435,7 @@ impl std::iter::Sum for ClockTime {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::format::FormattedValue;
+    use crate::{Signed, UnsignedIntoSigned};
-    use crate::Signed;
 
     const CT_1: ClockTime = ClockTime::from_nseconds(1);
     const CT_2: ClockTime = ClockTime::from_nseconds(2);

gstreamer/src/format.rs

@@ -210,6 +210,79 @@ where
     }
 }
 
+// rustdoc-stripper-ignore-next
+/// A trait implemented on unsigned types which can be converted into [`crate::Signed`]s.
+pub trait UnsignedIntoSigned: Copy + Sized {
+    type Signed;
+
+    // rustdoc-stripper-ignore-next
+    /// Converts `self` into a `Signed` matching the given `sign`.
+    fn into_signed(self, sign: i32) -> Self::Signed {
+        if sign.is_positive() {
+            self.into_positive()
+        } else {
+            self.into_negative()
+        }
+    }
+
+    // rustdoc-stripper-ignore-next
+    /// Converts `self` into a `Signed::Positive`.
+    fn into_positive(self) -> Self::Signed;
+
+    // rustdoc-stripper-ignore-next
+    /// Converts `self` into a `Signed::Negative`.
+    fn into_negative(self) -> Self::Signed;
+}
+
+impl_unsigned_int_into_signed!(u64);
+impl_signed_ops!(u64, 0);
+
+impl_unsigned_int_into_signed!(usize);
+impl_signed_ops!(usize, 0);
+
+impl_unsigned_int_into_signed!(u32);
+impl_signed_ops!(u32, 0);
+
+impl From<i64> for Signed<u64> {
+    fn from(val: i64) -> Signed<u64> {
+        skip_assert_initialized!();
+        match val {
+            positive if positive.is_positive() => Signed::Positive(positive as u64),
+            i64::MIN => {
+                // `i64::MIN.abs()` can't be represented as an `i64`
+                Signed::Negative((-(i64::MIN as i128)) as u64)
+            }
+            negative => Signed::Negative((-negative) as u64),
+        }
+    }
+}
+
+impl From<isize> for Signed<usize> {
+    fn from(val: isize) -> Signed<usize> {
+        skip_assert_initialized!();
+        match val {
+            positive if positive.is_positive() => Signed::Positive(positive as usize),
+            isize::MIN => {
+                // `isize::MIN.abs()` can't be represented as an `isize`
+                Signed::Negative((-(isize::MIN as i128)) as usize)
+            }
+            negative => Signed::Negative((-negative) as usize),
+        }
+    }
+}
+
+// `i32::MIN.abs()` can't be represented as an `i32`
+impl From<i32> for Signed<u32> {
+    fn from(val: i32) -> Signed<u32> {
+        skip_assert_initialized!();
+        if val.is_positive() {
+            Signed::Positive(val as u32)
+        } else {
+            Signed::Negative((-(val as i64)) as u32)
+        }
+    }
+}
+
 impl Signed<ClockTime> {
     // rustdoc-stripper-ignore-next
     /// Returns the `self` in nanoseconds.
@@ -288,8 +361,6 @@ impl Signed<ClockTime> {
     }
 }
 
-impl_signed_ops!(u64, 0);
-
 #[derive(PartialEq, Eq, Hash, Clone, Copy, Debug)]
 #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
 pub enum GenericFormattedValue {
@@ -359,10 +430,6 @@ pub trait FormattedValue: Copy + Clone + Sized + Into<GenericFormattedValue> + '
     /// Type which allows building a `FormattedValue` of this format from any raw value.
     type FullRange: FormattedValueFullRange + From<Self>;
 
-    // rustdoc-stripper-ignore-next
-    /// The `Signed` type for this `FormattedValue`.
-    type Signed;
-
     #[doc(alias = "get_default_format")]
     fn default_format() -> Format;
 
@@ -370,17 +437,6 @@ pub trait FormattedValue: Copy + Clone + Sized + Into<GenericFormattedValue> + '
     fn format(&self) -> Format;
 
     unsafe fn into_raw_value(self) -> i64;
-
-    fn into_positive(self) -> Self::Signed;
-    fn into_negative(self) -> Self::Signed;
-
-    fn into_signed(self, sign: i32) -> Self::Signed {
-        if sign < 0 {
-            self.into_negative()
-        } else {
-            self.into_positive()
-        }
-    }
 }
 
 // rustdoc-stripper-ignore-next
@@ -614,12 +670,10 @@ impl CompatibleFormattedValue<GenericFormattedValue> for GenericFormattedValue {
 }
 
 impl FormattedValue for GenericFormattedValue {
-    type FullRange = GenericFormattedValue;
-
     // The intrinsic value for `GenericFormattedValue` is also
     // `GenericFormattedValue`. We can't dissociate the `Option`
     // from the variants' inner type since they are not all `Option`s.
-    type Signed = Signed<GenericFormattedValue>;
+    type FullRange = GenericFormattedValue;
 
     fn default_format() -> Format {
         Format::Undefined
@@ -632,14 +686,6 @@ impl FormattedValue for GenericFormattedValue {
     unsafe fn into_raw_value(self) -> i64 {
         self.value()
     }
-
-    fn into_positive(self) -> Signed<GenericFormattedValue> {
-        Signed::Positive(self)
-    }
-
-    fn into_negative(self) -> Signed<GenericFormattedValue> {
-        Signed::Negative(self)
-    }
 }
 
 impl FormattedValueFullRange for GenericFormattedValue {
@@ -719,6 +765,36 @@ impl GenericFormattedValue {
 
 impl FormattedValueIntrinsic for GenericFormattedValue {}
 
+impl UnsignedIntoSigned for GenericFormattedValue {
+    type Signed = Signed<GenericFormattedValue>;
+
+    #[track_caller]
+    fn into_positive(self) -> Signed<GenericFormattedValue> {
+        match self {
+            GenericFormattedValue::Undefined(_) => {
+                unimplemented!("`GenericFormattedValue::Undefined` is already signed")
+            }
+            GenericFormattedValue::Other(..) => {
+                unimplemented!("`GenericFormattedValue::Other` is already signed")
+            }
+            unsigned_inner => Signed::Positive(unsigned_inner),
+        }
+    }
+
+    #[track_caller]
+    fn into_negative(self) -> Signed<GenericFormattedValue> {
+        match self {
+            GenericFormattedValue::Undefined(_) => {
+                unimplemented!("`GenericFormattedValue::Undefined` is already signed")
+            }
+            GenericFormattedValue::Other(..) => {
+                unimplemented!("`GenericFormattedValue::Other` is already signed")
+            }
+            unsigned_inner => Signed::Negative(unsigned_inner),
+        }
+    }
+}
+
 impl_common_ops_for_newtype_uint!(Default, u64);
 impl_format_value_traits!(Default, Default, Default, u64);
 option_glib_newtype_from_to!(Default, u64::MAX);
@@ -748,7 +824,6 @@ glib_newtype_display!(
 
 impl FormattedValue for Undefined {
     type FullRange = Undefined;
-    type Signed = Signed<Undefined>;
 
     fn default_format() -> Format {
         Format::Undefined
@@ -761,14 +836,6 @@ impl FormattedValue for Undefined {
     unsafe fn into_raw_value(self) -> i64 {
         self.0
     }
-
-    fn into_positive(self) -> Signed<Undefined> {
-        Signed::Positive(self)
-    }
-
-    fn into_negative(self) -> Signed<Undefined> {
-        Signed::Negative(self)
-    }
 }
 
 impl FormattedValueFullRange for Undefined {
@@ -842,6 +909,13 @@ impl AsMut<i64> for Undefined {
     }
 }
 
+impl From<Undefined> for Signed<u64> {
+    fn from(val: Undefined) -> Signed<u64> {
+        skip_assert_initialized!();
+        val.0.into()
+    }
+}
+
 glib_newtype_display!(Undefined, DisplayableUndefined, "(Undefined)");
 
 impl_common_ops_for_newtype_uint!(Percent, u32);
@@ -849,7 +923,6 @@ glib_newtype_display!(Percent, DisplayablePercent, DisplayableOptionPercent, "%"
 
 impl FormattedValue for Option<Percent> {
     type FullRange = Option<Percent>;
-    type Signed = Option<Signed<Percent>>;
 
     fn default_format() -> Format {
         Format::Percent
@@ -862,14 +935,6 @@ impl FormattedValue for Option<Percent> {
     unsafe fn into_raw_value(self) -> i64 {
         self.map_or(-1, |v| v.0 as i64)
     }
-
-    fn into_positive(self) -> Option<Signed<Percent>> {
-        Some(Signed::Positive(self?))
-    }
-
-    fn into_negative(self) -> Option<Signed<Percent>> {
-        Some(Signed::Negative(self?))
-    }
 }
 
 impl FormattedValueFullRange for Option<Percent> {
@@ -895,7 +960,6 @@ impl From<Percent> for GenericFormattedValue {
 
 impl FormattedValue for Percent {
     type FullRange = Option<Percent>;
-    type Signed = Signed<Percent>;
 
     fn default_format() -> Format {
         Format::Percent
@@ -908,14 +972,6 @@ impl FormattedValue for Percent {
     unsafe fn into_raw_value(self) -> i64 {
         self.0 as i64
     }
-
-    fn into_positive(self) -> Signed<Percent> {
-        Signed::Positive(self)
-    }
-
-    fn into_negative(self) -> Signed<Percent> {
-        Signed::Negative(self)
-    }
 }
 
 impl TryFrom<u64> for Percent {
@@ -1177,19 +1233,19 @@ mod tests {
         assert!(!signed.is_positive());
         assert!(signed.positive().is_none());
 
-        let und = Undefined(1);
+        let def = Default(1);
 
-        let signed = und.into_positive();
+        let signed = def.into_positive();
-        assert_eq!(signed, Signed::Positive(und));
+        assert_eq!(signed, Signed::Positive(def));
         assert!(signed.is_positive());
-        assert_eq!(signed.positive(), Some(und));
+        assert_eq!(signed.positive(), Some(def));
         assert!(!signed.is_negative());
         assert!(signed.negative().is_none());
 
-        let signed = und.into_negative();
+        let signed = def.into_negative();
-        assert_eq!(signed, Signed::Negative(und));
+        assert_eq!(signed, Signed::Negative(def));
         assert!(signed.is_negative());
-        assert_eq!(signed.negative(), Some(und));
+        assert_eq!(signed.negative(), Some(def));
         assert!(!signed.is_positive());
         assert!(signed.positive().is_none());
     }

gstreamer/src/lib.rs

@@ -228,7 +228,7 @@ pub mod format;
 pub use crate::format::{
     CompatibleFormattedValue, FormattedValue, FormattedValueFullRange, FormattedValueIntrinsic,
     GenericFormattedValue, Signed, SpecificFormattedValue, SpecificFormattedValueFullRange,
-    SpecificFormattedValueIntrinsic,
+    SpecificFormattedValueIntrinsic, UnsignedIntoSigned,
 };
 #[cfg(feature = "serde")]
 mod format_serde;
@@ -354,6 +354,7 @@ pub mod prelude {
     pub use crate::format::{
         CompatibleFormattedValue, FormattedValue, FormattedValueFullRange, FormattedValueIntrinsic,
         SpecificFormattedValue, SpecificFormattedValueFullRange, SpecificFormattedValueIntrinsic,
+        UnsignedIntoSigned,
     };
     pub use crate::utils::Displayable;
 

gstreamer/src/macros.rs

@@ -209,12 +209,43 @@ macro_rules! impl_non_trait_op_inner_type(
     };
 );
 
+macro_rules! impl_unsigned_int_into_signed(
+    ($name:ident) => {
+        impl crate::UnsignedIntoSigned for $name {
+            type Signed = crate::Signed<$name>;
+
+            fn into_positive(self) -> Self::Signed {
+                crate::Signed::Positive(self)
+            }
+
+            fn into_negative(self) -> Self::Signed {
+                crate::Signed::Negative(self)
+            }
+        }
+
+        impl crate::UnsignedIntoSigned for Option<$name> {
+            type Signed = Option<crate::Signed<$name>>;
+
+            fn into_positive(self) -> Self::Signed {
+                Some(self?.into_positive())
+            }
+
+            fn into_negative(self) -> Self::Signed {
+                Some(self?.into_negative())
+            }
+        }
+    };
+);
+
 macro_rules! impl_common_ops_for_newtype_uint(
     ($name:ident, $inner_type:ty) => {
         impl $name {
             pub const ZERO: Self = Self(0);
             pub const NONE: Option<Self> = None;
 
+            pub const MAX_SIGNED: crate::Signed::<$name> = crate::Signed::Positive(Self::MAX);
+            pub const MIN_SIGNED: crate::Signed::<$name> = crate::Signed::Negative(Self::MAX);
+
             pub const fn is_zero(self) -> bool {
                 self.0 == Self::ZERO.0
             }
@@ -234,6 +265,7 @@ macro_rules! impl_common_ops_for_newtype_uint(
 
         impl_non_trait_op_inner_type!($name, $inner_type);
 
+        impl_unsigned_int_into_signed!($name);
         impl_signed_ops!($name, $name::ZERO);
 
         impl<ND: Borrow<$inner_type>> MulDiv<ND> for $name {
@@ -551,13 +583,6 @@ macro_rules! impl_signed_ops(
             }
         }
 
-        impl From<$type> for crate::Signed<$type> {
-            fn from(val: $type) -> Self {
-                skip_assert_initialized!();
-                crate::Signed::Positive(val)
-            }
-        }
-
         impl PartialOrd<crate::Signed<$type>> for crate::Signed<$type> {
             fn partial_cmp(&self, other: &crate::Signed<$type>) -> Option<std::cmp::Ordering> {
                 Some(self.cmp(other))
@@ -594,7 +619,6 @@ macro_rules! impl_format_value_traits(
     ($name:ident, $format:ident, $format_value:ident, $inner_type:ty) => {
         impl FormattedValue for Option<$name> {
             type FullRange = Option<$name>;
-            type Signed = Option<crate::Signed<$name>>;
 
             fn default_format() -> Format {
                 Format::$format
@@ -607,14 +631,6 @@ macro_rules! impl_format_value_traits(
             unsafe fn into_raw_value(self) -> i64 {
                 IntoGlib::into_glib(self) as i64
             }
-
-            fn into_positive(self) -> Option<crate::Signed<$name>> {
-                Some(crate::Signed::Positive(self?))
-            }
-
-            fn into_negative(self) -> Option<crate::Signed<$name>> {
-                Some(crate::Signed::Negative(self?))
-            }
         }
 
         impl FormattedValueFullRange for Option<$name> {
@@ -639,7 +655,6 @@ macro_rules! impl_format_value_traits(
         }
         impl FormattedValue for $name {
             type FullRange = Option<$name>;
-            type Signed = crate::Signed<$name>;
 
             fn default_format() -> Format {
                 Format::$format
@@ -652,14 +667,6 @@ macro_rules! impl_format_value_traits(
             unsafe fn into_raw_value(self) -> i64 {
                 IntoGlib::into_glib(self) as i64
             }
-
-            fn into_positive(self) -> crate::Signed<$name> {
-                crate::Signed::Positive(self)
-            }
-
-            fn into_negative(self) -> crate::Signed<$name> {
-                crate::Signed::Negative(self)
-            }
         }
 
         impl SpecificFormattedValue for Option<$name> {}

gstreamer/src/segment.rs

@@ -6,6 +6,7 @@ use crate::SeekFlags;
 use crate::SeekType;
 use crate::{
     CompatibleFormattedValue, FormattedValue, FormattedValueFullRange, FormattedValueIntrinsic,
+    UnsignedIntoSigned,
 };
 use glib::translate::*;
 use glib::StaticType;
@@ -195,28 +196,6 @@ impl<T: FormattedValueIntrinsic> FormattedSegment<T> {
         }
     }
 
-    #[doc(alias = "gst_segment_position_from_running_time_full")]
-    pub fn position_from_running_time_full(
-        &self,
-        running_time: impl CompatibleFormattedValue<T>,
-    ) -> <T::FullRange as FormattedValue>::Signed {
-        let running_time = running_time
-            .try_into_checked_explicit(self.format())
-            .unwrap();
-
-        unsafe {
-            let mut position = mem::MaybeUninit::uninit();
-            let sign = ffi::gst_segment_position_from_running_time_full(
-                &self.0,
-                self.format().into_glib(),
-                running_time.into_raw_value() as u64,
-                position.as_mut_ptr(),
-            );
-
-            T::FullRange::from_raw(self.format(), position.assume_init() as i64).into_signed(sign)
-        }
-    }
-
     #[doc(alias = "gst_segment_position_from_stream_time")]
     pub fn position_from_stream_time(
         &self,
@@ -238,28 +217,6 @@ impl<T: FormattedValueIntrinsic> FormattedSegment<T> {
         }
     }
 
-    #[doc(alias = "gst_segment_position_from_stream_time_full")]
-    pub fn position_from_stream_time_full(
-        &self,
-        stream_time: impl CompatibleFormattedValue<T>,
-    ) -> <T::FullRange as FormattedValue>::Signed {
-        let stream_time = stream_time
-            .try_into_checked_explicit(self.format())
-            .unwrap();
-
-        unsafe {
-            let mut position = mem::MaybeUninit::uninit();
-            let sign = ffi::gst_segment_position_from_stream_time_full(
-                &self.0,
-                self.format().into_glib(),
-                stream_time.into_raw_value() as u64,
-                position.as_mut_ptr(),
-            );
-
-            T::FullRange::from_raw(self.format(), position.assume_init() as i64).into_signed(sign)
-        }
-    }
-
     #[doc(alias = "gst_segment_set_running_time")]
     pub fn set_running_time(
         &mut self,
@@ -297,27 +254,6 @@ impl<T: FormattedValueIntrinsic> FormattedSegment<T> {
         }
     }
 
-    #[doc(alias = "gst_segment_to_running_time_full")]
-    pub fn to_running_time_full(
-        &self,
-        position: impl CompatibleFormattedValue<T>,
-    ) -> <T::FullRange as FormattedValue>::Signed {
-        let position = position.try_into_checked_explicit(self.format()).unwrap();
-
-        unsafe {
-            let mut running_time = mem::MaybeUninit::uninit();
-            let sign = ffi::gst_segment_to_running_time_full(
-                &self.0,
-                self.format().into_glib(),
-                position.into_raw_value() as u64,
-                running_time.as_mut_ptr(),
-            );
-
-            T::FullRange::from_raw(self.format(), running_time.assume_init() as i64)
-                .into_signed(sign)
-        }
-    }
-
     #[doc(alias = "gst_segment_to_stream_time")]
     pub fn to_stream_time(&self, position: impl CompatibleFormattedValue<T>) -> T::FullRange {
         let position = position.try_into_checked_explicit(self.format()).unwrap();
@@ -334,27 +270,6 @@ impl<T: FormattedValueIntrinsic> FormattedSegment<T> {
         }
     }
 
-    #[doc(alias = "gst_segment_to_stream_time_full")]
-    pub fn to_stream_time_full(
-        &self,
-        position: impl CompatibleFormattedValue<T>,
-    ) -> <T::FullRange as FormattedValue>::Signed {
-        let position = position.try_into_checked_explicit(self.format()).unwrap();
-
-        unsafe {
-            let mut stream_time = mem::MaybeUninit::uninit();
-            let sign = ffi::gst_segment_to_stream_time_full(
-                &self.0,
-                self.format().into_glib(),
-                position.into_raw_value() as u64,
-                stream_time.as_mut_ptr(),
-            );
-
-            T::FullRange::from_raw(self.format(), stream_time.assume_init() as i64)
-                .into_signed(sign)
-        }
-    }
-
     #[doc(alias = "get_flags")]
     pub fn flags(&self) -> crate::SegmentFlags {
         unsafe { from_glib(self.0.flags) }
@@ -470,6 +385,98 @@ impl<T: FormattedValueIntrinsic> PartialEq for FormattedSegment<T> {
     }
 }
 
+impl<T> FormattedSegment<T>
+where
+    T: FormattedValueIntrinsic,
+    <T as FormattedValue>::FullRange: UnsignedIntoSigned,
+{
+    #[doc(alias = "gst_segment_position_from_running_time_full")]
+    pub fn position_from_running_time_full(
+        &self,
+        running_time: impl CompatibleFormattedValue<T>,
+    ) -> <T::FullRange as UnsignedIntoSigned>::Signed {
+        let running_time = running_time
+            .try_into_checked_explicit(self.format())
+            .unwrap();
+
+        unsafe {
+            let mut position = mem::MaybeUninit::uninit();
+            let sign = ffi::gst_segment_position_from_running_time_full(
+                &self.0,
+                self.format().into_glib(),
+                running_time.into_raw_value() as u64,
+                position.as_mut_ptr(),
+            );
+
+            T::FullRange::from_raw(self.format(), position.assume_init() as i64).into_signed(sign)
+        }
+    }
+
+    #[doc(alias = "gst_segment_position_from_stream_time_full")]
+    pub fn position_from_stream_time_full(
+        &self,
+        stream_time: impl CompatibleFormattedValue<T>,
+    ) -> <T::FullRange as UnsignedIntoSigned>::Signed {
+        let stream_time = stream_time
+            .try_into_checked_explicit(self.format())
+            .unwrap();
+
+        unsafe {
+            let mut position = mem::MaybeUninit::uninit();
+            let sign = ffi::gst_segment_position_from_stream_time_full(
+                &self.0,
+                self.format().into_glib(),
+                stream_time.into_raw_value() as u64,
+                position.as_mut_ptr(),
+            );
+
+            T::FullRange::from_raw(self.format(), position.assume_init() as i64).into_signed(sign)
+        }
+    }
+
+    #[doc(alias = "gst_segment_to_running_time_full")]
+    pub fn to_running_time_full(
+        &self,
+        position: impl CompatibleFormattedValue<T>,
+    ) -> <T::FullRange as UnsignedIntoSigned>::Signed {
+        let position = position.try_into_checked_explicit(self.format()).unwrap();
+
+        unsafe {
+            let mut running_time = mem::MaybeUninit::uninit();
+            let sign = ffi::gst_segment_to_running_time_full(
+                &self.0,
+                self.format().into_glib(),
+                position.into_raw_value() as u64,
+                running_time.as_mut_ptr(),
+            );
+
+            T::FullRange::from_raw(self.format(), running_time.assume_init() as i64)
+                .into_signed(sign)
+        }
+    }
+
+    #[doc(alias = "gst_segment_to_stream_time_full")]
+    pub fn to_stream_time_full(
+        &self,
+        position: impl CompatibleFormattedValue<T>,
+    ) -> <T::FullRange as UnsignedIntoSigned>::Signed {
+        let position = position.try_into_checked_explicit(self.format()).unwrap();
+
+        unsafe {
+            let mut stream_time = mem::MaybeUninit::uninit();
+            let sign = ffi::gst_segment_to_stream_time_full(
+                &self.0,
+                self.format().into_glib(),
+                position.into_raw_value() as u64,
+                stream_time.as_mut_ptr(),
+            );
+
+            T::FullRange::from_raw(self.format(), stream_time.assume_init() as i64)
+                .into_signed(sign)
+        }
+    }
+}
+
 impl<T: FormattedValueIntrinsic> Eq for FormattedSegment<T> {}
 
 unsafe impl<T: FormattedValueIntrinsic> Send for FormattedSegment<T> {}