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

Restart capture when screen resolution changes #4469

Closed
wants to merge 2 commits into from

Conversation

yume-chan
Copy link
Contributor

Fixes #1918
Fixes #161
Might affect #4152

Found this API by accident when researching #4446 (comment).

Some devices have a resolution option in their Settings app, and on all devices the resolution can be changed by adb shell wm size 1080x1920. This event will react to both cases.

Tested on Xiaomi Mi 11 (Android 13, both system settings and wm size), Samsung Galaxy S9 (Android 10, both system settings and wm size), Onyx BOOX Nova Pro (Android 6.0.1, only wm size).

@yume-chan yume-chan changed the base branch from master to dev November 27, 2023 09:10
@yume-chan yume-chan force-pushed the feat/resolution-change branch from a1869de to e90abc8 Compare November 27, 2023 09:11
Copy link
Collaborator

@rom1v rom1v left a comment

Choose a reason for hiding this comment

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

Great 👍 I will test later.

screenInfo = ScreenInfo.computeScreenInfo(displayInfo.getRotation(), deviceSize, crop, maxSize, lockVideoOrientation);

if (foldListener != null) {
foldListener.onFoldChanged(displayId, false);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why notifying fold changed? (btw, in theory, the size might change but folded might remain true)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I forgot to add a new event.

If Device.RotationListener, Device.FoldListener and this new listener all do the same thing, maybe merge into one is enough?

@rom1v
Copy link
Collaborator

rom1v commented Nov 27, 2023

For consistency with the other listeners where we use the aidl directly, maybe we could use IDisplayManagerCallback.aidl. This is what the wrapper DisplayListeners use internally (with an additional Handler).

https://github.com/aosp-mirror/platform_frameworks_base/blob/ad73c57d691ad2cfd48b201db6cf8b053d2207ba/core/java/android/hardware/display/DisplayManagerGlobal.java#L1099-L1108

@yume-chan
Copy link
Contributor Author

yume-chan commented Nov 27, 2023

maybe we could use IDisplayManagerCallback.aidl

I created a new branch to use IDisplayManager.registerCallback(IDisplayManagerCallback): https://github.com/Genymobile/scrcpy/compare/master...yume-chan:scrcpy:feat/resolution-change-2?expand=1

It throws an error when called:

C:\Users\simon\Downloads\scrcpy-win64-v2.3> ./scrcpy -s 0123456789ABCDEF
scrcpy 2.3 <https://github.com/Genymobile/scrcpy>
INFO: ADB device found:
INFO:     -->   (usb)      0123456789ABCDEF            device  NovaPro
INFO:           (usb)              9719052f            device  ONEPLUS_A6000
C:\Users\simon\Downloads\scrcpy-win64-v2.3\scrcpy-server: 1 file pushed, 0 skipped. 10.9 MB/s (66675 bytes in 0.006s)
[server] INFO: Device: [Onyx] Onyx NovaPro (Android 6.0.1)
[server] ERROR: java.lang.reflect.InvocationTargetException
java.lang.AssertionError: java.lang.reflect.InvocationTargetException
        at com.genymobile.scrcpy.wrappers.DisplayManager.registerCallback(DisplayManager.java:112)
        at com.genymobile.scrcpy.Device.<init>(Device.java:94)
        at com.genymobile.scrcpy.Server.scrcpy(Server.java:114)
        at com.genymobile.scrcpy.Server.internalMain(Server.java:244)
        at com.genymobile.scrcpy.Server.main(Server.java:199)
        at com.android.internal.os.RuntimeInit.nativeFinishInit(Native Method)
        at com.android.internal.os.RuntimeInit.main(RuntimeInit.java:251)
Caused by: java.lang.reflect.InvocationTargetException
        at java.lang.reflect.Method.invoke(Native Method)
        at com.genymobile.scrcpy.wrappers.DisplayManager.registerCallback(DisplayManager.java:110)
        ... 6 more
Caused by: java.lang.SecurityException: The calling process has already registered an IDisplayManagerCallback.
        at android.os.Parcel.readException(Parcel.java:1620)
        at android.os.Parcel.readException(Parcel.java:1573)
        at android.hardware.display.IDisplayManager$Stub$Proxy.registerCallback(IDisplayManager.java:305)
        ... 8 more
ERROR: Server connection failed

Until Android 12, one process can only have one IDisplayManagerCallback: https://cs.android.com/android/platform/superproject/+/android-11.0.0_r1:frameworks/base/services/core/java/com/android/server/display/DisplayManagerService.java;l=660-665;drc=b28fb721f5be06818bedc8601e02118ddcbd4739

@rom1v
Copy link
Collaborator

rom1v commented Nov 30, 2023

Oops, it seems this has changed in Android 14:

[server] INFO: Device: [Google] google Pixel 8 (Android 14)
[server] ERROR: java.lang.NoSuchMethodException: android.hardware.display.DisplayManagerGlobal.registerDisplayListener [interface android.hardware.display.DisplayManager$DisplayListener, class android.os.Handler]
java.lang.AssertionError: java.lang.NoSuchMethodException: android.hardware.display.DisplayManagerGlobal.registerDisplayListener [interface android.hardware.display.DisplayManager$DisplayListener, class android.os.Handler]
	at com.genymobile.scrcpy.wrappers.DisplayManager.registerDisplayListener(DisplayManager.java:127)
	at com.genymobile.scrcpy.Device.<init>(Device.java:96)
	at com.genymobile.scrcpy.Server.scrcpy(Server.java:114)
	at com.genymobile.scrcpy.Server.internalMain(Server.java:244)
	at com.genymobile.scrcpy.Server.main(Server.java:199)
	at com.android.internal.os.RuntimeInit.nativeFinishInit(Native Method)
	at com.android.internal.os.RuntimeInit.main(RuntimeInit.java:359)
Caused by: java.lang.NoSuchMethodException: android.hardware.display.DisplayManagerGlobal.registerDisplayListener [interface android.hardware.display.DisplayManager$DisplayListener, class android.os.Handler]
	at java.lang.Class.getMethod(Class.java:2937)
	at java.lang.Class.getMethod(Class.java:2449)
	at com.genymobile.scrcpy.wrappers.DisplayManager.registerDisplayListener(DisplayManager.java:124)
	... 6 more

EDIT: aosp-mirror/platform_frameworks_base@204eba6

@rom1v
Copy link
Collaborator

rom1v commented Nov 30, 2023

Oh, this callback is called also on device rotation (and probably also on fold, although I have no foldable device to test).

So we can probably remove the other callbacks completely.

@rom1v
Copy link
Collaborator

rom1v commented Nov 30, 2023

@AdoPi (as author or #3979), could you check that with this PR + this diff:

diff --git a/server/src/main/java/com/genymobile/scrcpy/Device.java b/server/src/main/java/com/genymobile/scrcpy/Device.java
index 756af86e3..c75e2a8d7 100644
--- a/server/src/main/java/com/genymobile/scrcpy/Device.java
+++ b/server/src/main/java/com/genymobile/scrcpy/Device.java
@@ -106,6 +106,7 @@ public final class Device {
 
             @Override
             public void onDisplayChanged(int displayId) {
+                Ln.i("=== onDisplayChanged(" + displayId + ")");
                 if (Device.this.displayId != displayId) {
                     return;
                 }

If you fold the device, is the log printed?

@yume-chan
Copy link
Contributor Author

Oops, it seems this has changed in Android 14:

Oh, I was originally using DeviceManager.registerDisplayListener, it always has an overload with only two parameters: https://cs.android.com/android/platform/superproject/main/+/main:frameworks/base/core/java/android/hardware/display/DisplayManager.java;l=726-761;drc=b3691fab2356133dfc7e11c213732ffef9a85315

Higher level API changes less, but is more prone to OEM modifications. It's hard to balance.

So we can probably remove the other callbacks completely.

I removed rotation listener, and will wait for test results from foldable devices.

rom1v added a commit that referenced this pull request Dec 1, 2023
When a display is folded or unfolded, the maxSize may have been updated
since the option was passed, and deviceSize must be updated.

Refs #4469 <#4469>
@rom1v
Copy link
Collaborator

rom1v commented Dec 1, 2023

Thank you 👍

For clarity, I extracted the rotation listener (and fold listener) to separate commits: reschange.3.

I also made minor changes, like removing unused onDisplayAdded() and onDisplayRemoved().

However, I just noticed that it does not work at all on Android 14: it builds, but the onDisplayChanged() event is never received, either on wm size changes or on rotation (on your branch or after my refactors). 😞

It works correctly on Android <= 13.

@yume-chan
Copy link
Contributor Author

yume-chan commented Dec 3, 2023

However, I just noticed that it does not work at all on Android 14

The reason is Android 14 no longer sends display events to "dead" (we don't have any window or service so we are considered as dead) apps: https://cs.android.com/android/platform/superproject/main/+/main:frameworks/base/services/core/java/com/android/server/display/DisplayManagerService.java;l=2878-2891;drc=d4035b6eb1192f71af7adae47bf17d22c7356fa1

It works if I enable foreground workaround (and keeping it running like camera). #4213 (comment) doesn't work.

In my app I'm experimenting showing foreground workaround on virtual displays (#1887 (comment)) to get camera capturing working on Android 11. Maybe it will help this on Android 14.

rom1v added a commit that referenced this pull request Dec 4, 2023
When a display is folded or unfolded, the maxSize may have been updated
since the option was passed, and deviceSize must be updated.

Refs #4469 <#4469>
@rom1v rom1v mentioned this pull request Oct 16, 2024
rom1v added a commit that referenced this pull request Oct 21, 2024
Replace RotationWatcher and FoldListener by a single DisplayListener,
which is notified whenever the display size or dpi changes.

Refs #4469 <#4469>

Co-authored-by: Simon Chan <[email protected]>
@rom1v
Copy link
Collaborator

rom1v commented Oct 21, 2024

@yume-chan For some reason, display_changed works everywhere, including on Android 14 🎉

It also fixes: #5362 (comment)

@rom1v
Copy link
Collaborator

rom1v commented Oct 21, 2024

For some reason, display_changed works everywhere

Oh no, not everywhere. It works on Android 14 on my device, but not on another tablet with Android 14. I think there are some early Android 14 versions which are broken.

I don't know how to handle that properly.

@rom1v
Copy link
Collaborator

rom1v commented Oct 21, 2024

It might have been fixed by https://android.googlesource.com/platform/frameworks/base.git/+/f2daa46634f6a1e5e329041f07a27dbc894d71b2%5E%21/, but the first tag with this commit is android-15.0.0_r1, while my device has Android 14, so maybe not.

rom1v added a commit that referenced this pull request Oct 21, 2024
Replace RotationWatcher and DisplayFoldListener by a single
DisplayListener, which is notified whenever the display size or dpi
changes.

Still use the old version specifically for Android 14, where
DisplayListener may be broken (it is fixed in recent Android 14
upgrades), until we receive the first DisplayListener event (which
proves that it works).

Refs #4469 <#4469>

Co-authored-by: Simon Chan <[email protected]>
@rom1v
Copy link
Collaborator

rom1v commented Oct 21, 2024

With this commit, it works on all versions (AFAIK): 3174ce2 (branch display_changed.3)

Feedback welcome 😉

There is a fallback specifically for Android 14 (which still listens to rotation and display fold events), but only until it receives the first DisplayListener event (so we can be sure this is an upgrade of Android 14 that works). This avoids to reset the encoding twice (one on rotation event, one on display event).

It's a bit hacky though.

rom1v added a commit that referenced this pull request Oct 22, 2024
Replace RotationWatcher and DisplayFoldListener by a single
DisplayListener, which is notified whenever the display size or dpi
changes.

Still use the old version specifically for Android 14, where
DisplayListener may be broken (it is fixed in recent Android 14
upgrades), until we receive the first DisplayListener event (which
proves that it works).

Refs #4469 <#4469>

Co-authored-by: Simon Chan <[email protected]>
@rom1v
Copy link
Collaborator

rom1v commented Oct 22, 2024

Not so good: onDisplayChanged() is also called when the screen turns off/on 😞

@yume-chan
Copy link
Contributor Author

Does display power state change its size? Can we compare the size and only restart when it changes?

@rom1v
Copy link
Collaborator

rom1v commented Oct 26, 2024

Yes, I did that on a local branch.

rom1v added a commit that referenced this pull request Oct 28, 2024
Replace RotationWatcher and DisplayFoldListener by a single
DisplayListener, which is notified whenever the display size or dpi
changes.

Still use the old mechanism specifically for Android 14, where
DisplayListener may be broken (it is fixed in recent Android 14
upgrades), until we receive the first DisplayListener event (which
proves that it works).

Fixes #161 <#161>
Fixes #1918 <#1918>
Fixes #4152 <#4152>
Refs #4469 <#4469>

Co-authored-by: Simon Chan <[email protected]>
rom1v added a commit that referenced this pull request Oct 28, 2024
Replace RotationWatcher and DisplayFoldListener by a single
DisplayListener, which is notified whenever the display size or dpi
changes.

Still use the old mechanism specifically for Android 14, where
DisplayListener may be broken (it is fixed in recent Android 14
upgrades), until we receive the first DisplayListener event (which
proves that it works).

Fixes #161 <#161>
Fixes #1918 <#1918>
Fixes #4152 <#4152>
Fixes #5362 comment <#5362 (comment)>
Refs #4469 <#4469>

Co-authored-by: Simon Chan <[email protected]>
rom1v added a commit that referenced this pull request Oct 30, 2024
Replace RotationWatcher and DisplayFoldListener by a single
DisplayListener, which is notified whenever the display size or dpi
changes.

However, the DisplayListener mechanism is broken in the first versions
of Android 14 (it is fixed in android-14.0.0_r29 by commit [1]), so
continue to use the old mechanism specifically for Android 14 (where
DisplayListener may be broken), until we receive the first
"display changed" event (which proves that it works).

[1]: <https://android.googlesource.com/platform/frameworks/base.git/+/5653c6b5875df599307c3e6bfae32fb2fc17ca1f%5E%21/>

Fixes #161 <#161>
Fixes #1918 <#1918>
Fixes #4152 <#4152>
Fixes #5362 comment <#5362 (comment)>
Refs #4469 <#4469>
PR #5415 <#5415>

Co-authored-by: Simon Chan <[email protected]>
rom1v added a commit that referenced this pull request Oct 31, 2024
Replace RotationWatcher and DisplayFoldListener by a single
DisplayListener, which is notified whenever the display size or dpi
changes.

However, the DisplayListener mechanism is broken in the first versions
of Android 14 (it is fixed in android-14.0.0_r29 by commit [1]), so
continue to use the old mechanism specifically for Android 14 (where
DisplayListener may be broken), until we receive the first
"display changed" event (which proves that it works).

[1]: <https://android.googlesource.com/platform/frameworks/base.git/+/5653c6b5875df599307c3e6bfae32fb2fc17ca1f%5E%21/>

Fixes #161 <#161>
Fixes #1918 <#1918>
Fixes #4152 <#4152>
Fixes #5362 comment <#5362 (comment)>
Refs #4469 <#4469>
PR #5415 <#5415>

Co-authored-by: Simon Chan <[email protected]>
@yume-chan yume-chan closed this Nov 3, 2024
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.

Doesn't sync correctly after use adb shell wm size. Support device resolution changes
2 participants