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

Listen to display changed events #5415

Merged
merged 2 commits into from
Oct 31, 2024
Merged

Listen to display changed events #5415

merged 2 commits into from
Oct 31, 2024

Conversation

rom1v
Copy link
Collaborator

@rom1v rom1v commented Oct 28, 2024

Whenever the display size changes, scrcpy must reset the capture/encoding to stream a video at the new size.

The display size may change for several reasons:

  • device rotation
  • device folding/unfolding
  • resolution change

Currently, scrcpy only listened for device rotation and device folding/unfolding events.

This was sometimes insufficient:

Instead of listening to specific events that could cause a display size change, @yume-chan submitted #4469 to listen to display changed events directly. It was almost ready to be merged, but we noticed that it didn't work on Android 14: #4469 (comment) (the events were never received)

In recent Android 14 upgrades, it seems the behavior is fixed (the display changed events are correctly received).

But we must still support the devices with a broken version of Android 14, so this PR implements the following behavior:

  • on Android != 14: only use display changed events
  • on Android 14: register rotation/fold listeners (like before) and listen to display changed events. On the first display changed event received, we know that the device has an Android 14 version where the DisplayListener works, so unregister rotation/fold listeners (to avoid resetting the capture/encoding twice)

The display changed events are triggered for many reasons, not only the size. This PR stores the current size and reset only if the new size is different.

@yume-chan Please review 😉

@yume-chan
Copy link
Contributor

LGTM.

I wanted to manually trigger a display changed event (without changing any user-observable properties), so we can detect whether it works proactively. But I can't find a candidate (in DisplayInfo.equals). Maybe it can be done on a virtual display, but I guess that would take too much time.

Another thing I don't understand is how f2daa46634f6a1e5e329041f07a27dbc894d71b2 fixed the issue. Because it means com.android.shell app is not considered dead by Android system?

@rom1v
Copy link
Collaborator Author

rom1v commented Oct 30, 2024

Another thing I don't understand is how f2daa46634f6a1e5e329041f07a27dbc894d71b2 fixed the issue. Because it means com.android.shell app is not considered dead by Android system?

I found this commit which seemed related, but this is probably not this one that fixes the issue, since it was introduced in Android 15 but now recent versions of Android 14 work: #4469 (comment)

@yume-chan
Copy link
Contributor

yume-chan commented Oct 30, 2024

It's actually fixed in 5653c6b5875df599307c3e6bfae32fb2fc17ca1f. Looks like Android Studio's screen mirroring is also using this display changed event.

I don't have any other questions.

@yume-chan
Copy link
Contributor

I tested it on a Redmi K70 Pro running Android 14, display changed event doesn't work.

But when changing screen resolution, the screen content is resized to fit the video, unlike Android 13 which clips or leaves blank areas in the video.

When screen aspect ratio changes, the screen content is centered with black bars on sides.

Also, touch inputs are shifted after changing screen resolution in v2.7, but works correctly after #5370, despite the coordinate in generated input events are for the old screen size.

I guess the new screen capture API already makes it much better even without display changed events.

@rom1v
Copy link
Collaborator Author

rom1v commented Oct 30, 2024

I tested it on a Redmi K70 Pro running Android 14, display changed event doesn't work.

But when changing screen resolution, the screen content is resized to fit the video, unlike Android 13 which clips or leaves blank areas in the video.

When screen aspect ratio changes, the screen content is centered with black bars on sides.

On master (2.7), on current dev (including #5370) or on this PR?

but works correctly after #5370, despite the coordinate in generated input events are for the old screen size.

That's unexpected, if the coordinates and the current display size do not match, the event should be discarded with a warning:

Ln.w("Ignore touch event, it was generated for a different device size");

@yume-chan
Copy link
Contributor

On master (2.7), on current dev (including #5370) or on this PR?

All give the same behavior.

That's unexpected, if the coordinates and the current display size do not match, the event should be discarded with a warning:

Because display change event doesn't work, the server doesn't know the sizes are mismatched.

rom1v added a commit that referenced this pull request Oct 30, 2024
If a reset request is pending when a new encoding starts, then it is
implicitly fulfilled.

PR #5415 <#5415>
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
Copy link
Collaborator Author

rom1v commented Oct 30, 2024

I fixed an issue with the "session size" (it stored the downscaled size when -m was passed, but compared to the source display size, instead it must always use the source display size).

And I mentioned the commit which fixed the issue in AOSP in the commit message.

rom1v and others added 2 commits October 31, 2024 20:12
If a reset request is pending when a new encoding starts, then it is
implicitly fulfilled.

PR #5415 <#5415>
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]>
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.

2 participants