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

Improved loging in case of clamping visual position #12516

Merged
merged 4 commits into from
Jan 20, 2024

Conversation

daschuer
Copy link
Member

@daschuer daschuer commented Jan 5, 2024

It turns out that the log spam discovered by @m0dB in #12470 is actually a bug.

This PR improves the log output and the negative side that is not a transport issue but a vsync issue.

It happens if the rendere requests a position of an already missed vsync cycle.

A solution is to detect that and advance to the next vsync cycle that has not yet passed.

@daschuer
Copy link
Member Author

daschuer commented Jan 5, 2024

The problematic function is

https://github.com/daschuer/mixxx/blob/f610d58a25b6f97172b097fb2a7a79147d863f7d/src/waveform/vsyncthread.cpp#L188

Here we need to advance to the next interval if we have missed the vsync interval the values are prepared for.

@m0dB
What was the reasoning for the combined sync and render step in the PLL case. I have good results with the combined timer and pll version when I start the timers after swap.

I think it depends on the drivers implementation of vsync. When I remember correctly, the rendering waits to not introduce tearing for two reasons:

  1. Swap is delayed just after the previous picture has been displayed.
  2. rendering waits, when both buffers are occupied. One is in use for display, the other has already a pending swap.

Maybe macOS has a workaround for 1. and constantly hits 2.

I have seen both with different drivers.

Idea: Maybe we can use the PLL everwhere and just make the sync moment an option?

Next I will contribute a PR that moves the position request into the right interval in case of vsync misses.

@daschuer
Copy link
Member Author

daschuer commented Jan 7, 2024

Requesting the right position was easy, I have added it here. Even the free mode is now working properly with ~500 Hz.

@daschuer daschuer added this to the 2.4.0 milestone Jan 7, 2024
Comment on lines 73 to 75
// The negative offset is limited to -data.m_callbackEntrytoDac to not
// More negative values indicated an outdated request, that is anyway no
// longer valid. Probably cause bay a vsync issue.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there something missing in the first line?
Please fix the typos.

@daschuer
Copy link
Member Author

daschuer commented Jan 8, 2024

Done.

Copy link
Member

@ywwg ywwg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

works fine on my machine

@m0dB
Copy link
Contributor

m0dB commented Jan 20, 2024

Works fine here too, with both ST_TIMER and ST_PLL.

@JoergAtGithub
Copy link
Member

Works for me too! Thank you!

@JoergAtGithub JoergAtGithub merged commit 85f3f06 into mixxxdj:2.4 Jan 20, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants