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

allow doubled avatar resolution #6611

Merged
merged 1 commit into from
Mar 4, 2025
Merged

allow doubled avatar resolution #6611

merged 1 commit into from
Mar 4, 2025

Conversation

r10s
Copy link
Contributor

@r10s r10s commented Mar 3, 2025

up to now, avatars were restricted to 20k, 256x256 pixel. resulting in often poor avatar quality.
(the limitation comes from times
where unencrypted outlook inner headers were a thing; this has changed meanwhile)

to increase quality while keeping a reasonable small size, for "balanced quality",
we increase the allowed width to 512x512 and the allowed size to 60k.

for "worse quality", things stay unchanged at 128x128 pixel and 20k.

up to now, avatars were restricted to 20k, 256x256 pixel.
resulting in often poor avatar quality.
(the limitation comes from times
where unencrypted outlook inner headers were a thing;
this has changed meanwhile)

to increase quality while keeping a reasonable small size,
for "balanced quality",
we increase the allowed width to 512x512 and the allowed size to 60k.

for "worse quality", things stay unchanged at 128x128 pixel and 20k.
@r10s r10s requested review from link2xt, adbenitez and iequidoo March 3, 2025 22:28
@iequidoo
Copy link
Collaborator

iequidoo commented Mar 4, 2025

Why not send avatars in the same size as images? The avatar (500 kB maximum) will be sent to every chat eventually, but if the user wants to save traffic, they can resize the original image. I even created a PR some time ago, #5863, it should be either revived or closed

@r10s
Copy link
Contributor Author

r10s commented Mar 4, 2025

if the user wants to save traffic, they can resize the original image.

we do not expect the average user to think about that. so it is up to us to use a reasonable amount of traffic and to not be wasteful

as the avatar is sent around more frequently as images, it seems reasonable to not go from 20k to 500k, but to sth in between and more moderate. idea is to show the avatar in the profile larger, let's see if that is sufficient, in some tests it was not bad. we can always increase further :)

@r10s r10s merged commit d6209e0 into main Mar 4, 2025
29 checks passed
@r10s r10s deleted the r10s/higher-avatar-quality branch March 4, 2025 21:23
@iequidoo
Copy link
Collaborator

iequidoo commented Mar 4, 2025

Anyway, i'd suggest one more simplification -- to remove a linear size limit (i.e. use one for images, 1280 px), the byte size limit should be sufficient

@r10s
Copy link
Contributor Author

r10s commented Mar 4, 2025

maybe. however, trying to encode 1280x1280 px to 60 kb will fail probably often, so that one ends up in <= 512x512 px anyways. so, not sure if that is worth the refactoring

@iequidoo
Copy link
Collaborator

iequidoo commented Mar 6, 2025

maybe. however, trying to encode 1280x1280 px to 60 kb will fail probably often, so that one ends up in <= 512x512 px anyways. so, not sure if that is worth the refactoring

I've found some cropped photos on my phone which are ~60 kB, but greater than 512x512, so it seems this limit should be lifted at least, but removing it at all would simplify the code.

EDIT: Finally i think 512 px are fine, greater limit would cause more iterations while trying to fit into 60 kB.

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.

2 participants