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

Fix ctrl-s (screenshot window) does not work, #4822 #4830

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

low-batt
Copy link
Contributor

This commit will change the MPVController.mpvInitRendering method to enable advanced control when creating the mpv renderer context. Advanced control must be enabled for this variation of the screenshot command to work.


Description:

@low-batt low-batt linked an issue Feb 23, 2024 that may be closed by this pull request
1 task
This commit will change the MPVController.mpvInitRendering method to
enable advanced control when creating the mpv renderer context. Advanced
control must be enabled for this variation of the screenshot command to
work.
@low-batt
Copy link
Contributor Author

Rebased with develop and fixed merge conflicts.

@lhc70000
Copy link
Member

I remember that it was disabled on purpose. We need to take a further look.

@low-batt
Copy link
Contributor Author

YES, this change needs a close look. This was disabled as a fix for issue #2149. In digging through old mpv commits I thought I saw some mpv fixes that might have fixed the hang. I did confirm those fixes were made after this workaround was added. There is a lot of discussion in the associated issue.

We should definitely merge PR #4819 first (as long as that one passes review). This PR corrects many data races detected by TSAN by moving work off of the MPVController queue and on to the main thread. As a lot of that is small I thought it better to access the data only from the main thread rather than adding locks to coordinate access.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ctrl-s (screenshot window) does not work
2 participants