From 22e1fe3aa4f9bead5c75b35e4b26cd2e3ff53cdf Mon Sep 17 00:00:00 2001 From: Andreas Molzer Date: Sun, 2 Feb 2020 21:41:38 +0100 Subject: [PATCH 1/4] Encapsulate the delay ratio Two major reasons: * This avoids the public dependency on `num_ratio`. * The internal representation can be changed. This already proved necessary once (from u16 rational) and in this manner we are free to do it again by adding an inner enum representation. Note that we already have rounding semantics for the meaning of Delay as gif rounds to the nearest 10ms when writing frames. Any more or less precise representation can be made to round to the closest u32 ratio. --- src/animation.rs | 47 ++++++++++++++++++++++++++++++++++++++++------- src/gif.rs | 4 ++-- src/lib.rs | 2 +- 3 files changed, 43 insertions(+), 10 deletions(-) diff --git a/src/animation.rs b/src/animation.rs index a4629b1d1e..ccff7c8fb1 100644 --- a/src/animation.rs +++ b/src/animation.rs @@ -37,7 +37,7 @@ impl<'a> Iterator for Frames<'a> { #[derive(Clone)] pub struct Frame { /// Delay between the frames in milliseconds - delay_ms: Ratio, + delay: Delay, /// x offset left: u32, /// y offset @@ -45,11 +45,17 @@ pub struct Frame { buffer: RgbaImage, } +/// The delay of a frame relative to the previous one. +#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd)] +pub struct Delay { + ratio: Ratio, +} + impl Frame { - /// Contructs a new frame + /// Contructs a new frame without any delay. pub fn new(buffer: RgbaImage) -> Frame { Frame { - delay_ms: Ratio::from_integer(0), + delay: Delay::from_ratio(Ratio::from_integer(0)), left: 0, top: 0, buffer, @@ -57,9 +63,9 @@ impl Frame { } /// Contructs a new frame - pub fn from_parts(buffer: RgbaImage, left: u32, top: u32, delay_ms: Ratio) -> Frame { + pub fn from_parts(buffer: RgbaImage, left: u32, top: u32, delay: Delay) -> Frame { Frame { - delay_ms, + delay, left, top, buffer, @@ -67,8 +73,8 @@ impl Frame { } /// Delay of this frame - pub fn delay_ms(&self) -> Ratio { - self.delay_ms + pub fn delay(&self) -> Delay { + self.delay } /// Returns the image buffer @@ -91,3 +97,30 @@ impl Frame { self.top } } + +impl Delay { + /// Create a delay from a ratio of milliseconds. + /// + /// # Examples + /// + /// ``` + /// use image::Delay; + /// let delay_10ms = Delay::from_num_denom_ms(10, 1); + /// ``` + pub fn from_num_denom_ms(numerator: u32, denominator: u32) -> Self { + Delay { ratio: Ratio::new_raw(numerator, denominator) } + } + + /// The numerator and denominator of the delay in milliseconds. + pub fn num_denom_ms(self) -> (u32, u32) { + (*self.ratio.numer(), *self.ratio.denom()) + } + + pub(crate) fn from_ratio(ratio: Ratio) -> Self { + Delay { ratio } + } + + pub(crate) fn into_ratio(self) -> Ratio { + self.ratio + } +} diff --git a/src/gif.rs b/src/gif.rs index b650a81b84..5b4f21704b 100644 --- a/src/gif.rs +++ b/src/gif.rs @@ -244,7 +244,7 @@ impl Iterator for GifFrameIterator { } let frame = animation::Frame::from_parts( - image_buffer.clone(), 0, 0, delay + image_buffer.clone(), 0, 0, animation::Delay::from_ratio(delay), ); match dispose { @@ -375,7 +375,7 @@ impl Encoder { -> ImageResult> { // get the delay before converting img_frame - let frame_delay = img_frame.delay_ms().to_integer(); + let frame_delay = img_frame.delay().into_ratio().to_integer(); // convert img_frame into RgbaImage let mut rbga_frame = img_frame.into_buffer(); let (width, height) = self.gif_dimensions( diff --git a/src/lib.rs b/src/lib.rs index 9b11d8c5a7..4f232227ab 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -68,7 +68,7 @@ pub use crate::dynimage::{load_from_memory, load_from_memory_with_format, open, pub use crate::dynimage::DynamicImage; -pub use crate::animation::{Frame, Frames}; +pub use crate::animation::{Delay, Frame, Frames}; // More detailed error type pub mod error; From 68ad0cc24730625be35e1c70a6b6e7da249b9a7f Mon Sep 17 00:00:00 2001 From: Andreas Molzer Date: Wed, 5 Feb 2020 21:02:29 +0100 Subject: [PATCH 2/4] Add conversion of Delay into time::Duration With the original available as a millisecond ratio this can easily be done to the accuracy of nanoseconds, which is the finest available. --- src/animation.rs | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/src/animation.rs b/src/animation.rs index ccff7c8fb1..8a7239005e 100644 --- a/src/animation.rs +++ b/src/animation.rs @@ -1,4 +1,5 @@ use std::iter::Iterator; +use std::time::Duration; use num_rational::Ratio; @@ -124,3 +125,33 @@ impl Delay { self.ratio } } + +impl From for Duration { + fn from(delay: Delay) -> Self { + let ratio = delay.into_ratio(); + let ms = ratio.to_integer(); + let rest = ratio.numer() % ratio.denom(); + let nanos = (u64::from(rest) * 1_000_000) / u64::from(*ratio.denom()); + Duration::from_millis(ms.into()) + Duration::from_nanos(nanos) + } +} + +#[cfg(test)] +mod tests { + use super::{Delay, Duration, Ratio}; + + #[test] + fn simple() { + let second = Delay::from_num_denom_ms(1000, 1); + assert_eq!(Duration::from(second), Duration::from_secs(1)); + } + + #[test] + fn fps_30() { + let thirtieth = Delay::from_num_denom_ms(1000, 30); + let duration = Duration::from(thirtieth); + assert_eq!(duration.as_secs(), 0); + assert_eq!(duration.subsec_millis(), 33); + assert_eq!(duration.subsec_nanos(), 33_333_333); + } +} From 633f24771415fce014eb363d4eb1ae3e18fb2264 Mon Sep 17 00:00:00 2001 From: Andreas Molzer Date: Wed, 5 Feb 2020 21:44:39 +0100 Subject: [PATCH 3/4] Align naming to ratio --- src/animation.rs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/animation.rs b/src/animation.rs index 8a7239005e..7de3a93249 100644 --- a/src/animation.rs +++ b/src/animation.rs @@ -106,14 +106,17 @@ impl Delay { /// /// ``` /// use image::Delay; - /// let delay_10ms = Delay::from_num_denom_ms(10, 1); + /// let delay_10ms = Delay::from_numer_denom_ms(10, 1); /// ``` - pub fn from_num_denom_ms(numerator: u32, denominator: u32) -> Self { + pub fn from_numer_denom_ms(numerator: u32, denominator: u32) -> Self { Delay { ratio: Ratio::new_raw(numerator, denominator) } } /// The numerator and denominator of the delay in milliseconds. - pub fn num_denom_ms(self) -> (u32, u32) { + /// + /// This is guaranteed to be an exact conversion if the `Delay` was previously created with the + /// `from_numer_denom_ms` constructor. + pub fn numer_denom_ms(self) -> (u32, u32) { (*self.ratio.numer(), *self.ratio.denom()) } @@ -142,13 +145,13 @@ mod tests { #[test] fn simple() { - let second = Delay::from_num_denom_ms(1000, 1); + let second = Delay::from_numer_denom_ms(1000, 1); assert_eq!(Duration::from(second), Duration::from_secs(1)); } #[test] fn fps_30() { - let thirtieth = Delay::from_num_denom_ms(1000, 30); + let thirtieth = Delay::from_numer_denom_ms(1000, 30); let duration = Duration::from(thirtieth); assert_eq!(duration.as_secs(), 0); assert_eq!(duration.subsec_millis(), 33); From b6dacabb8cb2f78e05ba06e96ef0ce36ca334e83 Mon Sep 17 00:00:00 2001 From: Andreas Molzer Date: Wed, 5 Feb 2020 21:47:56 +0100 Subject: [PATCH 4/4] Add a saturating conversion from time::Duration Given the prior usage, it does not make much sense to provide an exact conversion. However, it should be clear that precision is not finite and the representation is not exactly that of a Duration. The implementation turned out to be fairly involved as the num-fraction crate does not provide approximation algorithms for fractions. No fear, we code them ourselves with extensive testing to cover much more than the required accuracy tests. --- src/animation.rs | 170 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 170 insertions(+) diff --git a/src/animation.rs b/src/animation.rs index 7de3a93249..a10c0f611f 100644 --- a/src/animation.rs +++ b/src/animation.rs @@ -112,6 +112,45 @@ impl Delay { Delay { ratio: Ratio::new_raw(numerator, denominator) } } + /// Convert from a duration, clamped between 0 and an implemented defined maximum. + /// + /// The maximum is *at least* `i32::MAX` milliseconds. It should be noted that the accuracy of + /// the result may be relative and very large delays have a coarse resolution. + /// + /// # Examples + /// + /// ``` + /// use std::time::Duration; + /// use image::Delay; + /// + /// let duration = Duration::from_millis(20); + /// let delay = Delay::from_saturating_duration(duration); + /// ``` + pub fn from_saturating_duration(duration: Duration) -> Self { + // A few notes: The largest number we can represent as a ratio is u32::MAX but we can + // sometimes represent much smaller numbers. + // + // We can represent duration as `millis+a/b` (where a < b, b > 0). + // We must thus bound b with `b·millis + (b-1) <= u32::MAX` or + // > `0 < b <= (u32::MAX + 1)/(millis + 1)` + // Corollary: millis <= u32::MAX + + const MILLIS_BOUND: u128 = u32::max_value() as u128; + + let millis = duration.as_millis().min(MILLIS_BOUND); + let submillis = (duration.as_nanos() % 1_000_000) as u32; + + let max_b = if millis > 0 { + ((MILLIS_BOUND + 1)/(millis + 1)) as u32 + } else { + MILLIS_BOUND as u32 + }; + let millis = millis as u32; + + let (a, b) = Self::closest_bounded_fraction(max_b, submillis, 1_000_000); + Self::from_numer_denom_ms(a + b*millis, b) + } + /// The numerator and denominator of the delay in milliseconds. /// /// This is guaranteed to be an exact conversion if the `Delay` was previously created with the @@ -127,6 +166,93 @@ impl Delay { pub(crate) fn into_ratio(self) -> Ratio { self.ratio } + + /// Given some fraction, compute an approximation with denominator bounded. + /// + /// Note that `denom_bound` bounds nominator and denominator of all intermediate + /// approximations and the end result. + fn closest_bounded_fraction(denom_bound: u32, nom: u32, denom: u32) -> (u32, u32) { + use std::cmp::Ordering::{self, *}; + assert!(0 < denom); + assert!(0 < denom_bound); + assert!(nom < denom); + + // Avoid a few type troubles. All intermediate results are bounded by `denom_bound` which + // is in turn bounded by u32::MAX. Representing with u64 allows multiplication of any two + // values without fears of overflow. + + // Compare two fractions whose parts fit into a u32. + fn compare_fraction((an, ad): (u64, u64), (bn, bd): (u64, u64)) -> Ordering { + (an*bd).cmp(&(bn*ad)) + } + + // Computes the nominator of the absolute difference between two such fractions. + fn abs_diff_nom((an, ad): (u64, u64), (bn, bd): (u64, u64)) -> u64 { + let c0 = an*bd; + let c1 = ad*bn; + + let d0 = c0.max(c1); + let d1 = c0.min(c1); + d0 - d1 + } + + let exact = (u64::from(nom), u64::from(denom)); + // The lower bound fraction, numerator and denominator. + let mut lower = (0u64, 1u64); + // The upper bound fraction, numerator and denominator. + let mut upper = (1u64, 1u64); + // The closest approximation for now. + let mut guess = (u64::from(nom*2 > denom), 1u64); + + // loop invariant: ad, bd <= denom_bound + // iterates the Farey sequence. + loop { + // Break if we are done. + if compare_fraction(guess, exact) == Equal { + break; + } + + // Break if next Farey number is out-of-range. + if u64::from(denom_bound) - lower.1 < upper.1 { + break; + } + + // Next Farey approximation n between a and b + let next = (lower.0 + upper.0, lower.1 + upper.1); + // if F < n then replace the upper bound, else replace lower. + if compare_fraction(exact, next) == Less { + upper = next; + } else { + lower = next; + } + + // Now correct the closest guess. + // In other words, if |c - f| > |n - f| then replace it with the new guess. + // This favors the guess with smaller denominator on equality. + + // |g - f| = |g_diff_nom|/(gd*fd); + let g_diff_nom = abs_diff_nom(guess, exact); + // |n - f| = |n_diff_nom|/(nd*fd); + let n_diff_nom = abs_diff_nom(next, exact); + + // The difference |n - f| is smaller than |g - f| if either the integral part of the + // fraction |n_diff_nom|/nd is smaller than the one of |g_diff_nom|/gd or if they are + // the same but the fractional part is larger. + if match (n_diff_nom/next.1).cmp(&(g_diff_nom/guess.1)) { + Less => true, + Greater => false, + // Note that the nominator for the fractional part is smaller than its denominator + // which is smaller than u32 and can't overflow the multiplication with the other + // denominator, that is we can compare these fractions by multiplication with the + // respective other denominator. + Equal => compare_fraction((n_diff_nom%next.1, next.1), (g_diff_nom%guess.1, guess.1)) == Less, + } { + guess = next; + } + } + + (guess.0 as u32, guess.1 as u32) + } } impl From for Duration { @@ -157,4 +283,48 @@ mod tests { assert_eq!(duration.subsec_millis(), 33); assert_eq!(duration.subsec_nanos(), 33_333_333); } + + #[test] + fn duration_outlier() { + let oob = Duration::from_secs(0xFFFF_FFFF); + let delay = Delay::from_saturating_duration(oob); + assert_eq!(delay.numer_denom_ms(), (0xFFFF_FFFF, 1)); + } + + #[test] + fn duration_approx() { + let oob = Duration::from_millis(0xFFFF_FFFF) + Duration::from_micros(1); + let delay = Delay::from_saturating_duration(oob); + assert_eq!(delay.numer_denom_ms(), (0xFFFF_FFFF, 1)); + + let inbounds = Duration::from_millis(0xFFFF_FFFF) - Duration::from_micros(1); + let delay = Delay::from_saturating_duration(inbounds); + assert_eq!(delay.numer_denom_ms(), (0xFFFF_FFFF, 1)); + + let fine = Duration::from_millis(0xFFFF_FFFF/1000) + Duration::from_micros(0xFFFF_FFFF%1000); + let delay = Delay::from_saturating_duration(fine); + // Funnily, 0xFFFF_FFFF is divisble by 5, thus we compare with a `Ratio`. + assert_eq!(delay.into_ratio(), Ratio::new(0xFFFF_FFFF, 1000)); + } + + #[test] + fn precise() { + // The ratio has only 32 bits in the numerator, too imprecise to get more than 11 digits + // correct. But it may be expressed as 1_000_000/3 instead. + let exceed = Duration::from_secs(333) + Duration::from_nanos(333_333_333); + let delay = Delay::from_saturating_duration(exceed); + assert_eq!(Duration::from(delay), exceed); + } + + + #[test] + fn small() { + // Not quite a delay of `1 ms`. + let delay = Delay::from_numer_denom_ms(1 << 16, (1 << 16) + 1); + let duration = Duration::from(delay); + assert_eq!(duration.as_millis(), 0); + // Not precisely the original but should be smaller than 0. + let delay = Delay::from_saturating_duration(duration); + assert_eq!(delay.into_ratio().to_integer(), 0); + } }