recode animated webp avatars

Reencoding animated WebPs into JPEG loses all frames except the first one.

.github/workflows/ci.yml

@@ -20,7 +20,7 @@ permissions: {}
 
 env:
   RUSTFLAGS: -Dwarnings
-  RUST_VERSION: 1.99.0
+  RUST_VERSION: 1.98.1
 
   # Minimum Supported Rust Version
   MSRV: 1.89.0

Cargo.lock

@@ -194,9 +194,9 @@ dependencies = [
 
 [[package]]
 name = "astral-tokio-tar"
-version = "0.7.0"
+version = "0.6.4"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "6f2e989b33246fe9240d39accf4dd9a01e0b6c1f3ce9dd095e0a47fa02505523"
+checksum = "b18457efd137254e016bbde5e1d88df61c4e1a5ae2223746e56123bac6af2463"
 dependencies = [
  "futures-core",
  "libc",
@@ -205,7 +205,6 @@ dependencies = [
  "rustix 1.1.4",
  "tokio",
  "tokio-stream",
- "zerocopy 0.8.59",
 ]
 
 [[package]]
@@ -1485,7 +1484,6 @@ dependencies = [
  "deltachat",
  "deltachat-jsonrpc",
  "futures-lite",
- "libc",
  "log",
  "serde",
  "serde_json",
@@ -4597,7 +4595,7 @@ version = "0.2.20"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "77957b295656769bb8ad2b6a6b09d897d94f05c41b069aede1fcdaa675eaea04"
 dependencies = [
- "zerocopy 0.7.35",
+ "zerocopy",
 ]
 
 [[package]]
@@ -5814,7 +5812,7 @@ dependencies = [
  "sha3",
  "signature",
  "typenum",
- "zerocopy 0.7.35",
+ "zerocopy",
 ]
 
 [[package]]
@@ -7584,16 +7582,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "1b9b4fd18abc82b8136838da5d50bae7bdea537c574d8dc1a34ed098d6c166f0"
 dependencies = [
  "byteorder",
- "zerocopy-derive 0.7.35",
-]
-
-[[package]]
-name = "zerocopy"
-version = "0.8.59"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "6df92bf3d9227be3d53173901ddbffac2babc27ae50f397776ffd6dc33f800cb"
-dependencies = [
- "zerocopy-derive 0.8.59",
+ "zerocopy-derive",
 ]
 
 [[package]]
@@ -7607,17 +7596,6 @@ dependencies = [
  "syn 2.0.118",
 ]
 
-[[package]]
-name = "zerocopy-derive"
-version = "0.8.59"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "ac4f328cf2f05d084e496c3e9c3f33ed0a183656a16e1fcec4d464d8373aec82"
-dependencies = [
- "proc-macro2",
- "quote",
- "syn 2.0.118",
-]
-
 [[package]]
 name = "zerofrom"
 version = "0.1.5"

Cargo.toml

@@ -100,7 +100,7 @@ thiserror = { workspace = true }
 tokio-io-timeout = "1.2.1"
 tokio-rustls = { version = "0.26.2", default-features = false, features = ["tls12", "brotli"] }
 tokio-stream = { version = "0.1.17", features = ["fs"] }
-astral-tokio-tar = { version = "0.7.0", default-features = false }
+astral-tokio-tar = { version = "0.6.3", default-features = false }
 tokio-util = { workspace = true }
 tokio = { workspace = true, features = ["fs", "rt-multi-thread", "macros"] }
 toml = "0.9"

deltachat-rpc-client/src/deltachat_rpc_client/rpc.py

@@ -149,10 +149,7 @@ class Rpc:
     def close(self) -> None:
         """Terminate RPC server process and wait until the reader loop finishes."""
         self.closing = True
-        # JSON-RPC error may happen if RPC server process has crashed already.
-        # We still want to shutdown all threads in this case.
-        with contextlib.suppress(JsonRpcError):
-            self.stop_io_for_all_accounts()
+        self.stop_io_for_all_accounts()
         # Let `events_loop` stop cleanly on `closing` before the pipe goes away,
         # otherwise it might exit through an "RPC server closed" error instead.
         self.events_thread.join()

deltachat-rpc-client/tests/test_memory.py

@@ -1,33 +0,0 @@
-import os
-import sys
-
-import pytest
-
-
-def anonymous_mib(pid):
-    with open(f"/proc/{pid}/smaps_rollup") as f:
-        for line in f:
-            if line.startswith("Anonymous:"):
-                return int(line.split()[1]) // 1024
-    raise LookupError("Anonymous")
-
-
-@pytest.mark.skipif(sys.platform != "linux", reason="reads /proc")
-def test_attachment_memory_is_returned(acf, rpc, tmp_path):
-    # See also comments for `tune_malloc` in `deltachat-jsonrpc-client/src/main.rs`
-    ac1, ac2 = acf.get_online_accounts(2)
-    chat1 = acf.get_accepted_chat(ac1, ac2)
-    chat2 = ac2.create_chat(ac1)
-    blob = tmp_path / "blob.bin"
-    blob.write_bytes(os.urandom(20 << 20))
-    before = anonymous_mib(rpc.process.pid)
-
-    for sender_chat, receiver in ((chat1, ac2), (chat2, ac1)):
-        sender_chat.send_file(str(blob))
-        event = receiver.wait_for_incoming_msg_event()
-        assert receiver.get_message_by_id(event.msg_id).get_snapshot().file_bytes == 20 << 20
-    for ac in (ac1, ac2):
-        rpc.wait_for_all_work_done(ac.id)
-
-    grown = anonymous_mib(rpc.process.pid) - before
-    assert grown < 64, f"the server kept {grown} MiB after two 20 MiB attachments"

deltachat-rpc-client/tests/test_something.py

@@ -35,19 +35,6 @@ def test_sleep(rpc) -> None:
         assert sleep_5_future in pending
 
 
-def test_shutdown(rpc) -> None:
-    """Test RPC client shutdown if RPC server process is terminated.
-
-    This is a regression test, at the time of adding it RPC client
-    did not terminate all threads and python process did not exit.
-    Shutting down all threads is tested implicitly by pytest-timeout.
-    """
-    rpc.process.kill()
-    rpc.process.wait()
-    with pytest.raises(JsonRpcError):
-        rpc.get_system_info()
-
-
 def test_email_address_validity(rpc) -> None:
     valid_addresses = [
         "email@example.com",

deltachat-rpc-server/Cargo.toml

@@ -14,7 +14,6 @@ deltachat = { workspace = true }
 
 anyhow = { workspace = true }
 futures-lite = { workspace = true }
-libc = { workspace = true }
 log = { workspace = true }
 serde_json = { workspace = true }
 serde = { workspace = true, features = ["derive"] }

deltachat-rpc-server/src/main.rs

@@ -22,27 +22,8 @@ use tokio::task::JoinHandle;
 use tokio_util::sync::CancellationToken;
 use yerpc::{RpcClient, RpcSession};
 
-/// Keeps glibc from holding on to the buffers of large messages.
-///
-/// glibc dynamically raises its mmap threshold to the size of the first large chunk
-/// that is freed, up to 32 MiB, and then serves buffers of that size
-/// from its arenas, where they stay after being freed.
-/// A fixed threshold disables the adjustment, so every allocation
-/// above it is mapped on its own and returned to the kernel on free.
-#[cfg(all(target_os = "linux", target_env = "gnu"))]
-fn tune_malloc() {
-    unsafe {
-        // <https://man7.org/linux/man-pages/man3/mallopt.3.html>, M_MMAP_THRESHOLD.
-        libc::mallopt(libc::M_MMAP_THRESHOLD, 128 * 1024);
-    }
-}
-
-#[cfg(not(all(target_os = "linux", target_env = "gnu")))]
-fn tune_malloc() {}
-
 #[tokio::main(flavor = "multi_thread")]
 async fn main() {
-    tune_malloc();
     // Logs from `log` crate and traces from `tracing` crate
     // are configurable with `RUST_LOG` environment variable
     // and go to stderr to avoid interfering with JSON-RPC using stdout.

deny.toml

@@ -113,8 +113,6 @@ skip = [
      { name = "windows_x86_64_gnu" },
      { name = "windows_x86_64_gnullvm" },
      { name = "windows_x86_64_msvc" },
-     { name = "zerocopy-derive", version = "0.7.35" },
-     { name = "zerocopy", version = "0.7.35" },
 ]
 
 

src/blob.rs

@@ -350,7 +350,24 @@ impl<'a> BlobObject<'a> {
                 *vt = Viewtype::Image;
                 return Ok(name);
             }
-            let mut img = imgreader.decode().context("image decode failure")?;
+
+            let mut img = match fmt {
+                image::ImageFormat::WebP => {
+                    // `with_guessed_format()` restores file position,
+                    // so `buf_reader` is at the beginning of the file.
+                    let buf_reader = imgreader.into_inner();
+                    let webp_decoder = image::codecs::webp::WebPDecoder::new(buf_reader)
+                        .context("Failed to create WebP decoder")?;
+
+                    // If WebP has animation, do not try to recode it.
+                    // Recoding into JPEG will result in losing the animation.
+                    if !is_avatar && webp_decoder.has_animation() {
+                        return Ok(name);
+                    }
+                    DynamicImage::from_decoder(webp_decoder)?
+                }
+                _ => imgreader.decode().context("Failed to decode image")?,
+            };
             let orientation = exif
                 .as_ref()
                 .map(|exif| exif_orientation(exif, context))

src/blob/blob_tests.rs

@@ -4,7 +4,9 @@ use super::*;
 use crate::message::{Message, Viewtype};
 use crate::param::Param;
 use crate::sql;
-use crate::test_utils::{self, AVATAR_64x64_BYTES, AVATAR_64x64_DEDUPLICATED, TestContext};
+use crate::test_utils::{
+    self, AVATAR_64x64_BYTES, AVATAR_64x64_DEDUPLICATED, TestContext, TestContextManager,
+};
 use crate::tools::SystemTime;
 
 fn check_image_size(path: impl AsRef<Path>, width: u32, height: u32) -> image::DynamicImage {
@@ -738,6 +740,66 @@ async fn test_send_gif_as_sticker() -> Result<()> {
     Ok(())
 }
 
+/// Tests that animated WebP is sent without reencoding.
+#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+async fn test_send_animated_webp_as_image() -> Result<()> {
+    let bytes = include_bytes!("../../test-data/image/animated.webp");
+    let (width, height) = (1280u32, 531u32);
+    let mut tcm = TestContextManager::new();
+    let alice = &tcm.alice().await;
+    let bob = &tcm.bob().await;
+    alice
+        .set_config(
+            Config::MediaQuality,
+            Some(&(MediaQuality::Worse as i32).to_string()),
+        )
+        .await?;
+    let file = alice.get_blobdir().join("file").with_extension("gif");
+    fs::write(&file, &bytes)
+        .await
+        .context("Failed to write file")?;
+    let mut msg = Message::new(Viewtype::Image);
+    msg.set_file_and_deduplicate(alice, &file, Some("file.webp"), None)?;
+
+    let chat = alice.create_chat(bob).await;
+    let sent = alice.send_msg(chat.id, &mut msg).await;
+    let bob_msg = bob.recv_msg(&sent).await;
+    assert_eq!(bob_msg.get_viewtype(), Viewtype::Image);
+    assert_eq!(bob_msg.get_width() as u32, width);
+    assert_eq!(bob_msg.get_height() as u32, height);
+    assert_eq!(
+        bob_msg.get_filebytes(bob).await?.unwrap(),
+        bytes.len() as u64
+    );
+    Ok(())
+}
+
+/// Tests that if user sets animated WebP as an avatar, it may be recoded.
+///
+/// We don't want to recode animated WebPs into JPEG and lose animation,
+/// but for avatars we don't want animation and transparency anyway.
+#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+async fn test_recode_animated_webp_avatar() -> Result<()> {
+    let mut tcm = TestContextManager::new();
+    let t = &tcm.alice().await;
+
+    let avatar_src = t.dir.path().join("avatar.webp");
+    let avatar_bytes = include_bytes!("../../test-data/image/animated.webp");
+    fs::write(&avatar_src, avatar_bytes).await.unwrap();
+
+    t.set_config(Config::Selfavatar, Some(avatar_src.to_str().unwrap()))
+        .await?;
+    let avatar_blob = t.get_config(Config::Selfavatar).await?.unwrap();
+    assert!(avatar_blob.ends_with(".jpg"));
+
+    let scaled_avatar_size = fs::metadata(&avatar_blob).await.unwrap().len();
+    assert!(
+        scaled_avatar_size < avatar_bytes.len() as u64,
+        "Animated WebP avatar must be recoded"
+    );
+    Ok(())
+}
+
 #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
 async fn test_create_and_deduplicate() -> Result<()> {
     let t = TestContext::new().await;

src/smtp.rs

@@ -68,8 +68,6 @@ impl Smtp {
             // separate task to avoid waiting for reply or timeout.
             task::spawn(async move { transport.quit().await });
         }
-        self.transport_id = None;
-        self.from = None;
         self.last_success = None;
     }