WEBPs are thumbnailed as JPEGs, losing transparency/animation

[hiddeninthesand]

Description

Exactly as it sounds, WEBPs that have transparency get thumbnailed as JPEGs, making them look terrible. Tracked it down to #4382, where WEBPs were universally marked to use JPEG for thumbnailing instead of PNGs, or WEBPs, though I assume that last one is more of a Synapse limitation than an oversight. Intended behavior is for transparent images to have transparent thumbnails, including WEBPs.

Steps to reproduce

• Post a transparent WEBP into an unencrypted room
• See a terrible blocky black background

Version information

• Homeserver:

Homeserver was matrix.org

• Version: 1.51.0rc1 (b=matrix-org-hotfixes,7977b7f6a)

• Install method: Not sure what they do.

• Platform: Not sure.

[clokep]

#7586 (comment) has some thoughts on why this was chosen.

[hiddeninthesand]

Correct me if I'm wrong, but the comment you linked to says that small size should be chosen over quality, which I agree with, but removing transparency drastically alters a thumbnail compared to the source, and this often isn't even done well. Given how many other services are switching to using WEBPs, users will probably start noticing this deficiency more and more often. I agree that they should be treated as two different algorithms, but as it is right now this should air on the side of caution.

[clokep]

It quite possibly makes sense to check for transparency before just choosing to use jpegs, that would be a nice improvement.

I was mostly attempting to cross-link to the previous conversation about why those defaults were chosen!

[hiddeninthesand]

I see, thanks for the context though, it definitely helps illuminate the decision making!

[clokep]

As noted in #11899, this also causes lose of animation.

[hiddeninthesand]

Is this an enhancement? I thought this was a bug.

[clokep]

Is this an enhancement? I thought this was a bug.

This is currently acting as designed, the description for enhancement vs. defect might help:

Enhancement: New features, changes in functionality, improvements in performance, or user-facing enhancements.

Defect: Bugs, crashes, hangs, security vulnerabilities, or other reported issues.

This change probably lives somewhere in-between though and it is a bit of judgement call.

[ashkitten]

this is still an issue, particularly in conjuction with matrix-appservice-discord which uses webp for user avatars. matrix-media-repo handles this just fine, using png thumbnails for webp images. if you absolutely need lossy compression, you could thumbnail as webp in lossy mode (or jpeg-xl, which supports transparency and is slowly gaining support), but either way it feels very unacceptable to be turning transparent images into black blocky messes.

[tastytea]

This is really bad in conjunction with WebP emojis in MSC2545: Image Packs. 😢

This patch seems to work fine:

thumbnail-WebP-as-WebP.patch

From 0817ab5338858f1fb605b16813ce53d568bfca9f Mon Sep 17 00:00:00 2001
Date: Thu, 27 Oct 2022 18:44:06 +0200
Subject: [PATCH] thumbnail WebP as WebP

---
 synapse/config/repository.py         | 6 +++++-
 synapse/rest/media/v1/thumbnailer.py | 2 +-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/synapse/config/repository.py b/synapse/config/repository.py
index e4759711e..6701aad7c 100644
--- a/synapse/config/repository.py
+++ b/synapse/config/repository.py
@@ -47,7 +47,7 @@ THUMBNAIL_SIZE_YAML = """\
 THUMBNAIL_SUPPORTED_MEDIA_FORMAT_MAP = {
     "image/jpeg": "jpeg",
     "image/jpg": "jpeg",
-    "image/webp": "jpeg",
+    "image/webp": "webp",
     # Thumbnails can only be jpeg or png. We choose png thumbnails for gif
     # because it can have transparency.
     "image/gif": "png",
@@ -102,6 +102,10 @@ def parse_thumbnail_requirements(
                 requirement.append(
                     ThumbnailRequirement(width, height, method, "image/png")
                 )
+            elif thumbnail_format == "webp":
+                requirement.append(
+                    ThumbnailRequirement(width, height, method, "image/webp")
+                )
             else:
                 raise Exception(
                     "Unknown thumbnail mapping from %s to %s. This is a Synapse problem, please report!"
diff --git a/synapse/rest/media/v1/thumbnailer.py b/synapse/rest/media/v1/thumbnailer.py
index 9b93b9b4f..5ed7f5257 100644
--- a/synapse/rest/media/v1/thumbnailer.py
+++ b/synapse/rest/media/v1/thumbnailer.py
@@ -39,7 +39,7 @@ class ThumbnailError(Exception):

 class Thumbnailer:

-    FORMATS = {"image/jpeg": "JPEG", "image/png": "PNG"}
+    FORMATS = {"image/jpeg": "JPEG", "image/png": "PNG", "image/webp": "WEBP"}

     @staticmethod
     def set_limits(max_image_pixels: int) -> None:
--
2.37.3

[HarHarLinks]

Related: matrix-org/matrix-spec#453

I was about to say that some platforms (looking at iOS) may not support WebP, but it seems that it is supported since iOS 14.