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

Mouse clicks don't work #2190

Open
2 tasks done
CnoJenesis opened this issue Mar 11, 2021 · 15 comments
Open
2 tasks done

Mouse clicks don't work #2190

CnoJenesis opened this issue Mar 11, 2021 · 15 comments

Comments

@CnoJenesis
Copy link

  • I have read the FAQ.

  • I have searched in existing issues.

  • OS: Windows

  • scrcpy version: v1.17

  • installation method: Windows release

  • device model: Oppo F7 CPH1819

  • Android version: 10

Everytime I try to click on my screen it says "WARN: Ignore touch event, it was generated for a different device size." Keyboard inputs do work for some reason. My phone's screen resolution is 1080x2280 but it only displays 910x1920. When I use scrcpy -m 1920 it works but it doesn't when I do scrcpy -m 2280.

@rom1v
Copy link
Collaborator

rom1v commented Mar 11, 2021

When I use scrcpy -m 1920 it works

Yes, that's the correct workaround.

See #1645 (comment) for more technical details.

TL;DR: the device encoder decides to encode at a different resolution from the one requested, and we can't know.

@LBensman
Copy link

@rom1v, I haven't looked into your decoding code, so pardon if I'm off-base here, but H.264 is able to carry resolution changes (even mid-stream), and decoder should be alerting of the event. In CUDA APIs this is done via callback that is raised when NAL Unit is passed into cuvidParseVideoData(...) and it calls back into onSequenceCallBack(CUVIDEOFORMAT*) handler with parameter containing format changes. There you can detect a change in resolution (along with chroma and codec changes), and adjust to the new resolution (with CUDA it would be destroying current and creating new decoder).

I wonder if that's what may be happening when device's encoder refuses requested resolution and supplies its own.

@rom1v
Copy link
Collaborator

rom1v commented Mar 11, 2021

@LBensman Yes, the decoder on the client-side knows. Btw, the scrcpy window is automatically resized:

INFO: Initial texture: 1080x2336
INFO: New texture: 888x1920

But the device itself does not parse or decode the video stream (it encodes it), it just knows the device size and the size parameters it gives to the encoder. To avoid mismatch events when the device is rotated (for ex you click on a landscape window, but the device just got rotated to portrait when it handles the events), the events generated for a different device size are ignored. Hence the problem if the encoder decides to use another resolution on its own.

@LBensman
Copy link

@rom1v, oh, I think I misread the problem. Are you saying that because encoder changes the resolution it encodes to a one that's different from the actual device resolution, when that resolution is used when sending input touch commands, it's ignored b/c it doesn't match the actual device resolution?

@rom1v
Copy link
Collaborator

rom1v commented Mar 11, 2021

Yes, that's it.

(with "device resolution" replaced by "requested resolution", for example if you explicitly pass -m)

@LBensman
Copy link

Oh, I see, and so rotation events you pick up from the resolution change in decoded stream and use it back for input. So the issue is that stream resolution is overloaded with input resolution and encoder's change invalidates the input one.

Short of implementing a whole protocol on the stream to make it able to mux H.264 with w/e other output would be useful from the device, I have a rough suggestion for the fix: retain resolution you receive in your header on initial receipt of data on connect, and flip the w<=>h whenever you detect flip of dimensions in the decode stream, but don't use those values. So, going by above example (mixing very pseudo code with actual sample values 😜):

/// Initial texture: 1080x2336
devRes = {1080, 2336}
orientation=devRes.w/devRes.h;
/// New texture: 888x1920
onDecoderResolutionChange(streamRes) => {
    if (orientation != streamRes.w/streamRes.h) {
        swap(devRes.w, devRes.h);
        orientation=devRes.w/devRes.h;
    }
};

This way you'll send touch commands in device's resolution and still maintain check that orientation is the target on device side.

@rom1v
Copy link
Collaborator

rom1v commented Mar 11, 2021

This is an error to send events for a size different than expected. It could be due to orientation changes, or ratio changes (foldable devices?) or whatever.

To fix the problem, the server should get a way to know that the encoder decided to encode to a different resolution on its own.

But I think it's just a device encoder bug, other encoders typically fail if they can't support the resolution. And the solution is the same: limit the resolution to what the encoder supports.

It could be great to automatically detect that, but in practice I have no reliable solution.

@yume-chan
Copy link
Contributor

It could be due to orientation changes, or ratio changes (foldable devices?) or whatever.

But shouldn't the client always know when resolution changes, so the reported touch coordinates are always correct?

I mean, I wonder in which situation a well-behaved client may still send touch events for a different resolution.

@rom1v
Copy link
Collaborator

rom1v commented Mar 18, 2021

But shouldn't the client always know when resolution changes, so the reported touch coordinates are always correct?

The client always knows the resolution when it generates the events, but it may have changed before the event is injected to the device.

device       R  T
   \  \  \  \ \/
    \  \  \  \/\
     \  \  \ /\ \
      \  \  X  \ \
client     t
  • \ are frames sent from the device to the client
  • R is a device rotation
  • t is a touch event generated on the client
  • T is the touch event received and injected to the device

t is generated for the old rotation, but injected (T) once the device is rotated.

The correct solution for the device encoder producing a different resolution is probably to check codec capabilities beforehand. It requires some refactors on the server though, because currently the requested size is not supposed to change.

@yume-chan
Copy link
Contributor

Since the server re-initialize the encoder after device rotation, how about sending the "correct" resolution to client through the control stream?

image

@rom1v
Copy link
Collaborator

rom1v commented Mar 18, 2021

That does not change anything.

How do you detect that the touch is invalid on your schema (to "throw away")?

@yume-chan
Copy link
Contributor

yume-chan commented Mar 18, 2021

The server still only tracks the "correct" resolution. But the client now have both video resolution and "correct" resolution.

It could be great to automatically detect that, but in practice I have no reliable solution.

So now the client knows that the encoder encodes video in different resolution. It can map mouse coordinates from video resolution to "correct" resolution (if it still adjusts its window size according to video resolution) and send to server.

For the timing issue, the server must send resolution change event after video stream in new resolution has started. Between this short period of time, the client will map mouse coordinates incorrectly, but these touch events contains incorrect resolution so those will be thrown away anyway.

EDIT: Or the client can ignore video aspect ratio completely, adjust its window size according to "correct" resolution, and stretch the video to fill.

@rom1v
Copy link
Collaborator

rom1v commented Mar 18, 2021

IIUC, you say that the client should notify the server that the resolution has changed? This should not be necessary, assuming that the encoder fulfil the resolution requested by the server. In practice, it does not for very few devices, but I think that we can detect that before running the encoding (as I said at the end of #2190 (comment)).

EDIT: Or the client can ignore video aspect ratio completely, adjust its window size according to "correct" resolution, and stretch the video to fill.

This is what I wanted to avoid (because it could click at a wrong location).

@LBensman
Copy link

To fix the problem, the server should get a way to know that the encoder decided to encode to a different resolution on its own.

Encoder is just doing its thing, and is only about getting a visual of the device screen, and should not be required to match physical dimensions (i.e. it being buggy, or limited power of encoding hardware, etc), and thus it's problematic (albeit convenient) to rely on it on communicating true device dimensions. So it sounds like to solve it, it needs decoupling of "control dimensions" from "encode dimensions." I.e. it sounds like it needs a separate bidirectional channel to send commands to server and for server to communicate its events back to client, independent of the display channel that the encoder is shipping its data on.

@zhubinsheng
Copy link

m

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

No branches or pull requests

5 participants