Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Increase/validate precision of converted videos #359

Open
obilodeau opened this issue Sep 9, 2021 · 1 comment · May be fixed by #364
Open

Increase/validate precision of converted videos #359

obilodeau opened this issue Sep 9, 2021 · 1 comment · May be fixed by #364
Assignees
Labels
help wanted Extra attention is needed investigate Needs more thought / experience

Comments

@obilodeau
Copy link
Member

I believe that the way we encode frames we create the possibility of a drift between the real time and the resulting video times. This is exacerbated on large captures.

In Mp4EventHandler's init:

        self.delta = 1000 // fps  # ms per frame

then:

        nframes = (dt // self.delta)
        if nframes > 0:
            for _ in range(nframes):
                self.writeFrame()

However:

In [18]: 1000//30
Out[18]: 33

In [19]: 1000/30
Out[19]: 33.333333333333336

There's a 0.3ms per 1000ms drift opportunity. On a long capture this amount to a very large maximum theoretical drift:

In [26]: ((1000/30-1000//30) * 12*60*60) / 60
Out[26]: 240.0000000000017

Further investigation: I will record an RDP session playing a high precision clock and timer on a screen for a long time and compare after video encoding.

@obilodeau obilodeau added help wanted Extra attention is needed investigate Needs more thought / experience labels Sep 9, 2021
@obilodeau
Copy link
Member Author

@h3xstream suggested to use a fps of 25 (which can divide 1000 cleanly) and see. What I like about this solution is that it would also make the conversion faster and video files smaller. Remembering some captures I looked at, we clearly don't do 30 fps over RDP anyway.

@alxbl alxbl self-assigned this Oct 16, 2021
@alxbl alxbl linked a pull request Oct 24, 2021 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed investigate Needs more thought / experience
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants