diff --git a/frontend/src/lib/components/SettingsPanel.svelte b/frontend/src/lib/components/SettingsPanel.svelte index 37e9370..e3f42eb 100644 --- a/frontend/src/lib/components/SettingsPanel.svelte +++ b/frontend/src/lib/components/SettingsPanel.svelte @@ -10,36 +10,37 @@ let saveMessage = $state(null); let activeTab = $state('face'); - // Config state + // Config state - matches backend nested structure let config = $state({ processing: { - resize_size: 512, - face_resolution_threshold: 80, - pose_threshold: 25.0, - ear_threshold: 0.2, max_workers: 4, - keep_intermediates: false, - brightness: null, + face_resolution: { + enabled: true, + min_size: 80, + }, + brightness: { + enabled: false, + min_brightness: 0.1, + max_brightness: 0.95, + }, + output: { + size: 512, + keep_intermediates: false, + }, + alignment: { + enabled: false, + left_eye_pos: [0.35, 0.4], + right_eye_pos: [0.65, 0.4], + }, }, video: { - framerate: 15, enabled: true, + framerate: 15, codec: 'libx264', crf: 23, }, }); - // Helper to ensure brightness config exists - function ensureBrightnessConfig() { - if (!config.processing.brightness) { - config.processing.brightness = { - enabled: false, - min_brightness: 0.1, - max_brightness: 0.95, - }; - } - } - async function loadConfig() { loading = true; error = null; @@ -81,17 +82,29 @@ // Default configuration values (must match backend defaults in config.rs) const DEFAULT_CONFIG = { processing: { - resize_size: 512, - face_resolution_threshold: 80, - pose_threshold: 25.0, - ear_threshold: 0.2, max_workers: 4, - keep_intermediates: false, - brightness: null, + face_resolution: { + enabled: true, + min_size: 80, + }, + brightness: { + enabled: false, + min_brightness: 0.1, + max_brightness: 0.95, + }, + output: { + size: 512, + keep_intermediates: false, + }, + alignment: { + enabled: false, + left_eye_pos: [0.35, 0.4], + right_eye_pos: [0.65, 0.4], + }, }, video: { - framerate: 15, enabled: true, + framerate: 15, codec: 'libx264', crf: 23, }, @@ -159,72 +172,47 @@
{#if activeTab === 'face'}
-
- -
+ +
+
+ Face Resolution - {config.processing.face_resolution_threshold}px
+ + {#if config.processing.face_resolution.enabled} +
+ +
+ + {config.processing.face_resolution.min_size}px +
+
+ {/if}
-
- -
- - {config.processing.ear_threshold.toFixed(2)} -
-
- -
- -
- - {config.processing.pose_threshold}° -
-
- - +
Brightness Filter { - ensureBrightnessConfig(); - config.processing.brightness.enabled = e.target.checked; - }} + bind:checked={config.processing.brightness.enabled} />
- {#if config.processing.brightness?.enabled} + {#if config.processing.brightness.enabled}
@@ -254,16 +238,12 @@
{ - ensureBrightnessConfig(); - config.processing.brightness.max_brightness = parseFloat(e.target.value); - }} + bind:value={config.processing.brightness.max_brightness} min="0.5" max="1.0" step="0.05" /> - {(config.processing.brightness?.max_brightness ?? 0.95).toFixed(2)} + {config.processing.brightness.max_brightness.toFixed(2)}
{/if} @@ -279,12 +259,12 @@
- {config.processing.resize_size}px + {config.processing.output.size}px
@@ -310,7 +290,7 @@ Keep Debug Images Save intermediate processing visualizations - +
{:else if activeTab === 'video'} @@ -565,9 +545,15 @@ /* Settings section with header */ .setting-section { - margin-top: 1rem; - padding-top: 1rem; - border-top: 1px solid #333; + margin-bottom: 1rem; + padding-bottom: 1rem; + border-bottom: 1px solid #333; + } + + .setting-section:last-child { + margin-bottom: 0; + padding-bottom: 0; + border-bottom: none; } .section-header { diff --git a/src/config.rs b/src/config.rs index 0f20cc5..69d3502 100644 --- a/src/config.rs +++ b/src/config.rs @@ -4,10 +4,26 @@ //! 1. TOML file (config.toml) //! 2. Environment variables (prefixed with IMMICH_) //! 3. Programmatic overrides +//! +//! ## Step Configuration Pattern +//! +//! Each pipeline step has its own optional sub-config with an `enabled` flag: +//! ```ignore +//! processing: +//! face_resolution: +//! enabled: true +//! min_size: 80 +//! brightness: +//! enabled: false +//! min_brightness: 0.1 +//! max_brightness: 0.95 +//! ``` use serde::{Deserialize, Serialize}; use std::path::PathBuf; +use crate::error::{Error, Result}; + /// Main configuration struct. /// /// This is the single source of truth for all configuration. @@ -67,37 +83,40 @@ impl Default for ApiConfig { } } -/// Face processing parameters. +// ============================================================================ +// Step Configurations +// ============================================================================ + +/// Face resolution validation configuration. #[derive(Debug, Clone, Serialize, Deserialize)] -#[serde(default)] -pub struct ProcessingConfig { - /// Output image size (width and height in pixels). - pub resize_size: u32, +pub struct FaceResolutionConfig { + /// Whether face resolution validation is enabled. + pub enabled: bool, /// Minimum face width/height in pixels. - pub face_resolution_threshold: u32, + /// Faces smaller than this will be skipped. + pub min_size: u32, +} - /// Maximum allowed head yaw angle in degrees. - pub pose_threshold: f32, +impl Default for FaceResolutionConfig { + fn default() -> Self { + Self { + enabled: true, + min_size: 80, + } + } +} - /// Eye Aspect Ratio threshold for eye visibility. - pub ear_threshold: f32, - - /// Target position for left eye as (x%, y%). - pub left_eye_pos: (f32, f32), - - /// Target position for right eye as (x%, y%). - pub right_eye_pos: (f32, f32), - - /// Number of parallel workers for processing. - pub max_workers: usize, - - /// Keep intermediate images (original, cropped) for inspection. - pub keep_intermediates: bool, - - /// Brightness validation settings (optional). - #[serde(skip_serializing_if = "Option::is_none")] - pub brightness: Option, +impl FaceResolutionConfig { + /// Validate the configuration values. + pub fn validate(&self) -> Result<()> { + if self.enabled && self.min_size == 0 { + return Err(Error::Config( + "Face resolution min_size must be greater than 0".to_string(), + )); + } + Ok(()) + } } /// Brightness validation configuration. @@ -125,56 +144,207 @@ impl Default for BrightnessConfig { } } +impl BrightnessConfig { + /// Validate the configuration values. + pub fn validate(&self) -> Result<()> { + if !self.enabled { + return Ok(()); + } + if self.min_brightness < 0.0 || self.min_brightness > 1.0 { + return Err(Error::Config( + "Brightness min_brightness must be between 0.0 and 1.0".to_string(), + )); + } + if self.max_brightness < 0.0 || self.max_brightness > 1.0 { + return Err(Error::Config( + "Brightness max_brightness must be between 0.0 and 1.0".to_string(), + )); + } + if self.min_brightness >= self.max_brightness { + return Err(Error::Config( + "Brightness min_brightness must be less than max_brightness".to_string(), + )); + } + Ok(()) + } +} + +/// Output/resize configuration. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct OutputConfig { + /// Output image size (width and height in pixels). + pub size: u32, + + /// Keep intermediate images (original, cropped) for inspection. + pub keep_intermediates: bool, +} + +impl Default for OutputConfig { + fn default() -> Self { + Self { + size: 512, + keep_intermediates: false, + } + } +} + +impl OutputConfig { + /// Validate the configuration values. + pub fn validate(&self) -> Result<()> { + if self.size < 64 { + return Err(Error::Config( + "Output size must be at least 64 pixels".to_string(), + )); + } + if self.size > 4096 { + return Err(Error::Config( + "Output size must be at most 4096 pixels".to_string(), + )); + } + Ok(()) + } +} + +/// Face alignment configuration (for future landmark-based alignment). +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct AlignmentConfig { + /// Whether face alignment is enabled. + pub enabled: bool, + + /// Target position for left eye as (x%, y%) of output image. + pub left_eye_pos: (f32, f32), + + /// Target position for right eye as (x%, y%) of output image. + pub right_eye_pos: (f32, f32), +} + +impl Default for AlignmentConfig { + fn default() -> Self { + Self { + enabled: false, + left_eye_pos: (0.35, 0.4), + right_eye_pos: (0.65, 0.4), + } + } +} + +// ============================================================================ +// Main Processing Configuration +// ============================================================================ + +/// Face processing parameters. +/// +/// Each pipeline step has its own sub-configuration with an `enabled` flag. +/// This allows fine-grained control over which steps run and their parameters. +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(default)] +pub struct ProcessingConfig { + /// Number of parallel workers for processing. + pub max_workers: usize, + + /// Face resolution validation settings. + #[serde(default)] + pub face_resolution: FaceResolutionConfig, + + /// Brightness validation settings. + #[serde(default)] + pub brightness: BrightnessConfig, + + /// Output image settings. + #[serde(default)] + pub output: OutputConfig, + + /// Face alignment settings (requires landmarks - future feature). + #[serde(default)] + pub alignment: AlignmentConfig, +} + impl Default for ProcessingConfig { fn default() -> Self { Self { - resize_size: 512, - face_resolution_threshold: 80, - pose_threshold: 25.0, - ear_threshold: 0.2, - left_eye_pos: (0.35, 0.4), - right_eye_pos: (0.65, 0.4), max_workers: num_cpus(), - keep_intermediates: false, - brightness: None, + face_resolution: FaceResolutionConfig::default(), + brightness: BrightnessConfig::default(), + output: OutputConfig::default(), + alignment: AlignmentConfig::default(), } } } +impl ProcessingConfig { + /// Validate all step configurations. + pub fn validate(&self) -> Result<()> { + self.face_resolution.validate()?; + self.brightness.validate()?; + self.output.validate()?; + Ok(()) + } +} + fn num_cpus() -> usize { std::thread::available_parallelism() .map(|p| p.get()) .unwrap_or(4) } +// ============================================================================ +// Video Configuration +// ============================================================================ + /// Video compilation settings. #[derive(Debug, Clone, Serialize, Deserialize)] #[serde(default)] pub struct VideoConfig { - /// Output video framerate. - pub framerate: u32, - /// Whether to compile video after processing. pub enabled: bool, + /// Output video framerate. + pub framerate: u32, + /// Video codec (e.g., "libx264"). pub codec: String, - /// Constant Rate Factor for quality (lower = better, 18-28 recommended). + /// Constant Rate Factor for quality (0-51, lower = better, 18-28 recommended). pub crf: u32, } impl Default for VideoConfig { fn default() -> Self { Self { - framerate: 15, enabled: true, + framerate: 15, codec: "libx264".to_string(), crf: 23, } } } +impl VideoConfig { + /// Validate the configuration values. + pub fn validate(&self) -> Result<()> { + if self.framerate == 0 { + return Err(Error::Config( + "Video framerate must be greater than 0".to_string(), + )); + } + if self.framerate > 120 { + return Err(Error::Config( + "Video framerate must be at most 120".to_string(), + )); + } + if self.crf > 51 { + return Err(Error::Config( + "Video CRF must be between 0 and 51".to_string(), + )); + } + Ok(()) + } +} + +// ============================================================================ +// Persistence +// ============================================================================ + /// Persistable configuration (excludes sensitive API credentials). /// /// This struct contains only the settings that can safely be written to disk. @@ -194,12 +364,16 @@ impl From<&Config> for PersistableConfig { } } +// ============================================================================ +// Config Loading and Validation +// ============================================================================ + impl Config { /// Load configuration from a TOML file. - pub fn from_file(path: impl AsRef) -> crate::error::Result { + pub fn from_file(path: impl AsRef) -> Result { let content = std::fs::read_to_string(path)?; let config: Config = - toml::from_str(&content).map_err(|e| crate::error::Error::Config(e.to_string()))?; + toml::from_str(&content).map_err(|e| Error::Config(e.to_string()))?; Ok(config) } @@ -237,12 +411,12 @@ impl Config { // Processing settings if let Ok(val) = std::env::var("RESIZE_SIZE") { if let Ok(size) = val.parse() { - self.processing.resize_size = size; + self.processing.output.size = size; } } if let Ok(val) = std::env::var("FACE_RESOLUTION_THRESHOLD") { if let Ok(threshold) = val.parse() { - self.processing.face_resolution_threshold = threshold; + self.processing.face_resolution.min_size = threshold; } } if let Ok(val) = std::env::var("MAX_WORKERS") { @@ -251,7 +425,8 @@ impl Config { } } if let Ok(val) = std::env::var("KEEP_INTERMEDIATES") { - self.processing.keep_intermediates = val.eq_ignore_ascii_case("true") || val == "1"; + self.processing.output.keep_intermediates = + val.eq_ignore_ascii_case("true") || val == "1"; } // Video settings @@ -272,23 +447,20 @@ impl Config { self } - /// Validate the configuration. - pub fn validate(&self) -> crate::error::Result<()> { + /// Validate the entire configuration. + pub fn validate(&self) -> Result<()> { + // API validation if self.api.api_key.is_empty() { - return Err(crate::error::Error::Config( - "API key is required".to_string(), - )); + return Err(Error::Config("API key is required".to_string())); } if self.api.base_url.is_empty() { - return Err(crate::error::Error::Config( - "Base URL is required".to_string(), - )); - } - if self.processing.resize_size == 0 { - return Err(crate::error::Error::Config( - "Resize size must be greater than 0".to_string(), - )); + return Err(Error::Config("Base URL is required".to_string())); } + + // Delegate to sub-config validation + self.processing.validate()?; + self.video.validate()?; + Ok(()) } @@ -296,15 +468,19 @@ impl Config { /// /// 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) -> crate::error::Result<()> { + pub fn save_to_file(&self, path: impl AsRef) -> 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)))?; + .map_err(|e| Error::Config(format!("Failed to serialize config: {}", e)))?; std::fs::write(path, content)?; Ok(()) } } +// ============================================================================ +// Tests +// ============================================================================ + #[cfg(test)] mod tests { use super::*; @@ -312,8 +488,8 @@ mod tests { #[test] fn test_default_config() { let config = Config::default(); - assert_eq!(config.processing.resize_size, 512); - assert_eq!(config.processing.face_resolution_threshold, 80); + assert_eq!(config.processing.output.size, 512); + assert_eq!(config.processing.face_resolution.min_size, 80); assert_eq!(config.video.framerate, 15); } @@ -324,25 +500,83 @@ mod tests { assert!(result.is_err()); } + #[test] + fn test_brightness_validation() { + let mut config = BrightnessConfig::default(); + config.enabled = true; + + // Valid config + config.min_brightness = 0.1; + config.max_brightness = 0.9; + assert!(config.validate().is_ok()); + + // Invalid: min >= max + config.min_brightness = 0.9; + config.max_brightness = 0.1; + assert!(config.validate().is_err()); + + // Invalid: out of range + config.min_brightness = -0.1; + config.max_brightness = 0.9; + assert!(config.validate().is_err()); + + // Disabled config skips validation + config.enabled = false; + assert!(config.validate().is_ok()); + } + + #[test] + fn test_video_validation() { + let mut config = VideoConfig::default(); + + // Valid + assert!(config.validate().is_ok()); + + // Invalid CRF + config.crf = 52; + assert!(config.validate().is_err()); + + // Invalid framerate + config.crf = 23; + config.framerate = 0; + assert!(config.validate().is_err()); + } + + #[test] + fn test_output_validation() { + let mut config = OutputConfig::default(); + + // Valid + assert!(config.validate().is_ok()); + + // Too small + config.size = 32; + assert!(config.validate().is_err()); + + // Too large + config.size = 8192; + assert!(config.validate().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.processing.output.size = 256; + config.processing.face_resolution.min_size = 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"); + let temp_path = temp_dir.join("test_config_new.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("size = 256")); + assert!(content.contains("min_size = 100")); assert!(content.contains("framerate = 30")); assert!(content.contains("crf = 18")); diff --git a/src/job/mod.rs b/src/job/mod.rs index 8d60cae..3bc63e1 100644 --- a/src/job/mod.rs +++ b/src/job/mod.rs @@ -135,7 +135,7 @@ async fn run_job_inner( tokio::fs::create_dir_all(&images_dir).await?; // Create debug directories if enabled - let debug = if config.processing.keep_intermediates { + let debug = if config.processing.output.keep_intermediates { let debug_base = person_dir.join("debug"); let crop_dir = debug_base.join("crop"); @@ -499,7 +499,7 @@ async fn process_single_asset( ctx = ctx.with_bytes(image_bytes); // Determine debug directory - let debug_dir = if config.processing.keep_intermediates { + let debug_dir = if config.processing.output.keep_intermediates { output_dirs.debug.as_ref().and_then(|d| d.crop.as_ref().map(|p| p.parent().unwrap().to_path_buf())) } else { None diff --git a/src/pipeline/mod.rs b/src/pipeline/mod.rs index fe3c79a..1da5add 100644 --- a/src/pipeline/mod.rs +++ b/src/pipeline/mod.rs @@ -133,7 +133,7 @@ impl Pipeline { ctx = new_ctx; // Generate debug visualization if enabled - if config.processing.keep_intermediates { + if config.processing.output.keep_intermediates { if let Some(debug_img) = step.debug_visualize(&ctx) { ctx.add_debug_image(step.id(), debug_img); } diff --git a/src/pipeline/steps/brightness.rs b/src/pipeline/steps/brightness.rs index 8d5239c..670b8b1 100644 --- a/src/pipeline/steps/brightness.rs +++ b/src/pipeline/steps/brightness.rs @@ -53,6 +53,8 @@ impl ProcessingStep for BrightnessStep { } async fn execute(&self, mut ctx: PipelineContext, config: &Config) -> StepOutcome { + let step_config = &config.processing.brightness; + let image = match &ctx.image { Some(img) => img, None => { @@ -67,31 +69,29 @@ impl ProcessingStep for BrightnessStep { // Store computed brightness for potential use by other steps ctx.set_computed("brightness", ComputedValue::Float(brightness)); - // Check brightness thresholds if configured - if let Some(ref brightness_config) = config.processing.brightness { - if !brightness_config.enabled { - return StepOutcome::Continue(ctx); - } + // Skip validation if disabled + if !step_config.enabled { + return StepOutcome::Continue(ctx); + } - if brightness < brightness_config.min_brightness { - return StepOutcome::Skip { - reason: "too_dark".to_string(), - detail: Some(format!( - "{:.2} (min: {:.2})", - brightness, brightness_config.min_brightness - )), - }; - } + if brightness < step_config.min_brightness { + return StepOutcome::Skip { + reason: "too_dark".to_string(), + detail: Some(format!( + "{:.2} (min: {:.2})", + brightness, step_config.min_brightness + )), + }; + } - if brightness > brightness_config.max_brightness { - return StepOutcome::Skip { - reason: "too_bright".to_string(), - detail: Some(format!( - "{:.2} (max: {:.2})", - brightness, brightness_config.max_brightness - )), - }; - } + if brightness > step_config.max_brightness { + return StepOutcome::Skip { + reason: "too_bright".to_string(), + detail: Some(format!( + "{:.2} (max: {:.2})", + brightness, step_config.max_brightness + )), + }; } StepOutcome::Continue(ctx) @@ -101,7 +101,6 @@ impl ProcessingStep for BrightnessStep { #[cfg(test)] mod tests { use super::*; - use crate::config::BrightnessConfig; use crate::immich_api::FaceData; use image::{DynamicImage, RgbImage, Rgb}; @@ -146,15 +145,15 @@ mod tests { } #[tokio::test] - async fn test_brightness_no_config() { + async fn test_brightness_disabled() { let step = BrightnessStep; let img = create_solid_image(128, 128, 128); let ctx = make_ctx_with_image(img); - let config = Config::default(); // No brightness config + let config = Config::default(); // brightness.enabled = false by default match step.execute(ctx, &config).await { StepOutcome::Continue(new_ctx) => { - // Should still compute brightness even without thresholds + // Should still compute brightness even when disabled let brightness = new_ctx.get_computed("brightness") .and_then(|v| v.as_float()) .unwrap(); @@ -171,11 +170,9 @@ mod tests { let ctx = make_ctx_with_image(img); let mut config = Config::default(); - config.processing.brightness = Some(BrightnessConfig { - enabled: true, - min_brightness: 0.2, - max_brightness: 0.9, - }); + config.processing.brightness.enabled = true; + config.processing.brightness.min_brightness = 0.2; + config.processing.brightness.max_brightness = 0.9; match step.execute(ctx, &config).await { StepOutcome::Skip { reason, .. } => { @@ -192,11 +189,9 @@ mod tests { let ctx = make_ctx_with_image(img); let mut config = Config::default(); - config.processing.brightness = Some(BrightnessConfig { - enabled: true, - min_brightness: 0.1, - max_brightness: 0.9, - }); + config.processing.brightness.enabled = true; + config.processing.brightness.min_brightness = 0.1; + config.processing.brightness.max_brightness = 0.9; match step.execute(ctx, &config).await { StepOutcome::Skip { reason, .. } => { @@ -207,21 +202,19 @@ mod tests { } #[tokio::test] - async fn test_brightness_disabled() { + async fn test_brightness_within_range() { let step = BrightnessStep; - let img = create_solid_image(10, 10, 10); // Very dark + let img = create_solid_image(128, 128, 128); // Mid-gray let ctx = make_ctx_with_image(img); let mut config = Config::default(); - config.processing.brightness = Some(BrightnessConfig { - enabled: false, // Disabled - min_brightness: 0.2, - max_brightness: 0.9, - }); + config.processing.brightness.enabled = true; + config.processing.brightness.min_brightness = 0.1; + config.processing.brightness.max_brightness = 0.9; match step.execute(ctx, &config).await { - StepOutcome::Continue(_) => {} // Should continue even though image is dark - _ => panic!("Expected Continue when disabled"), + StepOutcome::Continue(_) => {} // Should pass + _ => panic!("Expected Continue for mid-range brightness"), } } } diff --git a/src/pipeline/steps/crop.rs b/src/pipeline/steps/crop.rs index c41b7b6..8a7ba45 100644 --- a/src/pipeline/steps/crop.rs +++ b/src/pipeline/steps/crop.rs @@ -35,7 +35,7 @@ impl ProcessingStep for CropFaceStep { // Crop returns (full_res_crop, resized). We want the full_res_crop here // and let the resize step handle the final sizing. - match crop_face_with_intermediate(image, &ctx.face_data, config.processing.resize_size) { + match crop_face_with_intermediate(image, &ctx.face_data, config.processing.output.size) { Ok((cropped_full, _resized)) => { // Use the full-resolution cropped image; resize step will handle final size ctx.image = Some(cropped_full); diff --git a/src/pipeline/steps/face_resolution.rs b/src/pipeline/steps/face_resolution.rs index 4e54f46..78181cc 100644 --- a/src/pipeline/steps/face_resolution.rs +++ b/src/pipeline/steps/face_resolution.rs @@ -23,6 +23,13 @@ impl ProcessingStep for FaceResolutionStep { } async fn execute(&self, mut ctx: PipelineContext, config: &Config) -> StepOutcome { + let step_config = &config.processing.face_resolution; + + // Skip this step if disabled + if !step_config.enabled { + return StepOutcome::Continue(ctx); + } + let face_data = &ctx.face_data; // Calculate face size in pixels from bounding box @@ -33,7 +40,7 @@ impl ProcessingStep for FaceResolutionStep { // Store computed value for later steps or debugging ctx.set_computed("face_size", ComputedValue::Int(face_size)); - let threshold = config.processing.face_resolution_threshold as i32; + let threshold = step_config.min_size as i32; if face_size < threshold { return StepOutcome::Skip { @@ -95,4 +102,18 @@ mod tests { _ => panic!("Expected Continue"), } } + + #[tokio::test] + async fn test_step_disabled() { + let step = FaceResolutionStep; + let ctx = make_ctx(50.0); // Would normally be too small + + let mut config = Config::default(); + config.processing.face_resolution.enabled = false; + + match step.execute(ctx, &config).await { + StepOutcome::Continue(_) => {} // Should pass through when disabled + _ => panic!("Expected Continue when disabled"), + } + } } diff --git a/src/pipeline/steps/resize.rs b/src/pipeline/steps/resize.rs index 031287c..1670bdc 100644 --- a/src/pipeline/steps/resize.rs +++ b/src/pipeline/steps/resize.rs @@ -31,7 +31,7 @@ impl ProcessingStep for ResizeStep { } }; - let output_size = config.processing.resize_size; + let output_size = config.processing.output.size; // Resize to square output let resized = image.resize_exact(output_size, output_size, FilterType::Lanczos3); @@ -72,7 +72,7 @@ mod tests { let ctx = make_ctx_with_image(img); let mut config = Config::default(); - config.processing.resize_size = 512; + config.processing.output.size = 512; match step.execute(ctx, &config).await { StepOutcome::Continue(new_ctx) => { @@ -113,7 +113,7 @@ mod tests { let ctx = make_ctx_with_image(img); let mut config = Config::default(); - config.processing.resize_size = 256; + config.processing.output.size = 256; match step.execute(ctx, &config).await { StepOutcome::Continue(new_ctx) => { diff --git a/src/web/handlers/config.rs b/src/web/handlers/config.rs index 6c912da..b581019 100644 --- a/src/web/handlers/config.rs +++ b/src/web/handlers/config.rs @@ -1,29 +1,13 @@ //! Configuration endpoints. -use crate::config::{ProcessingConfig, VideoConfig}; -use crate::web::state::{AppState, JobStatus}; -use axum::{ - extract::State, - http::StatusCode, - response::Json, +use crate::config::{ + AlignmentConfig, BrightnessConfig, FaceResolutionConfig, OutputConfig, ProcessingConfig, + VideoConfig, }; +use crate::web::state::{AppState, JobStatus}; +use axum::{extract::State, http::StatusCode, response::Json}; 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) -> Self { - Self { - field, - message: message.into(), - } - } -} - /// Configuration response (excludes sensitive API credentials). #[derive(Serialize)] pub struct ConfigResponse { @@ -42,6 +26,8 @@ pub async fn get_config(State(state): State) -> Json { } /// Configuration update request. +/// +/// Uses the same nested structure as the config itself for consistency. #[derive(Deserialize)] pub struct ConfigUpdateRequest { pub processing: Option, @@ -51,57 +37,39 @@ pub struct ConfigUpdateRequest { /// Processing configuration update fields. #[derive(Deserialize)] pub struct ProcessingConfigUpdate { - pub resize_size: Option, - pub face_resolution_threshold: Option, - pub pose_threshold: Option, - pub ear_threshold: Option, pub max_workers: Option, - pub keep_intermediates: Option, + pub face_resolution: Option, + pub brightness: Option, + pub output: Option, + pub alignment: Option, } /// Video configuration update fields. #[derive(Deserialize)] pub struct VideoConfigUpdate { - pub framerate: Option, pub enabled: Option, + pub framerate: Option, pub codec: Option, pub crf: Option, } +/// Validation error with field name and message. +struct ValidationError { + field: &'static str, + message: String, +} + +impl ValidationError { + fn new(field: &'static str, message: impl Into) -> Self { + Self { + field, + message: message.into(), + } + } +} + /// 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( @@ -110,6 +78,60 @@ fn validate_processing_config(proc: &ProcessingConfigUpdate) -> Result<(), Valid )); } } + + if let Some(ref fr) = proc.face_resolution { + if fr.enabled && fr.min_size == 0 { + return Err(ValidationError::new( + "processing.face_resolution.min_size", + "must be greater than 0 when enabled", + )); + } + if fr.min_size > 1000 { + return Err(ValidationError::new( + "processing.face_resolution.min_size", + format!("must be at most 1000, got {}", fr.min_size), + )); + } + } + + if let Some(ref br) = proc.brightness { + if br.enabled { + if br.min_brightness < 0.0 || br.min_brightness > 1.0 { + return Err(ValidationError::new( + "processing.brightness.min_brightness", + format!("must be between 0.0 and 1.0, got {}", br.min_brightness), + )); + } + if br.max_brightness < 0.0 || br.max_brightness > 1.0 { + return Err(ValidationError::new( + "processing.brightness.max_brightness", + format!("must be between 0.0 and 1.0, got {}", br.max_brightness), + )); + } + if br.min_brightness >= br.max_brightness { + return Err(ValidationError::new( + "processing.brightness", + "min_brightness must be less than max_brightness", + )); + } + } + } + + if let Some(ref out) = proc.output { + if out.size < 64 { + return Err(ValidationError::new( + "processing.output.size", + format!("must be at least 64, got {}", out.size), + )); + } + if out.size > 4096 { + return Err(ValidationError::new( + "processing.output.size", + format!("must be at most 4096, got {}", out.size), + )); + } + } + Ok(()) } @@ -132,7 +154,6 @@ fn validate_video_config(vid: &VideoConfigUpdate) -> Result<(), ValidationError> } } 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( @@ -187,33 +208,30 @@ pub async fn update_config( let mut config = state.config.write().await; if let Some(proc) = update.processing { - if let Some(v) = proc.resize_size { - config.processing.resize_size = v; - } - if let Some(v) = proc.face_resolution_threshold { - config.processing.face_resolution_threshold = v; - } - if let Some(v) = proc.pose_threshold { - config.processing.pose_threshold = v; - } - if let Some(v) = proc.ear_threshold { - config.processing.ear_threshold = v; - } if let Some(v) = proc.max_workers { config.processing.max_workers = v; } - if let Some(v) = proc.keep_intermediates { - config.processing.keep_intermediates = v; + if let Some(v) = proc.face_resolution { + config.processing.face_resolution = v; + } + if let Some(v) = proc.brightness { + config.processing.brightness = v; + } + if let Some(v) = proc.output { + config.processing.output = v; + } + if let Some(v) = proc.alignment { + config.processing.alignment = v; } } if let Some(vid) = update.video { - if let Some(v) = vid.framerate { - config.video.framerate = v; - } if let Some(v) = vid.enabled { config.video.enabled = v; } + if let Some(v) = vid.framerate { + config.video.framerate = v; + } if let Some(v) = vid.codec { config.video.codec = v; }