diff --git a/crates/pdfs-fuse/src/control.rs b/crates/pdfs-fuse/src/control.rs index e08b5a0..5fe3bb9 100644 --- a/crates/pdfs-fuse/src/control.rs +++ b/crates/pdfs-fuse/src/control.rs @@ -915,6 +915,7 @@ fn handle_control_conn(core: &Core, username: &str, mountpoint: &Path, stream: U capture_time, ..Default::default() }; + let thumbnails = core.upload_thumbnails(Path::new(&source_path), &name); let guard = core.transfers .begin(name.clone(), "", TransferDirection::Upload, len); @@ -924,7 +925,7 @@ fn handle_control_conn(core: &Core, username: &str, mountpoint: &Path, stream: U &media_type, reader, len as i64, - Vec::new(), + thumbnails, metadata, false, )) { diff --git a/crates/pdfs-fuse/src/photos.rs b/crates/pdfs-fuse/src/photos.rs index e3a2d27..55210ee 100644 --- a/crates/pdfs-fuse/src/photos.rs +++ b/crates/pdfs-fuse/src/photos.rs @@ -434,6 +434,16 @@ fn exiftool_query( serde_json::from_slice(&output).map_err(|_| RawPreviewUnavailable) } +/// Whether [`upload_thumbnails`] can make a thumbnail for a file `name` of +/// `size` bytes, asked before the file is on disk: an image small enough to +/// decode, or a video while ffmpeg is installed. +pub(crate) fn can_make_thumbnail(name: &str, size: u64) -> bool { + if is_video_name(name) { + return ffmpeg_available(); + } + is_thumbnail_image_name(name) && size <= UPLOAD_IMAGE_MAX_BYTES +} + /// Whether `name` is a video, whose thumbnail is a frame ffmpeg grabs rather /// than a picture the image decoder can scale. fn is_video_name(name: &str) -> bool { @@ -2109,9 +2119,10 @@ impl Core { #[cfg(test)] mod thumb_tests { use super::{ - RawTempFile, ScaleAttempt, THUMB_EDGE, UPLOAD_THUMB_MAX_BYTES, exiftool_binary, - fit_upload_thumb, is_video_name, ratio_of, scale_thumbnail, scale_thumbnail_with_exiftool, - similar_hashes, thumbnail_build_may_start, upload_thumbnails_with, video_thumbnail, + RawTempFile, ScaleAttempt, THUMB_EDGE, UPLOAD_IMAGE_MAX_BYTES, UPLOAD_THUMB_MAX_BYTES, + can_make_thumbnail, exiftool_binary, ffmpeg_available, fit_upload_thumb, is_video_name, + ratio_of, scale_thumbnail, scale_thumbnail_with_exiftool, similar_hashes, + thumbnail_build_may_start, upload_thumbnails_with, video_thumbnail, }; use pdfs_core::control::{ThumbnailBuildStatus, is_thumbnail_image_name}; use proton_drive_rs::ThumbnailType; @@ -2191,6 +2202,17 @@ mod thumb_tests { /// A phone stores a portrait photo as landscape pixels and an EXIF /// orientation; its thumbnail came out on its side (`docs/BUGS.md` B226). + /// A Takeout photo is copied out of its archive only when a thumbnail can + /// be made from the copy. + #[test] + fn only_a_photo_that_can_have_a_thumbnail_is_worth_copying_out() { + assert!(can_make_thumbnail("IMG_1.jpg", 4 << 20)); + assert!(can_make_thumbnail("Screenshot.png", 4 << 20)); + assert!(!can_make_thumbnail("IMG_1.jpg", UPLOAD_IMAGE_MAX_BYTES + 1)); + assert!(!can_make_thumbnail("notes.txt", 10)); + assert_eq!(can_make_thumbnail("clip.mp4", 1 << 30), ffmpeg_available()); + } + #[test] fn a_thumbnail_is_turned_the_way_its_exif_says() { let ScaleAttempt::Made(thumb) = scale_thumbnail( diff --git a/crates/pdfs-fuse/src/redate.rs b/crates/pdfs-fuse/src/redate.rs index 7f86890..b5bf463 100644 --- a/crates/pdfs-fuse/src/redate.rs +++ b/crates/pdfs-fuse/src/redate.rs @@ -190,6 +190,7 @@ impl Core { .map_err(|e| CoreError::internal(format!("stat {}: {e}", path.display())))? .len(); let media_type = media_type_of(&candidate.name).unwrap_or("application/octet-stream"); + let thumbnails = self.upload_thumbnails(&path, &candidate.name); let guard = self.transfers @@ -206,7 +207,7 @@ impl Core { media_type, reader, size as i64, - Vec::new(), + thumbnails, metadata, false, )); diff --git a/crates/pdfs-fuse/src/takeout.rs b/crates/pdfs-fuse/src/takeout.rs index 43e8a5b..650e938 100644 --- a/crates/pdfs-fuse/src/takeout.rs +++ b/crates/pdfs-fuse/src/takeout.rs @@ -25,7 +25,7 @@ use std::collections::{BTreeMap, HashMap}; use std::io::Read; use std::path::PathBuf; use std::sync::Arc; -use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; use pdfs_core::control::{ActivityKind, ImportSummary, TransferDirection}; use pdfs_core::takeout::{Bucket, TakeoutPhoto, TakeoutScan, scan_archives}; @@ -36,6 +36,7 @@ use sha1::{Digest, Sha1}; use tracing::{info, warn}; use super::Core; +use crate::photos::can_make_thumbnail; use crate::transfers::CountingReader; /// The one import this daemon may be running, and how the last one ended. @@ -136,6 +137,30 @@ fn read_entry( } impl Core { + /// Copy archive entry `entry` into the thumbnail staging folder and return + /// its path. The caller removes it. + fn stage_takeout_entry( + &self, + archive: &mut zip::ZipArchive>, + entry: &str, + ) -> CoreResult { + static STAGED: AtomicU64 = AtomicU64::new(0); + let path = self.cache.raw_thumbnail_staging_dir().join(format!( + "takeout-{}-{}", + std::process::id(), + STAGED.fetch_add(1, Ordering::Relaxed) + )); + let copied = read_entry(archive, entry, |reader| { + let mut file = std::fs::File::create(&path)?; + std::io::copy(reader, &mut file) + })?; + if let Err(e) = copied { + let _ = std::fs::remove_file(&path); + return Err(CoreError::internal(format!("copy {entry}: {e}"))); + } + Ok(path) + } + /// Import Google Photos Takeout archives. Blocking; run on its own thread. /// /// `cancel` stops the run at the next photo boundary — an upload already on @@ -321,18 +346,57 @@ impl Core { capture_time: photo.capture_time, ..Default::default() }; - let uploaded = read_entry(&mut archive, &photo.entry, |reader| { - let counting = CountingReader::new(reader, &guard); - self.rt.block_on(photos.upload_photo_from( - &photo.name, - &photo.media_type, - counting, - photo.size as i64, - Vec::new(), - metadata, - false, - )) - }); + // A thumbnail is made from a file, so a photo that can have one + // is copied out of the archive first and uploaded from the copy. + // Without one, Proton's apps show a blank tile until they make + // their own. + let staged = can_make_thumbnail(&photo.name, photo.size) + .then(|| self.stage_takeout_entry(&mut archive, &photo.entry)) + .and_then(|staged| { + staged + .inspect_err(|e| { + warn!(name = photo.name, error = %e, + "copying a takeout photo out failed; uploading it without a thumbnail") + }) + .ok() + }); + let uploaded = match &staged { + Some(path) => { + let thumbnails = self.upload_thumbnails(path, &photo.name); + std::fs::File::open(path) + .map_err(|e| { + CoreError::internal(format!("open {}: {e}", path.display())) + }) + .map(|file| { + let counting = + CountingReader::new(std::io::BufReader::new(file), &guard); + self.rt.block_on(photos.upload_photo_from( + &photo.name, + &photo.media_type, + counting, + photo.size as i64, + thumbnails, + metadata, + false, + )) + }) + } + None => read_entry(&mut archive, &photo.entry, |reader| { + let counting = CountingReader::new(reader, &guard); + self.rt.block_on(photos.upload_photo_from( + &photo.name, + &photo.media_type, + counting, + photo.size as i64, + Vec::new(), + metadata, + false, + )) + }), + }; + if let Some(path) = staged { + let _ = std::fs::remove_file(path); + } drop(guard); job.step(); diff --git a/docs/BUGS.md b/docs/BUGS.md index 1286357..93b971d 100644 --- a/docs/BUGS.md +++ b/docs/BUGS.md @@ -100,6 +100,7 @@ a live account since. | Entry | Title | |---|---| +| [B236](#b236--imported-re-dated-and-app-uploaded-photos-have-no-thumbnail) | Imported, re-dated and app-uploaded photos have no thumbnail | | [B235](#b235--scanning-a-large-takeout-folder-takes-half-a-minute) | Scanning a large Takeout folder takes half a minute | | [B234](#b234--any-folder-can-become-a-sync-root-the-home-folder-included) | Any folder can become a sync root, the home folder included | | [B233](#b233--signing-out-leaves-the-session-valid-on-proton) | Signing out leaves the session valid on Proton | @@ -338,6 +339,32 @@ a live account since. Newest first. +### B236 — Imported, re-dated and app-uploaded photos have no thumbnail + +**Status:** Fixed, not verified live +**Found:** 2026-10-16, architecture audit (`FINDINGS.md` 2.4.2). +**Where:** `Core::import_takeout` and `stage_takeout_entry` in `crates/pdfs-fuse/src/takeout.rs`; +`Core::redate_one` in `redate.rs`; the `UploadPhoto` request in `control.rs`; +`can_make_thumbnail` in `photos.rs` + +**Repro:** Import a Google Takeout export, re-date a photo, or upload a photo from the app's +gallery. Look at the new photos in Proton's web or phone app. + +**Cause.** All three paths uploaded with an empty thumbnail list. The drain and the mirror sync +make thumbnails for what they upload, but these three did not, so the photos showed as blank +tiles in Proton's apps. The 3.0 changelog says imports carry thumbnails. + +**Fix.** The re-date and the app's upload make thumbnails from the file they upload. A Takeout +photo that can have a thumbnail (an image small enough to decode, or a video while ffmpeg is +installed) is copied out of the archive into the private staging folder first. The thumbnail is +made from the copy, the upload reads the copy, and the copy is removed. When the copy fails, the +photo is uploaded from the archive without a thumbnail, as before. + +**Test:** `photos::thumb_tests::only_a_photo_that_can_have_a_thumbnail_is_worth_copying_out`; the +thumbnail itself is covered by the existing `thumb_tests`. + +--- + ### B235 — Scanning a large Takeout folder takes half a minute **Status:** Fixed, not verified live