diff --git a/CLAUDE.md b/CLAUDE.md index 1b967ec..bfa3f73 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -888,6 +888,18 @@ Rust's serde names and Dart's `value` strings are the wire format Dart enum's `outputBitDepth` drives the "your N-bit source will be output as M-bit" warning. +### VapourSynth Frame Cache (issue #107) + +**Neither template sets `core.max_cache_size` on its own.** VapourSynth sizes +the cache from the machine's memory (8192 MB on a 16 GB Mac); both templates +used to pin it to 1024, which starved QTGMC's threads on HD sources (1.9 fps +instead of 14.7 at 1080i Very Slow). The only assignment is the +`{{#MAX_CACHE_SIZE}}` block, emitted when the user ticks **Settings → Output → +VapourSynth Cache → Override max cache size** (`EncodingSettings.maxCacheSizeMb`, +MB, null = off). `EncodingSettings::effective_max_cache_size_mb` treats 0 as +off. `test_162` asserts both scripts. Don't reintroduce a hard-coded value — +SD never reaches the cap, so a too-low one passes every SD test. + ### Temporary Files Directory Scratch files default to the system temp directory; the user can redirect them diff --git a/app/lib/models/encoding_settings.dart b/app/lib/models/encoding_settings.dart index 605ef68..09be932 100644 --- a/app/lib/models/encoding_settings.dart +++ b/app/lib/models/encoding_settings.dart @@ -165,6 +165,11 @@ class EncodingSettings { /// User-supplied VapourSynth, injected after every built-in pass. Same /// footing as customFfmpegArgs, and gated behind advanced mode. final String customVapoursynth; + + /// Override for VapourSynth's frame cache limit (`core.max_cache_size`), in + /// MB. Null leaves VapourSynth on its own default, which it sizes from the + /// machine's memory. + final int? maxCacheSizeMb; final ContainerFormat container; /// Output directory. If null, uses the same directory as the input file. @@ -190,6 +195,7 @@ class EncodingSettings { this.proresQuantMat, this.customFfmpegArgs = '', this.customVapoursynth = '', + this.maxCacheSizeMb, this.container = ContainerFormat.mkv, this.outputDirectory, this.filenamePattern = '{input_filename}_processed', @@ -292,6 +298,8 @@ class EncodingSettings { bool clearProresQuantMat = false, String? customFfmpegArgs, String? customVapoursynth, + int? maxCacheSizeMb, + bool clearMaxCacheSizeMb = false, ContainerFormat? container, String? outputDirectory, bool clearOutputDirectory = false, @@ -322,6 +330,8 @@ class EncodingSettings { clearProresQuantMat ? null : (proresQuantMat ?? this.proresQuantMat), customFfmpegArgs: customFfmpegArgs ?? this.customFfmpegArgs, customVapoursynth: customVapoursynth ?? this.customVapoursynth, + maxCacheSizeMb: + clearMaxCacheSizeMb ? null : (maxCacheSizeMb ?? this.maxCacheSizeMb), container: container ?? this.container, outputDirectory: clearOutputDirectory ? null : (outputDirectory ?? this.outputDirectory), filenamePattern: filenamePattern ?? this.filenamePattern, diff --git a/app/lib/views/settings/settings_dialog.dart b/app/lib/views/settings/settings_dialog.dart index 2c57fa1..0be3639 100644 --- a/app/lib/views/settings/settings_dialog.dart +++ b/app/lib/views/settings/settings_dialog.dart @@ -712,6 +712,10 @@ class _OutputSettingsTabState extends State<_OutputSettingsTab> { late TextEditingController _filenamePatternController; late TextEditingController _customFfmpegArgsController; late TextEditingController _customVapoursynthController; + final TextEditingController _maxCacheSizeController = TextEditingController(); + + /// What the cache override starts at when first ticked, in MB. + static const int _kDefaultMaxCacheSizeMb = 4096; // Intel-Mac VideoToolbox uses a native target-bitrate control (no -q:v mode). final TextEditingController _vtBitrateController = TextEditingController(); @@ -767,6 +771,7 @@ class _OutputSettingsTabState extends State<_OutputSettingsTab> { _filenamePatternController.dispose(); _customFfmpegArgsController.dispose(); _customVapoursynthController.dispose(); + _maxCacheSizeController.dispose(); _vtBitrateController.dispose(); _vtBitrateFocus.dispose(); super.dispose(); @@ -788,6 +793,13 @@ class _OutputSettingsTabState extends State<_OutputSettingsTab> { if (_customVapoursynthController.text != settings.customVapoursynth) { _customVapoursynthController.text = settings.customVapoursynth; } + // Compared by value, not text: an empty field mid-edit keeps the last + // valid size and must not be refilled under the cursor. + if (settings.maxCacheSizeMb != null && + int.tryParse(_maxCacheSizeController.text) != + settings.maxCacheSizeMb) { + _maxCacheSizeController.text = '${settings.maxCacheSizeMb}'; + } return ListView( padding: const EdgeInsets.all(16), @@ -1329,12 +1341,76 @@ class _OutputSettingsTabState extends State<_OutputSettingsTab> { ], ), ), + + _buildSection( + context, + title: 'VapourSynth Cache', + child: _buildMaxCacheSize(context, viewModel, settings), + ), ], ); }, ); } + /// The frame cache override: off by default, because VapourSynth sizes its + /// own cache from the machine's memory. + Widget _buildMaxCacheSize( + BuildContext context, MainViewModel viewModel, EncodingSettings settings) { + final hint = Theme.of(context).textTheme.bodySmall?.copyWith( + color: Theme.of(context).colorScheme.onSurface.withValues(alpha: 0.6), + ); + + return Column( + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + CheckboxListTile( + contentPadding: EdgeInsets.zero, + controlAffinity: ListTileControlAffinity.leading, + value: settings.maxCacheSizeMb != null, + title: const Text('Override max cache size'), + subtitle: Text( + 'VapourSynth chooses how much memory to use for cached frames ' + 'based on this computer. Set a limit yourself only to hold memory ' + 'use down, or to raise it if the log reports "memory usage still ' + 'over the limit". Too low a limit makes processing much slower.', + style: hint, + ), + onChanged: (value) => viewModel.updateEncodingSettings( + (value ?? false) + ? settings.copyWith(maxCacheSizeMb: _kDefaultMaxCacheSizeMb) + : settings.copyWith(clearMaxCacheSizeMb: true), + ), + ), + if (settings.maxCacheSizeMb != null) + Padding( + padding: const EdgeInsets.only(left: 32, top: 4), + child: SizedBox( + width: 160, + child: TextField( + controller: _maxCacheSizeController, + keyboardType: TextInputType.number, + inputFormatters: [FilteringTextInputFormatter.digitsOnly], + decoration: const InputDecoration( + border: OutlineInputBorder(), + isDense: true, + suffixText: 'MB', + ), + onChanged: (value) { + final mb = int.tryParse(value); + if (mb != null && mb > 0) { + viewModel.updateEncodingSettings( + settings.copyWith(maxCacheSizeMb: mb), + ); + } + }, + ), + ), + ), + ], + ); + } + /// Warning shown when the chosen output colour format would reduce a /// higher-bit-depth source's precision — the 8-bit formats always do, 4:2:2 /// 10-bit only for a source deeper than 10, and "Match source" never. Returns diff --git a/app/test/integration_filter_parameters_test.dart b/app/test/integration_filter_parameters_test.dart index d314981..da9782a 100644 --- a/app/test/integration_filter_parameters_test.dart +++ b/app/test/integration_filter_parameters_test.dart @@ -809,6 +809,24 @@ void main() { print(' PASS'); }, timeout: const Timeout(Duration(minutes: 2))); + // --- FRAME CACHE (core.max_cache_size, issue #107) --- + test('max cache size: written only when overridden', () async { + final job = buildJob(testName: 'max_cache_default'); + print(' Generating script without the override...'); + final plain = await generateScriptViaWorker(job); + expect(plain, isNot(contains('max_cache_size =')), + reason: 'VapourSynth sizes its own cache unless told otherwise'); + expect(plain, isNot(contains('MAX_CACHE_SIZE'))); + + print(' Generating script with a 6000 MB override...'); + final overridden = await generateScriptViaWorker(job.copyWith( + encodingSettings: + job.encodingSettings.copyWith(maxCacheSizeMb: 6000), + )); + expect(overridden, contains('core.max_cache_size = 6000')); + print(' PASS'); + }, timeout: const Timeout(Duration(minutes: 2))); + // --- DESCRATCH (core.descratch.DeScratch) --- test('descratch: DeScratch params', () async { loadSchema('descratch'); // confirm schema parses diff --git a/app/test/max_cache_size_test.dart b/app/test/max_cache_size_test.dart new file mode 100644 index 0000000..10c79c1 --- /dev/null +++ b/app/test/max_cache_size_test.dart @@ -0,0 +1,38 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:vapourbox/models/encoding_settings.dart'; + +// The frame cache override (issue #107). Off by default: the worker only +// writes `core.max_cache_size` into the script when this is set, and a fixed +// 1 GB cap is what made HD deinterlacing run at ~1 fps. + +void main() { + test('is off by default, leaving the cache to VapourSynth', () { + expect(const EncodingSettings().maxCacheSizeMb, isNull); + expect(const EncodingSettings().toJson()['maxCacheSizeMb'], isNull); + }); + + group('copyWith', () { + const set = EncodingSettings(maxCacheSizeMb: 6000); + + test('carries the override through an unrelated edit', () { + expect(set.copyWith(audioMode: AudioMode.none).maxCacheSizeMb, 6000); + }); + + test('can clear it, so unticking the box really turns it off', () { + expect(set.copyWith(clearMaxCacheSizeMb: true).maxCacheSizeMb, isNull); + }); + }); + + group('round trip', () { + test('survives JSON under the name the worker reads', () { + final json = const EncodingSettings(maxCacheSizeMb: 6000).toJson(); + expect(json['maxCacheSizeMb'], 6000); + expect(EncodingSettings.fromJson(json).maxCacheSizeMb, 6000); + }); + + test('an older preset without the field still loads', () { + final json = const EncodingSettings().toJson()..remove('maxCacheSizeMb'); + expect(EncodingSettings.fromJson(json).maxCacheSizeMb, isNull); + }); + }); +} diff --git a/docs/ENGINEERING_NOTES.md b/docs/ENGINEERING_NOTES.md index b4ebe6e..6cec031 100644 --- a/docs/ENGINEERING_NOTES.md +++ b/docs/ENGINEERING_NOTES.md @@ -2157,6 +2157,38 @@ case-for-case to it. Depth is deliberately not warned about alone: ProRes is always 10-bit, so that would fire on most ProRes jobs and become wallpaper. --- +### The 1 GB frame cache cap (issue #107, 2026-10-05) + +Both templates set `core.max_cache_size = 1024` from 2026-01-18, on the premise +that VapourSynth "defaults to minimal caching". It does not: the bundled R78 +reports 8192 MB on a 16 GB Mac, so the line cut the cache to an eighth. A user +deinterlacing a 107-minute HD AVI saw ~1 fps and a log full of `memory usage +still over the limit, flushing pipeline`; overriding the value through Custom +VapourSynth took them to ~14 fps. + +Measured with bare `haf.QTGMC` on a synthetic interlaced 4:2:2 source under the +bundled vspipe (macos-arm64, 8 threads, deps 1.11.0): + +| Source | Preset | Cache (MB) | fps | Peak RSS | +|---|---|---|---|---| +| 1920x1080 | Slower | 1024 | 6.5 | 1.7 GB | +| 1920x1080 | Slower | 2048 | 17.2 | 2.5 GB | +| 1920x1080 | Slower | 4096 | 17.2 | 2.7 GB | +| 1920x1080 | Slower | default (8192) | 18.0 | 2.8 GB | +| 1920x1080 | Very Slow | 1024 | 1.9 | 2.2 GB | +| 1920x1080 | Very Slow | default (8192) | 14.7 | 3.8 GB | +| 720x576 | Slower | 1024 | 92.8 | 0.7 GB | +| 720x576 | Slower | default (8192) | 89.5 | 0.7 GB | + +CPU time was the same with and without the cap (119 s vs 123 s for 1080 Slower) +while wall time went from 47 s to 17 s — threads waiting on evicted frames, not +extra work. SD never reaches 1 GB, which is why no test noticed. The limit is a +ceiling, not an allocation: peak memory stayed well under it. + +The templates now leave the cache alone, and the user can set a limit in +Settings → Output. Not measured: the default VapourSynth picks on Windows and +Linux, or on an 8 GB machine. + ### Testing a deps change before publishing A PR that changes `deps/` has a chicken-and-egg problem: `ci-test.yml` and diff --git a/worker/src/models/video_job.rs b/worker/src/models/video_job.rs index 05763bc..c4f4b42 100644 --- a/worker/src/models/video_job.rs +++ b/worker/src/models/video_job.rs @@ -279,6 +279,12 @@ pub struct EncodingSettings { #[serde(default)] pub custom_vapoursynth: String, + /// Override for VapourSynth's frame cache limit (`core.max_cache_size`), + /// in MB. `None` leaves VapourSynth on its own default, which it sizes + /// from the machine's memory — the right answer almost always. + #[serde(default)] + pub max_cache_size_mb: Option, + /// Output container format #[serde(default)] pub container: ContainerFormat, @@ -576,6 +582,15 @@ impl ChromaSubsampling { } } +impl EncodingSettings { + /// The cache override to write into the script, if any. Zero is not a + /// usable limit — VapourSynth would flush on every frame — so it means + /// "no override", the same as leaving it unset. + pub fn effective_max_cache_size_mb(&self) -> Option { + self.max_cache_size_mb.filter(|mb| *mb > 0) + } +} + impl Default for EncodingSettings { fn default() -> Self { Self { @@ -588,6 +603,7 @@ impl Default for EncodingSettings { chroma_subsampling: ChromaSubsampling::default(), custom_ffmpeg_args: String::new(), custom_vapoursynth: String::new(), + max_cache_size_mb: None, container: ContainerFormat::default(), video_bitrate_kbps: None, prores_vendor_apl0: false, diff --git a/worker/src/script_generator.rs b/worker/src/script_generator.rs index 99a3540..2ff2a0b 100644 --- a/worker/src/script_generator.rs +++ b/worker/src/script_generator.rs @@ -1671,6 +1671,20 @@ impl ScriptGenerator { script = remove_block("{{#CHROMA_FIXES}}", "{{/CHROMA_FIXES}}", script); } + // ==================================================================== + // FRAME CACHE OVERRIDE + // ==================================================================== + match job.encoding_settings.effective_max_cache_size_mb() { + Some(mb) => { + script = script.replace("{{#MAX_CACHE_SIZE}}", ""); + script = script.replace("{{/MAX_CACHE_SIZE}}", ""); + script = script.replace("{{MAX_CACHE_SIZE}}", &mb.to_string()); + } + None => { + script = remove_block("{{#MAX_CACHE_SIZE}}", "{{/MAX_CACHE_SIZE}}", script); + } + } + // ==================================================================== // CUSTOM VAPOURSYNTH // ==================================================================== diff --git a/worker/templates/pipeline_template.vpy b/worker/templates/pipeline_template.vpy index 06c084a..568bb99 100644 --- a/worker/templates/pipeline_template.vpy +++ b/worker/templates/pipeline_template.vpy @@ -11,9 +11,12 @@ import sys core = vs.core -# Configure cache size for optimal performance with temporal filters -# 1GB default, can be adjusted based on system memory -core.max_cache_size = 1024 +# VapourSynth sizes its own frame cache from the machine's memory. Only an +# explicit override (Settings -> Output) replaces it: a fixed cap starved +# QTGMC's threads on HD sources (issue #107). +{{#MAX_CACHE_SIZE}} +core.max_cache_size = {{MAX_CACHE_SIZE}} +{{/MAX_CACHE_SIZE}} # Load input video from stdin pipe (FFmpeg decodes → raw frames → VapourSynth) # This eliminates FFMS2 indexing which blocks on large/NAS files. diff --git a/worker/templates/preview_template.vpy b/worker/templates/preview_template.vpy index 73be6d7..fc56be3 100644 --- a/worker/templates/preview_template.vpy +++ b/worker/templates/preview_template.vpy @@ -11,8 +11,12 @@ import os core = vs.core -# Configure cache size for optimal performance with temporal filters -core.max_cache_size = 1024 +# VapourSynth sizes its own frame cache from the machine's memory. Only an +# explicit override (Settings -> Output) replaces it: a fixed cap starved +# QTGMC's threads on HD sources (issue #107). +{{#MAX_CACHE_SIZE}} +core.max_cache_size = {{MAX_CACHE_SIZE}} +{{/MAX_CACHE_SIZE}} # Load raw frames piped from FFmpeg via stdin sys.path.insert(0, r"{{PIPE_SOURCE_DIR}}") diff --git a/worker/tests/filter_integration_test.rs b/worker/tests/filter_integration_test.rs index d418b3a..2a6186e 100644 --- a/worker/tests/filter_integration_test.rs +++ b/worker/tests/filter_integration_test.rs @@ -5968,3 +5968,49 @@ fn test_161_each_deinterlace_method_emits_only_its_own_block() { } } } + +/// The script leaves VapourSynth's frame cache alone unless the user overrides it. +/// +/// Both templates used to pin it to 1 GB, an eighth of what VapourSynth picks +/// for itself on a 16 GB machine, which starved QTGMC on HD sources (issue +/// #107: ~1 fps instead of ~14). Asserted on the encode and preview scripts, +/// since they are separate templates. +#[test] +fn test_162_max_cache_size_is_only_set_when_overridden() { + create_output_dir(); + let scripts = |job: &VideoJob| { + let generator = ScriptGenerator::new().expect("Failed to create generator"); + let params = PreviewParams { + width: 720, + height: 480, + pix_fmt: "yuv420p".to_string(), + num_frames: 15, + fps_num: 30000, + fps_den: 1001, + output_index: 7, + }; + let preview = generator.generate_preview(job, ¶ms).expect("preview script"); + [ + script_text(job), + std::fs::read_to_string(&preview).expect("read preview script"), + ] + }; + + let mut job = create_base_job("test_162_max_cache_default"); + for script in scripts(&job) { + assert!(!script.contains("max_cache_size ="), "no override, no assignment"); + assert!(!script.contains("MAX_CACHE_SIZE"), "placeholder left behind"); + } + + job.encoding_settings.max_cache_size_mb = Some(6000); + for script in scripts(&job) { + assert!(script.contains("core.max_cache_size = 6000")); + assert!(!script.contains("MAX_CACHE_SIZE"), "placeholder left behind"); + } + + // Zero would make VapourSynth flush constantly; it means "no override". + job.encoding_settings.max_cache_size_mb = Some(0); + for script in scripts(&job) { + assert!(!script.contains("max_cache_size =")); + } +}