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

Add Hextile depth 16 (rgb565) support #1779

Open
linkunfa opened this issue May 19, 2023 · 7 comments
Open

Add Hextile depth 16 (rgb565) support #1779

linkunfa opened this issue May 19, 2023 · 7 comments
Labels

Comments

@linkunfa
Copy link

Is your feature request related to a problem? Please describe.
There is depth parameter in HextileDecoder.decodeRect() functoin, but it is not used. If server only supports Hextile depth 16 (rgb565), then it can not work with noVNC since HextileDecoder only supports depth 24 (rgb888).

Describe the solution you'd like
Add depth 16 support in HextileDecoder, and set the depth based on the server-pixel-format in ServerInit message.

Describe alternatives you've considered
Wonder if this kind of feature is planned to support.

@CendioOssman
Copy link
Member

The VNC protocol requires servers to support all depths, so we've kept noVNC simple by just supporting the standard 24-bit depth. If your server requires 16-bit, then I'm afraid it is buggy.

Which server is this? Is it something embedded?

@linkunfa
Copy link
Author

The server is a Board Management Controller (BMC) running OpenBMC system. It actually supports all depths, but only Hextile depth 16 can use hardware encoding to get better performance because of hardware restrictions.

@CendioOssman
Copy link
Member

I see. I'm afraid it is unlikely we'll add that complexity just to improve performance on those servers, but we can keep this feature request open in case someone wants to see what can be done.

@linkunfa
Copy link
Author

Understand. I've modified HextileDecoder to support depth 16 and verified that it works on those servers: Nuvoton-Israel@f619657

Wonder if this change could be upstream.

@CendioOssman
Copy link
Member

Thanks for a suggested patch!
We'll have a look and get back to you once we've had a closer inspection.

@milomai
Copy link

milomai commented Aug 11, 2023

any update? We need 16-bit supported because we use cellular network to transfer vnc data, reduce cellular data usage is important to us.

@CendioOssman
Copy link
Member

That is non-trivial code. And it also needs to be adjusted more, since the server could now in theory send any codec in 16-bit mode. So I'm not sold. Especially since it is a performance tweak, and not something functional.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants