Skip to content

Conversation

@Hocuri
Copy link
Collaborator

@Hocuri Hocuri commented Apr 29, 2021

  • Currently, group images are compressed as well because it was easier to implement that way.

  • Currently, in the unlikely case that the avatar is compressed down to 20x20 pixels but still bigger than 20KB, the user doesn't get any indication of this, the avatar simply isn't changed (at least on Android).

    If we want to change this, the easiest way is probably to let dc_set_config() in the ffi call error!() if Selfavatar can't be set. The same might make sense for some or all other configs. BUUUUUT: At least Android doesn't show error!() toasts anymore, probably because they were used too often and too spammy.

  • The factor by which we scale down if the file is too big is 1.5.

I hope everything is clear 😂 there were surprisingly many small problems.

@link2xt
Copy link
Collaborator

link2xt commented May 1, 2021

@Hocuri I have merged #2232, could you rebase this PR on top of master?

@Hocuri Hocuri force-pushed the chat-user-avatar-base64-20kb branch from d35795a to f79c652 Compare May 1, 2021 08:31
src/blob.rs Outdated
mut blob_abs: PathBuf,
mut img_wh: u32,
max_bytes: Option<usize>,
) -> Result<String, BlobError> {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's better to explicitly return Option<String> and use None instead of special empty string value.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have pushed this change to this branch

@Hocuri Hocuri changed the title [WIP] Compress avatar to below 20k Compress avatar to below 20k May 2, 2021
@Hocuri Hocuri merged commit b7864f2 into master May 2, 2021
@Hocuri Hocuri deleted the chat-user-avatar-base64-20kb branch May 2, 2021 17:54
@r10s
Copy link
Contributor

r10s commented May 2, 2021

nice, just sent avatars with outlook :)

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.

4 participants