feat: Send avatars as large as usual images when possible

src/blob.rs

@@ -1,6 +1,6 @@
 //! # Blob directory management.
 
-use core::cmp::max;
+use std::cmp::{max, min};
 use std::ffi::OsStr;
 use std::fmt;
 use std::io::{Cursor, Seek};
@@ -14,13 +14,10 @@ use futures::StreamExt;
 use image::codecs::jpeg::JpegEncoder;
 use image::ImageReader;
 use image::{DynamicImage, GenericImage, GenericImageView, ImageFormat, Pixel, Rgba};
-use num_traits::FromPrimitive;
 use tokio::io::AsyncWriteExt;
 use tokio::{fs, io};
 use tokio_stream::wrappers::ReadDirStream;
 
-use crate::config::Config;
-use crate::constants::{self, MediaQuality};
 use crate::context::Context;
 use crate::events::EventType;
 use crate::log::LogExt;
@@ -346,28 +343,33 @@ impl<'a> BlobObject<'a> {
         Ok(blob.as_name().to_string())
     }
 
-    pub async fn recode_to_avatar_size(&mut self, context: &Context) -> Result<()> {
+    /// Recodes an avatar pointed by a [BlobObject] so that it fits into limits on the image width,
+    /// height and file size specified by the config.
+    ///
+    /// * `protected`: Whether the resulting avatar is going to be used in a protected context,
+    ///   i.e. in a protected chat or stored locally, and therefore may be larger.
+    pub async fn recode_to_avatar_size(
+        &mut self,
+        context: &Context,
+        protected: bool,
+    ) -> Result<()> {
         let blob_abs = self.to_abs_path();
-
-        let img_wh =
-            match MediaQuality::from_i32(context.get_config_int(Config::MediaQuality).await?)
-                .unwrap_or_default()
-            {
-                MediaQuality::Balanced => constants::BALANCED_AVATAR_SIZE,
-                MediaQuality::Worse => constants::WORSE_AVATAR_SIZE,
-            };
-
-        let maybe_sticker = &mut false;
-        let strict_limits = true;
+        let (img_wh, max_bytes) = context.max_image_wh_and_size().await?;
         // max_bytes is 20_000 bytes: Outlook servers don't allow headers larger than 32k.
         // 32 / 4 * 3 = 24k if you account for base64 encoding. To be safe, we reduced this to 20k.
+        let max_bytes = match protected {
+            false => min(max_bytes, 20_000),
+            true => max_bytes,
+        };
+        let maybe_sticker = &mut false;
+        let is_avatar = true;
         if let Some(new_name) = self.recode_to_size(
             context,
             blob_abs,
             maybe_sticker,
             img_wh,
-            20_000,
-            strict_limits,
+            max_bytes,
+            is_avatar,
         )? {
             self.name = new_name;
         }
@@ -387,43 +389,32 @@ impl<'a> BlobObject<'a> {
         maybe_sticker: &mut bool,
     ) -> Result<()> {
         let blob_abs = self.to_abs_path();
-        let (img_wh, max_bytes) =
-            match MediaQuality::from_i32(context.get_config_int(Config::MediaQuality).await?)
-                .unwrap_or_default()
-            {
-                MediaQuality::Balanced => (
-                    constants::BALANCED_IMAGE_SIZE,
-                    constants::BALANCED_IMAGE_BYTES,
-                ),
-                MediaQuality::Worse => (constants::WORSE_IMAGE_SIZE, constants::WORSE_IMAGE_BYTES),
-            };
-        let strict_limits = false;
+        let (img_wh, max_bytes) = context.max_image_wh_and_size().await?;
+        let is_avatar = false;
         if let Some(new_name) = self.recode_to_size(
             context,
             blob_abs,
             maybe_sticker,
             img_wh,
             max_bytes,
-            strict_limits,
+            is_avatar,
         )? {
             self.name = new_name;
         }
         Ok(())
     }
 
-    /// If `!strict_limits`, then if `max_bytes` is exceeded, reduce the image to `img_wh` and just
-    /// proceed with the result.
     fn recode_to_size(
         &mut self,
         context: &Context,
         mut blob_abs: PathBuf,
         maybe_sticker: &mut bool,
         mut img_wh: u32,
         max_bytes: usize,
-        strict_limits: bool,
+        is_avatar: bool,
     ) -> Result<Option<String>> {
         // Add white background only to avatars to spare the CPU.
-        let mut add_white_bg = img_wh <= constants::BALANCED_AVATAR_SIZE;
+        let mut add_white_bg = is_avatar;
         let mut no_exif = false;
         let no_exif_ref = &mut no_exif;
         let res = tokio::task::block_in_place(move || {
@@ -493,7 +484,7 @@ impl<'a> BlobObject<'a> {
             // also `Viewtype::Gif` (maybe renamed to `Animation`) should be used for animated
             // images.
             let do_scale = exceeds_max_bytes
-                || strict_limits
+                || is_avatar
                     && (exceeds_wh
                         || exif.is_some() && {
                             if mem::take(&mut add_white_bg) {
@@ -530,7 +521,7 @@ impl<'a> BlobObject<'a> {
                         ofmt.clone(),
                         max_bytes,
                         &mut encoded,
-                    )? && strict_limits
+                    )? && is_avatar
                     {
                         if img_wh < 20 {
                             return Err(format_err!(
@@ -579,7 +570,7 @@ impl<'a> BlobObject<'a> {
         match res {
             Ok(_) => res,
             Err(err) => {
-                if !strict_limits && no_exif {
+                if !is_avatar && no_exif {
                     warn!(
                         context,
                         "Cannot recode image, using original data: {err:#}.",
@@ -761,6 +752,8 @@ mod tests {
 
     use super::*;
     use crate::chat::{self, create_group_chat, ProtectionStatus};
+    use crate::config::Config;
+    use crate::constants::{self, MediaQuality};
     use crate::message::{Message, Viewtype};
     use crate::test_utils::{self, TestContext};
 
@@ -984,14 +977,14 @@ mod tests {
             let mut blob = BlobObject::new_from_path(&t, &avatar_src).await.unwrap();
             let img_wh = 128;
             let maybe_sticker = &mut false;
-            let strict_limits = true;
+            let is_avatar = true;
             blob.recode_to_size(
                 &t,
                 blob.to_abs_path(),
                 maybe_sticker,
                 img_wh,
                 20_000,
-                strict_limits,
+                is_avatar,
             )
             .unwrap();
             tokio::task::block_in_place(move || {
@@ -1015,16 +1008,18 @@ mod tests {
             .await
             .unwrap();
         assert!(avatar_blob.exists());
-        assert!(fs::metadata(&avatar_blob).await.unwrap().len() < avatar_bytes.len() as u64);
+        {
+            let avatar_blob = &avatar_blob;
+            tokio::task::block_in_place(move || {
+                let (_, exif) = image_metadata(&std::fs::File::open(avatar_blob).unwrap()).unwrap();
+                assert!(exif.is_none());
+            });
+        }
         let avatar_cfg = t.get_config(Config::Selfavatar).await.unwrap();
         assert_eq!(avatar_cfg, avatar_blob.to_str().map(|s| s.to_string()));
 
         check_image_size(avatar_src, 1000, 1000);
-        check_image_size(
-            &avatar_blob,
-            constants::BALANCED_AVATAR_SIZE,
-            constants::BALANCED_AVATAR_SIZE,
-        );
+        check_image_size(&avatar_blob, 1000, 1000);
 
         async fn file_size(path_buf: &Path) -> u64 {
             let file = File::open(path_buf).await.unwrap();
@@ -1033,16 +1028,9 @@ mod tests {
 
         let mut blob = BlobObject::new_from_path(&t, &avatar_blob).await.unwrap();
         let maybe_sticker = &mut false;
-        let strict_limits = true;
-        blob.recode_to_size(
-            &t,
-            blob.to_abs_path(),
-            maybe_sticker,
-            1000,
-            3000,
-            strict_limits,
-        )
-        .unwrap();
+        let is_avatar = true;
+        blob.recode_to_size(&t, blob.to_abs_path(), maybe_sticker, 1000, 3000, is_avatar)
+            .unwrap();
         assert!(file_size(&avatar_blob).await <= 3000);
         assert!(file_size(&avatar_blob).await > 2000);
         tokio::task::block_in_place(move || {
@@ -1071,11 +1059,7 @@ mod tests {
             avatar_src.with_extension("png").to_str().unwrap()
         );
 
-        check_image_size(
-            avatar_cfg,
-            constants::BALANCED_AVATAR_SIZE,
-            constants::BALANCED_AVATAR_SIZE,
-        );
+        check_image_size(avatar_cfg, 900, 900);
     }
 
     #[tokio::test(flavor = "multi_thread", worker_threads = 2)]

src/chat.rs

@@ -4106,7 +4106,9 @@ pub async fn set_chat_profile_image(
         msg.text = stock_str::msg_grp_img_deleted(context, ContactId::SELF).await;
     } else {
         let mut image_blob = BlobObject::new_from_path(context, Path::new(new_image)).await?;
-        image_blob.recode_to_avatar_size(context).await?;
+        image_blob
+            .recode_to_avatar_size(context, chat.is_protected() || chat.is_self_talk())
+            .await?;
         chat.param.set(Param::ProfileImage, image_blob.as_name());
         msg.param.set(Param::Arg, image_blob.as_name());
         msg.text = stock_str::msg_grp_img_changed(context, ContactId::SELF).await;

src/config.rs

@@ -7,13 +7,14 @@ use std::str::FromStr;
 use anyhow::{ensure, Context as _, Result};
 use base64::Engine as _;
 use deltachat_contact_tools::{addr_cmp, sanitize_single_line};
+use num_traits::FromPrimitive;
 use serde::{Deserialize, Serialize};
 use strum::{EnumProperty, IntoEnumIterator};
 use strum_macros::{AsRefStr, Display, EnumIter, EnumString};
 use tokio::fs;
 
 use crate::blob::BlobObject;
-use crate::constants;
+use crate::constants::{self, MediaQuality};
 use crate::context::Context;
 use crate::events::EventType;
 use crate::log::LogExt;
@@ -537,6 +538,18 @@ impl Context {
         }
     }
 
+    pub(crate) async fn max_image_wh_and_size(&self) -> Result<(u32, usize)> {
+        match MediaQuality::from_i32(self.get_config_int(Config::MediaQuality).await?)
+            .unwrap_or_default()
+        {
+            MediaQuality::Balanced => Ok((
+                constants::BALANCED_IMAGE_SIZE,
+                constants::BALANCED_IMAGE_BYTES,
+            )),
+            MediaQuality::Worse => Ok((constants::WORSE_IMAGE_SIZE, constants::WORSE_IMAGE_BYTES)),
+        }
+    }
+
     /// Executes [`SyncData::Config`] item sent by other device.
     pub(crate) async fn sync_config(&self, key: &Config, value: &str) -> Result<()> {
         let config_value;
@@ -621,7 +634,8 @@ impl Context {
                 match value {
                     Some(path) => {
                         let mut blob = BlobObject::new_from_path(self, path.as_ref()).await?;
-                        blob.recode_to_avatar_size(self).await?;
+                        let protected = true;
+                        blob.recode_to_avatar_size(self, protected).await?;
                         self.sql
                             .set_raw_config(key.as_ref(), Some(blob.as_name()))
                             .await?;

src/constants.rs

@@ -198,10 +198,6 @@ pub(crate) const DC_FETCH_EXISTING_MSGS_COUNT: i64 = 100;
 pub const BALANCED_IMAGE_BYTES: usize = 500_000;
 pub const WORSE_IMAGE_BYTES: usize = 130_000;
 
-// max. width/height of an avatar
-pub(crate) const BALANCED_AVATAR_SIZE: u32 = 256;
-pub(crate) const WORSE_AVATAR_SIZE: u32 = 128;
-
 // max. width/height of images scaled down because of being too huge
 pub const BALANCED_IMAGE_SIZE: u32 = 1280;
 pub const WORSE_IMAGE_SIZE: u32 = 640;

src/sql.rs

@@ -264,7 +264,8 @@ impl Sql {
         if recode_avatar {
             if let Some(avatar) = context.get_config(Config::Selfavatar).await? {
                 let mut blob = BlobObject::new_from_path(context, avatar.as_ref()).await?;
-                match blob.recode_to_avatar_size(context).await {
+                let protected = true;
+                match blob.recode_to_avatar_size(context, protected).await {
                     Ok(()) => {
                         context
                             .set_config_internal(Config::Selfavatar, Some(&avatar))

src/tests/verified_chats.rs

@@ -1,6 +1,7 @@
 use anyhow::Result;
 use pretty_assertions::assert_eq;
 
+use crate::blob;
 use crate::chat::{self, add_contact_to_chat, Chat, ProtectionStatus};
 use crate::chatlist::Chatlist;
 use crate::config::Config;
@@ -909,6 +910,51 @@ async fn test_no_unencrypted_name_if_verified() -> Result<()> {
     Ok(())
 }
 
+#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+async fn test_contact_avatar() -> Result<()> {
+    let mut tcm = TestContextManager::new();
+    let alice = &tcm.alice().await;
+    enable_verified_oneonone_chats(&[alice]).await;
+    let bob = &tcm.bob().await;
+    mark_as_verified(alice, bob).await;
+    let alice_bob_chat = alice.create_chat(bob).await;
+    let file = alice.dir.path().join("avatar.jpg");
+    let bytes = include_bytes!("../../test-data/image/avatar1000x1000.jpg");
+    tokio::fs::write(&file, bytes).await?;
+    alice
+        .set_config(Config::Selfavatar, Some(file.to_str().unwrap()))
+        .await?;
+    let sent_msg = alice
+        .send_text(alice_bob_chat.id, "hello, I have a new avatar")
+        .await;
+    bob.recv_msg(&sent_msg).await;
+    let bob_alice_contact = bob.add_or_lookup_contact(alice).await;
+    let avatar_path = bob_alice_contact.get_profile_image(bob).await?.unwrap();
+    tokio::task::block_in_place(move || {
+        let (_, exif) = blob::image_metadata(&std::fs::File::open(&avatar_path)?)?;
+        assert!(exif.is_none());
+        let img = image::open(&avatar_path)?;
+        assert_eq!(img.width(), 1000);
+        assert_eq!(img.height(), 1000);
+        Result::<()>::Ok(())
+    })?;
+    Ok(())
+}
+
+#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+async fn test_self_chat() -> Result<()> {
+    let mut tcm = TestContextManager::new();
+    let alice = &tcm.alice().await;
+    enable_verified_oneonone_chats(&[alice]).await;
+    let chat = alice.get_self_chat().await;
+    assert!(!chat.is_protected());
+    assert_eq!(
+        chat.id.is_protected(alice).await?,
+        ProtectionStatus::Unprotected
+    );
+    Ok(())
+}
+
 // ============== Helper Functions ==============
 
 async fn assert_verified(this: &TestContext, other: &TestContext, protected: ProtectionStatus) {