github-actions[bot]
commented
1 week ago Download the artifacts for this pull request:
Closed kasper93 closed 1 week ago
github-actions[bot]
commented
1 week ago Download the artifacts for this pull request:
Dudemanguy
commented
1 week ago Didn't check the math but wouldn't 1 only allow exact integers? e.g. 23.976 fps against a 72.00 HZ is surely desirable so you'd still want that to work.
kasper93
commented
1 week ago Didn't check the math but wouldn't 1 only allow exact integers? e.g. 23.976 fps against a 72.00 HZ is surely desirable so you'd still want that to work.
I'm not sure what is 72 Hz in your example. But let me explain what is going on in this code.
It tries find speed adjustment for video to match multiple of display fps. So for example for 144 Hz:
factor 1 tries to match 144 Hz factor 2 tries to match 288 Hz factor 3 tries to match 432 Hz factor 4 tries to match 576 Hz factor 5 tries to match 720 Hz and so on...
Assuming your example was about 72 Hz display. It would match at factor 1 because speed adjustment is below threshold. So it would get adjusted to 24 fps @ 72 Hz. Also this is perfect case, because after small adjustment we have multiple of display Hz, so factor doesn't even matter.
Now more interesting case is when we cannot adjust speed enough (due to limit) and start going into multiples of display Hz. For example. 25 fps @ 144 Hz would try to:
| factor | speed change % | video fps after adjust | vsync ratio |
|---|---|---|---|
| no adjustment | 0.0000 | 25.0000 | 5.7600 |
| 1 | -4.0000 | 24.0000 | 6.0000 |
| 2 | -4.0000 | 24.0000 | 6.0000 |
| 3 | 1.6471 | 25.4118 | 5.6667 |
| 4 | 0.1739 | 25.0435 | 5.7500 |
| 5 | -0.6897 | 24.8276 | 5.8000 |
1-3 is rejected, because it is too big speed change and we end up using factor 4. Which is targeting video fps to be multiple of 576 Hz. 576 / 25.0435 = 23.
Sync code will fit number of reapeats for current vsync duration. So if we cannot target multiple of this duration we will accumulate error either way, and have at some point 5 instead of 6 repeats. note that in all those cases it would do the same, but if we adjust the speed to factor 4, we in theory after repeating frames 23 times (6+6+6+5), cancel out error and align on vsync again. We also have drift and another sync innacuracies, so it won't be perfect anyway. I made this draft, because after doing 2nd take on this, it might actually be better this way, I'm not sure though if 5 is not too big number, but theoretically it makes sense actually.
Another example would be 23.976 fps on 60 Hz. With factor 2 it would adjust it to 24 fps, which makes it clean 3:2 rate. Else we would accumulate error and after a while (every 17 seconds) we would need to do drop one repeat.
Either way, I think we can skip this patch probably, I still not sure if this is the best way to adjust speed, but it is what we have.
Allow changing the video speed only if it results in an integer multiple of the display FPS by default. It makes little sense to adjust the video speed if it does not result in the target multiple.
For example, a 25 fps video on a 144 Hz display would be sped up by 0.17% before this change, resulting in a ratio of approximately 5.75 instead of 5.76 without the speed change...