-
-
Notifications
You must be signed in to change notification settings - Fork 10.4k
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 incorrect capture when using DisplayManager API #4840
base: dev
Are you sure you want to change the base?
Conversation
Thank you for your contribution.
The commit 7e3b935 destroyed/created a new display on every rotation. On the contrary, your PR keeps the virtual display instance (instead of releasing/creating a new one). Could you please explain a bit more the problem? |
When execute Instead of releasing/creating a new VirtualDisplay (avoiding displayId increase), I do the same job at scrcpy/server/src/main/java/com/genymobile/scrcpy/ScreenCapture.java Lines 55 to 57 in dcea343
When executing But if we use DisplayManager API directly create a VirtualDisplay and set it to Because Android 14 lock orientation doesn't work as expected <#4011>, so we use (It seems a little bit hard to understand while I am poor at English writing 😴) And I suggest use |
Thank you for the detailed answer. I will read in details a bit later. Just a quick question:
Why is it a problem that an internal id is incremented? IIUC it's not the "display id" as in |
Nope, it's the "display id" as in |
I remember someone mentioning that when the "display ID" is over 1000, the system may encounter issues and need a reboot to fix it. |
BTW, in Android 14
also created a new display id |
Indeed (but I think this is true only for Android 14, on my older devices it does not create a new display id). |
With your commit from this PR, and this additional hack to force using the DisplayManager API: diff --git a/server/src/main/java/com/genymobile/scrcpy/ScreenCapture.java b/server/src/main/java/com/genymobile/scrcpy/ScreenCapture.java
index 6f9c4d338..d04fad572 100644
--- a/server/src/main/java/com/genymobile/scrcpy/ScreenCapture.java
+++ b/server/src/main/java/com/genymobile/scrcpy/ScreenCapture.java
@@ -41,6 +41,7 @@ public class ScreenCapture extends SurfaceCapture implements Device.RotationList
}
try {
+ if (true) throw new Exception("fake");
display = createDisplay();
setDisplaySurface(display, surface, videoRotation, contentRect, unlockedVideoRect, layerStack);
Ln.d("Display: using SurfaceControl API"); I can reproduce the problem that was fixed by 7e3b935 on a Pixel 8. 😞 |
Could you please post the screenshot of the incorrect capture? |
In fact, it is not a corrupted image: on device rotation, sometimes (not always, so there's probably a race condition) it just stops sending frames (so the physical device is portrait and the screen is updated, but the scrcpy window is landscape with the last landscape image, frozen), and the video stream is restored when I rotate the device screen one more time. In practice, I can only reproduce if the current active application is the (default) camera app. |
scrcpy/server/src/main/java/com/genymobile/scrcpy/ScreenCapture.java Lines 62 to 63 in dcea343
I think this should fix it 😑 |
Nope, with or without these lines, I can reproduce the problem. Besides, this is probably not an "incorrect" rotation value, it looks like a race condition (80% of times, it works correctly…). |
😮 Do you encounter this problem with current |
When using DisplayManager API without scrcpy/server/src/main/java/com/genymobile/scrcpy/ScreenCapture.java Lines 62 to 63 in dcea343
this problem occurs occasionally. (without passing |
😄 |
Fix the incorrect capture like this
![image](https://private-user-images.githubusercontent.com/52876220/323218336-d17b5388-2a15-4d00-aa9e-ec00fc56e07f.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MjU1OTI5NDMsIm5iZiI6MTcyNTU5MjY0MywicGF0aCI6Ii81Mjg3NjIyMC8zMjMyMTgzMzYtZDE3YjUzODgtMmExNS00ZDAwLWFhOWUtZWMwMGZjNTZlMDdmLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNDA5MDYlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQwOTA2VDAzMTcyM1omWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTEwY2E0MTBkODFjZDhkM2RlM2UzYzcwMDk4OTVlNTRmZjBmOGJlOTg0Mjg2ODUzMWYyNjQ2ZTYxMzhjNTM2NjImWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0JmFjdG9yX2lkPTAma2V5X2lkPTAmcmVwb19pZD0wIn0.5VlDmIVNd_CSzcQoPDVSjaoQaVVUxMXwlF-ajXMETgQ)