TASVideos / tasvideos

The code for the live TASVideos website
https://tasvideos.org/
GNU General Public License v3.0
62 stars 29 forks source link

Support newer OMR format #1682

Closed fsvgm777 closed 1 year ago

fsvgm777 commented 1 year ago

Fixes #1586.

Also fixes a rounding issue when calculating the total frame count, as it'd result in the total run time being wrong most of the time.

Included are also additional system detection tests to account for the recent format changes.

I've tested it locally, and it works fine.

fsvgm777 commented 1 year ago

After a brief discussion on Discord, I am not sure whether the rounding should be changed back (Math.Ceiling) or if the rounding should be kept (with the midpoint rounding changed to AwayFromZero to fix more rounding issues). As the format doesn't exactly have a concept of a frame per se (in fact, from my understanding of the format, it's cycle-based, which can then be converted to seconds), I wouldn't even be opposed to put in a frame rate override. Opinions?

My main thing with changing it back to Math.Ceiling is that it would then result in the wrong run time being displayed on the site (e.g. Skooter would have a run time of 5:41.44 instead of 5:41.42, and I'm more convinced the latter is right, based on how the time is calculated (you end up with 341.42295459339107 seconds after dividing the last key matrix state in that particular movie (1173253276800) by 3579545.0, and then by 960.0).

MBilderbeek commented 1 year ago

I'd approve this if I had the power 🙂