Added settigns validation, made skip reason more expansible

This commit is contained in:
Arnaud_Cayrol 2026-01-31 09:42:24 +01:00
parent 41506d6a3a
commit a6d491d3c2
7 changed files with 321 additions and 88 deletions

View file

@ -9,8 +9,38 @@
import ResultsView from './lib/components/ResultsView.svelte';
import SettingsPanel from './lib/components/SettingsPanel.svelte';
// LocalStorage keys
const STORAGE_KEY_PERSON = 'immich-timelapse-selected-person';
// Load persisted state from localStorage
function loadPersistedPersonId() {
try {
const stored = localStorage.getItem(STORAGE_KEY_PERSON);
if (stored) {
const parsed = JSON.parse(stored);
return parsed?.id || null;
}
} catch (e) {
console.warn('Failed to load persisted person:', e);
}
return null;
}
function persistSelectedPerson(person) {
try {
if (person) {
localStorage.setItem(STORAGE_KEY_PERSON, JSON.stringify({ id: person.id, name: person.name }));
} else {
localStorage.removeItem(STORAGE_KEY_PERSON);
}
} catch (e) {
console.warn('Failed to persist person:', e);
}
}
let connectionOk = $state(false);
let selectedPerson = $state(null);
let initialSelectedPersonId = $state(loadPersistedPersonId());
let jobStatus = $state('idle');
let progress = $state({ completed: 0, total: 0, message: '' });
let pollInterval = $state(null);
@ -93,6 +123,7 @@
function handlePersonSelect(person) {
selectedPerson = person;
persistSelectedPerson(person);
}
function handleJobUpdate(data) {
@ -194,6 +225,7 @@
<PeopleSelector
onselect={handlePersonSelect}
disabled={isJobRunning}
initialSelectedId={initialSelectedPersonId}
/>
{#if selectedPerson && !isJobRunning}

View file

@ -1,13 +1,13 @@
<script>
import { onMount } from 'svelte';
let { disabled = false, onselect } = $props();
let { disabled = false, onselect, initialSelectedId = null } = $props();
let people = $state([]);
let loading = $state(true);
let error = $state(null);
let searchQuery = $state('');
let selectedId = $state(null);
let selectedId = $state(initialSelectedId);
let filteredPeople = $derived(
people.filter(p => {
@ -24,6 +24,20 @@
const res = await fetch('/api/people');
if (!res.ok) throw new Error('Failed to load people');
people = await res.json();
// If we have an initial selection, find and notify parent about the full person object
if (initialSelectedId && !selectedId) {
selectedId = initialSelectedId;
}
if (selectedId) {
const person = people.find(p => p.id === selectedId);
if (person) {
onselect?.(person);
} else {
// Person no longer exists, clear selection
selectedId = null;
}
}
} catch (e) {
error = e.message;
} finally {

View file

@ -30,36 +30,32 @@
// Person being processed
let personDisplay = $derived(progress.person_name || progress.person_id || null);
// Skip statistics from the backend
let skipStats = $derived(progress.skip_stats || {
face_too_small: 0,
eyes_closed: 0,
head_turned: 0,
too_dark: 0,
too_bright: 0,
no_face_detected: 0,
download_failed: 0,
decode_failed: 0,
crop_failed: 0,
total: 0,
});
// Convert snake_case to readable label (e.g., "face_too_small" -> "Face too small")
function formatLabel(key) {
return key
.split('_')
.map((word, i) => i === 0 ? word.charAt(0).toUpperCase() + word.slice(1) : word)
.join(' ');
}
// Skip statistics from the backend - dynamically handle any keys
let skipStats = $derived(progress.skip_stats || {});
// Calculate total from all numeric values in skip_stats
let skipTotal = $derived(
Object.values(skipStats).reduce((sum, val) => sum + (typeof val === 'number' ? val : 0), 0)
);
// Calculate kept (successful) count: completed - skipped
let keptCount = $derived(Math.max(0, progress.completed - skipStats.total));
let keptCount = $derived(Math.max(0, progress.completed - skipTotal));
// Filter to only show non-zero skip reasons
// Dynamically build skip reasons from whatever the backend sends
// Filter to only show non-zero counts
let skipReasons = $derived(
[
{ label: 'Face too small', count: skipStats.face_too_small },
{ label: 'Eyes closed', count: skipStats.eyes_closed },
{ label: 'Head turned', count: skipStats.head_turned },
{ label: 'Too dark', count: skipStats.too_dark },
{ label: 'Too bright', count: skipStats.too_bright },
{ label: 'No face detected', count: skipStats.no_face_detected },
{ label: 'Download failed', count: skipStats.download_failed },
{ label: 'Decode failed', count: skipStats.decode_failed },
{ label: 'Crop failed', count: skipStats.crop_failed },
].filter(r => r.count > 0)
Object.entries(skipStats)
.filter(([_, count]) => typeof count === 'number' && count > 0)
.map(([key, count]) => ({ label: formatLabel(key), count }))
.sort((a, b) => b.count - a.count) // Sort by count descending
);
async function cancelProcessing() {
@ -104,7 +100,7 @@
<div class="skip-header">
<span class="skip-label">Discarded:Kept</span>
<span class="skip-totals">
<span class="skipped-count">{skipStats.total}</span>
<span class="skipped-count">{skipTotal}</span>
<span class="separator">:</span>
<span class="kept-count">{keptCount}</span>
</span>

View file

@ -145,6 +145,25 @@ impl Default for VideoConfig {
}
}
/// Persistable configuration (excludes sensitive API credentials).
///
/// This struct contains only the settings that can safely be written to disk.
/// API credentials should always come from environment variables.
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct PersistableConfig {
pub processing: ProcessingConfig,
pub video: VideoConfig,
}
impl From<&Config> for PersistableConfig {
fn from(config: &Config) -> Self {
Self {
processing: config.processing.clone(),
video: config.video.clone(),
}
}
}
impl Config {
/// Load configuration from a TOML file.
pub fn from_file(path: impl AsRef<std::path::Path>) -> crate::error::Result<Self> {
@ -242,6 +261,18 @@ impl Config {
}
Ok(())
}
/// Save processing and video configuration to a TOML file.
///
/// Only saves `processing` and `video` sections - API credentials are
/// intentionally excluded as they should come from environment variables.
pub fn save_to_file(&self, path: impl AsRef<std::path::Path>) -> crate::error::Result<()> {
let persistable = PersistableConfig::from(self);
let content = toml::to_string_pretty(&persistable)
.map_err(|e| crate::error::Error::Config(format!("Failed to serialize config: {}", e)))?;
std::fs::write(path, content)?;
Ok(())
}
}
#[cfg(test)]
@ -262,4 +293,34 @@ mod tests {
let result = config.validate();
assert!(result.is_err());
}
#[test]
fn test_save_and_load_config() {
// Create a config with custom values
let mut config = Config::default();
config.processing.resize_size = 256;
config.processing.face_resolution_threshold = 100;
config.video.framerate = 30;
config.video.crf = 18;
// Save to a temp file
let temp_dir = std::env::temp_dir();
let temp_path = temp_dir.join("test_config.toml");
config.save_to_file(&temp_path).expect("Failed to save config");
// Verify file was created and contains expected content
let content = std::fs::read_to_string(&temp_path).expect("Failed to read config");
assert!(content.contains("resize_size = 256"));
assert!(content.contains("face_resolution_threshold = 100"));
assert!(content.contains("framerate = 30"));
assert!(content.contains("crf = 18"));
// Verify API credentials are NOT in the file
assert!(!content.contains("api_key"));
assert!(!content.contains("base_url"));
// Clean up
let _ = std::fs::remove_file(&temp_path);
}
}

View file

@ -15,7 +15,7 @@ use crate::face_processing::{AssetResult, ProcessedFace, SkipReason};
use crate::immich_api::{Asset, FaceData, ImmichClient};
use crate::utils::sanitize_folder_name;
use crate::video::compile_timelapse;
use crate::web::{AppState, JobStatus, Progress, SkipStats};
use crate::web::{AppState, AtomicSkipStats, JobStatus, Progress, SkipStats};
use image::ImageFormat;
use std::io::Cursor;
@ -241,16 +241,8 @@ async fn run_job_inner(
let config = Arc::new(config);
let output_dirs = Arc::new(output_dirs);
// Atomic counters for real-time skip statistics
let skip_face_too_small = Arc::new(AtomicU32::new(0));
let skip_eyes_closed = Arc::new(AtomicU32::new(0));
let skip_head_turned = Arc::new(AtomicU32::new(0));
let skip_too_dark = Arc::new(AtomicU32::new(0));
let skip_too_bright = Arc::new(AtomicU32::new(0));
let skip_no_face = Arc::new(AtomicU32::new(0));
let skip_download_failed = Arc::new(AtomicU32::new(0));
let skip_decode_failed = Arc::new(AtomicU32::new(0));
let skip_crop_failed = Arc::new(AtomicU32::new(0));
// Atomic counters for real-time skip statistics (consolidated into single struct)
let skip_stats = Arc::new(AtomicSkipStats::new());
let mut handles = Vec::with_capacity(assets_with_faces.len());
@ -275,16 +267,8 @@ async fn run_job_inner(
let state = state.clone();
let task_cancel_token = cancel_token.clone();
// Clone skip counters for this task
let skip_face_too_small = skip_face_too_small.clone();
let skip_eyes_closed = skip_eyes_closed.clone();
let skip_head_turned = skip_head_turned.clone();
let skip_too_dark = skip_too_dark.clone();
let skip_too_bright = skip_too_bright.clone();
let skip_no_face = skip_no_face.clone();
let skip_download_failed = skip_download_failed.clone();
let skip_decode_failed = skip_decode_failed.clone();
let skip_crop_failed = skip_crop_failed.clone();
// Clone skip stats for this task
let skip_stats = skip_stats.clone();
// Clone person info for this task
let person_id = task_person_id.clone();
@ -313,34 +297,13 @@ async fn run_job_inner(
// Update skip counters based on result
if let AssetResult::Skipped { reason, .. } = &result {
match reason {
SkipReason::FaceTooSmall => skip_face_too_small.fetch_add(1, Ordering::SeqCst),
SkipReason::EyesClosed => skip_eyes_closed.fetch_add(1, Ordering::SeqCst),
SkipReason::HeadTurned => skip_head_turned.fetch_add(1, Ordering::SeqCst),
SkipReason::TooDark => skip_too_dark.fetch_add(1, Ordering::SeqCst),
SkipReason::TooBright => skip_too_bright.fetch_add(1, Ordering::SeqCst),
SkipReason::NoFaceDetected => skip_no_face.fetch_add(1, Ordering::SeqCst),
SkipReason::DownloadFailed => skip_download_failed.fetch_add(1, Ordering::SeqCst),
SkipReason::DecodeFailed => skip_decode_failed.fetch_add(1, Ordering::SeqCst),
SkipReason::CropFailed => skip_crop_failed.fetch_add(1, Ordering::SeqCst),
SkipReason::Cancelled => 0, // Don't count cancellation
};
skip_stats.increment(reason);
}
// Update progress with current skip stats (only if not cancelled)
if !task_cancel_token.is_cancelled() {
let done = completed.fetch_add(1, Ordering::SeqCst) + 1;
let current_skip_stats = SkipStats {
face_too_small: skip_face_too_small.load(Ordering::SeqCst),
eyes_closed: skip_eyes_closed.load(Ordering::SeqCst),
head_turned: skip_head_turned.load(Ordering::SeqCst),
too_dark: skip_too_dark.load(Ordering::SeqCst),
too_bright: skip_too_bright.load(Ordering::SeqCst),
no_face_detected: skip_no_face.load(Ordering::SeqCst),
download_failed: skip_download_failed.load(Ordering::SeqCst),
decode_failed: skip_decode_failed.load(Ordering::SeqCst),
crop_failed: skip_crop_failed.load(Ordering::SeqCst),
};
let current_skip_stats = skip_stats.snapshot();
state
.update_progress(Progress {
status: JobStatus::Running,
@ -378,17 +341,7 @@ async fn run_job_inner(
}
// Get final skip statistics from atomic counters
let skip_stats = SkipStats {
face_too_small: skip_face_too_small.load(Ordering::SeqCst),
eyes_closed: skip_eyes_closed.load(Ordering::SeqCst),
head_turned: skip_head_turned.load(Ordering::SeqCst),
too_dark: skip_too_dark.load(Ordering::SeqCst),
too_bright: skip_too_bright.load(Ordering::SeqCst),
no_face_detected: skip_no_face.load(Ordering::SeqCst),
download_failed: skip_download_failed.load(Ordering::SeqCst),
decode_failed: skip_decode_failed.load(Ordering::SeqCst),
crop_failed: skip_crop_failed.load(Ordering::SeqCst),
};
let final_skip_stats = skip_stats.snapshot();
// Count results
let successful = results
@ -399,7 +352,7 @@ async fn run_job_inner(
.iter()
.filter(|r| matches!(r, AssetResult::Error { .. }))
.count();
let skipped = skip_stats.total();
let skipped = final_skip_stats.total();
tracing::info!(
"Processing complete: {} successful, {} skipped, {} errors",
@ -415,7 +368,7 @@ async fn run_job_inner(
completed: total,
total,
message: Some("Processing complete, preparing video...".to_string()),
skip_stats: skip_stats.clone(),
skip_stats: final_skip_stats.clone(),
person_id: Some(params.person_id.clone()),
person_name: params.person_name.clone(),
})
@ -440,7 +393,7 @@ async fn run_job_inner(
completed: 0,
total: successful as u32,
message: Some("Compiling video...".to_string()),
skip_stats: skip_stats.clone(),
skip_stats: final_skip_stats.clone(),
person_id: Some(params.person_id.clone()),
person_name: params.person_name.clone(),
})

View file

@ -9,6 +9,21 @@ use axum::{
};
use serde::{Deserialize, Serialize};
/// Validation error with field name and message.
struct ValidationError {
field: &'static str,
message: String,
}
impl ValidationError {
fn new(field: &'static str, message: impl Into<String>) -> Self {
Self {
field,
message: message.into(),
}
}
}
/// Configuration response (excludes sensitive API credentials).
#[derive(Serialize)]
pub struct ConfigResponse {
@ -53,6 +68,86 @@ pub struct VideoConfigUpdate {
pub crf: Option<u32>,
}
/// Validate processing configuration update values.
fn validate_processing_config(proc: &ProcessingConfigUpdate) -> Result<(), ValidationError> {
if let Some(v) = proc.resize_size {
if v < 64 || v > 4096 {
return Err(ValidationError::new(
"processing.resize_size",
format!("must be between 64 and 4096, got {}", v),
));
}
}
if let Some(v) = proc.face_resolution_threshold {
if v == 0 || v > 1000 {
return Err(ValidationError::new(
"processing.face_resolution_threshold",
format!("must be between 1 and 1000, got {}", v),
));
}
}
if let Some(v) = proc.pose_threshold {
if v <= 0.0 || v > 90.0 {
return Err(ValidationError::new(
"processing.pose_threshold",
format!("must be between 0 and 90 degrees, got {}", v),
));
}
}
if let Some(v) = proc.ear_threshold {
if v <= 0.0 || v > 1.0 {
return Err(ValidationError::new(
"processing.ear_threshold",
format!("must be between 0 and 1, got {}", v),
));
}
}
if let Some(v) = proc.max_workers {
if v == 0 || v > 64 {
return Err(ValidationError::new(
"processing.max_workers",
format!("must be between 1 and 64, got {}", v),
));
}
}
Ok(())
}
/// Validate video configuration update values.
fn validate_video_config(vid: &VideoConfigUpdate) -> Result<(), ValidationError> {
if let Some(v) = vid.framerate {
if v == 0 || v > 120 {
return Err(ValidationError::new(
"video.framerate",
format!("must be between 1 and 120, got {}", v),
));
}
}
if let Some(v) = vid.crf {
if v > 51 {
return Err(ValidationError::new(
"video.crf",
format!("must be between 0 and 51, got {}", v),
));
}
}
if let Some(ref v) = vid.codec {
// Allow common video codecs
let valid_codecs = ["libx264", "libx265", "libvpx", "libvpx-vp9", "libaom-av1"];
if !valid_codecs.contains(&v.as_str()) {
return Err(ValidationError::new(
"video.codec",
format!(
"must be one of: {}, got '{}'",
valid_codecs.join(", "),
v
),
));
}
}
Ok(())
}
/// Update configuration.
pub async fn update_config(
State(state): State<AppState>,
@ -69,6 +164,24 @@ pub async fn update_config(
}
}
// Validate input before updating
if let Some(ref proc) = update.processing {
validate_processing_config(proc).map_err(|e| {
(
StatusCode::BAD_REQUEST,
format!("Invalid {}: {}", e.field, e.message),
)
})?;
}
if let Some(ref vid) = update.video {
validate_video_config(vid).map_err(|e| {
(
StatusCode::BAD_REQUEST,
format!("Invalid {}: {}", e.field, e.message),
)
})?;
}
// Update config
{
let mut config = state.config.write().await;
@ -109,7 +222,12 @@ pub async fn update_config(
}
}
tracing::info!("Configuration updated");
// Persist to file (non-blocking, best-effort)
if let Err(e) = config.save_to_file("config.toml") {
tracing::warn!("Failed to persist config to file: {}", e);
} else {
tracing::info!("Configuration updated and persisted to config.toml");
}
}
// Return updated config

View file

@ -1,6 +1,7 @@
//! Application state for the web server.
use crate::config::Config;
use std::sync::atomic::{AtomicU32, Ordering};
use std::sync::Arc;
use tokio::sync::{broadcast, RwLock};
use tokio_util::sync::CancellationToken;
@ -54,6 +55,64 @@ impl SkipStats {
}
}
/// Atomic version of SkipStats for thread-safe concurrent updates.
///
/// This consolidates all skip counters into a single struct that can be
/// shared across async tasks with Arc, avoiding the need to clone multiple
/// individual atomic counters.
#[derive(Debug, Default)]
pub struct AtomicSkipStats {
pub face_too_small: AtomicU32,
pub eyes_closed: AtomicU32,
pub head_turned: AtomicU32,
pub too_dark: AtomicU32,
pub too_bright: AtomicU32,
pub no_face_detected: AtomicU32,
pub download_failed: AtomicU32,
pub decode_failed: AtomicU32,
pub crop_failed: AtomicU32,
}
impl AtomicSkipStats {
/// Create a new AtomicSkipStats with all counters at zero.
pub fn new() -> Self {
Self::default()
}
/// Increment a specific counter and return the new value.
pub fn increment(&self, reason: &crate::face_processing::SkipReason) -> u32 {
use crate::face_processing::SkipReason;
let counter = match reason {
SkipReason::FaceTooSmall => &self.face_too_small,
SkipReason::EyesClosed => &self.eyes_closed,
SkipReason::HeadTurned => &self.head_turned,
SkipReason::TooDark => &self.too_dark,
SkipReason::TooBright => &self.too_bright,
SkipReason::NoFaceDetected => &self.no_face_detected,
SkipReason::DownloadFailed => &self.download_failed,
SkipReason::DecodeFailed => &self.decode_failed,
SkipReason::CropFailed => &self.crop_failed,
SkipReason::Cancelled => return 0, // Don't count cancellation
};
counter.fetch_add(1, Ordering::SeqCst) + 1
}
/// Get a snapshot of the current stats as a non-atomic SkipStats.
pub fn snapshot(&self) -> SkipStats {
SkipStats {
face_too_small: self.face_too_small.load(Ordering::SeqCst),
eyes_closed: self.eyes_closed.load(Ordering::SeqCst),
head_turned: self.head_turned.load(Ordering::SeqCst),
too_dark: self.too_dark.load(Ordering::SeqCst),
too_bright: self.too_bright.load(Ordering::SeqCst),
no_face_detected: self.no_face_detected.load(Ordering::SeqCst),
download_failed: self.download_failed.load(Ordering::SeqCst),
decode_failed: self.decode_failed.load(Ordering::SeqCst),
crop_failed: self.crop_failed.load(Ordering::SeqCst),
}
}
}
/// Progress information for a running job.
#[derive(Debug, Clone)]
pub struct Progress {