mike-jumper commented on a change in pull request #322:
URL: https://github.com/apache/guacamole-server/pull/322#discussion_r552435569
##########
File path: src/libguac/encode-webp.c
##########
@@ -192,10 +192,10 @@ int guac_webp_write(guac_socket* socket, guac_stream*
stream,
return -1;
/* Add additional tuning */
- config.lossless = lossless;
- config.quality = quality;
- config.thread_level = 1; /* Multi threaded */
- config.method = 2; /* Compression method (0=fast/larger, 6=slow/smaller) */
+ config.lossless = 1; /* force lossless */
Review comment:
The `guac_webp_write()` function should definitely not ignore its
`lossless` parameter entirely, as that's essentially crippling a library
function. If lossless WebP should be used across the board wherever WebP would
be used, it should be the surface implementation making that decision, not the
internals of the generic WebP writer.
##########
File path: src/libguac/encode-webp.c
##########
@@ -192,10 +192,10 @@ int guac_webp_write(guac_socket* socket, guac_stream*
stream,
return -1;
/* Add additional tuning */
- config.lossless = lossless;
- config.quality = quality;
- config.thread_level = 1; /* Multi threaded */
- config.method = 2; /* Compression method (0=fast/larger, 6=slow/smaller) */
+ config.lossless = 1; /* force lossless */
+ config.quality = 30.f; /* prefer fast encoding over size */
Review comment:
The `quality` parameter should not be ignored, either. If `30.f` is the
desired value here, shouldn't that be what's passed in?
##########
File path: src/libguac/encode-webp.c
##########
@@ -192,10 +192,10 @@ int guac_webp_write(guac_socket* socket, guac_stream*
stream,
return -1;
/* Add additional tuning */
- config.lossless = lossless;
- config.quality = quality;
- config.thread_level = 1; /* Multi threaded */
- config.method = 2; /* Compression method (0=fast/larger, 6=slow/smaller) */
+ config.lossless = 1; /* force lossless */
+ config.quality = 30.f; /* prefer fast encoding over size */
+ config.thread_level = 0; /* Disable Multi thread */
Review comment:
Why?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
