Make the thumbnails while the originals are still in hand
An import read every byte of every original, uploaded them, and then left the grid to fetch a preview range back out of each one over the network — for files that had been on this machine minutes earlier. The pixels are now made from the bytes already in memory, on the fast path FR-CAT-3 names: locate the camera's own embedded JPEG, decode a few hundred KB, downscale, orient. Never a demosaic. A file with no usable preview yields none, which is not an error and simply leaves the grid to fetch one later. The filing waits for the upload, and only the filing. The shard store is keyed by `oc:fileid` (FR-NC-5) and that does not exist until the server has the file — so the thumbnail is made from the local copy and held until an id can be attached to it. One listing per folder supplies every id at once, rather than a PROPFIND per photograph over a link that may be mobile data. Best-effort throughout: a thumbnail that cannot be filed costs the grid one preview fetch later, and failing an import over it would be the wrong trade. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Claude Opus 5
+243
-19
@@ -35,6 +35,7 @@ use std::sync::Arc;
|
||||
|
||||
use dr_ingest::{Candidate, DupKey, Imported, Ingest, Options, Report, Shot};
|
||||
use dr_plat::{DirRef, LocalStorage, Storage, WritableStorage};
|
||||
use dr_sync::{RemoteBackend, RemotePath};
|
||||
use dr_sync_nextcloud::{AppCredentials, NextcloudBackend};
|
||||
use dr_types::{FormatFilter, RootId};
|
||||
|
||||
@@ -85,6 +86,9 @@ pub struct Upload {
|
||||
/// The library folder on the server. The dated folders from the template
|
||||
/// are created beneath it, the same ones the local copy went into.
|
||||
pub library: String,
|
||||
/// Where the thumbnail shards live, so a thumbnail made during the import
|
||||
/// can be filed under the server's `oc:fileid` once the upload assigns one.
|
||||
pub thumbs: PathBuf,
|
||||
}
|
||||
|
||||
impl std::fmt::Debug for Upload {
|
||||
@@ -93,6 +97,7 @@ impl std::fmt::Debug for Upload {
|
||||
f.debug_struct("Upload")
|
||||
.field("user_id", &self.user_id)
|
||||
.field("library", &self.library)
|
||||
.field("thumbs", &self.thumbs)
|
||||
.finish_non_exhaustive()
|
||||
}
|
||||
}
|
||||
@@ -141,6 +146,20 @@ pub struct Outcome {
|
||||
pub retirable: usize,
|
||||
/// Originals confirmed on the server.
|
||||
pub uploaded: usize,
|
||||
/// Thumbnails made from the imported files and filed in the shard store.
|
||||
///
|
||||
/// Made here rather than left to the grid, which would otherwise fetch a
|
||||
/// preview range out of every freshly uploaded original — over the network,
|
||||
/// for files that were on this machine minutes earlier (FR-CAT-3).
|
||||
pub thumbnails: usize,
|
||||
/// Originals the server already held, so nothing was transferred.
|
||||
///
|
||||
/// The re-inserted card: the shoot went up last week and the card has not
|
||||
/// been formatted since. Counted apart from `uploaded` because they are
|
||||
/// different answers to "what did this cost me", and apart from
|
||||
/// `duplicates` because these *were* copied locally — only the upload was
|
||||
/// skipped.
|
||||
pub already_on_server: usize,
|
||||
/// Originals that stayed local because the upload did not go through.
|
||||
///
|
||||
/// Not a failure of the import: the photographs are on disk and verified,
|
||||
@@ -224,11 +243,15 @@ fn run(request: Request, cancel: &Cancel, tx: &Sender<Message>) -> Result<(), St
|
||||
// and its digest checked, so nothing from here on can cost the user a
|
||||
// photograph — only a transfer.
|
||||
if let Some(upload) = &request.upload {
|
||||
let sent = upload_all(upload, &library, &report.imported, cancel, tx);
|
||||
outcome.uploaded = sent.len();
|
||||
let (sent, thumbnails) = upload_all(upload, &library, &report.imported, cancel, tx);
|
||||
outcome.thumbnails = thumbnails;
|
||||
outcome.uploaded = sent.iter().filter(|s| s.transferred).count();
|
||||
outcome.already_on_server = sent.len() - outcome.uploaded;
|
||||
outcome.upload_failed = report.imported.len() - sent.len();
|
||||
// The card keeps its copies of anything the server did not take.
|
||||
outcome.retirable = outcome.retirable.min(outcome.uploaded);
|
||||
// The card keeps its copies of anything the server does not have. A
|
||||
// photograph it already held counts as safe — that is the whole claim
|
||||
// `AlreadyThere` makes.
|
||||
outcome.retirable = outcome.retirable.min(sent.len());
|
||||
|
||||
// The digests, so the *next* import can answer the content tier
|
||||
// without reading anything.
|
||||
@@ -249,8 +272,8 @@ fn run(request: Request, cancel: &Cancel, tx: &Sender<Message>) -> Result<(), St
|
||||
|
||||
/// Send the imported originals to the server, in their dated folders.
|
||||
///
|
||||
/// Returns the remote path and digest of each original the server confirmed.
|
||||
/// A failure here is reported
|
||||
/// Returns one [`Sent`] per original the server confirmed it has, whether this
|
||||
/// run put it there or it was already up. A failure here is reported
|
||||
/// and not propagated: the import succeeded, and turning "the network was
|
||||
/// slow" into a failed import would misdescribe what happened to the
|
||||
/// photographs and, on a move-import, would be the difference between a card
|
||||
@@ -261,12 +284,12 @@ fn upload_all(
|
||||
imported: &[Imported],
|
||||
cancel: &Cancel,
|
||||
tx: &Sender<Message>,
|
||||
) -> Vec<(String, String)> {
|
||||
) -> (Vec<Sent>, usize) {
|
||||
let rt = match crate::net_runtime::build() {
|
||||
Ok(rt) => rt,
|
||||
Err(e) => {
|
||||
log::warn!("no runtime for the upload: {e}");
|
||||
return Vec::new();
|
||||
return (Vec::new(), 0);
|
||||
}
|
||||
};
|
||||
|
||||
@@ -275,12 +298,12 @@ fn upload_all(
|
||||
Ok(b) => b,
|
||||
Err(e) => {
|
||||
log::warn!("connecting to upload: {e}");
|
||||
return Vec::new();
|
||||
return (Vec::new(), 0);
|
||||
}
|
||||
};
|
||||
let root = dr_sync::RemotePath::new(&upload.library);
|
||||
|
||||
let mut sent = Vec::new();
|
||||
let mut sent: Vec<Sent> = Vec::new();
|
||||
for (i, image) in imported.iter().enumerate() {
|
||||
if cancel.load(Ordering::Relaxed) {
|
||||
// Everything not yet sent stays local, and the card keeps its
|
||||
@@ -302,22 +325,159 @@ fn upload_all(
|
||||
}
|
||||
};
|
||||
|
||||
// Before the body is handed over: `upload_original` takes it by
|
||||
// value, and re-reading the file to make a thumbnail afterwards
|
||||
// would be a second full read of an 80 MB original.
|
||||
let thumbnail = make_thumbnail(&bytes);
|
||||
|
||||
// The same folder segments the local copy went into, so the two
|
||||
// libraries have the same shape (FR-NC-7a).
|
||||
match dr_sync::upload_original(&backend, &root, &image.folders, &image.name, bytes)
|
||||
.await
|
||||
{
|
||||
Ok((path, _)) => {
|
||||
log::info!("uploaded {path}");
|
||||
sent.push((path.as_str().to_string(), image.digest.clone()));
|
||||
Ok(placed) => {
|
||||
let transferred = matches!(placed, dr_sync::Placed::Uploaded { .. });
|
||||
if transferred {
|
||||
log::info!("uploaded {}", placed.path());
|
||||
} else {
|
||||
log::info!("already on the server: {}", placed.path());
|
||||
}
|
||||
sent.push(Sent {
|
||||
remote_path: placed.path().as_str().to_string(),
|
||||
digest: image.digest.clone(),
|
||||
transferred,
|
||||
// Made from the bytes already in hand rather than from
|
||||
// a later range fetch of what we just sent up.
|
||||
thumbnail,
|
||||
});
|
||||
}
|
||||
Err(e) => log::warn!("uploading {}: {e}", image.name),
|
||||
}
|
||||
}
|
||||
sent
|
||||
let filed = file_thumbnails(&backend, &root, imported, &sent, &upload.thumbs).await;
|
||||
(sent, filed)
|
||||
})
|
||||
}
|
||||
|
||||
/// One original the server has, after the upload phase.
|
||||
struct Sent {
|
||||
remote_path: String,
|
||||
digest: String,
|
||||
/// Whether this run transferred it, as against finding it already there.
|
||||
transferred: bool,
|
||||
/// The thumbnail made from the local copy, waiting for an `oc:fileid` to
|
||||
/// be filed under. `None` where the file carried no usable preview.
|
||||
thumbnail: Option<dr_thumbs::Thumbnail>,
|
||||
}
|
||||
|
||||
/// TRACES: FR-CAT-3 | FR-CULL-2
|
||||
/// A grid thumbnail from an original's own embedded preview.
|
||||
///
|
||||
/// The fast path FR-CAT-3 names: cameras write a JPEG preview into every RAW,
|
||||
/// so this is a locate, a JPEG decode of a few hundred KB and a downscale —
|
||||
/// never a demosaic. `None` where the file carries no usable preview, which is
|
||||
/// not an error and simply leaves the grid to fetch one later.
|
||||
///
|
||||
/// Oriented after the downscale, for the same reason the grid's own path does
|
||||
/// it in that order: the permutation then moves thumbnail-sized bytes rather
|
||||
/// than the full preview's. An embedded preview is written in the sensor's
|
||||
/// orientation, so without this every portrait frame lies on its side — and
|
||||
/// the store is keyed by file and size alone, so it would stay that way.
|
||||
fn make_thumbnail(bytes: &[u8]) -> Option<dr_thumbs::Thumbnail> {
|
||||
let mut preview = dr_decode::extract_preview(bytes, dr_decode::PreviewSize::Thumbnail).ok()?;
|
||||
preview.downscale_to(dr_thumbs::ThumbSize::Grid.edge());
|
||||
preview.apply_orientation(dr_decode::orientation(bytes).unwrap_or_default());
|
||||
let encoded = dr_thumbs::encode_rgba(preview.width, preview.height, &preview.rgba).ok()?;
|
||||
Some(dr_thumbs::Thumbnail {
|
||||
width: preview.width,
|
||||
height: preview.height,
|
||||
bytes: encoded,
|
||||
})
|
||||
}
|
||||
|
||||
/// TRACES: FR-CAT-3 | FR-NC-5
|
||||
/// File the thumbnails made during the import under the ids the server gave.
|
||||
///
|
||||
/// **The `oc:fileid` is the reason this happens after the upload rather than
|
||||
/// during the copy.** The shard store is keyed by it (FR-NC-5), and it does not
|
||||
/// exist until the server has the file. So the pixels are made from the bytes
|
||||
/// in hand — which is the point, they are never fetched back — and only the
|
||||
/// *key* waits for the upload.
|
||||
///
|
||||
/// One listing per folder rather than a `PROPFIND` per file: a day's import is
|
||||
/// one request, where per-file probing would be one per photograph over a link
|
||||
/// that may be mobile data.
|
||||
///
|
||||
/// Best-effort throughout. A thumbnail that cannot be filed costs the grid one
|
||||
/// preview fetch later; failing the import over it would be the wrong trade.
|
||||
async fn file_thumbnails(
|
||||
backend: &NextcloudBackend,
|
||||
library: &RemotePath,
|
||||
imported: &[Imported],
|
||||
sent: &[Sent],
|
||||
thumbs_dir: &Path,
|
||||
) -> usize {
|
||||
use dr_sync::RemoteId;
|
||||
|
||||
if sent.iter().all(|s| s.thumbnail.is_none()) {
|
||||
return 0;
|
||||
}
|
||||
let mut store = match dr_thumbs::ThumbStore::open(thumbs_dir) {
|
||||
Ok(s) => s,
|
||||
Err(e) => {
|
||||
log::warn!("no thumbnail store for the import: {e}");
|
||||
return 0;
|
||||
}
|
||||
};
|
||||
|
||||
// Group by folder so each is listed once.
|
||||
let mut by_folder: std::collections::BTreeMap<String, Vec<&Sent>> = Default::default();
|
||||
for (image, item) in imported.iter().zip(sent.iter()) {
|
||||
if item.thumbnail.is_some() {
|
||||
by_folder
|
||||
.entry(image.folders.join("/"))
|
||||
.or_default()
|
||||
.push(item);
|
||||
}
|
||||
}
|
||||
|
||||
let mut filed = 0;
|
||||
for (folder, items) in by_folder {
|
||||
let dir = dr_sync::destination(library, &[folder]);
|
||||
let entries = match backend.list(&dir, None).await {
|
||||
Ok(e) => e,
|
||||
Err(e) => {
|
||||
log::warn!("listing {dir} for file ids: {e}");
|
||||
continue;
|
||||
}
|
||||
};
|
||||
// Path to id, for the folder we just wrote into.
|
||||
let ids: std::collections::HashMap<&str, u64> = entries
|
||||
.iter()
|
||||
.filter_map(|e| match e.id {
|
||||
RemoteId::Stable(id) => Some((e.path.as_str(), id)),
|
||||
// A backend without stable ids cannot key the shard store at
|
||||
// all; the grid falls back to fetching previews.
|
||||
RemoteId::Path(_) => None,
|
||||
})
|
||||
.collect();
|
||||
|
||||
for item in items {
|
||||
let Some(&file_id) = ids.get(item.remote_path.as_str()) else {
|
||||
continue;
|
||||
};
|
||||
let Some(thumb) = item.thumbnail.as_ref() else {
|
||||
continue;
|
||||
};
|
||||
match store.put(file_id, dr_thumbs::ThumbSize::Grid, thumb) {
|
||||
Ok(_) => filed += 1,
|
||||
Err(e) => log::warn!("storing the thumbnail for {file_id}: {e}"),
|
||||
}
|
||||
}
|
||||
}
|
||||
filed
|
||||
}
|
||||
|
||||
/// Read an imported file back out of the library.
|
||||
fn read_all(library: &LocalStorage, image: &Imported) -> Result<Vec<u8>, String> {
|
||||
use std::io::Read;
|
||||
@@ -430,7 +590,7 @@ fn is_duplicate(conn: &rusqlite::Connection, key: &DupKey) -> bool {
|
||||
///
|
||||
/// A scan never reads a whole file, so `content_hash` is only ever filled in
|
||||
/// by something that had a reason to read every byte — which an import did.
|
||||
fn record_digests(conn: &rusqlite::Connection, library: &str, sent: &[(String, String)]) {
|
||||
fn record_digests(conn: &rusqlite::Connection, library: &str, sent: &[Sent]) {
|
||||
// The library's row, resolved the same way the remote scan resolves it:
|
||||
// one root per library folder, keyed by label.
|
||||
let root: Option<i64> = conn
|
||||
@@ -445,9 +605,14 @@ fn record_digests(conn: &rusqlite::Connection, library: &str, sent: &[(String, S
|
||||
// nothing to attach a digest to. Not an error.
|
||||
return;
|
||||
};
|
||||
for (path, digest) in sent {
|
||||
if let Err(e) = dr_catalog::set_content_hash(conn, root as u64, path, digest) {
|
||||
log::warn!("recording the digest of {path}: {e}");
|
||||
for item in sent {
|
||||
// Recorded for the already-there ones too, and especially for them:
|
||||
// that row has no digest precisely because no import ever read the
|
||||
// file, which is what left the content tier unable to answer.
|
||||
if let Err(e) =
|
||||
dr_catalog::set_content_hash(conn, root as u64, &item.remote_path, &item.digest)
|
||||
{
|
||||
log::warn!("recording the digest of {}: {e}", item.remote_path);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -473,7 +638,9 @@ pub fn summarise(report: &Report) -> Outcome {
|
||||
retirable: report.retirable.len(),
|
||||
// Filled in by the upload phase, which runs after this.
|
||||
uploaded: 0,
|
||||
already_on_server: 0,
|
||||
upload_failed: 0,
|
||||
thumbnails: 0,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -521,8 +688,14 @@ pub fn describe(outcome: &Outcome) -> String {
|
||||
// What the server got. Said plainly rather than folded into the import
|
||||
// count: "imported" and "uploaded" are different promises, and a user on a
|
||||
// failing connection needs to know which one held.
|
||||
if outcome.uploaded > 0 || outcome.upload_failed > 0 {
|
||||
if outcome.uploaded > 0 || outcome.upload_failed > 0 || outcome.already_on_server > 0 {
|
||||
out.push_str(&format!(". {} uploaded", outcome.uploaded));
|
||||
if outcome.already_on_server > 0 {
|
||||
out.push_str(&format!(
|
||||
", {} already on the server",
|
||||
outcome.already_on_server
|
||||
));
|
||||
}
|
||||
if outcome.upload_failed > 0 {
|
||||
out.push_str(&format!(
|
||||
", {} still only on this computer",
|
||||
@@ -531,6 +704,12 @@ pub fn describe(outcome: &Outcome) -> String {
|
||||
}
|
||||
}
|
||||
|
||||
// Worth saying: it is why the grid fills in immediately after an import
|
||||
// rather than fetching a preview out of every original that just went up.
|
||||
if outcome.thumbnails > 0 {
|
||||
out.push_str(&format!(", {} thumbnails made", outcome.thumbnails));
|
||||
}
|
||||
|
||||
// FR-NC-7a: the mtime fallback is reported rather than silent.
|
||||
if !outcome.undated.is_empty() {
|
||||
out.push_str(&format!(
|
||||
@@ -734,6 +913,51 @@ mod tests {
|
||||
assert!(describe(&many).contains("were filed"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_card_the_server_already_has_says_so_rather_than_claiming_an_upload() {
|
||||
// The re-inserted card. Reporting "0 uploaded" alone would read as a
|
||||
// failure; reporting them as uploaded would be a lie about what the
|
||||
// run cost and about what is newly backed up.
|
||||
let o = Outcome {
|
||||
already_on_server: 12,
|
||||
..outcome()
|
||||
};
|
||||
let text = describe(&o);
|
||||
assert!(
|
||||
text.contains("0 uploaded, 12 already on the server"),
|
||||
"{text}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn an_upload_that_did_not_go_through_is_not_silent() {
|
||||
let o = Outcome {
|
||||
uploaded: 2,
|
||||
upload_failed: 1,
|
||||
..outcome()
|
||||
};
|
||||
let text = describe(&o);
|
||||
assert!(text.contains("2 uploaded"), "{text}");
|
||||
assert!(text.contains("1 still only on this computer"), "{text}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn thumbnails_are_mentioned_only_when_some_were_made() {
|
||||
let o = Outcome {
|
||||
uploaded: 3,
|
||||
thumbnails: 3,
|
||||
..outcome()
|
||||
};
|
||||
assert!(
|
||||
describe(&o).contains("3 thumbnails made"),
|
||||
"{}",
|
||||
describe(&o)
|
||||
);
|
||||
// A card of files with no embedded preview makes none, and a report
|
||||
// that says "0 thumbnails made" is noise.
|
||||
assert!(!describe(&outcome()).contains("thumbnails"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn several_days_on_one_card_are_counted_rather_than_listed() {
|
||||
let o = Outcome {
|
||||
|
||||
@@ -323,6 +323,12 @@ where
|
||||
let context = context.clone();
|
||||
window.on_import_open(move || {
|
||||
let Some(w) = weak.upgrade() else { return };
|
||||
// The button that reaches here is hidden where this is false, so
|
||||
// arriving anyway means a key binding or a stale window state.
|
||||
// Refused rather than shown empty.
|
||||
if !dr_plat::imports_supported() {
|
||||
return;
|
||||
}
|
||||
// Re-read the settings file for the same reason the settings page
|
||||
// does: another instance may have written it since.
|
||||
*ctl.settings.settings.borrow_mut() = ctl.settings.store.load();
|
||||
|
||||
@@ -908,6 +908,12 @@ pub fn run(paths: Vec<PathBuf>) -> Result<()> {
|
||||
// this runs once at startup, where a library can be opened, closed and
|
||||
// re-opened for the whole life of the window.
|
||||
{
|
||||
// Whether the header offers Import at all. A fact about the platform,
|
||||
// set once: it cannot change while the window is open, and on Android
|
||||
// it is false because there is no card to reach and nothing to write
|
||||
// through (`dr_plat::imports_supported`).
|
||||
window.set_import_supported(dr_plat::imports_supported());
|
||||
|
||||
let import = import_ui::ImportController::new(settings.clone(), activity.clone());
|
||||
let library_for_context = library.clone();
|
||||
let weak = window.as_weak();
|
||||
@@ -927,6 +933,10 @@ pub fn run(paths: Vec<PathBuf>) -> Result<()> {
|
||||
credentials: creds,
|
||||
user_id: session.user_id.clone(),
|
||||
library: session.root.clone(),
|
||||
// The same shard store the grid reads and the sync
|
||||
// pushes, so a thumbnail made during an import is the
|
||||
// one every other client gets.
|
||||
thumbs: library::thumbs_dir(&session.server, &session.user_id),
|
||||
}),
|
||||
})
|
||||
},
|
||||
|
||||
Reference in New Issue
Block a user