Fix incorrect capture when using DisplayManager API

[eiyooooo]

Fix the incorrect capture like this

[rom1v]

Thank you for your contribution.

Fix the same incorrect capture occur before 7e3b9359322fff65bd350febfdc02a76186981cd when using DisplayManager API

The commit 7e3b9359322fff65bd350febfdc02a76186981cd 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?

[eiyooooo]

Could you please explain a bit more the problem?

When execute SurfaceControl.createDisplay on Android 14, it essentially calls DisplayManager.createVirtualDisplay. When execute SurfaceControl.setDisplayProjection on Android 14, it essentially calls DisplayManager.resizeVirtualDisplay. When execute SurfaceControl.setDisplayLayerStack on Android 14, it essentially calls DisplayManager.setDisplayIdToMirror.

Instead of releasing/creating a new VirtualDisplay (avoiding displayId increase), I do the same job at https://github.com/Genymobile/scrcpy/blob/dcea34350673491a49597d2b5a8c33d273e2b1f4/server/src/main/java/com/genymobile/scrcpy/ScreenCapture.java#L55-L57

When executing DisplayManager.setDisplayIdToMirror, it will copy the configuration of the original display to the created display (including the rotation). (It probably can only happen once to each created display, so you need to destroy the created display and create a new one to copy the rotation at https://github.com/Genymobile/scrcpy/commit/7e3b9359322fff65bd350febfdc02a76186981cd.)

But if we use DisplayManager API directly create a VirtualDisplay and set it to VIRTUAL_DISPLAY_FLAG_AUTO_MIRROR, orientation and displayRect will automatically be computed based on configuration changes. 

Because Android 14 lock orientation doesn't work as expected <#4011>, so we use videoRect. But videoRect is already rotated according to the device rotation, so we need to freeze the VirtualDisplay's rotation to 0 to avoid a double rotation.

(It seems a little bit hard to understand while I am poor at English writing :sleeping:)

And I suggest use DisplayManager API directly on Android 14 and above, instead of trying the SurfaceControl API first, because it basically does the same thing. If we use DisplayManager API, we could have a better control of the created VirtualDisplay.

[rom1v]

Thank you for the detailed answer. I will read in details a bit later.

Just a quick question:

Instead of releasing/creating a new VirtualDisplay (avoiding displayId increase)

Why is it a problem that an internal id is incremented? IIUC it's not the "display id" as in scrcpy --display-id=, is it?

[eiyooooo]

IIUC it's not the "display id" as in scrcpy --display-id=, is it?

Nope, it's the "display id" as in scrcpy --display-id=:wink:

[eiyooooo]

Why is it a problem that an internal id is incremented?

I remember someone mentioning that when the "display ID" is over 1000, the system may encounter issues and need a reboot to fix it.

[rom1v]

Oh, I'm had never noticed that mirroring the device created a new display id (listed by scrcpy --list-displays) with createVirtualDisplay() (be3f949aa5c4dcad276ac0f2b36671a3309ce12f).

Maybe there is another method which does not create a new display id (maybe refs #4848, but I did not understand the issue).

[eiyooooo]

Oh, I'm had never noticed that mirroring the device created a new display id (listed by scrcpy --list-displays) with createVirtualDisplay()

BTW, in Android 14

https://github.com/Genymobile/scrcpy/blob/be3f949aa5c4dcad276ac0f2b36671a3309ce12f/server/src/main/java/com/genymobile/scrcpy/ScreenCapture.java#L48

also created a new display id

[rom1v]

Indeed (but I think this is true only for Android 14, on my older devices it does not create a new display id).

[rom1v]

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 7e3b9359322fff65bd350febfdc02a76186981cd on a Pixel 8. :disappointed:

[eiyooooo]

I can reproduce the problem that was fixed by 7e3b935 on a Pixel 8. 😞

Could you please post the screenshot of the incorrect capture?

[rom1v]

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.

[eiyooooo]

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.

https://github.com/Genymobile/scrcpy/blob/dcea34350673491a49597d2b5a8c33d273e2b1f4/server/src/main/java/com/genymobile/scrcpy/ScreenCapture.java#L62-L63

I think this should fix it :expressionless:

[rom1v]

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…).

[eiyooooo]

Fix the same incorrect capture occur before 7e3b935 when using DisplayManager API

So this pr is not about the incorrect capture occur before 7e3b935

Here are the incorrect capture I met

[rom1v]

:open_mouth: Do you encounter this problem with current dev branch on rotation, without passing --crop or --lock-video-orientation?

[eiyooooo]

😮 Do you encounter this problem with current dev branch on rotation, without passing --crop or --lock-video-orientation?

When using DisplayManager API without

https://github.com/Genymobile/scrcpy/blob/dcea34350673491a49597d2b5a8c33d273e2b1f4/server/src/main/java/com/genymobile/scrcpy/ScreenCapture.java#L62-L63

this problem occurs occasionally.

(without passing --crop or --lock-video-orientation)

[eiyooooo]

I can reproduce the problem that was fixed by 7e3b935 on a Pixel 8. 😞

With my commit from this PR, I can reproduce your bug fixed by 7e3b935 too. :disappointed_relieved:

So I think releasing/creating a new one still necessary.

[eiyooooo]

😄