From e138a0de8e94087e92c4cb0732953117dfeed3ad Mon Sep 17 00:00:00 2001 From: Scott Morris Date: Fri, 3 Apr 2026 14:35:31 -0400 Subject: [PATCH 1/8] feat: add DVD text subtitle render jobs **What changed** - add explicit `RenderTextSubtitles` build jobs and subtitle render-mode support in the Rust and TypeScript models - prepare text subtitle streams through FFmpeg, compose them onto authored title MPEGs with `spumux`, and surface the work on the Build page - preserve mixed bitmap and text subtitle mapping order in the planner and add regression coverage for the ordering and text-render planning paths - improve render-step diagnostics with a host-font hint when subtitle composition fails **Why** - issue `#17` needs real DVD text subtitle authoring instead of warning-only handling - explicit render jobs and order-aware planning make the build output and progress reporting match the authored subtitle intent more closely - stronger tests and clearer diagnostics reduce the risk of silent subtitle regressions during authoring --- apps/spindle/src/pages/BuildPage.tsx | 2 + apps/spindle/src/types/project.ts | 17 + .../src/build/executor.rs | 234 ++++++++++++- .../src/build/ffmpeg.rs | 22 ++ .../src/build/planner.rs | 314 ++++++++++++++++-- .../src/build/types.rs | 20 +- .../src/desktop.rs | 24 +- .../src/models.rs | 17 + 8 files changed, 606 insertions(+), 44 deletions(-) diff --git a/apps/spindle/src/pages/BuildPage.tsx b/apps/spindle/src/pages/BuildPage.tsx index b46ad3e..08e8e79 100644 --- a/apps/spindle/src/pages/BuildPage.tsx +++ b/apps/spindle/src/pages/BuildPage.tsx @@ -363,6 +363,8 @@ function getJobIcon(job: BuildJob): string { return '\u{1F517}'; case 'extractSubtitles': return '\u{1F4DD}'; + case 'renderTextSubtitles': + return '\u{1F5E8}'; case 'authorDvd': return '\u{1F4BF}'; case 'createIso': diff --git a/apps/spindle/src/types/project.ts b/apps/spindle/src/types/project.ts index 19c0cf9..a6ae8ea 100644 --- a/apps/spindle/src/types/project.ts +++ b/apps/spindle/src/types/project.ts @@ -306,6 +306,7 @@ export interface BuildSettings { generateIso: boolean; safetyMarginBytes: number; allocationStrategy: AllocationStrategy; + subtitleRenderMode?: 'one-pass' | 'two-pass'; } // ── Validation ────────────────────────────────────────────────────────────── @@ -390,6 +391,22 @@ export type BuildJob = command: string[]; label: string; } + | { + type: 'renderTextSubtitles'; + titleId: string; + titleName: string; + sourcePath: string; + sourceStreamIndex: number; + inputPath: string; + outputPath: string; + subtitlePath: string; + prepareCommand: string[]; + spumuxXml: string; + command: string[]; + label: string; + renderMode: 'one-pass' | 'two-pass'; + fontFamily: string; + } | { type: 'authorDvd'; xmlPath: string; diff --git a/plugins/tauri-plugin-spindle-project/src/build/executor.rs b/plugins/tauri-plugin-spindle-project/src/build/executor.rs index 183c6ee..1759c21 100644 --- a/plugins/tauri-plugin-spindle-project/src/build/executor.rs +++ b/plugins/tauri-plugin-spindle-project/src/build/executor.rs @@ -194,6 +194,7 @@ where i, total, &label, + "FFmpeg transcode", &mut on_progress, ) { Ok(output) => { @@ -214,6 +215,101 @@ where } } } + BuildJob::RenderTextSubtitles { + prepare_command, + spumux_xml, + command, + input_path, + output_path, + subtitle_path, + font_family, + .. + } => { + log_lines.push(format!(" $ {}", prepare_command.join(" "))); + on_progress(BuildProgress { + job_index: i, + total_jobs: total, + current_label: label.clone(), + status: BuildJobStatus::Running, + output: None, + step_label: Some("Prepare subtitle text".to_string()), + step_percent: None, + step_detail: Some(subtitle_path.clone()), + step_status: Some(BuildJobStatus::Running), + }); + + match run_ffmpeg_command( + prepare_command, + None, + i, + total, + &label, + "Prepare subtitle text", + &mut on_progress, + ) { + Ok(output) => { + if !output.is_empty() { + log_lines.push(output); + } + } + Err(msg) => { + log_lines.push(msg.clone()); + on_progress(BuildProgress::job( + i, + total, + label, + BuildJobStatus::Failed, + Some(msg.clone()), + )); + return failure(plan, log_lines, i, msg); + } + } + + let xml_path = command + .last() + .cloned() + .unwrap_or_else(|| format!("{output_path}.xml")); + if let Err(e) = std::fs::write(&xml_path, spumux_xml) { + let msg = format!("Failed to write subtitle render XML: {e}"); + log_lines.push(msg.clone()); + return failure(plan, log_lines, i, msg); + } + log_lines.push(format!(" Wrote {xml_path}")); + + on_progress(BuildProgress { + job_index: i, + total_jobs: total, + current_label: label.clone(), + status: BuildJobStatus::Running, + output: None, + step_label: Some("Compose DVD subtitle stream".to_string()), + step_percent: None, + step_detail: Some(output_path.clone()), + step_status: Some(BuildJobStatus::Running), + }); + + match run_spumux_command(command, input_path, output_path) { + Ok(output) => { + if !output.is_empty() { + log_lines.push(output); + } + } + Err(msg) => { + let msg = format!( + "{msg}\nText subtitle rendering uses the host font \"{font_family}\". Confirm that Fontconfig can resolve it on this machine." + ); + log_lines.push(msg.clone()); + on_progress(BuildProgress::job( + i, + total, + label, + BuildJobStatus::Failed, + Some(msg.clone()), + )); + return failure(plan, log_lines, i, msg); + } + } + } BuildJob::AuthorDvd { xml_path, command, .. } => { @@ -332,6 +428,7 @@ fn run_ffmpeg_command( job_index: usize, total_jobs: usize, label: &str, + step_label: &str, on_progress: &mut F, ) -> std::result::Result where @@ -407,7 +504,7 @@ where current_label: label.to_string(), status: BuildJobStatus::Running, output: None, - step_label: Some("FFmpeg transcode".to_string()), + step_label: Some(step_label.to_string()), step_percent: pct, step_detail: Some(detail), step_status: Some(BuildJobStatus::Running), @@ -548,7 +645,7 @@ mod tests { use crate::build::test_support::{test_menu_with_action, test_project}; use crate::build::{execute_build_plan, generate_build_plan}; - use crate::models::PlaybackAction; + use crate::models::{PlaybackAction, SubtitleStreamInfo, SubtitleType}; use super::reset_workspace_directory; @@ -697,4 +794,137 @@ mod tests { fs::remove_dir_all(&output_dir).unwrap(); } + + #[test] + #[ignore = "requires ffmpeg, ffprobe, spumux, and dvdauthor on PATH"] + fn execute_build_plan_smoke_authors_text_subtitle_stream() { + let Some(ffmpeg_bin) = find_tool_on_path("ffmpeg") else { + eprintln!("Skipping smoke test because `ffmpeg` is not available on PATH."); + return; + }; + let Some(ffprobe_bin) = find_tool_on_path("ffprobe") else { + eprintln!("Skipping smoke test because `ffprobe` is not available on PATH."); + return; + }; + if find_tool_on_path("spumux").is_none() || find_tool_on_path("dvdauthor").is_none() { + eprintln!( + "Skipping smoke test because `spumux` and/or `dvdauthor` are not available on PATH." + ); + return; + } + + let output_dir = unique_temp_dir("build-text-subtitle-smoke"); + let source_path = output_dir.join("source.mkv"); + let subtitle_path = output_dir.join("subtitle.srt"); + fs::create_dir_all(&output_dir).unwrap(); + fs::write( + &subtitle_path, + "1\n00:00:00,000 --> 00:00:01,000\nHello from text subtitles.\n", + ) + .unwrap(); + + let ffmpeg_status = Command::new(ffmpeg_bin) + .args([ + "-y", + "-f", + "lavfi", + "-i", + "color=c=black:s=640x360:d=1.5", + "-f", + "lavfi", + "-i", + "anullsrc=r=48000:cl=stereo", + "-i", + ]) + .arg(&subtitle_path) + .args([ + "-shortest", + "-c:v", + "libx264", + "-pix_fmt", + "yuv420p", + "-c:a", + "aac", + "-b:a", + "128k", + "-c:s", + "srt", + ]) + .arg(&source_path) + .status() + .expect("ffmpeg should launch for text subtitle smoke test fixture generation"); + assert!( + ffmpeg_status.success(), + "ffmpeg fixture generation failed with status {ffmpeg_status}" + ); + + let mut project = test_project(); + project.assets[0].source_path = source_path.display().to_string(); + project.assets[0].file_name = "source.mkv".to_string(); + project.assets[0].duration_secs = Some(1.5); + project.assets[0].subtitle_streams = vec![SubtitleStreamInfo { + index: 2, + codec: "subrip".to_string(), + language: Some("eng".to_string()), + subtitle_type: SubtitleType::Text, + title: Some("English".to_string()), + }]; + project.disc.titlesets[0].titles[0].subtitle_mappings.push( + crate::models::SubtitleTrackMapping { + id: "sm-text".to_string(), + source_stream_index: 2, + label: "English".to_string(), + language: "eng".to_string(), + order_index: 0, + is_default: false, + is_forced: false, + }, + ); + + let plan = generate_build_plan(&project, output_dir.to_str().unwrap(), true).unwrap(); + let result = execute_build_plan(&plan, |_| {}); + + if !result.success { + panic!( + "expected text subtitle smoke build to succeed\n{}", + result.log_lines.join("\n") + ); + } + + let authored_title_path = PathBuf::from(&plan.working_directory) + .join("titles") + .join("title-1.mpg"); + assert!( + authored_title_path.exists(), + "expected authored title MPEG at {}", + authored_title_path.display() + ); + + let ffprobe_output = Command::new(ffprobe_bin) + .args([ + "-v", + "error", + "-select_streams", + "s", + "-show_entries", + "stream=codec_name", + "-of", + "csv=p=0", + ]) + .arg(&authored_title_path) + .output() + .expect("ffprobe should inspect authored title MPEG"); + assert!( + ffprobe_output.status.success(), + "ffprobe failed with status {}", + ffprobe_output.status + ); + let subtitle_codecs = String::from_utf8_lossy(&ffprobe_output.stdout); + assert!( + subtitle_codecs.lines().any(|line| line.trim() == "dvd_subtitle"), + "expected authored title MPEG to include a dvd_subtitle stream, got:\n{subtitle_codecs}" + ); + + fs::remove_dir_all(&output_dir).unwrap(); + } } diff --git a/plugins/tauri-plugin-spindle-project/src/build/ffmpeg.rs b/plugins/tauri-plugin-spindle-project/src/build/ffmpeg.rs index fb21518..24870b7 100644 --- a/plugins/tauri-plugin-spindle-project/src/build/ffmpeg.rs +++ b/plugins/tauri-plugin-spindle-project/src/build/ffmpeg.rs @@ -187,6 +187,28 @@ pub(crate) fn build_ffmpeg_transcode_command( cmd } +/// Build an FFmpeg command that normalises a text subtitle stream to SRT. +/// +/// This gives the first-pass text subtitle path one stable text format that +/// `spumux` can render with a host font during subtitle composition. +pub(crate) fn build_ffmpeg_text_subtitle_prepare_command( + source_path: &str, + output_path: &Path, + source_stream_index: u32, +) -> Vec { + vec![ + "ffmpeg".to_string(), + "-y".to_string(), + "-i".to_string(), + source_path.to_string(), + "-map".to_string(), + format!("0:{source_stream_index}"), + "-f".to_string(), + "srt".to_string(), + output_path.display().to_string(), + ] +} + fn choose_output_fps(source_fps: Option, standard: VideoStandard) -> f64 { match standard { VideoStandard::Pal => 25.0, diff --git a/plugins/tauri-plugin-spindle-project/src/build/planner.rs b/plugins/tauri-plugin-spindle-project/src/build/planner.rs index fe81578..be41c45 100644 --- a/plugins/tauri-plugin-spindle-project/src/build/planner.rs +++ b/plugins/tauri-plugin-spindle-project/src/build/planner.rs @@ -9,10 +9,10 @@ use std::path::PathBuf; use crate::models::*; use super::authoring::generate_dvdauthor_xml; -use super::ffmpeg::build_ffmpeg_transcode_command; +use super::ffmpeg::{build_ffmpeg_text_subtitle_prepare_command, build_ffmpeg_transcode_command}; use super::menu::{authorable_menus, build_ffmpeg_menu_command, generate_spumux_xml}; use super::types::{BuildJob, BuildPlan, BuildSummary, MenuOverlayButton}; -use super::util::sanitise_filename; +use super::util::{sanitise_filename, xml_escape}; struct BuildPaths { output_dir: PathBuf, @@ -30,6 +30,11 @@ struct MenuPaths { select_image_path: PathBuf, } +struct TitlePaths { + base_video_path: PathBuf, + authored_video_path: PathBuf, +} + impl BuildPaths { fn new(output_dir: &str) -> Self { let output_dir = PathBuf::from(output_dir); @@ -66,9 +71,36 @@ impl BuildPaths { ] } - fn title_video_path(&self, title_id: &str) -> PathBuf { - self.titles_dir - .join(format!("{}.mpg", sanitise_filename(title_id))) + fn title_paths(&self, title_id: &str) -> TitlePaths { + let base_name = sanitise_filename(title_id); + TitlePaths { + base_video_path: self.titles_dir.join(format!("{base_name}_base.mpg")), + authored_video_path: self.titles_dir.join(format!("{base_name}.mpg")), + } + } + + fn subtitle_text_path(&self, title_id: &str, source_stream_index: u32) -> PathBuf { + self.subtitles_dir.join(format!( + "{}_sub_{}.srt", + sanitise_filename(title_id), + source_stream_index + )) + } + + fn title_subtitle_xml_path(&self, title_id: &str, stream_index: usize) -> PathBuf { + self.subtitles_dir.join(format!( + "{}_sub_{}.xml", + sanitise_filename(title_id), + stream_index + )) + } + + fn title_subtitle_stage_path(&self, title_id: &str, stream_index: usize) -> PathBuf { + self.titles_dir.join(format!( + "{}_substage_{}.mpg", + sanitise_filename(title_id), + stream_index + )) } fn menu_paths(&self, menu_id: &str) -> MenuPaths { @@ -132,15 +164,11 @@ pub fn generate_build_plan_with_options( skip_sidecar: bool, skip_unsupported_streams: bool, ) -> crate::Result { - // When skip_unsupported_streams is enabled, strip text subtitle mappings - // so the build proceeds without them. - let owned_project; - let project = if skip_unsupported_streams { - owned_project = strip_unsupported_subtitle_mappings(project); - &owned_project - } else { - project - }; + let mut owned_project = project.clone(); + if skip_unsupported_streams { + strip_unsupported_subtitle_mappings(&mut owned_project); + } + let project = &owned_project; let paths = BuildPaths::new(output_dir); let tools = ResolvedToolchain::resolve(skip_sidecar); @@ -203,18 +231,44 @@ pub fn generate_build_plan_with_options( subtitle_key.join(","), ); - if let Some(existing_output) = transcode_cache.get(&cache_key) { - // Reuse by symlinking to the existing transcode output - let link_path = paths.title_video_path(&title.id); - jobs.push(BuildJob::LinkTitle { - title_id: title.id.clone(), - title_name: title.name.clone(), - source_path: existing_output.display().to_string(), - link_path: link_path.display().to_string(), - label: format!("Link \"{}\" (shared transcode)", title.name), - }); - } else { - let output_path = paths.title_video_path(&title.id); + let text_subtitle_mappings: Vec<_> = title + .subtitle_mappings + .iter() + .enumerate() + .filter_map(|(stream_index, sm)| { + asset + .subtitle_streams + .iter() + .any(|stream| { + stream.index == sm.source_stream_index + && stream.subtitle_type == SubtitleType::Text + }) + .then_some((stream_index, sm)) + }) + .collect(); + let has_text_subtitles = !text_subtitle_mappings.is_empty(); + let title_paths = paths.title_paths(&title.id); + + if !has_text_subtitles { + if let Some(existing_output) = transcode_cache.get(&cache_key) { + // Reuse by symlinking to the existing transcode output + jobs.push(BuildJob::LinkTitle { + title_id: title.id.clone(), + title_name: title.name.clone(), + source_path: existing_output.display().to_string(), + link_path: title_paths.authored_video_path.display().to_string(), + label: format!("Link \"{}\" (shared transcode)", title.name), + }); + continue; + } + } + + { + let output_path = if has_text_subtitles { + title_paths.base_video_path.clone() + } else { + title_paths.authored_video_path.clone() + }; let video_info = title .video_mapping @@ -231,7 +285,9 @@ pub fn generate_build_plan_with_options( ); command[0] = tools.ffmpeg.clone(); - transcode_cache.insert(cache_key, output_path.clone()); + if !has_text_subtitles { + transcode_cache.insert(cache_key, output_path.clone()); + } transcode_count += 1; jobs.push(BuildJob::TranscodeTitle { @@ -243,6 +299,65 @@ pub fn generate_build_plan_with_options( label: format!("Transcode \"{}\"", title.name), duration_secs: asset.duration_secs, }); + + if has_text_subtitles { + let profile = title.video_output_profile.unwrap_or(VideoOutputProfile { + raster: VideoRaster::FullD1, + aspect: AspectMode::SixteenByNine, + }); + let mut current_input = output_path; + let font_family = "Sans".to_string(); + + for (text_job_index, (stream_index, sm)) in + text_subtitle_mappings.iter().enumerate() + { + let subtitle_path = paths.subtitle_text_path(&title.id, sm.source_stream_index); + let mut prepare_command = build_ffmpeg_text_subtitle_prepare_command( + &asset.source_path, + &subtitle_path, + sm.source_stream_index, + ); + prepare_command[0] = tools.ffmpeg.clone(); + + let output_path = if text_job_index + 1 == text_subtitle_mappings.len() { + title_paths.authored_video_path.clone() + } else { + paths.title_subtitle_stage_path(&title.id, *stream_index) + }; + let xml_path = paths.title_subtitle_xml_path(&title.id, *stream_index); + let spumux_xml = generate_text_subtitle_spumux_xml( + &subtitle_path, + project.disc.standard, + profile, + &font_family, + ); + + jobs.push(BuildJob::RenderTextSubtitles { + title_id: title.id.clone(), + title_name: title.name.clone(), + source_path: asset.source_path.clone(), + source_stream_index: sm.source_stream_index, + input_path: current_input.display().to_string(), + output_path: output_path.display().to_string(), + subtitle_path: subtitle_path.display().to_string(), + prepare_command, + spumux_xml, + command: vec![ + tools.spumux.clone(), + "-m".to_string(), + "dvd".to_string(), + "-s".to_string(), + stream_index.to_string(), + xml_path.display().to_string(), + ], + label: format!("Render subtitle \"{}\" for \"{}\"", sm.label, title.name), + render_mode: project.build_settings.subtitle_render_mode, + font_family: font_family.clone(), + }); + + current_input = output_path; + } + } } } @@ -376,11 +491,34 @@ pub fn generate_build_plan_with_options( }) } -/// Remove subtitle mappings that reference text-based (non-bitmap) source streams. -fn strip_unsupported_subtitle_mappings(project: &SpindleProjectFile) -> SpindleProjectFile { +fn generate_text_subtitle_spumux_xml( + subtitle_path: &std::path::Path, + standard: VideoStandard, + profile: VideoOutputProfile, + font_family: &str, +) -> String { + let format_str = match standard { + VideoStandard::Ntsc => "NTSC", + VideoStandard::Pal => "PAL", + }; + let (width, height) = profile.raster.resolution(standard); + let aspect = match profile.aspect { + AspectMode::FourByThree => "4:3", + AspectMode::SixteenByNine => "16:9", + }; + let fontsize = ((height as f64) * 0.05).round().clamp(24.0, 36.0); + + format!( + "\n\n \n \n \n\n", + xml_escape(&subtitle_path.display().to_string()), + xml_escape(font_family), + ) +} + +/// Remove subtitle mappings that cannot be authored by the current build. +fn strip_unsupported_subtitle_mappings(project: &mut SpindleProjectFile) { let assets: HashMap<&str, &Asset> = project.assets.iter().map(|a| (a.id.as_str(), a)).collect(); - let mut project = project.clone(); for titleset in &mut project.disc.titlesets { for title in &mut titleset.titles { if let Some(asset) = title @@ -390,13 +528,13 @@ fn strip_unsupported_subtitle_mappings(project: &SpindleProjectFile) -> SpindleP { title.subtitle_mappings.retain(|sm| { asset.subtitle_streams.iter().any(|s| { - s.index == sm.source_stream_index && s.subtitle_type == SubtitleType::Bitmap + s.index == sm.source_stream_index + && matches!(s.subtitle_type, SubtitleType::Bitmap | SubtitleType::Text) }) }); } } } - project } #[cfg(test)] @@ -488,6 +626,118 @@ mod tests { .any(|job| matches!(job, BuildJob::ExtractSubtitles { .. }))); } + #[test] + fn build_plan_renders_text_subtitles_after_base_transcode() { + let mut project = test_project(); + project.assets[0].subtitle_streams.push(SubtitleStreamInfo { + index: 2, + codec: "subrip".to_string(), + language: Some("eng".to_string()), + subtitle_type: SubtitleType::Text, + title: Some("English".to_string()), + }); + project.disc.titlesets[0].titles[0] + .subtitle_mappings + .push(SubtitleTrackMapping { + id: "sm-text".to_string(), + source_stream_index: 2, + label: "English".to_string(), + language: "eng".to_string(), + order_index: 0, + is_default: false, + is_forced: false, + }); + + let plan = generate_build_plan(&project, "/tmp/dvd_output", false).unwrap(); + + assert!( + plan.jobs + .iter() + .any(|job| matches!(job, BuildJob::RenderTextSubtitles { .. })), + "expected explicit text subtitle render job" + ); + + let transcode_output = plan.jobs.iter().find_map(|job| match job { + BuildJob::TranscodeTitle { output_path, .. } => Some(output_path), + _ => None, + }); + assert!( + transcode_output.is_some_and(|path| path.contains("_base.mpg")), + "text subtitle titles should transcode to a base MPEG before composition" + ); + } + + #[test] + fn build_plan_preserves_mixed_subtitle_stream_order() { + let mut project = test_project(); + project.assets[0].subtitle_streams.push(SubtitleStreamInfo { + index: 2, + codec: "subrip".to_string(), + language: Some("eng".to_string()), + subtitle_type: SubtitleType::Text, + title: Some("English text".to_string()), + }); + project.assets[0].subtitle_streams.push(SubtitleStreamInfo { + index: 3, + codec: "dvd_subtitle".to_string(), + language: Some("fra".to_string()), + subtitle_type: SubtitleType::Bitmap, + title: Some("French bitmap".to_string()), + }); + project.disc.titlesets[0].titles[0].subtitle_mappings = vec![ + SubtitleTrackMapping { + id: "sm-text".to_string(), + source_stream_index: 2, + label: "English".to_string(), + language: "eng".to_string(), + order_index: 0, + is_default: false, + is_forced: false, + }, + SubtitleTrackMapping { + id: "sm-bitmap".to_string(), + source_stream_index: 3, + label: "French".to_string(), + language: "fra".to_string(), + order_index: 1, + is_default: false, + is_forced: false, + }, + ]; + + let plan = generate_build_plan(&project, "/tmp/dvd_output", false).unwrap(); + + let transcode_job = plan + .jobs + .iter() + .find_map(|job| match job { + BuildJob::TranscodeTitle { command, .. } => Some(command), + _ => None, + }) + .expect("expected transcode job"); + assert!( + transcode_job + .windows(2) + .any(|window| window == [String::from("-c:s:0"), String::from("dvd_subtitle")]), + "bitmap subtitle should keep the first ffmpeg subtitle slot when it is the first bitmap mapping" + ); + + let render_job = plan + .jobs + .iter() + .find_map(|job| match job { + BuildJob::RenderTextSubtitles { command, .. } => Some(command), + _ => None, + }) + .expect("expected text subtitle render job"); + assert!( + render_job + .windows(2) + .any(|window| { window == [String::from("-s"), String::from("0")] }), + "text subtitle should render into stream slot 0 when it is the first subtitle mapping" + ); + } + #[test] fn build_plan_deduplicates_identical_transcodes_with_different_mapping_ids() { let mut project = test_project(); diff --git a/plugins/tauri-plugin-spindle-project/src/build/types.rs b/plugins/tauri-plugin-spindle-project/src/build/types.rs index 83861c3..e131b02 100644 --- a/plugins/tauri-plugin-spindle-project/src/build/types.rs +++ b/plugins/tauri-plugin-spindle-project/src/build/types.rs @@ -5,7 +5,7 @@ use serde::{Deserialize, Serialize}; -use crate::models::VideoStandard; +use crate::models::{SubtitleRenderMode, VideoStandard}; /// A complete build plan for authoring a DVD-Video disc. #[derive(Debug, Clone, Serialize, Deserialize)] @@ -90,6 +90,22 @@ pub enum BuildJob { command: Vec, label: String, }, + /// Prepare and render a text subtitle mapping into the authored title MPEG. + RenderTextSubtitles { + title_id: String, + title_name: String, + source_path: String, + source_stream_index: u32, + input_path: String, + output_path: String, + subtitle_path: String, + prepare_command: Vec, + spumux_xml: String, + command: Vec, + label: String, + render_mode: SubtitleRenderMode, + font_family: String, + }, /// Symlink/copy a title's output from a shared transcode (deduplication). LinkTitle { title_id: String, @@ -114,6 +130,7 @@ impl BuildJob { BuildJob::TranscodeTitle { label, .. } | BuildJob::LinkTitle { label, .. } | BuildJob::ExtractSubtitles { label, .. } + | BuildJob::RenderTextSubtitles { label, .. } | BuildJob::RenderMenu { label, .. } | BuildJob::ComposeMenuHighlights { label, .. } | BuildJob::AuthorDvd { label, .. } @@ -126,6 +143,7 @@ impl BuildJob { BuildJob::PrepareWorkspace { .. } | BuildJob::LinkTitle { .. } => None, BuildJob::TranscodeTitle { command, .. } | BuildJob::ExtractSubtitles { command, .. } + | BuildJob::RenderTextSubtitles { command, .. } | BuildJob::RenderMenu { command, .. } | BuildJob::ComposeMenuHighlights { command, .. } | BuildJob::AuthorDvd { command, .. } diff --git a/plugins/tauri-plugin-spindle-project/src/desktop.rs b/plugins/tauri-plugin-spindle-project/src/desktop.rs index 645a27c..0567755 100644 --- a/plugins/tauri-plugin-spindle-project/src/desktop.rs +++ b/plugins/tauri-plugin-spindle-project/src/desktop.rs @@ -275,27 +275,33 @@ impl SpindleProject { } } - // Text-only subtitle warning - let has_text_subs = title.subtitle_mappings.iter().any(|sm| { - asset + let mut has_text_subs = false; + for sm in &title.subtitle_mappings { + if let Some(stream) = asset .subtitle_streams .iter() .find(|s| s.index == sm.source_stream_index) - .is_some_and(|s| s.subtitle_type == SubtitleType::Text) - }); + { + match stream.subtitle_type { + SubtitleType::Text => has_text_subs = true, + SubtitleType::Bitmap => {} + SubtitleType::Unknown => {} + } + } + } if has_text_subs { issues.push(ValidationIssue { - severity: IssueSeverity::Warning, - code: "subtitle.text-only-unsupported".to_string(), + severity: IssueSeverity::Info, + code: "subtitle.text-rendering-simplified".to_string(), message: format!( - "Title \"{}\" has text-based subtitle mappings that cannot yet be authored to DVD.", + "Title \"{}\" has text subtitle mappings that will be rendered with first-pass DVD-safe styling.", title.name ), context: Some(title.id.clone()), entity_type: Some("title".to_string()), entity_name: Some(title.name.clone()), - suggested_fix: Some("Text subtitle rendering is not yet supported. Remove text subtitles or provide bitmap subtitle sources.".to_string()), + suggested_fix: Some("First-pass subtitle rendering uses a host font and simplified DVD-safe styling. Review the authored disc output if subtitle appearance matters.".to_string()), }); } } diff --git a/plugins/tauri-plugin-spindle-project/src/models.rs b/plugins/tauri-plugin-spindle-project/src/models.rs index d634d5b..5744d85 100644 --- a/plugins/tauri-plugin-spindle-project/src/models.rs +++ b/plugins/tauri-plugin-spindle-project/src/models.rs @@ -662,6 +662,8 @@ pub struct BuildSettings { pub generate_iso: bool, pub safety_margin_bytes: u64, pub allocation_strategy: AllocationStrategy, + #[serde(default)] + pub subtitle_render_mode: SubtitleRenderMode, } impl Default for BuildSettings { @@ -672,6 +674,7 @@ impl Default for BuildSettings { // 50 MB default safety margin safety_margin_bytes: 50_000_000, allocation_strategy: AllocationStrategy::DurationWeighted, + subtitle_render_mode: SubtitleRenderMode::TwoPass, } } } @@ -685,6 +688,20 @@ pub enum AllocationStrategy { PriorityWeighted, } +/// High-level subtitle rendering mode for text subtitle authoring. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "kebab-case")] +pub enum SubtitleRenderMode { + OnePass, + TwoPass, +} + +impl Default for SubtitleRenderMode { + fn default() -> Self { + Self::TwoPass + } +} + // ── Command payloads ──────────────────────────────────────────────────────── /// Request to create a new project with initial settings. From c2151cd35da7321bd17ef2fda8fd91e2a2f631bb Mon Sep 17 00:00:00 2001 From: Scott Morris Date: Fri, 3 Apr 2026 14:35:41 -0400 Subject: [PATCH 2/8] docs: document DVD text subtitle rendering **What changed** - update the product summary and spec to describe first-pass text subtitle rendering as shipped DVD capability - revise the DVD authoring notes and subtitle plan so they describe the hybrid FFmpeg plus `spumux` seam that the build now uses - remove the outdated mixed-order limitation from the docs now that the planner keeps subtitle mapping order intact **Why** - issue `#17` changes the shipped subtitle authoring surface, so the repository docs need to describe the real behaviour and its remaining first-pass limits - keeping the docs aligned with the implementation avoids misleading review, QA, and release notes work --- README.md | 4 +-- SPEC.md | 2 +- docs/core-dvd-authoring-completion.md | 10 +++---- docs/text-subtitle-rendering-plan.md | 40 ++++++++------------------- 4 files changed, 19 insertions(+), 37 deletions(-) diff --git a/README.md b/README.md index d394114..62249ca 100644 --- a/README.md +++ b/README.md @@ -16,7 +16,7 @@ Current DVD authoring capabilities include: - authored menu routing for VMGM, titleset, and title-return paths, including keyboard-safe entry selection - asset inspection with embedded metadata title surfacing, compatibility explanations, and fix-oriented validation - DVD build planning and execution with diagnostics export and toolchain checks -- bitmap subtitle muxing, plus a developer option to skip unsupported text subtitle mappings during builds +- bitmap subtitle muxing plus first-pass text subtitle rendering for DVD authoring ## Workspace layout @@ -52,7 +52,7 @@ If Rust tooling is not installed locally, run Rust and Tauri commands through `g Current app behaviour also includes: - a persistent thumbnail cache stored in the app cache directory, with Settings controls to inspect and clear cached previews -- developer toggles to prefer host `PATH` tools over bundled sidecars and to skip unsupported subtitle mappings during builds +- developer toggles to prefer host `PATH` tools over bundled sidecars and to skip unsupported subtitle mappings during builds, mainly for unknown subtitle types or debugging - diagnostics bundle export including toolchain status, build logs, validation issues, project summary, and active developer options Build the frontend bundle: diff --git a/SPEC.md b/SPEC.md index a11f1c9..b081f4c 100644 --- a/SPEC.md +++ b/SPEC.md @@ -1181,6 +1181,7 @@ Because this app orchestrates binaries, the product should clearly communicate: - direct titleset editing with compatibility guidance - reversible subtitle track selection - bitmap subtitle authoring and muxing +- first-pass text subtitle rendering and DVD-safe conversion with simplified styling - `VIDEO_TS` export - optional ISO generation - build logs and diagnostics @@ -1197,7 +1198,6 @@ Because this app orchestrates binaries, the product should clearly communicate: - motion menus - autogenerated title, chapter, audio, and subtitle menu creation - menu themes and theme-aware generation -- text subtitle rendering and conversion - advanced VM command logic exposure - deep program/cell editing - Blu-ray authoring diff --git a/docs/core-dvd-authoring-completion.md b/docs/core-dvd-authoring-completion.md index e41f4d7..1e65c2a 100644 --- a/docs/core-dvd-authoring-completion.md +++ b/docs/core-dvd-authoring-completion.md @@ -53,13 +53,13 @@ When all source streams are already mapped, the picker is disabled. The subtitle ### 4. Subtitle Authoring and Export Pipeline -**Build planner** — `planner.rs` generates `ExtractSubtitles` jobs for each title with bitmap subtitle mappings. Only bitmap subtitles (`dvd_subtitle`, `dvdsub`, `hdmv_pgs_subtitle`, `pgssub`) are extracted; text subtitle rendering is out of scope. +**Build planner** — `planner.rs` muxes bitmap subtitle mappings during `TranscodeTitle`. Text subtitle mappings generate explicit `RenderTextSubtitles` jobs that first normalise the source stream to SRT, then compose DVD subtitle streams onto the authored title MPEG with `spumux` text rendering. -**FFmpeg extraction** — `ffmpeg.rs` provides `build_ffmpeg_subtitle_extract_command()` which generates: `ffmpeg -i source -map 0:{index} -c:s dvd_subtitle output.sub` +**Rendering path** — `ffmpeg.rs` provides a text-subtitle preparation command that exports supported text subtitle streams to SRT. `spumux` then renders that text into DVD subpictures using a host font and DVD-safe defaults. -**dvdauthor XML** — `authoring.rs` generates `` declarations with language attributes for each title's subtitle mappings within the titleset's `` section. +**dvdauthor XML** — `authoring.rs` generates `` declarations with language attributes for each title's subtitle mappings within the titleset's `` section. The first-pass text subtitle path keeps that seam by producing a final title MPEG that already contains the rendered subtitle streams. -**Build job type** — `ExtractSubtitles` variant added to `BuildJob` enum in `types.rs`, with title_id, title_name, source_path, output_path, command, and label fields. The frontend `BuildJob` union and `BuildPage.tsx` handle this job type for progress display. +**Build job types** — `TranscodeTitle` remains the bitmap subtitle mux point, and `RenderTextSubtitles` adds the explicit text subtitle rendering stage surfaced on the Build page. --- @@ -138,7 +138,7 @@ All fields use `#[serde(default)]` for backwards compatibility with existing pro | `titleset.format-mismatch` | Ensure all titles in this titleset use the same resolution and aspect ratio, or move mismatched titles to a separate titleset. | | `build.no-output-dir` | Set an output directory in the build settings to avoid being prompted each time. | | `subtitle.dangling-stream` | The source file may have changed. Remove this subtitle mapping or relink the asset. | -| `subtitle.text-only-unsupported` | Text subtitle rendering is not yet supported. Remove text subtitles or provide bitmap subtitle sources. | +| `subtitle.text-rendering-simplified` | Text subtitle rendering uses first-pass DVD-safe styling with a host font. | **Clickable issue navigation** — Validation issues in OverviewPage and BuildPage are clickable. Clicking an issue navigates to the relevant page (titles, menus, build, etc.) and auto-selects the affected entity using a `NavigationContext` provided by `App.tsx`. Entity type determines the target route: diff --git a/docs/text-subtitle-rendering-plan.md b/docs/text-subtitle-rendering-plan.md index 87a8ca3..2869d45 100644 --- a/docs/text-subtitle-rendering-plan.md +++ b/docs/text-subtitle-rendering-plan.md @@ -10,7 +10,13 @@ Enable text-based subtitle streams (SRT, ASS/SSA, WebVTT, `mov_text`) to be rend ### Current state -The bitmap subtitle path is fully functional: extraction via FFmpeg, muxing via spumux, and `` declarations in dvdauthor XML. Text subtitles are detected during inspection and classified as `SubtitleType::Text`, but the build pipeline filters them out. A `subtitle.text-only-unsupported` validation warning tells users that text subtitles cannot yet be authored. +The shipped first pass now keeps the current `dvdauthor` seam intact by: + +1. transcoding titles to DVD MPEG as before +2. normalising text subtitle mappings to SRT with FFmpeg +3. composing rendered DVD subtitle streams onto the authored title MPEG with `spumux` text subtitle rendering + +Text subtitles are no longer blocked outright, but the first pass stays honest about simplified styling and host-font dependence. ### DVD subtitle constraints @@ -38,36 +44,12 @@ From `inspect.rs::classify_subtitle_type`: ### Rendering approach -Use FFmpeg's subtitle filter chain to render text subtitles into bitmap subpictures. FFmpeg can: - -1. Decode all recognised text formats (SRT, ASS, SSA, WebVTT, `mov_text`). -2. Render styled text onto a transparent canvas via the `subtitles` or `ass` filter. -3. Output the result as a VOBsub stream (`-c:s dvd_subtitle`). - -The rendering pipeline is a two-step process per text subtitle mapping: +The implemented first pass uses a hybrid seam: -**Step 1 — Render text to bitmap video overlay** - -Generate a short video stream where each frame is a transparent canvas with the subtitle text rendered on it. FFmpeg's `subtitles` filter handles this when applied to a blank video input: - -``` -ffmpeg -f lavfi -i "color=c=black@0:s=720x480:d={duration}" \ - -vf "subtitles={source}:si={stream_index}:force_style='{style}'" \ - -c:v rawvideo -pix_fmt yuva420p \ - {temp_overlay}.nut -``` - -**Step 2 — Convert overlay to VOBsub** - -Quantise the rendered overlay to DVD's 4-colour palette and package as VOBsub: - -``` -ffmpeg -i {temp_overlay}.nut \ - -c:s dvd_subtitle \ - {output}.sub -``` +1. FFmpeg decodes the mapped text subtitle stream and exports it to SRT. +2. `spumux` renders that text input into DVD subtitle graphics while multiplexing the subtitle stream into the authored title MPEG. -An alternative single-pass approach may be viable depending on FFmpeg version capabilities. The two-step approach is safer and allows intermediate inspection. +This keeps the current authoring path compatible with `dvdauthor` and avoids relying on a direct FFmpeg text-to-`dvd_subtitle` path that is not available in the current toolchain. ### Style defaults From 2924555d96767784d4501a415dc7840e823d36a3 Mon Sep 17 00:00:00 2001 From: Scott Morris Date: Fri, 3 Apr 2026 19:18:15 -0400 Subject: [PATCH 3/8] fix: harden text subtitle fallback and font resolution **What changed** - restore the `skip unsupported streams` escape hatch so builds can strip text subtitle mappings instead of scheduling render jobs - resolve text subtitle fonts through Fontconfig from a small sans-serif shortlist rather than hard-coding a single alias - add validation for missing host subtitle fonts and a regression test that covers the skip-text fallback path - apply the formatting cleanup needed for the updated subtitle authoring docs and Rust sources **Why** - the review found that the existing fallback no longer let users author a subtitle-free disc when text subtitle rendering was unavailable - relying on one host-specific font alias made the new authoring path too machine-dependent and could fail late during build execution - early validation and a tested fallback make the text subtitle path safer without hiding the first-pass host-font constraint --- docs/core-dvd-authoring-completion.md | 44 ++++++------- .../src/build/planner.rs | 61 +++++++++++++++++-- .../src/desktop.rs | 15 +++++ .../src/models.rs | 9 +-- .../src/toolchain.rs | 30 +++++++++ 5 files changed, 125 insertions(+), 34 deletions(-) diff --git a/docs/core-dvd-authoring-completion.md b/docs/core-dvd-authoring-completion.md index 1e65c2a..b77fa06 100644 --- a/docs/core-dvd-authoring-completion.md +++ b/docs/core-dvd-authoring-completion.md @@ -117,28 +117,28 @@ All fields use `#[serde(default)]` for backwards compatibility with existing pro **Fix descriptions by rule:** -| Code | Suggested fix | -| -------------------------------- | ------------------------------------------------------------------------------------------------------------------------------ | -| `disc.no-titlesets` | Add at least one titleset to the disc. | -| `disc.no-titles` | Add titles in the Titles page to define the disc's playback structure. | -| `disc.no-first-play` | Set a first-play action on the overview page so the disc has a defined startup behaviour. | -| `title.no-source` | Open the title and assign a source asset from the Assets library. | -| `title.dangling-source` | Re-import the missing asset or assign a different source. | -| `title.no-video-mapping` | Select a video stream in the title's track mapping section. | -| `title.no-output-profile` | Choose a video output profile (resolution and aspect ratio) for this title. | -| `chapter.non-increasing` | Reorder or adjust chapter timestamps so they are strictly increasing. | -| `chapter.beyond-duration` | Move this chapter to a timestamp within the asset's duration or remove it. | -| `menu.no-buttons` | Add at least one button to define user interaction. | -| `menu.no-default-button` | Set a default button so the player knows which button to highlight on entry. | -| `menu.button-no-action` | Assign an action (play title, show menu, etc.) to this button. | -| `menu.dangling-title-ref` | Update the button action to point to an existing title or remove it. | -| `menu.dangling-menu-ref` | Update the button action to point to an existing menu or remove it. | -| `menu.dangling-nav-ref` | Remove the broken nav link or use auto-generate navigation to rebuild all links. | -| `menu.button-no-navigation` | Use the auto-generate navigation feature to create directional links for all buttons. | -| `titleset.format-mismatch` | Ensure all titles in this titleset use the same resolution and aspect ratio, or move mismatched titles to a separate titleset. | -| `build.no-output-dir` | Set an output directory in the build settings to avoid being prompted each time. | -| `subtitle.dangling-stream` | The source file may have changed. Remove this subtitle mapping or relink the asset. | -| `subtitle.text-rendering-simplified` | Text subtitle rendering uses first-pass DVD-safe styling with a host font. | +| Code | Suggested fix | +| ------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------ | +| `disc.no-titlesets` | Add at least one titleset to the disc. | +| `disc.no-titles` | Add titles in the Titles page to define the disc's playback structure. | +| `disc.no-first-play` | Set a first-play action on the overview page so the disc has a defined startup behaviour. | +| `title.no-source` | Open the title and assign a source asset from the Assets library. | +| `title.dangling-source` | Re-import the missing asset or assign a different source. | +| `title.no-video-mapping` | Select a video stream in the title's track mapping section. | +| `title.no-output-profile` | Choose a video output profile (resolution and aspect ratio) for this title. | +| `chapter.non-increasing` | Reorder or adjust chapter timestamps so they are strictly increasing. | +| `chapter.beyond-duration` | Move this chapter to a timestamp within the asset's duration or remove it. | +| `menu.no-buttons` | Add at least one button to define user interaction. | +| `menu.no-default-button` | Set a default button so the player knows which button to highlight on entry. | +| `menu.button-no-action` | Assign an action (play title, show menu, etc.) to this button. | +| `menu.dangling-title-ref` | Update the button action to point to an existing title or remove it. | +| `menu.dangling-menu-ref` | Update the button action to point to an existing menu or remove it. | +| `menu.dangling-nav-ref` | Remove the broken nav link or use auto-generate navigation to rebuild all links. | +| `menu.button-no-navigation` | Use the auto-generate navigation feature to create directional links for all buttons. | +| `titleset.format-mismatch` | Ensure all titles in this titleset use the same resolution and aspect ratio, or move mismatched titles to a separate titleset. | +| `build.no-output-dir` | Set an output directory in the build settings to avoid being prompted each time. | +| `subtitle.dangling-stream` | The source file may have changed. Remove this subtitle mapping or relink the asset. | +| `subtitle.text-rendering-simplified` | Text subtitle rendering uses first-pass DVD-safe styling with a host font. | **Clickable issue navigation** — Validation issues in OverviewPage and BuildPage are clickable. Clicking an issue navigates to the relevant page (titles, menus, build, etc.) and auto-selects the affected entity using a `NavigationContext` provided by `App.tsx`. Entity type determines the target route: diff --git a/plugins/tauri-plugin-spindle-project/src/build/planner.rs b/plugins/tauri-plugin-spindle-project/src/build/planner.rs index be41c45..aebe5f1 100644 --- a/plugins/tauri-plugin-spindle-project/src/build/planner.rs +++ b/plugins/tauri-plugin-spindle-project/src/build/planner.rs @@ -170,6 +170,8 @@ pub fn generate_build_plan_with_options( } let project = &owned_project; + let subtitle_font_family = crate::toolchain::resolve_text_subtitle_font(); + let paths = BuildPaths::new(output_dir); let tools = ResolvedToolchain::resolve(skip_sidecar); @@ -247,6 +249,12 @@ pub fn generate_build_plan_with_options( }) .collect(); let has_text_subtitles = !text_subtitle_mappings.is_empty(); + if has_text_subtitles && subtitle_font_family.is_none() { + return Err(crate::Error::Build(format!( + "Title \"{}\" has text subtitle mappings, but no host sans-serif font could be resolved with Fontconfig. Enable `skip unsupported streams` to author without text subtitles, or install a compatible font such as Noto Sans or Liberation Sans.", + title.name + ))); + } let title_paths = paths.title_paths(&title.id); if !has_text_subtitles { @@ -306,7 +314,9 @@ pub fn generate_build_plan_with_options( aspect: AspectMode::SixteenByNine, }); let mut current_input = output_path; - let font_family = "Sans".to_string(); + let font_family = subtitle_font_family + .clone() + .expect("text subtitle titles should have a resolved host font"); for (text_job_index, (stream_index, sm)) in text_subtitle_mappings.iter().enumerate() @@ -515,7 +525,7 @@ fn generate_text_subtitle_spumux_xml( ) } -/// Remove subtitle mappings that cannot be authored by the current build. +/// Remove subtitle mappings that the escape hatch should skip during build. fn strip_unsupported_subtitle_mappings(project: &mut SpindleProjectFile) { let assets: HashMap<&str, &Asset> = project.assets.iter().map(|a| (a.id.as_str(), a)).collect(); @@ -528,8 +538,7 @@ fn strip_unsupported_subtitle_mappings(project: &mut SpindleProjectFile) { { title.subtitle_mappings.retain(|sm| { asset.subtitle_streams.iter().any(|s| { - s.index == sm.source_stream_index - && matches!(s.subtitle_type, SubtitleType::Bitmap | SubtitleType::Text) + s.index == sm.source_stream_index && s.subtitle_type == SubtitleType::Bitmap }) }); } @@ -540,7 +549,7 @@ fn strip_unsupported_subtitle_mappings(project: &mut SpindleProjectFile) { #[cfg(test)] mod tests { use crate::build::test_support::{test_menu, test_project}; - use crate::build::{generate_build_plan, BuildJob}; + use crate::build::{generate_build_plan, generate_build_plan_with_options, BuildJob}; use crate::models::*; #[test] @@ -738,6 +747,48 @@ mod tests { ); } + #[test] + fn build_plan_skip_unsupported_streams_removes_text_subtitles() { + let mut project = test_project(); + project.assets[0].subtitle_streams.push(SubtitleStreamInfo { + index: 2, + codec: "subrip".to_string(), + language: Some("eng".to_string()), + subtitle_type: SubtitleType::Text, + title: Some("English text".to_string()), + }); + project.disc.titlesets[0].titles[0] + .subtitle_mappings + .push(SubtitleTrackMapping { + id: "sm-text".to_string(), + source_stream_index: 2, + label: "English".to_string(), + language: "eng".to_string(), + order_index: 0, + is_default: false, + is_forced: false, + }); + + let plan = + generate_build_plan_with_options(&project, "/tmp/dvd_output", false, true).unwrap(); + + assert!( + !plan + .jobs + .iter() + .any(|job| matches!(job, BuildJob::RenderTextSubtitles { .. })), + "skip unsupported streams should strip text subtitle render jobs" + ); + assert!( + plan.jobs + .iter() + .any(|job| matches!(job, BuildJob::TranscodeTitle { + output_path, .. + } if output_path.ends_with("title-1.mpg"))), + "text subtitle stripping should fall back to the direct title output path" + ); + } + #[test] fn build_plan_deduplicates_identical_transcodes_with_different_mapping_ids() { let mut project = test_project(); diff --git a/plugins/tauri-plugin-spindle-project/src/desktop.rs b/plugins/tauri-plugin-spindle-project/src/desktop.rs index 0567755..a5b8789 100644 --- a/plugins/tauri-plugin-spindle-project/src/desktop.rs +++ b/plugins/tauri-plugin-spindle-project/src/desktop.rs @@ -303,6 +303,21 @@ impl SpindleProject { entity_name: Some(title.name.clone()), suggested_fix: Some("First-pass subtitle rendering uses a host font and simplified DVD-safe styling. Review the authored disc output if subtitle appearance matters.".to_string()), }); + + if crate::toolchain::resolve_text_subtitle_font().is_none() { + issues.push(ValidationIssue { + severity: IssueSeverity::Warning, + code: "subtitle.host-font-unavailable".to_string(), + message: format!( + "Title \"{}\" has text subtitle mappings, but no compatible host sans-serif font could be resolved.", + title.name + ), + context: Some(title.id.clone()), + entity_type: Some("title".to_string()), + entity_name: Some(title.name.clone()), + suggested_fix: Some("Install a Fontconfig-visible sans-serif font such as Noto Sans or Liberation Sans, or enable the developer option to skip unsupported streams for a subtitle-free build.".to_string()), + }); + } } } } diff --git a/plugins/tauri-plugin-spindle-project/src/models.rs b/plugins/tauri-plugin-spindle-project/src/models.rs index 5744d85..cc9e840 100644 --- a/plugins/tauri-plugin-spindle-project/src/models.rs +++ b/plugins/tauri-plugin-spindle-project/src/models.rs @@ -689,19 +689,14 @@ pub enum AllocationStrategy { } /// High-level subtitle rendering mode for text subtitle authoring. -#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, Default)] #[serde(rename_all = "kebab-case")] pub enum SubtitleRenderMode { OnePass, + #[default] TwoPass, } -impl Default for SubtitleRenderMode { - fn default() -> Self { - Self::TwoPass - } -} - // ── Command payloads ──────────────────────────────────────────────────────── /// Request to create a new project with initial settings. diff --git a/plugins/tauri-plugin-spindle-project/src/toolchain.rs b/plugins/tauri-plugin-spindle-project/src/toolchain.rs index af9c31c..3a99ef5 100644 --- a/plugins/tauri-plugin-spindle-project/src/toolchain.rs +++ b/plugins/tauri-plugin-spindle-project/src/toolchain.rs @@ -8,6 +8,7 @@ // SPDX-License-Identifier: MIT use std::path::PathBuf; +use std::process::Command; /// Resolve the path to an external tool. /// @@ -28,6 +29,35 @@ pub fn resolve_tool(name: &str, skip_sidecar: bool) -> Option { path_lookup(name) } +/// Resolve a host font family for first-pass text subtitle rendering. +/// +/// Returns the first family that Fontconfig can match from a conservative +/// shortlist of sans-serif fonts commonly available on Linux desktops. +pub fn resolve_text_subtitle_font() -> Option { + let fontconfig = path_lookup("fc-match")?; + for family in [ + "Noto Sans", + "Liberation Sans", + "DejaVu Sans", + "Arial", + "Helvetica", + "Sans", + ] { + let output = Command::new(&fontconfig) + .args(["-f", "%{family[0]}", family]) + .output() + .ok()?; + if !output.status.success() { + continue; + } + let matched = String::from_utf8_lossy(&output.stdout).trim().to_string(); + if !matched.is_empty() { + return Some(matched); + } + } + None +} + /// Return the expected sidecar path: same directory as the running executable. fn sidecar_path(name: &str) -> Option { let exe = std::env::current_exe().ok()?; From 2d1a5cb3c1fbe2ca03637fc443a463d1d4780662 Mon Sep 17 00:00:00 2001 From: Scott Morris Date: Sat, 4 Apr 2026 12:26:35 -0400 Subject: [PATCH 4/8] fix: skip empty text subtitle passes **What changed** - treat zero-cue extracted text subtitle files as a handled runtime outcome during authoring - carry the current authored MPEG forward when a subtitle pass is skipped so later passes still chain correctly - add executor coverage for empty and whitespace-only prepared subtitle files **Why** - trimmed sample clips can legitimately include subtitle streams that have no cues in the authored range - builds should continue when subtitle extraction succeeds structurally but produces no usable subtitle text for the selected portion --- .../src/build/executor.rs | 194 +++++++++++++++++- 1 file changed, 191 insertions(+), 3 deletions(-) diff --git a/plugins/tauri-plugin-spindle-project/src/build/executor.rs b/plugins/tauri-plugin-spindle-project/src/build/executor.rs index 1759c21..e01bfb5 100644 --- a/plugins/tauri-plugin-spindle-project/src/build/executor.rs +++ b/plugins/tauri-plugin-spindle-project/src/build/executor.rs @@ -265,6 +265,58 @@ where } } + match subtitle_file_has_cues(subtitle_path) { + Ok(true) => {} + Ok(false) => { + if let Err(msg) = carry_title_stage_forward(input_path, output_path) { + log_lines.push(msg.clone()); + on_progress(BuildProgress::job( + i, + total, + label, + BuildJobStatus::Failed, + Some(msg.clone()), + )); + return failure(plan, log_lines, i, msg); + } + + let msg = format!( + "Skipped text subtitle render for {subtitle_path} because the extracted subtitle file had no cues in this authored range." + ); + log_lines.push(msg.clone()); + on_progress(BuildProgress { + job_index: i, + total_jobs: total, + current_label: label.clone(), + status: BuildJobStatus::Running, + output: Some(msg), + step_label: Some("Prepare subtitle text".to_string()), + step_percent: Some(100.0), + step_detail: Some(subtitle_path.clone()), + step_status: Some(BuildJobStatus::Complete), + }); + on_progress(BuildProgress::job( + i, + total, + label, + BuildJobStatus::Complete, + None, + )); + continue; + } + Err(msg) => { + log_lines.push(msg.clone()); + on_progress(BuildProgress::job( + i, + total, + label, + BuildJobStatus::Failed, + Some(msg.clone()), + )); + return failure(plan, log_lines, i, msg); + } + } + let xml_path = command .last() .cloned() @@ -415,6 +467,33 @@ fn reset_workspace_directory(path: &str) -> std::io::Result<()> { Ok(()) } +fn subtitle_file_has_cues(path: &str) -> Result { + let bytes = + std::fs::read(path).map_err(|e| format!("Failed to read prepared subtitle file: {e}"))?; + Ok(bytes.iter().any(|byte| !byte.is_ascii_whitespace())) +} + +fn carry_title_stage_forward(input_path: &str, output_path: &str) -> Result<(), String> { + if input_path == output_path { + return Ok(()); + } + + let src = Path::new(input_path); + let dst = Path::new(output_path); + if let Some(parent) = dst.parent() { + std::fs::create_dir_all(parent) + .map_err(|e| format!("Failed to prepare title stage directory: {e}"))?; + } + if dst.exists() { + std::fs::remove_file(dst) + .map_err(|e| format!("Failed to replace carried title stage output: {e}"))?; + } + + std::fs::hard_link(src, dst) + .or_else(|_| std::fs::copy(src, dst).map(|_| ())) + .map_err(|e| format!("Failed to carry title stage forward after empty subtitles: {e}")) +} + /// Run an FFmpeg command with streaming stderr, step-progress reporting, /// and cancellation support. /// @@ -644,10 +723,12 @@ mod tests { use std::time::{SystemTime, UNIX_EPOCH}; use crate::build::test_support::{test_menu_with_action, test_project}; - use crate::build::{execute_build_plan, generate_build_plan}; - use crate::models::{PlaybackAction, SubtitleStreamInfo, SubtitleType}; + use crate::build::{ + execute_build_plan, generate_build_plan, BuildJob, BuildPlan, BuildProgress, BuildSummary, + }; + use crate::models::{PlaybackAction, SubtitleRenderMode, SubtitleStreamInfo, SubtitleType}; - use super::reset_workspace_directory; + use super::{reset_workspace_directory, subtitle_file_has_cues}; fn unique_temp_dir(name: &str) -> PathBuf { let nanos = SystemTime::now() @@ -692,6 +773,113 @@ mod tests { fs::remove_dir_all(&output_dir).unwrap(); } + #[test] + fn subtitle_file_has_cues_rejects_empty_and_whitespace_only_files() { + let output_dir = unique_temp_dir("subtitle-cues"); + fs::create_dir_all(&output_dir).unwrap(); + let empty_path = output_dir.join("empty.srt"); + let whitespace_path = output_dir.join("whitespace.srt"); + let populated_path = output_dir.join("populated.srt"); + + fs::write(&empty_path, "").unwrap(); + fs::write(&whitespace_path, "\n \t\n").unwrap(); + fs::write( + &populated_path, + "1\n00:00:00,000 --> 00:00:01,000\nHello.\n", + ) + .unwrap(); + + assert!(!subtitle_file_has_cues(empty_path.to_str().unwrap()).unwrap()); + assert!(!subtitle_file_has_cues(whitespace_path.to_str().unwrap()).unwrap()); + assert!(subtitle_file_has_cues(populated_path.to_str().unwrap()).unwrap()); + + fs::remove_dir_all(&output_dir).unwrap(); + } + + #[test] + fn execute_build_plan_skips_empty_text_subtitle_passes() { + let output_dir = unique_temp_dir("empty-text-subtitle-pass"); + let working_dir = output_dir.join("_spindle_work"); + let input_path = working_dir.join("titles").join("title-1-base.mpg"); + let output_path = working_dir.join("titles").join("title-1.mpg"); + let subtitle_path = working_dir.join("subtitles").join("title-1_sub_2.srt"); + let xml_path = working_dir.join("subtitles").join("title-1_sub_2.xml"); + + fs::create_dir_all(input_path.parent().unwrap()).unwrap(); + fs::create_dir_all(subtitle_path.parent().unwrap()).unwrap(); + fs::write(&input_path, b"stub-mpeg-data").unwrap(); + + let plan = BuildPlan { + jobs: vec![BuildJob::RenderTextSubtitles { + title_id: "title-1".to_string(), + title_name: "Title 1".to_string(), + source_path: "/tmp/source.mkv".to_string(), + source_stream_index: 2, + input_path: input_path.display().to_string(), + output_path: output_path.display().to_string(), + subtitle_path: subtitle_path.display().to_string(), + prepare_command: vec![ + "python3".to_string(), + "-c".to_string(), + "from pathlib import Path; import sys; Path(sys.argv[-1]).write_text('')" + .to_string(), + subtitle_path.display().to_string(), + ], + spumux_xml: "".to_string(), + command: vec![ + "/bin/sh".to_string(), + "-c".to_string(), + "exit 99".to_string(), + xml_path.display().to_string(), + ], + label: "Render subtitle \"English (forced)\" for \"Title 1\"".to_string(), + render_mode: SubtitleRenderMode::TwoPass, + font_family: "Noto Sans".to_string(), + }], + output_directory: output_dir.display().to_string(), + working_directory: working_dir.display().to_string(), + dvdauthor_xml: String::new(), + summary: BuildSummary { + total_jobs: 1, + transcode_jobs: 0, + titles_count: 1, + menus_count: 0, + generate_iso: false, + estimated_commands: vec![], + }, + }; + + let mut progress_updates: Vec = Vec::new(); + let result = execute_build_plan(&plan, |progress| progress_updates.push(progress)); + + assert!(result.success, "expected build to succeed: {result:?}"); + assert_eq!( + fs::read(&output_path).unwrap(), + fs::read(&input_path).unwrap(), + "expected the prior title stage to carry forward unchanged" + ); + assert!( + !xml_path.exists(), + "expected spumux XML not to be written when subtitle extraction is empty" + ); + assert!( + result + .log_lines + .iter() + .any(|line| line.contains("had no cues")), + "expected build log to explain the skipped subtitle pass" + ); + assert!( + progress_updates.iter().any(|progress| progress + .output + .as_ref() + .is_some_and(|line| line.contains("had no cues"))), + "expected progress updates to mention the skipped subtitle pass" + ); + + fs::remove_dir_all(&output_dir).unwrap(); + } + #[test] #[ignore = "requires ffmpeg, spumux, and dvdauthor on PATH"] fn execute_build_plan_smoke_authors_titleset_menu_return_path() { From 583d20e570461e463afb15637bfe5ccc596c7712 Mon Sep 17 00:00:00 2001 From: Scott Morris Date: Sat, 4 Apr 2026 12:35:48 -0400 Subject: [PATCH 5/8] fix: normalise subtitle languages for dvdauthor **What changed** - normalise recognised three-letter subtitle languages to the two-letter codes that `dvdauthor` accepts in `` declarations - omit the `lang` attribute when a subtitle language cannot be made DVD-safe - add authoring coverage for normalised and invalid subtitle language values **Why** - real projects can carry ISO 639-2 style values such as `eng` and `fre` in subtitle mappings - `dvdauthor` rejects those values during final authoring, even when the rest of the build has succeeded --- .../src/build/authoring.rs | 119 ++++++++++++++++-- 1 file changed, 110 insertions(+), 9 deletions(-) diff --git a/plugins/tauri-plugin-spindle-project/src/build/authoring.rs b/plugins/tauri-plugin-spindle-project/src/build/authoring.rs index eef28a3..f0e9318 100644 --- a/plugins/tauri-plugin-spindle-project/src/build/authoring.rs +++ b/plugins/tauri-plugin-spindle-project/src/build/authoring.rs @@ -262,11 +262,14 @@ fn append_titles_section( .titles .iter() .find_map(|t| t.subtitle_mappings.get(i).map(|sm| sm.language.as_str())) - .unwrap_or("und"); - xml.push_str(&format!( - " \n", - xml_escape(lang) - )); + .and_then(dvdauthor_subpicture_language); + match lang { + Some(lang) => xml.push_str(&format!( + " \n", + xml_escape(&lang) + )), + None => xml.push_str(" \n"), + } } } @@ -321,6 +324,79 @@ fn append_title_pgc( xml.push_str(" \n"); Ok(()) } + +fn dvdauthor_subpicture_language(language: &str) -> Option { + let normalised = language.trim().to_ascii_lowercase(); + match normalised.as_str() { + "" | "und" | "nolang" => None, + "ab" | "aa" | "af" | "ak" | "sq" | "am" | "ar" | "an" | "hy" | "as" | "av" | "ae" + | "ay" | "az" | "ba" | "bm" | "eu" | "be" | "bn" | "bh" | "bi" | "bs" | "br" | "bg" + | "my" | "ca" | "cs" | "ch" | "ce" | "ny" | "zh" | "cu" | "cv" | "kw" | "co" | "cr" + | "hr" | "cy" | "da" | "de" | "dv" | "nl" | "dz" | "el" | "en" | "eo" | "et" | "ee" + | "fo" | "fa" | "fj" | "fi" | "fr" | "fy" | "ff" | "gd" | "ga" | "gl" | "gv" | "gn" + | "gu" | "ht" | "ha" | "he" | "hz" | "hi" | "ho" | "hu" | "ia" | "id" | "ie" | "ig" + | "ii" | "ik" | "io" | "is" | "it" | "iu" | "ja" | "jv" | "ka" | "kg" | "ki" | "kj" + | "kk" | "kl" | "km" | "kn" | "ko" | "kr" | "ks" | "ku" | "kv" | "ky" | "la" | "lb" + | "lg" | "li" | "ln" | "lo" | "lt" | "lu" | "lv" | "mg" | "mh" | "ml" | "mi" | "mk" + | "mr" | "ms" | "mt" | "mn" | "na" | "nv" | "nd" | "ne" | "ng" | "nb" | "nn" | "no" + | "nr" | "oc" | "oj" | "om" | "or" | "os" | "pa" | "pi" | "pl" | "ps" | "pt" + | "qu" | "rm" | "rn" | "ro" | "ru" | "rw" | "sa" | "sc" | "sd" | "se" | "sm" | "sg" + | "sr" | "sn" | "si" | "sk" | "sl" | "so" | "st" | "es" | "su" | "sw" | "ss" | "sv" + | "ta" | "te" | "tg" | "th" | "ti" | "tk" | "tl" | "tn" | "to" | "tr" | "ts" | "tt" + | "tw" | "ty" | "ug" | "uk" | "ur" | "uz" | "ve" | "vi" | "vo" | "wa" | "wo" | "xh" + | "yi" | "yo" | "za" | "zu" => Some(normalised), + "alb" | "sqi" => Some("sq".to_string()), + "ara" => Some("ar".to_string()), + "arm" | "hye" => Some("hy".to_string()), + "baq" | "eus" => Some("eu".to_string()), + "bul" => Some("bg".to_string()), + "bur" | "mya" => Some("my".to_string()), + "cat" => Some("ca".to_string()), + "chi" | "zho" => Some("zh".to_string()), + "hrv" => Some("hr".to_string()), + "cze" | "ces" => Some("cs".to_string()), + "dan" => Some("da".to_string()), + "dut" | "nld" => Some("nl".to_string()), + "eng" => Some("en".to_string()), + "est" => Some("et".to_string()), + "fin" => Some("fi".to_string()), + "fre" | "fra" => Some("fr".to_string()), + "geo" | "kat" => Some("ka".to_string()), + "ger" | "deu" => Some("de".to_string()), + "gre" | "ell" => Some("el".to_string()), + "heb" => Some("he".to_string()), + "hin" => Some("hi".to_string()), + "hun" => Some("hu".to_string()), + "ice" | "isl" => Some("is".to_string()), + "ind" => Some("id".to_string()), + "ita" => Some("it".to_string()), + "jpn" => Some("ja".to_string()), + "kor" => Some("ko".to_string()), + "lav" => Some("lv".to_string()), + "lit" => Some("lt".to_string()), + "mac" | "mkd" => Some("mk".to_string()), + "may" | "msa" => Some("ms".to_string()), + "mao" | "mri" => Some("mi".to_string()), + "nor" => Some("no".to_string()), + "nob" => Some("no".to_string()), + "nno" => Some("nn".to_string()), + "pol" => Some("pl".to_string()), + "por" => Some("pt".to_string()), + "rum" | "ron" => Some("ro".to_string()), + "rus" => Some("ru".to_string()), + "slo" | "slk" => Some("sk".to_string()), + "slv" => Some("sl".to_string()), + "spa" => Some("es".to_string()), + "srp" => Some("sr".to_string()), + "swe" => Some("sv".to_string()), + "tha" => Some("th".to_string()), + "tur" => Some("tr".to_string()), + "ukr" => Some("uk".to_string()), + "vie" => Some("vi".to_string()), + "wel" | "cym" => Some("cy".to_string()), + _ => None, + } +} #[cfg(test)] mod tests { use crate::build::generate_build_plan; @@ -393,7 +469,34 @@ mod tests { } #[test] - fn dvdauthor_xml_escapes_subpicture_language_values() { + fn dvdauthor_xml_normalises_subpicture_languages_for_dvdauthor() { + let mut project = test_project(); + project.assets[0].subtitle_streams.push(SubtitleStreamInfo { + index: 2, + codec: "dvd_subtitle".to_string(), + language: Some("eng".to_string()), + subtitle_type: SubtitleType::Bitmap, + title: None, + }); + project.disc.titlesets[0].titles[0] + .subtitle_mappings + .push(SubtitleTrackMapping { + id: "sm-1".to_string(), + source_stream_index: 2, + label: "English".to_string(), + language: "eng".to_string(), + order_index: 0, + is_default: false, + is_forced: false, + }); + + let plan = generate_build_plan(&project, "/tmp/dvd_output", false).unwrap(); + + assert!(plan.dvdauthor_xml.contains("")); + } + + #[test] + fn dvdauthor_xml_omits_invalid_subpicture_language_values() { let mut project = test_project(); project.assets[0].subtitle_streams.push(SubtitleStreamInfo { index: 2, @@ -416,9 +519,7 @@ mod tests { let plan = generate_build_plan(&project, "/tmp/dvd_output", false).unwrap(); - assert!(plan - .dvdauthor_xml - .contains("")); + assert!(plan.dvdauthor_xml.contains("")); } #[test] From e74b9a878a44aa23fd9e10c9679fa70036353706 Mon Sep 17 00:00:00 2001 From: Scott Morris Date: Sat, 4 Apr 2026 12:38:49 -0400 Subject: [PATCH 6/8] refactor: use isolang for dvd subtitle languages **What changed** - add `isolang` to normalise subtitle language codes before writing `dvdauthor` subpicture declarations - accept both ISO 639-1 and ISO 639-3 inputs, and trim simple region suffixes such as `en-US` - keep the existing fallback of omitting `lang` when a subtitle language cannot be mapped safely **Why** - a library-backed mapping is easier to trust and maintain than a local hand-written conversion table - this keeps the `dvdauthor` fix focused on behaviour while moving the code-to-code mapping burden onto a dedicated crate --- Cargo.lock | 10 +++ .../tauri-plugin-spindle-project/Cargo.toml | 1 + .../src/build/authoring.rs | 85 ++++--------------- 3 files changed, 27 insertions(+), 69 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f52ace9..b42d5d8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1951,6 +1951,15 @@ dependencies = [ "once_cell", ] +[[package]] +name = "isolang" +version = "2.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fe50d48c77760c55188549098b9a7f6e37ae980c586a24693d6b01c3b2010c3c" +dependencies = [ + "phf 0.11.3", +] + [[package]] name = "itoa" version = "1.0.18" @@ -4334,6 +4343,7 @@ name = "tauri-plugin-spindle-project" version = "0.2.0" dependencies = [ "chrono", + "isolang", "serde", "serde_json", "tauri", diff --git a/plugins/tauri-plugin-spindle-project/Cargo.toml b/plugins/tauri-plugin-spindle-project/Cargo.toml index 9a79285..f08756d 100644 --- a/plugins/tauri-plugin-spindle-project/Cargo.toml +++ b/plugins/tauri-plugin-spindle-project/Cargo.toml @@ -15,6 +15,7 @@ serde_json = "1" thiserror = "2" uuid = { version = "1", features = ["v4", "serde"] } chrono = { version = "0.4", features = ["serde"] } +isolang = "2.4" [build-dependencies] tauri-plugin = { version = "2", features = ["build"] } diff --git a/plugins/tauri-plugin-spindle-project/src/build/authoring.rs b/plugins/tauri-plugin-spindle-project/src/build/authoring.rs index f0e9318..012ce44 100644 --- a/plugins/tauri-plugin-spindle-project/src/build/authoring.rs +++ b/plugins/tauri-plugin-spindle-project/src/build/authoring.rs @@ -5,6 +5,8 @@ use std::path::Path; +use isolang::Language; + use crate::models::*; use super::dvd_navigation::{ @@ -326,76 +328,21 @@ fn append_title_pgc( } fn dvdauthor_subpicture_language(language: &str) -> Option { - let normalised = language.trim().to_ascii_lowercase(); - match normalised.as_str() { - "" | "und" | "nolang" => None, - "ab" | "aa" | "af" | "ak" | "sq" | "am" | "ar" | "an" | "hy" | "as" | "av" | "ae" - | "ay" | "az" | "ba" | "bm" | "eu" | "be" | "bn" | "bh" | "bi" | "bs" | "br" | "bg" - | "my" | "ca" | "cs" | "ch" | "ce" | "ny" | "zh" | "cu" | "cv" | "kw" | "co" | "cr" - | "hr" | "cy" | "da" | "de" | "dv" | "nl" | "dz" | "el" | "en" | "eo" | "et" | "ee" - | "fo" | "fa" | "fj" | "fi" | "fr" | "fy" | "ff" | "gd" | "ga" | "gl" | "gv" | "gn" - | "gu" | "ht" | "ha" | "he" | "hz" | "hi" | "ho" | "hu" | "ia" | "id" | "ie" | "ig" - | "ii" | "ik" | "io" | "is" | "it" | "iu" | "ja" | "jv" | "ka" | "kg" | "ki" | "kj" - | "kk" | "kl" | "km" | "kn" | "ko" | "kr" | "ks" | "ku" | "kv" | "ky" | "la" | "lb" - | "lg" | "li" | "ln" | "lo" | "lt" | "lu" | "lv" | "mg" | "mh" | "ml" | "mi" | "mk" - | "mr" | "ms" | "mt" | "mn" | "na" | "nv" | "nd" | "ne" | "ng" | "nb" | "nn" | "no" - | "nr" | "oc" | "oj" | "om" | "or" | "os" | "pa" | "pi" | "pl" | "ps" | "pt" - | "qu" | "rm" | "rn" | "ro" | "ru" | "rw" | "sa" | "sc" | "sd" | "se" | "sm" | "sg" - | "sr" | "sn" | "si" | "sk" | "sl" | "so" | "st" | "es" | "su" | "sw" | "ss" | "sv" - | "ta" | "te" | "tg" | "th" | "ti" | "tk" | "tl" | "tn" | "to" | "tr" | "ts" | "tt" - | "tw" | "ty" | "ug" | "uk" | "ur" | "uz" | "ve" | "vi" | "vo" | "wa" | "wo" | "xh" - | "yi" | "yo" | "za" | "zu" => Some(normalised), - "alb" | "sqi" => Some("sq".to_string()), - "ara" => Some("ar".to_string()), - "arm" | "hye" => Some("hy".to_string()), - "baq" | "eus" => Some("eu".to_string()), - "bul" => Some("bg".to_string()), - "bur" | "mya" => Some("my".to_string()), - "cat" => Some("ca".to_string()), - "chi" | "zho" => Some("zh".to_string()), - "hrv" => Some("hr".to_string()), - "cze" | "ces" => Some("cs".to_string()), - "dan" => Some("da".to_string()), - "dut" | "nld" => Some("nl".to_string()), - "eng" => Some("en".to_string()), - "est" => Some("et".to_string()), - "fin" => Some("fi".to_string()), - "fre" | "fra" => Some("fr".to_string()), - "geo" | "kat" => Some("ka".to_string()), - "ger" | "deu" => Some("de".to_string()), - "gre" | "ell" => Some("el".to_string()), - "heb" => Some("he".to_string()), - "hin" => Some("hi".to_string()), - "hun" => Some("hu".to_string()), - "ice" | "isl" => Some("is".to_string()), - "ind" => Some("id".to_string()), - "ita" => Some("it".to_string()), - "jpn" => Some("ja".to_string()), - "kor" => Some("ko".to_string()), - "lav" => Some("lv".to_string()), - "lit" => Some("lt".to_string()), - "mac" | "mkd" => Some("mk".to_string()), - "may" | "msa" => Some("ms".to_string()), - "mao" | "mri" => Some("mi".to_string()), - "nor" => Some("no".to_string()), - "nob" => Some("no".to_string()), - "nno" => Some("nn".to_string()), - "pol" => Some("pl".to_string()), - "por" => Some("pt".to_string()), - "rum" | "ron" => Some("ro".to_string()), - "rus" => Some("ru".to_string()), - "slo" | "slk" => Some("sk".to_string()), - "slv" => Some("sl".to_string()), - "spa" => Some("es".to_string()), - "srp" => Some("sr".to_string()), - "swe" => Some("sv".to_string()), - "tha" => Some("th".to_string()), - "tur" => Some("tr".to_string()), - "ukr" => Some("uk".to_string()), - "vie" => Some("vi".to_string()), - "wel" | "cym" => Some("cy".to_string()), - _ => None, + let normalised = language + .trim() + .split(['-', '_']) + .next() + .unwrap_or_default() + .to_ascii_lowercase(); + + if matches!(normalised.as_str(), "" | "und" | "nolang") { + return None; } + + Language::from_639_1(&normalised) + .and_then(|lang| lang.to_639_1()) + .or_else(|| Language::from_639_3(&normalised).and_then(|lang| lang.to_639_1())) + .map(str::to_string) } #[cfg(test)] mod tests { From 254ae48e4ffff198f2903881f5e387a1f23376b9 Mon Sep 17 00:00:00 2001 From: Scott Morris Date: Sat, 4 Apr 2026 13:00:50 -0400 Subject: [PATCH 7/8] fix: handle bibliographic subtitle language codes **What changed** - canonicalise common ISO 639-2/B bibliographic subtitle codes such as `fre` before passing them through `isolang` - document why the alias shim exists at the `dvdauthor` normalisation boundary - add regression coverage for the French bibliographic code path **Why** - source media metadata can legitimately report bibliographic language codes even when `dvdauthor` needs a two-letter output tag - `isolang` handles canonical 639-3 values well, but the bibliographic alias layer still needs a small compatibility shim --- .../src/build/authoring.rs | 63 ++++++++++++++++++- 1 file changed, 61 insertions(+), 2 deletions(-) diff --git a/plugins/tauri-plugin-spindle-project/src/build/authoring.rs b/plugins/tauri-plugin-spindle-project/src/build/authoring.rs index 012ce44..b9ea039 100644 --- a/plugins/tauri-plugin-spindle-project/src/build/authoring.rs +++ b/plugins/tauri-plugin-spindle-project/src/build/authoring.rs @@ -339,9 +339,37 @@ fn dvdauthor_subpicture_language(language: &str) -> Option { return None; } - Language::from_639_1(&normalised) + // FFprobe often surfaces ISO 639-2/B bibliographic codes from container metadata + // such as `fre`, while `isolang` resolves the canonical 639-3 form `fra`. + // Canonicalise the common bibliographic aliases here, then let `isolang` + // handle the real 639-1/639-3 conversion work. + let canonical = match normalised.as_str() { + "alb" => "sqi", + "arm" => "hye", + "baq" => "eus", + "bur" => "mya", + "chi" => "zho", + "cze" => "ces", + "dut" => "nld", + "fre" => "fra", + "geo" => "kat", + "ger" => "deu", + "gre" => "ell", + "ice" => "isl", + "mac" => "mkd", + "mao" => "mri", + "may" => "msa", + "per" => "fas", + "rum" => "ron", + "slo" => "slk", + "tib" => "bod", + "wel" => "cym", + _ => normalised.as_str(), + }; + + Language::from_639_1(canonical) .and_then(|lang| lang.to_639_1()) - .or_else(|| Language::from_639_3(&normalised).and_then(|lang| lang.to_639_1())) + .or_else(|| Language::from_639_3(canonical).and_then(|lang| lang.to_639_1())) .map(str::to_string) } #[cfg(test)] @@ -469,6 +497,37 @@ mod tests { assert!(plan.dvdauthor_xml.contains("")); } + #[test] + fn dvdauthor_xml_normalises_bibliographic_french_language_code() { + let mut project = test_project(); + project.assets[0].subtitle_streams.push(SubtitleStreamInfo { + index: 2, + codec: "dvd_subtitle".to_string(), + language: Some("fre".to_string()), + subtitle_type: SubtitleType::Bitmap, + title: None, + }); + project.disc.titlesets[0].titles[0] + .subtitle_mappings + .push(SubtitleTrackMapping { + id: "sm-1".to_string(), + source_stream_index: 2, + label: "French".to_string(), + language: "fre".to_string(), + order_index: 0, + is_default: false, + is_forced: false, + }); + + let plan = generate_build_plan(&project, "/tmp/dvd_output", false).unwrap(); + + assert!( + plan.dvdauthor_xml.contains(""), + "expected bibliographic French code to normalise to fr\n{}", + plan.dvdauthor_xml + ); + } + #[test] fn vmgm_menu_button_to_same_domain_menu_uses_jump_menu() { let mut project = test_project(); From 5f65ac620e9292bb1a7346046344bbc1d1e768b9 Mon Sep 17 00:00:00 2001 From: Scott Morris Date: Sat, 4 Apr 2026 13:20:30 -0400 Subject: [PATCH 8/8] fix: allow text subtitle builds without fontconfig **What changed** - stop failing build planning when Fontconfig cannot resolve a host subtitle font - fall back to a generic `sans-serif` font-family hint for `spumux` when `fc-match` is unavailable - update the validation warning so it reflects the fallback behaviour more accurately **Why** - `spumux` can still attempt generic family or path-based font resolution even when Fontconfig is not present - blocking planning on `fc-match` availability was stricter than the authoring toolchain itself and could reject otherwise valid text subtitle builds --- .../src/build/planner.rs | 12 +++--------- plugins/tauri-plugin-spindle-project/src/desktop.rs | 2 +- .../tauri-plugin-spindle-project/src/toolchain.rs | 9 +++++++++ 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/plugins/tauri-plugin-spindle-project/src/build/planner.rs b/plugins/tauri-plugin-spindle-project/src/build/planner.rs index aebe5f1..d06ec49 100644 --- a/plugins/tauri-plugin-spindle-project/src/build/planner.rs +++ b/plugins/tauri-plugin-spindle-project/src/build/planner.rs @@ -249,12 +249,6 @@ pub fn generate_build_plan_with_options( }) .collect(); let has_text_subtitles = !text_subtitle_mappings.is_empty(); - if has_text_subtitles && subtitle_font_family.is_none() { - return Err(crate::Error::Build(format!( - "Title \"{}\" has text subtitle mappings, but no host sans-serif font could be resolved with Fontconfig. Enable `skip unsupported streams` to author without text subtitles, or install a compatible font such as Noto Sans or Liberation Sans.", - title.name - ))); - } let title_paths = paths.title_paths(&title.id); if !has_text_subtitles { @@ -314,9 +308,9 @@ pub fn generate_build_plan_with_options( aspect: AspectMode::SixteenByNine, }); let mut current_input = output_path; - let font_family = subtitle_font_family - .clone() - .expect("text subtitle titles should have a resolved host font"); + let font_family = subtitle_font_family.clone().unwrap_or_else(|| { + crate::toolchain::default_text_subtitle_font_family().to_string() + }); for (text_job_index, (stream_index, sm)) in text_subtitle_mappings.iter().enumerate() diff --git a/plugins/tauri-plugin-spindle-project/src/desktop.rs b/plugins/tauri-plugin-spindle-project/src/desktop.rs index a5b8789..acc9d4f 100644 --- a/plugins/tauri-plugin-spindle-project/src/desktop.rs +++ b/plugins/tauri-plugin-spindle-project/src/desktop.rs @@ -315,7 +315,7 @@ impl SpindleProject { context: Some(title.id.clone()), entity_type: Some("title".to_string()), entity_name: Some(title.name.clone()), - suggested_fix: Some("Install a Fontconfig-visible sans-serif font such as Noto Sans or Liberation Sans, or enable the developer option to skip unsupported streams for a subtitle-free build.".to_string()), + suggested_fix: Some("Spindle will fall back to a generic sans-serif font hint, but installing a Fontconfig-visible font such as Noto Sans or Liberation Sans gives more predictable subtitle rendering.".to_string()), }); } } diff --git a/plugins/tauri-plugin-spindle-project/src/toolchain.rs b/plugins/tauri-plugin-spindle-project/src/toolchain.rs index 3a99ef5..7b65e23 100644 --- a/plugins/tauri-plugin-spindle-project/src/toolchain.rs +++ b/plugins/tauri-plugin-spindle-project/src/toolchain.rs @@ -58,6 +58,15 @@ pub fn resolve_text_subtitle_font() -> Option { None } +/// Default font-family hint used when Fontconfig is unavailable. +/// +/// `spumux` can still attempt to resolve generic family names or font paths +/// without Fontconfig, so planning should not fail solely because `fc-match` +/// is missing on the host. +pub fn default_text_subtitle_font_family() -> &'static str { + "sans-serif" +} + /// Return the expected sidecar path: same directory as the running executable. fn sidecar_path(name: &str) -> Option { let exe = std::env::current_exe().ok()?;