1909614 - TimeUnit::operator== is nonsense for large numerators and denominators

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Karl Tomlinson (:karlt)]

https://hg.mozilla.org/mozilla-central/rev/2f0238fa0a6b6167a7fbf5cbeb1803b579fdbe85 missed operator==

https://searchfox.org/mozilla-central/rev/dea459eb01a4c38b696ae3d31c1540e86365a937/dom/media/TimeUnits.cpp#181-186

Comment 1

[Karl Tomlinson (:karlt)]

Perhaps 128-bit arithmetic might be useful to make TimeIntervals more predictable.

Do we ever use bases > 2^32?
If not, 96-bit arithmetic would be sufficient.

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1817997

:padenot, since you are the author of the regressor, bug 1817997, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 3

[Karl Tomlinson (:karlt)]

If different precision is used in different situations, such as using 53-bit precision floating point sometimes in operator== and exact arithmetic when its easier, then this can lead to inconsistent results such as a < c when a == b == c.

Assuming denominators are either hard-coded or come from up-to-32-bit fields in media resources, 96-bit fixed point (exact) arithmetic could be sufficient.

Comment 4

[Karl Tomlinson (:karlt)]

When TimeUnit comparisons are between microseconds and tenths of microseconds, the fallback path is used for times greater than 10 days. (2^63/10^6/10^7/60/60/24).

If comparisons are between nanoseconds and tenths of microseconds, then the fallback path is used for times greater than 15 minutes (2^63/10^9/10^7/60).

Bug 1835044 comment 4 indicates that sites can use large start times (53 years).

Comment 5

[Karl Tomlinson (:karlt)]

Downgrading because the Reduce() operations provide that equal TimeUnits return true and unequal TimeUnits return true only if they differ only in a unique factor that moves from numerator in one argument to denominator in the other.

Comment 6

[Karl Tomlinson (:karlt)]

Attachment: Bug 1909614 use exact 96-bit integer arithmetic for TimeUnit comparison operators r?padenot

As well as removing 53-bit precision rounding in >= and <= operators, this
resolves a bug where operator== would sometimes returning true for very
different TimeUnits.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/557afaf1731b
use exact 96-bit integer arithmetic for TimeUnit comparison operators r=padenot

Comment 8

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/557afaf1731b

Comment 9

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:karlt, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox129 to wontfix.

For more information, please visit BugBot documentation.

Comment 10

[Karl Tomlinson (:karlt)]

I don't know of any particular real life situations hitting this, so i think this is better to have some time to ride the trains.