Fix to_systemtime sub-second precision for non-standard NTSC rates#74
Open
nickswalker wants to merge 1 commit into
Open
Fix to_systemtime sub-second precision for non-standard NTSC rates#74nickswalker wants to merge 1 commit into
nickswalker wants to merge 1 commit into
Conversation
System time is defined in the integer frame grid, so the sub-second calculation should always divide by _int_framerate. Using float(framerate) was imprecise for non-standard NTSC rates stored as rounded strings (e.g. "47.95" for 48000/1001), producing wrong millisecond values mid-second. Co-authored-by: cubicibo <55701024+cubicibo@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
to_systemtimecomputes the sub-second portion of the timestamp by dividing the frame count within the current second by the framerate:For NTSC rates,
float(self.framerate)is used, butself.frameratestores whatever string was passed in, or a float rounded to 2 decimal places. For a standard rate like 29.97 that's fine. For a non-standard NTSC multiple like 48000/1001 (≈47.952 fps), the setter stores"47.95", so the divisor is47.95instead of the correct integer framerate48:All
to_systemtimetests assert on whole-second boundaries, whereff = 0and thereforems = 0 / anything = 0, so the divisor never mattered. The rounding error for standard rates (29.97, 59.94, 119.88) is also small enough (1ms per frame) that it happens to survive the 3-decimal string format for the values tested.System time is defined in the integer frame grid. The divisor should always be
_int_framerate:@cubicibo fixed this as part of #69. Parting it out for easier review.