Aleksoid1978 / VideoRenderer

Внешний видео-рендерер
GNU General Public License v3.0
983 stars 108 forks source link

implement simplified frame syncing (fix issue #45) #130

Closed JTGaming closed 4 months ago

JTGaming commented 4 months ago

fixing issue #45.

The issue with MPC VR currently is that calling Render() twice in a row, as would be done for interlaced content, is that on displays where the refresh rate does not match the content frame rate, the frame delivery would not be consistent as the second field (Render(2)) would scan out on the next immediate refresh cycle, instead of waiting for it's proper turn.

Let's take this example: 240Hz display, 30fps content. If this was not interlaced, you would expect the renderer to present a new video frame every 240/30 = 8 refreshes. And if it was interlaced, you would expect it to present a new frame every 240/(30 * 2) = 4 refreshes, if frame doubling is enabled (which is by default). We would expect frames to render like this: Field 1 - blank - blank - blank - Field 2 - blank - blank - blank But that is not what happens as is. Instead, this is what we currently have: Field 1 - Field 2 - blank - blank - blank - blank - blank - blank give or take some variability, sometimes there is a single blank between Field 1 and Field 2.

This issue happens on any screen that has a refresh rate higher than double the interlaced frame rate when the 'double interlaced frame rate' option is on (25i on 60Hz too), though it gets more noticeable as the refresh rate increases. How do you test if you have this issue on your own screen with your particular content? Play a video and open the debug panel, then check the frametime graph - if you see a sawtooth pattern, then you're affected by this bug.

The simplest way I've found to fix this issue is by sleeping the renderer until we want the frame to be drawn. I've tried to keep the code short and easy to understand. The algorithm I've got in my own branch is much more fleshed out and adaptive, plus it removes nearly all frametime variance and variance introduced by the actual sleep() call itself, but the code is rather cryptic so this one that goes 90% of the way should suffice.

I haven't really noticed any downsides to having this always-on, since it does not run unless we finish processing the frame long before it should be rendered.

Aleksoid1978 commented 4 months ago

I think need set it's Optional.

Or, better, call SleepToSync() only when use double frame for deinterlacing.

Aleksoid1978 commented 4 months ago

Why microseconds ? Anyway, on Windows the maximum accuracy is ~1 ms

Aleksoid1978 commented 4 months ago

Я сделал реализацию с более простым и понятным кодом. simply_frame_syncing.zip

call SleepToSync() only before second call Render(2).

v0lt commented 4 months ago

Why can't I use the Sleep() function in SleepToSync() instead of std::this_thread::sleep_for()?

Aleksoid1978 commented 4 months ago

Why can't I use the Sleep() function in SleepToSync() instead of std::this_thread::sleep_for()?

Можно. И наверное так будет даже "проще".

Aleksoid1978 commented 4 months ago

Version with Sleep() simply_frame_syncing_Sleep.zip

JTGaming commented 4 months ago

You definitely could use miliseconds, I just copied some of the code from my branch where I use microseconds. Found there to be less variance on average that way.

Also, I would not recommend calling it only before the Render(2) call - you'd have to call it before both Render() calls to keep the frames synced to the same time target (Render(1) is aligned to around -7ms on average on my system, and Render(2) would then be synced to around 0ms - thus causing spiking between 7ms and 0ms).

If you want to limit it to interlaced content, I guess you could add a 'm_bDoubleFrames' check in SleepToSync, and only run the code if that is true. Or, add an option for this in the UI for users to decide if they want it on or not. I did not add such code to this PR as to not bloat the commit with multiple file changes for a single button.

clsid2 commented 4 months ago

Why can two progressive frames be rendered with proper timing and two interlaced frames not? Isn't there just a code bug that causes second field to be presented directly after first field?

JTGaming commented 4 months ago

I'm not too certain since I haven't really done much digging and I'm no video engineer, but my understanding is this: The frames are being sent to the renderer on a specific cadence (I believe that is done in CMpcVideoRenderer::Receive() with WaitForRenderTime()), and the renderer just churns through the frame and renders it out immediately.

With interlaced content, the renderer gets presented both fields at the same time in one frame, they are not seperated into two different frames (only one Receive() call, so WaitForRenderTime() only gets called before both fields are drawn, nothing in-between) - so the renderer goes through both fields back-to-back, sending them out immediately to the screen one after the other, without any wait time between the fields. And so this repeats, where every Field 1 gets drawn on time as CMpcVideoRenderer::Receive() syncs the frame, but Field 2 gets drawn immediately after because there is nothing being done in ProcessSample() to sync them.

Basically, if you are watching 30i content with frame-doubling enabled, the video itself would be shown as 60fps, but the frame delivery would be synced up to 30fps.