fix(backfill): convert exiftool-legacy rows whose only dates are zero
backfill_utc_dates refused any fs_time re-resolution of a tag-dated row, assuming exiftool had failed on the file. Files whose date tags are all 0000:00:00 (e.g. uv-prod-long.mp4) were read fine and simply have no date, so they sat unconverted and re-ran through exiftool every tick. resolve_dates_batch_report now also returns the paths exiftool read without error (-Error surfaces ExifTool:Error in the JSON). The drain lets fs_time stand for an exiftool-legacy row only when exiftool read the file; read errors / missing entries and kamadak exif-legacy rows keep refusing. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
783fcb8dbe
commit
55d2e888d6
3 files changed
+222
-18
No files matched your search
+143
-10
@@ -302,6 +302,32 @@ pub fn backfill_utc_dates(
|
||||
backfill_utc_dates_capped(library, exif_dao, reel_dao, display_tz, cursor, cap)
|
||||
}
|
||||
|
||||
/// Whether a re-resolution can stand in for a row's legacy date. Only an
|
||||
/// `fs_time` landing on a tag-dated row is suspect:
|
||||
///
|
||||
/// - legacy `exif` (kamadak-exif) — a different reader found a date there,
|
||||
/// so exiftool coming up empty says nothing about it: refuse.
|
||||
/// - legacy `exiftool` — refuse unless exiftool read the file this pass.
|
||||
/// If it did and found no usable date (e.g. a muxer's all-zero
|
||||
/// `CreateDate`s), the legacy value was never a real capture date and
|
||||
/// fs_time is the honest answer; without this the row re-runs through
|
||||
/// exiftool every tick and never converts. If it didn't (crash, read
|
||||
/// error), the legacy date may be real.
|
||||
fn utc_resolution_is_faithful(
|
||||
legacy_source: Option<&str>,
|
||||
resolved_source: date_resolver::DateSource,
|
||||
exiftool_read: bool,
|
||||
) -> bool {
|
||||
if resolved_source != date_resolver::DateSource::FsTime {
|
||||
return true;
|
||||
}
|
||||
match legacy_source {
|
||||
Some("exif") => false,
|
||||
Some("exiftool") => exiftool_read,
|
||||
_ => true,
|
||||
}
|
||||
}
|
||||
|
||||
fn backfill_utc_dates_capped(
|
||||
library: &libraries::Library,
|
||||
exif_dao: &Arc<Mutex<Box<dyn ExifDao>>>,
|
||||
@@ -390,14 +416,16 @@ fn backfill_utc_dates_capped(
|
||||
// One exiftool subprocess for the whole batch — byte-identical
|
||||
// call shape to the #117 audit preview.
|
||||
let paths: Vec<PathBuf> = filed.iter().map(|(_, p)| p.clone()).collect();
|
||||
let resolved = date_resolver::resolve_dates_batch(&paths, &HashMap::new());
|
||||
let batch = date_resolver::resolve_dates_batch_report(&paths, &HashMap::new());
|
||||
for (row, abs) in filed {
|
||||
let legacy_had_tag = matches!(
|
||||
row.date_taken_source.as_deref(),
|
||||
Some("exif") | Some("exiftool")
|
||||
);
|
||||
match resolved.get(&abs) {
|
||||
Some(rd) if !(legacy_had_tag && rd.source == date_resolver::DateSource::FsTime) => {
|
||||
match batch.resolved.get(&abs) {
|
||||
Some(rd)
|
||||
if utc_resolution_is_faithful(
|
||||
row.date_taken_source.as_deref(),
|
||||
rd.source,
|
||||
batch.exiftool_read.contains(&abs),
|
||||
) =>
|
||||
{
|
||||
match (rd.date_utc, rd.tz_source) {
|
||||
(Some(utc), Some(tz)) => conversions.push(Conversion {
|
||||
row,
|
||||
@@ -408,9 +436,9 @@ fn backfill_utc_dates_capped(
|
||||
_ => outcome.unresolved += 1,
|
||||
}
|
||||
}
|
||||
// No result, or a row whose legacy date came from a tag
|
||||
// now resolving to fs_time (exiftool failed on it): don't
|
||||
// bake a filesystem time in over a real capture date.
|
||||
// No result, or a tag-dated row now resolving to fs_time
|
||||
// without exiftool having read the file: don't bake a
|
||||
// filesystem time in over a real capture date.
|
||||
_ => {
|
||||
debug!(
|
||||
"utc_backfill: no faithful resolution for lib {} {}",
|
||||
@@ -1494,6 +1522,111 @@ mod tests {
|
||||
assert_eq!(progress[&1], 0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn utc_resolution_fs_time_over_tag_date_needs_an_exiftool_read() {
|
||||
use date_resolver::DateSource::{Exiftool, FsTime};
|
||||
// Anything but fs_time stands, whatever the legacy source.
|
||||
assert!(utc_resolution_is_faithful(
|
||||
Some("exiftool"),
|
||||
Exiftool,
|
||||
false
|
||||
));
|
||||
assert!(utc_resolution_is_faithful(Some("exif"), Exiftool, false));
|
||||
// fs_time over a non-tag legacy source stands.
|
||||
assert!(utc_resolution_is_faithful(Some("fs_time"), FsTime, false));
|
||||
assert!(utc_resolution_is_faithful(None, FsTime, false));
|
||||
// Legacy exiftool: only once exiftool read the file and found no
|
||||
// usable date.
|
||||
assert!(utc_resolution_is_faithful(Some("exiftool"), FsTime, true));
|
||||
assert!(!utc_resolution_is_faithful(Some("exiftool"), FsTime, false));
|
||||
// Legacy kamadak exif: never — exiftool's read doesn't vouch for it.
|
||||
assert!(!utc_resolution_is_faithful(Some("exif"), FsTime, true));
|
||||
assert!(!utc_resolution_is_faithful(Some("exif"), FsTime, false));
|
||||
}
|
||||
|
||||
/// Tiny H.264 MP4 with no metadata: exiftool reads it cleanly but every
|
||||
/// QuickTime / Track date is the `0000:00:00 00:00:00` placeholder.
|
||||
fn write_zero_date_video(root: &std::path::Path, rel: &str) {
|
||||
run_ok(
|
||||
std::process::Command::new("ffmpeg")
|
||||
.args([
|
||||
"-y",
|
||||
"-loglevel",
|
||||
"error",
|
||||
"-f",
|
||||
"lavfi",
|
||||
"-i",
|
||||
"color=c=black:s=32x32:d=0.4",
|
||||
"-c:v",
|
||||
"libx264",
|
||||
"-pix_fmt",
|
||||
"yuv420p",
|
||||
"-fflags",
|
||||
"+bitexact",
|
||||
"-map_metadata",
|
||||
"-1",
|
||||
])
|
||||
.arg(root.join(rel)),
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn utc_backfill_legacy_exiftool_row_with_only_zero_dates_converts_to_fs_time() {
|
||||
if !tools_available() {
|
||||
eprintln!("skipping: ffmpeg/exiftool not on PATH");
|
||||
return;
|
||||
}
|
||||
let (tmp, library, conn, exif_dao, _face_dao) = setup();
|
||||
let reel_dao = reel_dao_for(&conn);
|
||||
// Both legacy-exiftool rows land on fs_time. exiftool reads the
|
||||
// video cleanly and finds only zero dates (the prod
|
||||
// `uv-prod-long.mp4` case); the "jpg" is binary junk exiftool
|
||||
// reports a File format error for, so it never got a look inside.
|
||||
// (Printable junk wouldn't do: exiftool reads it as a text file.)
|
||||
write_zero_date_video(tmp.path(), "uv-long.mp4");
|
||||
write_image(
|
||||
tmp.path(),
|
||||
"unreadable.jpg",
|
||||
&[0, 1, 2, 3, 4, 5, 6, 7, 8, 9],
|
||||
);
|
||||
let video = insert_legacy(
|
||||
&exif_dao,
|
||||
1,
|
||||
"uv-long.mp4",
|
||||
Some(1_000_000_000),
|
||||
Some("exiftool"),
|
||||
);
|
||||
let junk = insert_legacy(
|
||||
&exif_dao,
|
||||
1,
|
||||
"unreadable.jpg",
|
||||
Some(1_000_000_000),
|
||||
Some("exiftool"),
|
||||
);
|
||||
|
||||
let mut cursor = 0;
|
||||
let out = backfill_utc_dates_capped(&library, &exif_dao, &reel_dao, NY, &mut cursor, 10);
|
||||
assert_eq!(out.converted, 1);
|
||||
assert_eq!(out.unresolved, 1);
|
||||
|
||||
let fs_secs = std::fs::metadata(tmp.path().join("uv-long.mp4"))
|
||||
.ok()
|
||||
.and_then(|m| crate::utils::earliest_fs_time(&m))
|
||||
.map(|t| chrono::DateTime::<chrono::Utc>::from(t).timestamp())
|
||||
.expect("fs time");
|
||||
let r = row(&conn, video);
|
||||
assert_eq!(r.date_utc, Some(fs_secs));
|
||||
assert_eq!(r.camera_offset_minutes, None);
|
||||
assert_eq!(r.date_taken_tz_source.as_deref(), Some("utc_direct"));
|
||||
// Legacy pair stays frozen.
|
||||
assert_eq!(r.date_taken, Some(1_000_000_000));
|
||||
assert_eq!(r.date_taken_source.as_deref(), Some("exiftool"));
|
||||
|
||||
// The read-error row keeps its legacy date and stays pending.
|
||||
assert_eq!(row(&conn, junk).date_utc, None);
|
||||
assert_eq!(remaining(&exif_dao, 1), 1);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn utc_backfill_unresolvable_rows_cannot_stall_the_rows_behind_them() {
|
||||
let (tmp, library, conn, exif_dao, _face_dao) = setup();
|
||||
|
||||
+75
-7
@@ -48,7 +48,7 @@
|
||||
//! In-class files invert that rule deliberately (ADR-0001): a renamed
|
||||
//! screenshot or a chat-export timestamp is the only honest date left.
|
||||
|
||||
use std::collections::HashMap;
|
||||
use std::collections::{HashMap, HashSet};
|
||||
use std::io::Write;
|
||||
use std::path::{Path, PathBuf};
|
||||
use std::process::{Command, Stdio};
|
||||
@@ -259,7 +259,30 @@ pub fn resolve_dates_batch(
|
||||
paths: &[PathBuf],
|
||||
prior_exif_priors: &HashMap<PathBuf, ExifPrior>,
|
||||
) -> HashMap<PathBuf, ResolvedDate> {
|
||||
resolve_dates_batch_report(paths, prior_exif_priors).resolved
|
||||
}
|
||||
|
||||
/// [`resolve_dates_batch`]'s result plus which files exiftool actually read.
|
||||
#[derive(Debug, Default)]
|
||||
pub struct BatchResolution {
|
||||
pub resolved: HashMap<PathBuf, ResolvedDate>,
|
||||
/// Paths exiftool returned an error-free entry for, whether or not it
|
||||
/// held a usable date. A path in here that still resolved to `fs_time`
|
||||
/// genuinely has no date tag (e.g. a muxer that wrote all-zero
|
||||
/// `CreateDate`s); a path missing from it was never read — exiftool
|
||||
/// absent, crashed, or reported an error for that file.
|
||||
pub exiftool_read: HashSet<PathBuf>,
|
||||
}
|
||||
|
||||
/// [`resolve_dates_batch`], also reporting which files exiftool read, so
|
||||
/// a caller can tell "no date tag in the file" from "exiftool didn't get
|
||||
/// a look at it" when the waterfall lands on `fs_time`.
|
||||
pub fn resolve_dates_batch_report(
|
||||
paths: &[PathBuf],
|
||||
prior_exif_priors: &HashMap<PathBuf, ExifPrior>,
|
||||
) -> BatchResolution {
|
||||
let mut out: HashMap<PathBuf, ResolvedDate> = HashMap::new();
|
||||
let mut exiftool_read: HashSet<PathBuf> = HashSet::new();
|
||||
let mut needs_exiftool: Vec<&Path> = Vec::with_capacity(paths.len());
|
||||
|
||||
for path in paths {
|
||||
@@ -282,7 +305,10 @@ pub fn resolve_dates_batch(
|
||||
// One subprocess for the whole batch; each entry is already fully
|
||||
// resolved (legacy + new-contract fields).
|
||||
for (path, resolved) in exiftool_dates_batch(&needs_exiftool) {
|
||||
out.insert(path, resolved);
|
||||
if let Some(resolved) = resolved {
|
||||
out.insert(path.clone(), resolved);
|
||||
}
|
||||
exiftool_read.insert(path);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -310,7 +336,10 @@ pub fn resolve_dates_batch(
|
||||
}
|
||||
}
|
||||
|
||||
out
|
||||
BatchResolution {
|
||||
resolved: out,
|
||||
exiftool_read,
|
||||
}
|
||||
}
|
||||
|
||||
/// Tag priority for exiftool extraction. First parseable value wins.
|
||||
@@ -384,9 +413,12 @@ pub fn exiftool_available() -> bool {
|
||||
/// Append the exiftool arguments shared by the single-file and batch
|
||||
/// invocations: raw date strings (no `-d %s` — the resolver interprets
|
||||
/// them itself) with group prefixes for classification, plus the offset
|
||||
/// / GPS evidence tags for the cascade.
|
||||
/// / GPS evidence tags for the cascade. `-Error` puts exiftool's per-file
|
||||
/// read error (`ExifTool:Error`, e.g. "File format error") into the JSON
|
||||
/// — under `-q` it otherwise only reaches stderr, and the file's entry
|
||||
/// looks like a clean read that found no tags.
|
||||
fn exiftool_common_args(cmd: &mut Command) {
|
||||
cmd.arg("-j").arg("-q").arg("-G1");
|
||||
cmd.arg("-j").arg("-q").arg("-G1").arg("-Error");
|
||||
for tag in EXIFTOOL_DATE_TAGS {
|
||||
cmd.arg(format!("-{}", tag));
|
||||
}
|
||||
@@ -430,7 +462,12 @@ fn exiftool_date_single(path: &Path) -> Option<ResolvedDate> {
|
||||
/// libraries with very long path components. Resolution is the same pure
|
||||
/// function as the single-file path, so the drain and ingest produce
|
||||
/// identical results for the same file.
|
||||
fn exiftool_dates_batch(paths: &[&Path]) -> HashMap<PathBuf, ResolvedDate> {
|
||||
///
|
||||
/// Keyed by every file exiftool read without error, `None` when the read
|
||||
/// found no usable date. Files it never reported (crash, partial output)
|
||||
/// or reported an error for are absent, unless the entry still carried a
|
||||
/// usable date.
|
||||
fn exiftool_dates_batch(paths: &[&Path]) -> HashMap<PathBuf, Option<ResolvedDate>> {
|
||||
let mut out = HashMap::new();
|
||||
if paths.is_empty() || !exiftool_available() {
|
||||
return out;
|
||||
@@ -481,7 +518,8 @@ fn exiftool_dates_batch(paths: &[&Path]) -> HashMap<PathBuf, ResolvedDate> {
|
||||
);
|
||||
}
|
||||
for file in parse_exiftool_payload(&output.stdout) {
|
||||
if let Some(resolved) = resolve_exiftool_payload(&file.date_tags, &file.evidence) {
|
||||
let resolved = resolve_exiftool_payload(&file.date_tags, &file.evidence);
|
||||
if resolved.is_some() || !file.read_error {
|
||||
out.insert(PathBuf::from(file.source_file), resolved);
|
||||
}
|
||||
}
|
||||
@@ -810,6 +848,8 @@ struct ExiftoolFileData {
|
||||
source_file: String,
|
||||
date_tags: Vec<RawDateTag>,
|
||||
evidence: ExiftoolEvidence,
|
||||
/// exiftool reported `ExifTool:Error` for this file (couldn't read it).
|
||||
read_error: bool,
|
||||
}
|
||||
|
||||
/// Split the exiftool `-j -G1` output into per-file date tags + evidence.
|
||||
@@ -841,11 +881,16 @@ fn parse_exiftool_payload(stdout: &[u8]) -> Vec<ExiftoolFileData> {
|
||||
source_file: source_file.to_string(),
|
||||
date_tags: Vec::new(),
|
||||
evidence: ExiftoolEvidence::default(),
|
||||
read_error: false,
|
||||
};
|
||||
for (key, value) in obj {
|
||||
let Some((group, tag)) = key.split_once(':') else {
|
||||
continue;
|
||||
};
|
||||
if group == "ExifTool" && tag == "Error" {
|
||||
data.read_error = true;
|
||||
continue;
|
||||
}
|
||||
let Some(v) = value.as_str() else {
|
||||
continue;
|
||||
};
|
||||
@@ -962,6 +1007,29 @@ mod tests {
|
||||
assert!((lon - (-74.006)).abs() < 1e-9);
|
||||
}
|
||||
|
||||
/// A clean read whose date tags are all exiftool's zero placeholder is
|
||||
/// "no usable date", not a read error; `ExifTool:Error` is the error
|
||||
/// flag and isn't mistaken for a date tag.
|
||||
#[test]
|
||||
fn parse_payload_flags_read_errors_not_zero_dates() {
|
||||
let json = br#"[{
|
||||
"SourceFile": "/lib/zero.mp4",
|
||||
"QuickTime:CreateDate": "0000:00:00 00:00:00",
|
||||
"Track1:TrackCreateDate": "0000:00:00 00:00:00"
|
||||
},
|
||||
{
|
||||
"SourceFile": "/lib/bad.jpg",
|
||||
"ExifTool:Error": "File format error"
|
||||
}]"#;
|
||||
let files = parse_exiftool_payload(json);
|
||||
assert_eq!(files.len(), 2);
|
||||
assert!(!files[0].read_error);
|
||||
assert_eq!(files[0].date_tags.len(), 2);
|
||||
assert!(resolve_exiftool_payload(&files[0].date_tags, &files[0].evidence).is_none());
|
||||
assert!(files[1].read_error);
|
||||
assert!(files[1].date_tags.is_empty());
|
||||
}
|
||||
|
||||
/// #163: exiftool's default print conversion renders hemisphere refs as
|
||||
/// words. Before the fix "West" / "South" never matched, so western /
|
||||
/// southern coordinates came out positive and the GPS offset lookup hit
|
||||
|
||||
Reference in new issue
Block a user