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

Force Discord avatars to be PNGs in a friendly matter #792

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

Miepee
Copy link
Contributor

@Miepee Miepee commented Jun 7, 2022

This can be considered a hack in order to fix #788

It was discussed a bit in #help:t2bot.io on what the root cause of the issue might be, and it was found out that the bridge creates webp thumbnails for users on t2bot.io, which then often (not always for some reason) aren't mirrored to matrix.org (and potentially other HS), causing Clients to just display a default/broken avatar.

This PR circumvents the root cause, by just forcing all avatars to be in PNG format, as then thumbnails get mirrored correctly.
Only users who update their avatar will create revalidation. This is done in order to not revalidate every user currently existing on the bridge which would cause quite a bit of network traffic.

An alternative solution would be to implement #743; have a config file with a wished image file format, which is then used for checking the discord profile picture. I however am not that proficient with JS nor can I self-host/test my changes, hence this primitive solution.
Even better would be to figure out why the thumbnails aren't generates/mirrored in the first place. Possibly is related to #770, but not sure.

Either way, I'm opening this PR so that people who are self-hosting and are encountering the issue have a solution they can apply until the underlying root causes can be found and properly fixed.

@AndrewFerr
Copy link
Member

WebP thumbnails work, but requires installing an extra library. On Debian, it's libwebp-dev, and it fixed the problem for me on my own instance of this bridge. I can't recall if it was the bridge or Synapse that was failing without the library, though.

If that still works, we can just document that library as an optional dependency & make PNG-only thumbnails a configuration setting.

And IIRC Discord sometimes sends image attachments as WebP too, not just avatars.

@Miepee
Copy link
Contributor Author

Miepee commented Jun 9, 2022

And IIRC Discord sometimes sends image attachments as WebP too, not just avatars.

Unless there's something new I'm not aware of, image attachments are always send as-is, so it's dependent on what the user sends, not Discord. And thumbnails are just generated via passing width and height parameters to the attachment URL.

Regarding libwebp am not sure about that one. The bridge did create webp thumbnails on the HS it was hosted on (in this case t2bot.io), they just weren't federated to other homeservers (I.e. the t2bot thumbnail exists, but matrix.org thumbnail does not).
Is that what the dependency is needed for, or would it be needed to create the thumbnails and upload it to the HS the bot is hosted on in the first place?

@AndrewFerr
Copy link
Member

Unless there's something new I'm not aware of, image attachments are always send as-is, so it's dependent on what the user sends, not Discord.

Ah, that makes sense. Though that suggests the bridge/homeservers ought to support WebP, lest images fail to be bridged.

The bridge did create webp thumbnails on the HS it was hosted on (in this case t2bot.io), they just weren't federated to other homeservers

It might be that Synapse is what requires the library, not the bridge. That would explain federation issues like what you've described (i.e. any homeserver that doesn't have the library will fail). I'll try to reproduce the problem to refresh my memory on what the actual error is.

@Miepee
Copy link
Contributor Author

Miepee commented Jun 10, 2022

Also worth noting that this issue only occurred "recently". It used to federate them just fine a month or so ago, and then it just suddenly... stopped. Which still makes me question libwebps role in it, since from my experience at least, people don't just remove dependencies for a running homeserver.

@Damaj301damaj-lol
Copy link

Seems like the matrix.org homeserver disabled webp support, which considering it is the biggest homeserver, it cannot be forgiven, so using the patch works

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

Successfully merging this pull request may close these issues.

Profile pictures of Discord users not showing up on Matrix
3 participants