You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@guacamole.apache.org by GitBox <gi...@apache.org> on 2021/01/06 08:36:06 UTC

[GitHub] [guacamole-server] mike-jumper commented on a change in pull request #322: Force fast WebP compression

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:
users@infra.apache.org