diff --git a/CLAUDE.md b/CLAUDE.md index 8b69393..3e9ef3d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -466,6 +466,34 @@ Presets save complete filter pipeline + encoding settings. Built-in presets (Fas --- +## Hardware Encoders (issue #51) + +Three invariants that are easy to break and hard to notice, because a wrong +value here shows up as "this GPU doesn't work" rather than as an error: + +- **The availability probe must use a large enough frame.** Hardware encoders + have minimum dimensions, and below them they fail — so a too-small probe + reports a *working* encoder as broken. AMD AMF rejects 64x64, which is exactly + what the probe used to send. `HardwareEncoderDetector.probeFrameSize` is now + 512 (AMF's floor is 192x128 on some ASICs); `hardware_encoder_probe_test.dart` + keeps it there. +- **Presets are per-family vocabularies and must not cross over.** x264 takes + `ultrafast…placebo`, NVENC `p1…p7`, QSV `veryfast…veryslow`, AMF + `speed|balanced|quality`. A saved preset or imported job config can pair a + codec with the wrong family's preset; ffmpeg then rejects the option and the + whole encode dies. `VideoCodec::normalized_preset` (Rust) substitutes the + family default — the Rust `available_presets`/`default_preset` must stay in + step with `availablePresets`/`defaultPreset` in `app/lib/models/video_job.dart`. +- **AMF is pinned to `nv12`.** Left to negotiate, a >8-bit source reaches the + encoder as p010; 10-bit HEVC encode exists only on some AMD ASICs, and where + it doesn't the AMF runtime faults (0xC0000005) instead of failing cleanly. + h264_amf has no 10-bit mode at all. Custom FFmpeg Arguments still override it + (a later `-pix_fmt` wins), which is the escape hatch for 10-bit HEVC on + hardware that supports it. + +None of the AMF behaviour can be verified in CI or on macOS — there is no AMD +hardware in the matrix — so changes here rest on reporter confirmation. + ## QTGMC Parameters Reference The most important parameters: diff --git a/app/lib/services/hardware_encoder_detector.dart b/app/lib/services/hardware_encoder_detector.dart index 9a898ca..1cc277e 100644 --- a/app/lib/services/hardware_encoder_detector.dart +++ b/app/lib/services/hardware_encoder_detector.dart @@ -125,14 +125,28 @@ class HardwareEncoderDetector extends ChangeNotifier { } } + /// Frame size for the functional probe. + /// + /// Hardware encoders have minimum dimensions and a tiny frame makes them + /// report a *false* failure: AMD AMF rejects 64x64, which had VapourBox + /// marking working AMF encoders as broken (issue #51). AMF's minimum is + /// 192x128 on some ASICs, and NVENC/QSV have their own floors, so probe at a + /// size comfortably above all of them. One frame, so the cost is unchanged. + static const int probeFrameSize = 512; + + /// The ffmpeg arguments used to functionally probe [codecValue]. Exposed for + /// tests so the frame-size guarantee above can't silently regress. + static List probeArgs(String codecValue) => [ + '-hide_banner', '-loglevel', 'error', + '-f', 'lavfi', + '-i', 'color=c=black:s=${probeFrameSize}x$probeFrameSize:r=5:d=1', + '-frames:v', '1', + '-c:v', codecValue, + '-f', 'null', '-', + ]; + Future _probe(String ffmpegPath, VideoCodec codec) async { - final args = [ - '-hide_banner', '-loglevel', 'error', - '-f', 'lavfi', '-i', 'color=c=black:s=64x64:r=5:d=1', - '-frames:v', '1', - '-c:v', codec.value, - '-f', 'null', '-', - ]; + final args = probeArgs(codec.value); var ok = false; String? errorLog; try { diff --git a/app/test/hardware_encoder_probe_test.dart b/app/test/hardware_encoder_probe_test.dart new file mode 100644 index 0000000..6fc20d7 --- /dev/null +++ b/app/test/hardware_encoder_probe_test.dart @@ -0,0 +1,37 @@ +// Guards the hardware-encoder functional probe against regressing to a frame +// size that hardware encoders reject (issue #51: AMD AMF fails at 64x64, so +// working AMF encoders were reported as broken). +// +// Run with: flutter test test/hardware_encoder_probe_test.dart + +import 'package:flutter_test/flutter_test.dart'; +import 'package:vapourbox/services/hardware_encoder_detector.dart'; + +void main() { + /// The largest minimum-dimension any bundled hardware encoder imposes. AMF is + /// the strictest at 192x128 on some ASICs; the probe must clear it with room + /// to spare on both axes. + const int strictestEncoderMinimum = 192; + + test('probes at a frame size every hardware encoder accepts', () { + expect( + HardwareEncoderDetector.probeFrameSize, + greaterThan(strictestEncoderMinimum), + reason: 'a frame below the encoder minimum makes a working encoder ' + 'report failure (issue #51)', + ); + // Odd dimensions are their own source of encoder rejections. + expect(HardwareEncoderDetector.probeFrameSize.isEven, isTrue); + }); + + test('probe args encode one frame with the requested codec', () { + final args = HardwareEncoderDetector.probeArgs('hevc_amf'); + + final size = HardwareEncoderDetector.probeFrameSize; + expect(args, containsAllInOrder(['-i', 'color=c=black:s=${size}x$size:r=5:d=1'])); + expect(args, containsAllInOrder(['-c:v', 'hevc_amf'])); + expect(args, containsAllInOrder(['-frames:v', '1'])); + // Discard the output — the probe only cares whether the encoder opens. + expect(args, containsAllInOrder(['-f', 'null', '-'])); + }); +} diff --git a/worker/src/models/video_job.rs b/worker/src/models/video_job.rs index f2bc9fa..50f0e55 100644 --- a/worker/src/models/video_job.rs +++ b/worker/src/models/video_job.rs @@ -456,10 +456,62 @@ impl VideoCodec { pub fn forced_pix_fmt(&self) -> Option<&'static str> { match self { VideoCodec::Huffyuv => Some("yuv422p"), + // AMF takes nv12 natively. Left to negotiate, ffmpeg will hand a + // >8-bit source to the encoder as p010, and 10-bit HEVC encode is + // only supported on some AMD ASICs — where it isn't, the AMF + // runtime faults (0xC0000005) instead of failing cleanly, which is + // one candidate for the crashes in issue #51. h264_amf has no + // 10-bit mode at all. Pinning nv12 makes the conversion explicit + // and identical on every card. Custom FFmpeg Arguments still wins: + // a later -pix_fmt overrides this one. + VideoCodec::H264Amf | VideoCodec::H265Amf => Some("nv12"), _ => None, } } + /// Encoder presets this codec accepts. Mirrors `availablePresets` in + /// `app/lib/models/video_job.dart` — keep the two in step. + pub fn available_presets(&self) -> &'static [&'static str] { + match self.encoder_family() { + EncoderFamily::Software => &[ + "ultrafast", "superfast", "veryfast", "faster", "fast", "medium", + "slow", "slower", "veryslow", "placebo", + ], + EncoderFamily::Nvenc => &["p1", "p2", "p3", "p4", "p5", "p6", "p7"], + EncoderFamily::Qsv => &[ + "veryfast", "faster", "fast", "medium", "slow", "slower", "veryslow", + ], + EncoderFamily::Amf => &["speed", "balanced", "quality"], + // VideoToolbox, ProRes and the lossless codecs take no preset. + _ => &[], + } + } + + /// The preset to fall back on when the configured one isn't accepted. + pub fn default_preset(&self) -> &'static str { + match self.encoder_family() { + EncoderFamily::Nvenc => "p4", + EncoderFamily::Amf => "balanced", + _ => "medium", + } + } + + /// The configured preset if this encoder accepts it, else its default. + /// + /// Preset vocabularies are per-family and don't overlap — x264's "medium" + /// means nothing to AMF, which wants speed/balanced/quality. A saved preset + /// or an imported job config can pair a codec with another family's preset, + /// and passing that through makes ffmpeg reject the option and fail the + /// whole encode. Falling back keeps the job running with a sane setting. + pub fn normalized_preset(&self, configured: &str) -> String { + let allowed = self.available_presets(); + if allowed.is_empty() || allowed.contains(&configured) { + configured.to_string() + } else { + self.default_preset().to_string() + } + } + /// Whether this codec produces H.264 output. pub fn is_h264(&self) -> bool { matches!(self, VideoCodec::H264 | VideoCodec::H264Nvenc | VideoCodec::H264Qsv | diff --git a/worker/src/pipeline_executor.rs b/worker/src/pipeline_executor.rs index 7c60a51..416fae6 100644 --- a/worker/src/pipeline_executor.rs +++ b/worker/src/pipeline_executor.rs @@ -810,6 +810,9 @@ impl PipelineExecutor { /// Build encoder-family-specific quality and preset arguments. fn build_encoder_quality_args(args: &mut Vec, job: &VideoJob) { let settings = &job.encoding_settings; + // Never pass another family's preset vocabulary to the encoder — ffmpeg + // rejects the option and the encode dies. See `normalized_preset`. + let preset = settings.codec.normalized_preset(&settings.encoder_preset); if let Some(profile) = settings.codec.prores_profile() { args.push("-profile:v".to_string()); args.push(profile.to_string()); @@ -817,7 +820,7 @@ impl PipelineExecutor { match settings.codec.encoder_family() { EncoderFamily::Software => { args.extend(["-crf".to_string(), settings.quality.to_string()]); - args.extend(["-preset".to_string(), settings.encoder_preset.clone()]); + args.extend(["-preset".to_string(), preset.clone()]); } EncoderFamily::Nvenc => { // VBR with constant quality (-cq) provides much better quality @@ -826,7 +829,7 @@ impl PipelineExecutor { args.extend(["-rc".to_string(), "vbr".to_string()]); args.extend(["-cq".to_string(), settings.quality.to_string()]); args.extend(["-b:v".to_string(), "0".to_string()]); - args.extend(["-preset".to_string(), settings.encoder_preset.clone()]); + args.extend(["-preset".to_string(), preset.clone()]); args.extend(["-tune".to_string(), "hq".to_string()]); args.extend(["-multipass".to_string(), "fullres".to_string()]); args.extend(["-rc-lookahead".to_string(), "32".to_string()]); @@ -840,7 +843,7 @@ impl PipelineExecutor { } EncoderFamily::Qsv => { args.extend(["-global_quality".to_string(), settings.quality.to_string()]); - args.extend(["-preset".to_string(), settings.encoder_preset.clone()]); + args.extend(["-preset".to_string(), preset.clone()]); } EncoderFamily::Videotoolbox => { // VideoToolbox's constant-quality mode (-q:v) is only @@ -872,7 +875,12 @@ impl PipelineExecutor { args.extend(["-rc".to_string(), "cqp".to_string()]); args.extend(["-qp_i".to_string(), settings.quality.to_string()]); args.extend(["-qp_p".to_string(), settings.quality.to_string()]); - args.extend(["-quality".to_string(), settings.encoder_preset.clone()]); + // B-frames were left on the encoder's default QP while I and + // P were pinned, so a GOP could mix a chosen quality with an + // unrelated one. AMF ignores the option on ASICs without + // B-frame support, so setting it is safe either way. + args.extend(["-qp_b".to_string(), settings.quality.to_string()]); + args.extend(["-quality".to_string(), preset.clone()]); } EncoderFamily::Lossless | EncoderFamily::ProRes => { // No quality/preset args needed @@ -1707,10 +1715,76 @@ mod tests { assert!(qp_p_idx.is_some(), "AMF should use -qp_p"); assert_eq!(args[qp_p_idx.unwrap() + 1], "20"); + let qp_b_idx = args.iter().position(|a| a == "-qp_b"); + assert!(qp_b_idx.is_some(), "AMF should pin -qp_b too (issue #51)"); + assert_eq!(args[qp_b_idx.unwrap() + 1], "20"); + let quality_idx = args.iter().position(|a| a == "-quality"); assert!(quality_idx.is_some(), "AMF should use -quality"); assert_eq!(args[quality_idx.unwrap() + 1], "balanced"); assert!(!args.contains(&"-crf".to_string()), "AMF should not use -crf"); } + + /// Issue #51: left to negotiate, a >8-bit source reaches AMF as p010 and + /// 10-bit HEVC encode only exists on some AMD ASICs — elsewhere the runtime + /// faults rather than failing cleanly. Pin the format both encoders always + /// support. + #[test] + fn test_ffmpeg_args_amf_forces_nv12() { + for codec in [VideoCodec::H264Amf, VideoCodec::H265Amf] { + let mut job = create_test_job("output.mp4"); + job.encoding_settings.codec = codec; + + let args = build_ffmpeg_args_for_test(&job); + + let idx = args.iter().position(|a| a == "-pix_fmt"); + assert!(idx.is_some(), "{:?} should force a pixel format", codec); + assert_eq!(args[idx.unwrap() + 1], "nv12", "codec {:?}", codec); + } + } + + /// A preset from another encoder family (a saved preset carrying x264's + /// "medium", say) must not reach ffmpeg — it rejects the option and the + /// whole encode fails. Each family falls back to its own default instead. + #[test] + fn test_ffmpeg_args_foreign_preset_falls_back() { + let cases = [ + (VideoCodec::H264Amf, "-quality", "balanced"), + (VideoCodec::H264Nvenc, "-preset", "p4"), + (VideoCodec::H264Qsv, "-preset", "medium"), + ]; + + for (codec, flag, expected) in cases { + let mut job = create_test_job("output.mp4"); + job.encoding_settings.codec = codec; + // "placebo" is x264-only — meaningless to every hardware family. + job.encoding_settings.encoder_preset = "placebo".to_string(); + + let args = build_ffmpeg_args_for_test(&job); + let idx = args.iter().position(|a| a == flag); + assert!(idx.is_some(), "{:?} should pass {}", codec, flag); + assert_eq!(args[idx.unwrap() + 1], expected, "codec {:?}", codec); + } + } + + /// A preset the encoder *does* accept passes through untouched. + #[test] + fn test_ffmpeg_args_valid_preset_preserved() { + let mut job = create_test_job("output.mp4"); + job.encoding_settings.codec = VideoCodec::H265Amf; + job.encoding_settings.encoder_preset = "speed".to_string(); + + let args = build_ffmpeg_args_for_test(&job); + let idx = args.iter().position(|a| a == "-quality").unwrap(); + assert_eq!(args[idx + 1], "speed"); + + // Software x264 keeps its own vocabulary. + let mut sw = create_test_job("output.mp4"); + sw.encoding_settings.codec = VideoCodec::H264; + sw.encoding_settings.encoder_preset = "veryslow".to_string(); + let sw_args = build_ffmpeg_args_for_test(&sw); + let sw_idx = sw_args.iter().position(|a| a == "-preset").unwrap(); + assert_eq!(sw_args[sw_idx + 1], "veryslow"); + } }