Refactor and improve Delay

[RunDevelopment]

Changes:

• Ratio is now private to the animation.rs module. Decoders and Co. now only use the public API of Delay.
• Use NonZeroU32 for the denominator. This should allow the compiler to remove a few div-by-zero panic paths and creates a niche.
• Added Delay::ZERO to represent a delay of 0 ms.
• Added Delay::from_millis(u32) -> Self and Delay::as_millis(self) -> u32. These mirror the functions on Duration for an easy-to-use API. They are also very efficient implementation-wise.
• More straightforward implementation of From<Delay> for Duration.
• Derive Ratio: PartialEq + Eq. Each ratio has a unique representation, so we don't need to go via cmp.
• Document panic of Delay::from_numer_denom_ms. Addresses M21 from Claude Code Review Report #2954.

I basically implemented what I think Delay should look like. Note that I didn't introduce any breaking changes (yet). I still don't like Delay::from_numer_denom_ms, but I'm also not entirely sure what should replace it.

Tell me what you think of these changes. If you like some but not others, I can revert some.

[197g]

I still don't like Delay::from_numer_denom_ms, but I'm also not entirely sure what should replace it.

It's not entirely clear if we need the flexibility of a denom: u32 parameter; a more constrained type (that should not be NonZeroU32 but a 'time-scale' of sorts) would avoid the zero panic and may be more expressive. Or rather it may move the panic to the construction of such a type / enumeration or an unwrapped option of it.

[RunDevelopment]

The only decoder that still uses Delay::from_numer_denom_ms is PNG. (GIF and WebP use Delay::from_millis.) Unfortunately, the PNG spec allows any fraction of u16 over non-zero u16 as the delay. So any sort of predefined time scales are a non-starter.

Of course, if we relax the constraints a little and allow approximations of the delay fraction, then we don't even really need predefined time scales. A Delay::from_nanos(u64) or even the existing Delay::from_saturating_duration would suffice. Do we require PNG (and similar) delay fractions to be exact?

(On that note: u32::MAX nanoseconds are 4.29 seconds. So most common delays can be represented exactly in nanosecond precision.)

[197g]

Expanding on the alternatives for time-scale:

• png's fctl only has 16-bit denominators and 0 "is to be treated as if it were 100"
• gif has a fixed timescale in u16 · 10ms.
• avif has a fixed timescale with delays in u32 ms.
• webp has a fixed timescale with offsets in i32 ms.
• vps/hevc define a 'clock tick' as its own u32/u32 quotient and use that as timescale. This is a good reason for hiding the implementation type to be honest. Neither of these numbers is supposed to be 0 but also the absolute frame offset may exceed u32::MAX if you were to use the denominator from this time point.

Also for all intents and purposes you can get quite good approximations with a microsecond scale. It would not matter to me if there were multiple ways to construct Delay as long as our interface for retrieving information from it is reasonably precise about roundtrips and precision.

[RunDevelopment]

Then should we just use u64 nanos instead of a fraction? Would be closer to Duration too.

Or we take the plunge and use Duration itself. No Delay anymore.

[197g]

Then should we just use u64 nanos instead of a fraction?

Not without doing the "as long as our interface for retrieving information from it is reasonably precise about roundtrips and precision" part, no. Also Lindy's law still applies, I don't think we need to rework our internals (rounding in a getter though, fine for me). But embracing different time-scale types as separate, possible constructors for Duration we could also introduce a change of Ration to a u64 numerator without regressing any existing representations; that should be sufficient to handle the format, only needing some approximation for the conversion back (saturating math probably). And then if ever need be we could make ratio be an enum field.

[RunDevelopment]

Not without doing the "as long as our interface for retrieving information from it is reasonably precise about roundtrips and precision" part, no.

What's your "reasonably precise" for roundtrips? Because ns seems like it'd be enough to me.

Also, could you please explain your idea for different type-scale types a bit more? I don't have an image of what you're trying to suggest yet.

[197g]

Not precise roundtrips, but for one reasonable implementation a good enough explanation of what will roundtrip; and how. For png it'd be at least surprising if re-encoding the same frame sequence could produce a very distinct result. For now that is simple enough: fractions expression as u16 are precise and those above can't have come from a png. Converting all decoded rations to ns inherently alreadys leads to a denominator larger than the PNG range on the way back, so the encoder always has to run in approxmation mode; it'll probably not always hit the input fraction then. That's one instance where ns-offset suffers in explainability..

With different time-scale types I mean making a public API to enable that promise use-case by use-case. So, bikeshedding names besides, we may have a constructor taking enum SiBasis { Seconds, Milliseconds } that will produce a Delay with the promise of round-tripping its numerator through Duration::{millis, seconds} (by using powers-of-ten denominators but that's not the user's concern). And separately a constructor with u32/u32 fractions. And then we can add the vp9 constructor (via a type representing clock-tick maybe) that promises to also roundtrip those into a particular getter on Delay. But we would allow ourselves to approximate at any cross-cutting use of Delay.

I don't know if it's a good idea. If explaining it here was already hard, it may not be an intuitive API.

[197g]

Since whatever idea was floating in my head aligns with making Duration more private in relation to decoders/ encoders, LGTM.

Derive Ratio: PartialEq + Eq. Each ratio has a unique representation, so we don't need to go via cmp.

Ah yes, we eagerly reduce now, right.

[RunDevelopment]

One last thing.

If we want the internal representation of Delay to somewhat abstracted away, then I feel like Delay::numer_denom_ms is an issue. It returns a u32 fraction, but it doesn't say that and there are other choices (e.g. a u16 fraction like for PNG). So it just exposes the internal representation. Maybe we should rename it to something like to_millis_ratio_32?

Same for Delay::from_numer_denom_ms. Maybe we should rename it to Delay::from_millis_fraction/ratio?

---

Also, I think I get the idea of your different time scales now. But as you said, I'm not sure if it's going to be worth the complexity. A u32/u32 fraction can represent all delays exactly (except probably hevc; haven't looked into that one yet), so let's see how far that gets us.