From 528eac51a3a5af996fd86ec40f908dc8d3a92d9a Mon Sep 17 00:00:00 2001 From: Andri Yngvason Date: Sun, 10 Jul 2022 12:41:18 +0000 Subject: [PATCH] Fix jpeg quality setting The encoding identifiers are supposed to be interpreted as a range. I interpreted the upper and lower limits as two discrete quality settings instead, which is wrong. --- include/common.h | 1 + include/encoder.h | 4 ++-- include/rfb-proto.h | 5 +++-- include/tight.h | 25 ------------------------- src/encoder.c | 6 +++--- src/server.c | 30 ++++++++---------------------- src/tight.c | 24 +++++------------------- 7 files changed, 22 insertions(+), 73 deletions(-) delete mode 100644 include/tight.h diff --git a/include/common.h b/include/common.h index a5184e6..11e8cc3 100644 --- a/include/common.h +++ b/include/common.h @@ -90,6 +90,7 @@ struct nvnc_client { bool is_qemu_key_ext_notified; struct encoder* encoder; uint32_t cursor_seq; + int quality; }; LIST_HEAD(nvnc_client_list, nvnc_client); diff --git a/include/encoder.h b/include/encoder.h index 9e13752..bd5544d 100644 --- a/include/encoder.h +++ b/include/encoder.h @@ -43,7 +43,7 @@ struct encoder_impl { void (*set_output_format)(struct encoder*, const struct rfb_pixel_format*); - void (*set_tight_quality)(struct encoder*, int quality); + void (*set_quality)(struct encoder*, int quality); int (*resize)(struct encoder*, uint16_t width, uint16_t height); @@ -78,7 +78,7 @@ enum encoder_kind encoder_get_kind(const struct encoder* self); void encoder_set_output_format(struct encoder* self, const struct rfb_pixel_format*); -void encoder_set_tight_quality(struct encoder* self, int value); +void encoder_set_quality(struct encoder* self, int value); int encoder_resize(struct encoder* self, uint16_t width, uint16_t height); diff --git a/include/rfb-proto.h b/include/rfb-proto.h index fcec224..d2e651f 100644 --- a/include/rfb-proto.h +++ b/include/rfb-proto.h @@ -63,12 +63,13 @@ enum rfb_encodings { RFB_ENCODING_OPEN_H264 = 50, RFB_ENCODING_CURSOR = -239, RFB_ENCODING_DESKTOPSIZE = -223, - RFB_ENCODING_JPEG_HIGHQ = -23, - RFB_ENCODING_JPEG_LOWQ = -32, RFB_ENCODING_QEMU_EXT_KEY_EVENT = -258, RFB_ENCODING_PTS = -1000, }; +#define RFB_ENCODING_JPEG_HIGHQ -23 +#define RFB_ENCODING_JPEG_LOWQ -32 + enum rfb_server_to_client_msg_type { RFB_SERVER_TO_CLIENT_FRAMEBUFFER_UPDATE = 0, RFB_SERVER_TO_CLIENT_SET_COLOUR_MAP_ENTRIES = 1, diff --git a/include/tight.h b/include/tight.h deleted file mode 100644 index b3ec658..0000000 --- a/include/tight.h +++ /dev/null @@ -1,25 +0,0 @@ -/* - * Copyright (c) 2019 - 2021 Andri Yngvason - * - * Permission to use, copy, modify, and/or distribute this software for any - * purpose with or without fee is hereby granted, provided that the above - * copyright notice and this permission notice appear in all copies. - * - * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES WITH - * REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY - * AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT, - * INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM - * LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE - * OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR - * PERFORMANCE OF THIS SOFTWARE. - */ - -#pragma once - -enum tight_quality { - TIGHT_QUALITY_UNSPEC = 0, - TIGHT_QUALITY_LOSSLESS, - TIGHT_QUALITY_LOW, - TIGHT_QUALITY_HIGH, -}; - diff --git a/src/encoder.c b/src/encoder.c index ed4071f..209c24c 100644 --- a/src/encoder.c +++ b/src/encoder.c @@ -93,10 +93,10 @@ void encoder_set_output_format(struct encoder* self, self->impl->set_output_format(self, pixfmt); } -void encoder_set_tight_quality(struct encoder* self, int value) +void encoder_set_quality(struct encoder* self, int value) { - if (self->impl->set_tight_quality) - self->impl->set_tight_quality(self, value); + if (self->impl->set_quality) + self->impl->set_quality(self, value); } int encoder_resize(struct encoder* self, uint16_t width, uint16_t height) diff --git a/src/server.c b/src/server.c index 6b578fc..1185ef3 100644 --- a/src/server.c +++ b/src/server.c @@ -26,7 +26,6 @@ #include "config.h" #include "usdt.h" #include "encoder.h" -#include "tight.h" #include "enc-util.h" #include "cursor.h" #include "logging.h" @@ -74,7 +73,6 @@ static int send_desktop_resize(struct nvnc_client* client, struct nvnc_fb* fb); static int send_qemu_key_ext_frame(struct nvnc_client* client); static enum rfb_encodings choose_frame_encoding(struct nvnc_client* client, struct nvnc_fb*); -static enum tight_quality client_get_tight_quality(struct nvnc_client* client); static void on_encode_frame_done(struct encoder*, struct rcbuf*, uint64_t pts); static bool client_has_encoding(const struct nvnc_client* client, enum rfb_encodings encoding); @@ -500,6 +498,8 @@ static int on_client_set_encodings(struct nvnc_client* client) sizeof(*msg) + n_encodings * 4) return 0; + client->quality = 10; + for (size_t i = 0; i < n_encodings; ++i) { enum rfb_encodings encoding = htonl(msg->encodings[i]); @@ -514,12 +514,14 @@ static int on_client_set_encodings(struct nvnc_client* client) case RFB_ENCODING_OPEN_H264: case RFB_ENCODING_CURSOR: case RFB_ENCODING_DESKTOPSIZE: - case RFB_ENCODING_JPEG_HIGHQ: - case RFB_ENCODING_JPEG_LOWQ: case RFB_ENCODING_QEMU_EXT_KEY_EVENT: case RFB_ENCODING_PTS: client->encodings[n++] = encoding; } + + if (RFB_ENCODING_JPEG_LOWQ <= encoding && + encoding <= RFB_ENCODING_JPEG_HIGHQ) + client->quality = encoding - RFB_ENCODING_JPEG_LOWQ; } client->n_encodings = n; @@ -717,8 +719,7 @@ static void process_fb_update_requests(struct nvnc_client* client) client_ref(client); - int q = client_get_tight_quality(client); - encoder_set_tight_quality(client->encoder, q); + encoder_set_quality(client->encoder, client->quality); encoder_set_output_format(client->encoder, &client->pixfmt); client->encoder->on_done = on_encode_frame_done; @@ -1109,6 +1110,7 @@ static void on_connection(void* obj) client->ref = 1; client->server = server; + client->quality = 10; /* default to lossless */ int fd = accept(server->fd, NULL, 0); if (fd < 0) { @@ -1367,22 +1369,6 @@ static enum rfb_encodings choose_frame_encoding(struct nvnc_client* client, return RFB_ENCODING_RAW; } -static enum tight_quality client_get_tight_quality(struct nvnc_client* client) -{ - if (client->pixfmt.bits_per_pixel != 16 && - client->pixfmt.bits_per_pixel != 32) - return TIGHT_QUALITY_LOSSLESS; - - for (size_t i = 0; i < client->n_encodings; ++i) - switch (client->encodings[i]) { - case RFB_ENCODING_JPEG_HIGHQ: return TIGHT_QUALITY_HIGH; - case RFB_ENCODING_JPEG_LOWQ: return TIGHT_QUALITY_LOW; - default:; - } - - return TIGHT_QUALITY_LOSSLESS; -} - static bool client_has_encoding(const struct nvnc_client* client, enum rfb_encodings encoding) { diff --git a/src/tight.c b/src/tight.c index 1858315..64d4c37 100644 --- a/src/tight.c +++ b/src/tight.c @@ -19,7 +19,6 @@ #include "common.h" #include "pixels.h" #include "vec.h" -#include "tight.h" #include "config.h" #include "enc-util.h" #include "fb.h" @@ -66,7 +65,7 @@ struct tight_encoder { uint32_t height; uint32_t grid_width; uint32_t grid_height; - enum tight_quality quality; + int quality; struct tight_tile* grid; @@ -350,13 +349,7 @@ static int tight_encode_tile_jpeg(struct tight_encoder* self, unsigned char* buffer = NULL; unsigned long size = 0; - int quality; /* 1 - 100 */ - - switch (self->quality) { - case TIGHT_QUALITY_HIGH: quality = 66; break; - case TIGHT_QUALITY_LOW: quality = 33; break; - default: abort(); - } + int quality = 11 * self->quality + 1; uint32_t fourcc = nvnc_fb_get_fourcc_format(self->fb); enum TJPF tjfmt = tight_get_jpeg_pixfmt(fourcc); @@ -411,17 +404,10 @@ static void tight_encode_tile(struct tight_encoder* self, tile->size = 0; #ifdef HAVE_JPEG - switch (self->quality) { - case TIGHT_QUALITY_LOSSLESS: + if (self->quality >= 10) { tight_encode_tile_basic(self, tile, x, y, width, height, gx % 4); - break; - case TIGHT_QUALITY_HIGH: - case TIGHT_QUALITY_LOW: - // TODO: Use more workers for jpeg + } else { tight_encode_tile_jpeg(self, tile, x, y, width, height); - break; - case TIGHT_QUALITY_UNSPEC: - abort(); } #else tight_encode_tile_basic(self, tile, x, y, width, height, gx % 4); @@ -620,7 +606,7 @@ static int tight_encoder_encode(struct encoder* encoder, struct nvnc_fb* fb, struct encoder_impl encoder_impl_tight = { .destroy = tight_encoder_destroy_wrapper, .set_output_format = tight_encoder_set_output_format, - .set_tight_quality = tight_encoder_set_quality, + .set_quality = tight_encoder_set_quality, .resize = tight_encoder_resize_wrapper, .encode = tight_encoder_encode, };