sibson / vncdotool

A command line VNC client and python library
Other
458 stars 123 forks source link

Improve Pixelformat handling #243

Closed pmhahn closed 1 year ago

pmhahn commented 1 year ago

This is my result from looking at #205, which tried to force ARD to use BRG;16 instead of RGB32. This PR introduces a new class PixelFormat based on dataclasses to pass around those many values of PIXEL_FORMAT.

As usual I found some more issues on my way:

The last one is problematic and incomplete, but I currently have no idea myself how to fix it for real: The bigger picture is that currently stream handling is wrong in several cases: As neither TCP nor the RFB/VNC protocol defines any framing, all messages passed from client-to-server and server-to-client must be understood and handled. While testing with gvncviewer as my client connecting LibVNCServer-0.9.14/examples/pnmshow24 as my server via vnclog I noticed, that they negotiated many more pseudo encoding than vnclog understands and can handle:

The pnmshow24 server supports many of them, but not all:

rfbProcessClientNormalMessage: ignoring unsupported encoding type Enc(0xFFFFFEFE)
rfbProcessClientNormalMessage: ignoring unsupported encoding type Enc(0xFFFFFEFB)
Enabling NewFBSize protocol extension for client 127.0.0.1
rfbProcessClientNormalMessage: ignoring unsupported encoding type Enc(0x574D5669)
rfbProcessClientNormalMessage: ignoring unsupported encoding type Enc(0xFFFFFEFD)
Enabling full-color cursor updates for client 127.0.0.1
Enabling X-style cursor updates for client 127.0.0.1
rfbProcessClientNormalMessage: ignoring unsupported encoding type Enc(0xFFFFFEFF)
Using tight encoding for client 127.0.0.1

The last one is problematic as this results in a FramebufferUpdate-Message with the Encoding.TIGHT pseudo-encoding, which vncdotool cannot handle:

INFO:twisted:x=3 y=3 width=8 height=7 <Encoding.PSEUDO_CURSOR: -239>
INFO:twisted:x=0 y=0 width=256 height=256 <Encoding.TIGHT: 7>
INFO:twisted:unknown encoding received <Encoding.TIGHT: 7>
INFO:twisted:x=181 y=48643 width=30721 height=60669 '<Encoding.UNKNOWN: 5785c57>'

Afterwards vnclog is out-of-synchronisation and starts logging many errors as most following bytes are seen as an unknown Server-to-Client message ID.

To handle this correctly vnclog needs to sit in-between of the client and the server and must rewrite some of those messages to filter out any AuthType, Encoding, … vncdotool does not understand.

That's too much for me to do as of now, but someone might work on that. At least the PR improves the situation, but there is more work to do. Feel free to merge now and create new issues for the newly identified issues — or delay this PR until we have some idea on how to fix them.

sibson commented 1 year ago

Better to have some forward progress we can build on than wait for perfect.