From 6254b5610187a1152451545efdfa20287f4ccf12 Mon Sep 17 00:00:00 2001 From: Stuart Cameron Date: Wed, 19 Aug 2026 10:43:17 +1000 Subject: [PATCH 1/2] fix(worker): NVENC and QSV cannot encode 4:2:2, so stop letting ffmpeg pick MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A 4:2:2 source killed every NVENC job on pre-Blackwell hardware (#74): zero frames, "YUV422P not supported / No capable devices found". Reported with a CineForm yuv422p10le capture on an RTX 4070 Super. An encoder's declared pix_fmt list is compiled into ffmpeg, but NVENC's real capabilities are queried from the driver at avcodec_open2. A recent ffmpeg built against NVENC SDK 13 advertises yuv422p on h264_nvenc for Blackwell's 4:2:2 support, so negotiation picks it for a 4:2:2 source and every earlier card rejects it. ffmpeg cannot negotiate its way out of this, because the list it negotiates against is wrong for the hardware in the machine. forced_pix_fmt therefore takes the format the pipeline will actually hand the encoder (VideoJob::encoder_input_pix_fmt: the output conversion when one is selected, the pipe format otherwise) and picks an encodable one. The NVENC/QSV arm is deliberately conditional where HuffYUV's and AMF's are not. Pinning NVENC unconditionally the way AMF is pinned would flatten a 10-bit 4:2:0 source to 8-bit for everyone it already serves correctly, so it fires only on 4:2:2, 4:4:4, or >8-bit into an H.264 encoder (neither family has a 10-bit H.264 mode). A 4:2:0 job emits no -pix_fmt at all, as before. HEVC keeps the depth via p010le; only the chroma has to go. QSV is included preventively rather than on a report — hevc_qsv advertises y210le on builds whose hardware may not have it. VideoToolbox is excluded: it never advertises a mode it lacks, so forcing a format would only discard chroma it could have kept. The substitution is logged, since it changes the output and should not also be invisible. Custom FFmpeg Arguments still win, which is the escape hatch for a card that really does have the mode. Neither encoder exists in CI or on macOS, so this rests on unit tests over the emitted arguments. Note the functional probe cannot catch this class at all: it encodes one yuv420p frame, so it correctly reports NVENC as available — the device works, only the format doesn't. --- worker/src/models/video_job.rs | 248 ++++++++++++++++++++++++++++++-- worker/src/pipeline_executor.rs | 127 +++++++++++++++- worker/src/pixel_format.rs | 12 +- 3 files changed, 372 insertions(+), 15 deletions(-) diff --git a/worker/src/models/video_job.rs b/worker/src/models/video_job.rs index 668950d..036c97a 100644 --- a/worker/src/models/video_job.rs +++ b/worker/src/models/video_job.rs @@ -103,6 +103,28 @@ impl VideoJob { ) } + /// The pixel format the encoder ffmpeg is handed on stdin. + /// + /// The output conversion is the last thing the `.vpy` does, so when one is + /// selected it decides the format outright. With `Original` nothing + /// converts and the pipe carries the format the decoder was asked for — + /// which is the *pipe* format, not necessarily the source's own (see + /// `pixel_format`, where an unreadable source is normalised on the way in). + /// + /// A pass that changes format mid-graph always restores it (Turn90, + /// DeScratch, LUTDeCrawl all convert back), so the graph's output format is + /// its input format. Custom VapourSynth could break that assumption, and + /// like everything else about custom code, it is the user's to get right. + pub fn encoder_input_pix_fmt(&self) -> String { + self.encoding_settings + .chroma_subsampling + .ffmpeg_pix_fmt() + .map(str::to_string) + .unwrap_or_else(|| { + crate::pixel_format::decode_pixel_format(self.input_pixel_format.as_deref()).name + }) + } + /// Get the effective processing pipeline. /// Uses processing_pipeline if set, otherwise creates one from legacy qtgmc_parameters. pub fn effective_pipeline(&self) -> ProcessingPipeline { @@ -414,6 +436,10 @@ pub enum ChromaSubsampling { Original, /// Convert to 8-bit YUV420 for maximum compatibility. Yuv420, + /// Convert to 10-bit YUV420 — the only 10-bit layout NVENC, QSV and AMF can + /// encode, so it keeps a 10-bit source's grading where 4:2:2 fails outright + /// on most GPUs (issue #74). + Yuv420P10, /// Convert to 8-bit YUV422 for higher chroma quality. Yuv422, /// Convert to 10-bit YUV422 — keeps a 10-bit source's precision while @@ -428,10 +454,29 @@ impl ChromaSubsampling { match self { ChromaSubsampling::Original => None, ChromaSubsampling::Yuv420 => Some("vs.YUV420P8"), + ChromaSubsampling::Yuv420P10 => Some("vs.YUV420P10"), ChromaSubsampling::Yuv422 => Some("vs.YUV422P8"), ChromaSubsampling::Yuv422P10 => Some("vs.YUV422P10"), } } + + /// The same format as an FFmpeg pixel-format name — what actually comes out + /// of the Y4M pipe once the conversion above has run. `None` for `Original`, + /// where the format is the source's and only the pipe knows it. + /// + /// Keep in step with [`vapoursynth_format`](Self::vapoursynth_format): the + /// two describe the same conversion, and + /// `chroma_subsampling_names_agree` fails if a variant gains one and not + /// the other. + pub fn ffmpeg_pix_fmt(&self) -> Option<&'static str> { + match self { + ChromaSubsampling::Original => None, + ChromaSubsampling::Yuv420 => Some("yuv420p"), + ChromaSubsampling::Yuv420P10 => Some("yuv420p10le"), + ChromaSubsampling::Yuv422 => Some("yuv422p"), + ChromaSubsampling::Yuv422P10 => Some("yuv422p10le"), + } + } } impl Default for EncodingSettings { @@ -577,23 +622,82 @@ impl VideoCodec { matches!(self.encoder_family(), EncoderFamily::Lossless) } - /// FFmpeg output pixel format to force for this codec, if it cannot accept - /// the pipeline's native format. Classic HuffYUV only supports yuv422p (not - /// yuv420p), so force conversion; ffvhuff and the others accept yuv420p. - pub fn forced_pix_fmt(&self) -> Option<&'static str> { + /// FFmpeg output pixel format to force for this codec, given the format the + /// pipeline will actually hand it (`encoder_input`, from + /// [`VideoJob::encoder_input_pix_fmt`]). `None` leaves the choice to + /// ffmpeg's own negotiation. + /// + /// Two of these are unconditional, because the encoder takes one format + /// whatever the source was. The NVENC/QSV arm is not: it must only fire + /// when the pipeline's format is genuinely unencodable, or it would drag a + /// perfectly good 10-bit 4:2:0 source down to 8-bit for no reason. + /// + /// **Do not assume ffmpeg's negotiation handles this.** An encoder's + /// declared pix_fmt list is static, but NVENC's real capabilities are + /// queried from the driver at `avcodec_open2`. A recent ffmpeg built + /// against NVENC SDK 13 advertises `yuv422p` on `h264_nvenc` for + /// Blackwell's 4:2:2 support, so negotiation happily picks it for a 4:2:2 + /// source — and every pre-Blackwell card then fails the job outright with + /// "YUV422P not supported / No capable devices found" (issue #74, on an + /// RTX 4070 Super). Negotiation cannot avoid this, because the list it + /// negotiates against is wrong for the hardware in the machine. + /// + /// Custom FFmpeg Arguments still wins in every case: they are appended + /// last, so a later `-pix_fmt` overrides this one. That is the escape hatch + /// for someone whose card really does have the mode we refuse to assume. + pub fn forced_pix_fmt(&self, encoder_input: &str) -> Option<&'static str> { match self { - VideoCodec::Huffyuv => Some("yuv422p"), + // Classic HuffYUV only supports yuv422p (not yuv420p); ffvhuff and + // the others accept yuv420p. + VideoCodec::Huffyuv => return 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, + // and identical on every card. + VideoCodec::H264Amf | VideoCodec::H265Amf => return Some("nv12"), + _ => {} + } + + let family = self.encoder_family(); + if !matches!(family, EncoderFamily::Nvenc | EncoderFamily::Qsv) { + return None; } + + let (class, depth) = crate::pixel_format::chroma_and_depth(encoder_input); + + // 4:2:0 is the only chroma layout every NVENC and QSV part encodes. + // 4:2:2 is Blackwell-only on NVENC and needs a recent VDENC on QSV; + // 4:4:4 needs a profile this pipeline never selects (`build_encoder_ + // quality_args` pins h264_nvenc to `-profile:v high`, which is 4:2:0). + let chroma_unsupported = class != crate::pixel_format::ChromaClass::C420; + // Neither family has a 10-bit H.264 mode at all. + let depth_unsupported = depth > 8 && self.is_h264(); + + if !chroma_unsupported && !depth_unsupported { + return None; + } + + // NVENC names planar 4:2:0 `yuv420p`; QSV's native format is the + // semi-planar `nv12`. Both are 8-bit 4:2:0 and either encoder accepts + // either, but each family's own name is the one that avoids a + // needless swscale hop. + let planar = matches!(family, EncoderFamily::Nvenc); + Some(if self.is_h264() { + // H.264: 8-bit 4:2:0 is the only option on either family. + if planar { "yuv420p" } else { "nv12" } + } else if depth > 8 { + // HEVC: drop the chroma, but keep the source's precision. Every + // NVENC from Pascal on, and every QSV with an HEVC VDENC, takes + // 10-bit 4:2:0. + "p010le" + } else if planar { + "yuv420p" + } else { + "nv12" + }) } /// Encoder presets this codec accepts. Mirrors `availablePresets` in @@ -836,6 +940,132 @@ mod tests { assert!(!VideoCodec::ProResHQ.is_hardware()); } + /// Issue #74: a 4:2:2 source killed every NVENC job on pre-Blackwell + /// hardware, because a recent ffmpeg advertises `yuv422p` on `h264_nvenc` + /// and only the driver knows the card can't do it. The pipeline must pick + /// the format itself rather than leaving it to negotiation. + #[test] + fn test_nvenc_cannot_be_handed_422() { + // The exact case reported: CineForm yuv422p10le -> h264_nvenc. + assert_eq!( + VideoCodec::H264Nvenc.forced_pix_fmt("yuv422p10le"), + Some("yuv420p") + ); + // HEVC drops the chroma but keeps the 10 bits. + assert_eq!( + VideoCodec::H265Nvenc.forced_pix_fmt("yuv422p10le"), + Some("p010le") + ); + // 8-bit 4:2:2 needs no depth change, only the chroma. + assert_eq!( + VideoCodec::H265Nvenc.forced_pix_fmt("yuv422p"), + Some("yuv420p") + ); + // 4:4:4 is equally unencodable: `build_encoder_quality_args` pins + // h264_nvenc to `-profile:v high`, which is a 4:2:0 profile. + assert_eq!( + VideoCodec::H264Nvenc.forced_pix_fmt("yuv444p"), + Some("yuv420p") + ); + } + + /// The guard must stay off for what already worked. Pinning NVENC + /// unconditionally (as AMF is pinned) would silently flatten a 10-bit + /// 4:2:0 source to 8-bit for everyone it currently serves correctly. + #[test] + fn test_nvenc_leaves_encodable_formats_alone() { + assert_eq!(VideoCodec::H264Nvenc.forced_pix_fmt("yuv420p"), None); + assert_eq!(VideoCodec::H265Nvenc.forced_pix_fmt("yuv420p"), None); + // 10-bit 4:2:0 into HEVC is exactly what NVENC is good at — untouched. + assert_eq!(VideoCodec::H265Nvenc.forced_pix_fmt("yuv420p10le"), None); + assert_eq!(VideoCodec::H265Nvenc.forced_pix_fmt("p010le"), None); + } + + /// H.264 has no 10-bit mode on either hardware family, so depth alone is + /// enough to force a conversion even when the chroma is already fine. + #[test] + fn test_h264_hardware_has_no_10_bit_mode() { + assert_eq!( + VideoCodec::H264Nvenc.forced_pix_fmt("yuv420p10le"), + Some("yuv420p") + ); + assert_eq!( + VideoCodec::H264Qsv.forced_pix_fmt("yuv420p10le"), + Some("nv12") + ); + } + + /// QSV is the same class of trap — `hevc_qsv` advertises 4:2:2 (`y210le`) + /// on builds where the hardware may not have it. Not reported, guarded on + /// the same reasoning. QSV's native layout is semi-planar, so it gets + /// nv12 where NVENC gets yuv420p. + #[test] + fn test_qsv_takes_its_own_native_formats() { + assert_eq!(VideoCodec::H264Qsv.forced_pix_fmt("yuv422p"), Some("nv12")); + assert_eq!( + VideoCodec::H265Qsv.forced_pix_fmt("yuv422p10le"), + Some("p010le") + ); + assert_eq!(VideoCodec::H265Qsv.forced_pix_fmt("yuv422p"), Some("nv12")); + assert_eq!(VideoCodec::H264Qsv.forced_pix_fmt("yuv420p"), None); + } + + /// The two unconditional pins predate this and must not become conditional: + /// they hold whatever the pipeline's format is. + #[test] + fn test_unconditional_pins_ignore_the_input_format() { + for fmt in ["yuv420p", "yuv422p10le", "yuv444p16le", "nv12"] { + assert_eq!(VideoCodec::Huffyuv.forced_pix_fmt(fmt), Some("yuv422p")); + assert_eq!(VideoCodec::H264Amf.forced_pix_fmt(fmt), Some("nv12")); + assert_eq!(VideoCodec::H265Amf.forced_pix_fmt(fmt), Some("nv12")); + } + } + + /// Software, ProRes, lossless and VideoToolbox negotiate correctly on their + /// own — VideoToolbox never advertises a mode it lacks, which is the whole + /// difference from NVENC. Forcing a format on them would only throw away + /// chroma they can keep. + #[test] + fn test_negotiating_encoders_are_left_alone() { + for codec in [ + VideoCodec::H264, + VideoCodec::H265, + VideoCodec::H264Videotoolbox, + VideoCodec::H265Videotoolbox, + VideoCodec::ProRes422, + VideoCodec::FFV1, + VideoCodec::Ffvhuff, + ] { + for fmt in ["yuv420p", "yuv422p10le", "yuv444p16le"] { + assert_eq!( + codec.forced_pix_fmt(fmt), + None, + "{codec:?} should negotiate {fmt} itself" + ); + } + } + } + + /// The two names for the output conversion describe the same thing, so a + /// variant gaining one and not the other is a bug — the ffmpeg name is what + /// decides whether a hardware encoder can take it. + #[test] + fn chroma_subsampling_names_agree() { + for cs in [ + ChromaSubsampling::Original, + ChromaSubsampling::Yuv420, + ChromaSubsampling::Yuv420P10, + ChromaSubsampling::Yuv422, + ChromaSubsampling::Yuv422P10, + ] { + assert_eq!( + cs.vapoursynth_format().is_some(), + cs.ffmpeg_pix_fmt().is_some(), + "{cs:?} declares one output format name but not the other" + ); + } + } + #[test] fn test_container_format_serialization() { assert_eq!( diff --git a/worker/src/pipeline_executor.rs b/worker/src/pipeline_executor.rs index 878fb10..cc9e7c0 100644 --- a/worker/src/pipeline_executor.rs +++ b/worker/src/pipeline_executor.rs @@ -835,8 +835,24 @@ impl PipelineExecutor { Self::build_encoder_quality_args(&mut args, job); // Force a compatible output pixel format for codecs that can't accept the - // pipeline's native format (e.g. classic HuffYUV requires yuv422p). - if let Some(pix_fmt) = settings.codec.forced_pix_fmt() { + // pipeline's native format (e.g. classic HuffYUV requires yuv422p, and + // NVENC/QSV cannot encode 4:2:2 on most hardware — issue #74). + let encoder_input = job.encoder_input_pix_fmt(); + if let Some(pix_fmt) = settings.codec.forced_pix_fmt(&encoder_input) { + if pix_fmt != encoder_input { + // Say so in the job log. A silent downconversion is the right + // behaviour — failing the whole encode helps nobody — but it + // changes the output, so it must not also be invisible. + self.reporter.send_log( + LogLevel::Info, + &format!( + "{} cannot encode {}; converting to {} for output", + settings.codec.display_name(), + encoder_input, + pix_fmt + ), + ); + } args.extend(["-pix_fmt".to_string(), pix_fmt.to_string()]); } @@ -1332,7 +1348,7 @@ impl Drop for PipelineExecutor { #[cfg(test)] mod tests { use super::*; - use crate::models::{AudioCodec, AudioQuality, EncodingSettings, QTGMCParameters, VideoCodec}; + use crate::models::{AudioCodec, AudioQuality, ChromaSubsampling, EncodingSettings, QTGMCParameters, VideoCodec}; use uuid::Uuid; /// A leftover `progress=end` must not end a run that has produced no frames. @@ -1525,8 +1541,9 @@ mod tests { // Encoder-family-specific quality and preset args PipelineExecutor::build_encoder_quality_args(&mut args, job); - // Force a compatible output pixel format (e.g. HuffYUV requires yuv422p) - if let Some(pix_fmt) = settings.codec.forced_pix_fmt() { + // Force a compatible output pixel format (e.g. HuffYUV requires yuv422p, + // NVENC/QSV cannot encode 4:2:2 on most hardware) + if let Some(pix_fmt) = settings.codec.forced_pix_fmt(&job.encoder_input_pix_fmt()) { args.extend(["-pix_fmt".to_string(), pix_fmt.to_string()]); } @@ -1948,6 +1965,106 @@ mod tests { assert!(!args.contains(&"-preset".to_string()), "HuffYUV should not have -preset"); } + /// Issue #74, end to end: the reported job was a CineForm `yuv422p10le` + /// source at the default "Match source" colour format, encoded with + /// h264_nvenc. With nothing forced, ffmpeg negotiated `yuv422p` (which the + /// encoder advertises) and the RTX 4070 Super rejected it at open, so the + /// job produced zero frames. + #[test] + fn test_nvenc_422_source_gets_an_encodable_pix_fmt() { + let mut job = create_test_job("output.mp4"); + job.encoding_settings.codec = VideoCodec::H264Nvenc; + job.input_pixel_format = Some("yuv422p10le".to_string()); + assert_eq!( + job.encoding_settings.chroma_subsampling, + ChromaSubsampling::Original, + "the reported job used the default colour format" + ); + + let args = build_ffmpeg_args_for_test(&job); + let idx = args + .iter() + .position(|a| a == "-pix_fmt") + .expect("NVENC must not be left to negotiate a 4:2:2 source"); + assert_eq!(args[idx + 1], "yuv420p"); + } + + /// The same source into HEVC keeps its 10 bits — only the chroma has to go. + #[test] + fn test_hevc_nvenc_422_source_keeps_10_bits() { + let mut job = create_test_job("output.mp4"); + job.encoding_settings.codec = VideoCodec::H265Nvenc; + job.input_pixel_format = Some("yuv422p10le".to_string()); + + let args = build_ffmpeg_args_for_test(&job); + let idx = args.iter().position(|a| a == "-pix_fmt").unwrap(); + assert_eq!(args[idx + 1], "p010le"); + } + + /// Choosing a 4:2:2 output format explicitly is the other route to the same + /// failure: the conversion is done by the `.vpy`, so the source format never + /// enters into it. + #[test] + fn test_nvenc_422_output_format_is_also_guarded() { + let mut job = create_test_job("output.mp4"); + job.encoding_settings.codec = VideoCodec::H265Nvenc; + job.encoding_settings.chroma_subsampling = ChromaSubsampling::Yuv422P10; + // An 8-bit 4:2:0 source — the pipeline is what makes it 4:2:2. + job.input_pixel_format = Some("yuv420p".to_string()); + + assert_eq!(job.encoder_input_pix_fmt(), "yuv422p10le"); + let args = build_ffmpeg_args_for_test(&job); + let idx = args.iter().position(|a| a == "-pix_fmt").unwrap(); + assert_eq!(args[idx + 1], "p010le"); + } + + /// A 4:2:0 source into NVENC must emit no `-pix_fmt` at all, so the fix + /// changes nothing for the jobs that already worked. + #[test] + fn test_nvenc_420_source_is_untouched() { + for fmt in ["yuv420p", "yuv420p10le"] { + let mut job = create_test_job("output.mp4"); + job.encoding_settings.codec = VideoCodec::H265Nvenc; + job.input_pixel_format = Some(fmt.to_string()); + + let args = build_ffmpeg_args_for_test(&job); + assert!( + !args.contains(&"-pix_fmt".to_string()), + "{fmt} is encodable; NVENC should negotiate it" + ); + } + } + + /// x264 handles 4:2:2 natively. Forcing a format here would throw away + /// chroma the encoder was perfectly able to keep. + #[test] + fn test_software_encoder_keeps_422() { + let mut job = create_test_job("output.mkv"); + job.encoding_settings.codec = VideoCodec::H264; + job.input_pixel_format = Some("yuv422p10le".to_string()); + + let args = build_ffmpeg_args_for_test(&job); + assert!(!args.contains(&"-pix_fmt".to_string())); + } + + /// Custom FFmpeg Arguments are appended last, so a user who knows their + /// card really does have 4:2:2 can still ask for it — ffmpeg takes the + /// later `-pix_fmt`. That is the escape hatch this guard relies on. + #[test] + fn test_custom_args_can_override_the_forced_format() { + let mut job = create_test_job("output.mp4"); + job.encoding_settings.codec = VideoCodec::H264Nvenc; + job.input_pixel_format = Some("yuv422p10le".to_string()); + job.encoding_settings.custom_ffmpeg_args = "-pix_fmt yuv422p".to_string(); + + let args = build_ffmpeg_args_for_test(&job); + let last = args + .iter() + .rposition(|a| a == "-pix_fmt") + .expect("both formats should be present"); + assert_eq!(args[last + 1], "yuv422p", "the user's choice must win"); + } + #[test] fn test_ffmpeg_args_video_codec_ffvhuff_no_forced_pix_fmt() { let mut job = create_test_job("output.mkv"); diff --git a/worker/src/pixel_format.rs b/worker/src/pixel_format.rs index 40536f1..694f860 100644 --- a/worker/src/pixel_format.rs +++ b/worker/src/pixel_format.rs @@ -49,7 +49,7 @@ pub const DEFAULT_FORMAT: &str = "yuv420p"; /// Chroma resolution class of a source format. Ordered so that a source is /// always mapped to a class at least as detailed as its own. #[derive(Debug, Clone, Copy, PartialEq, Eq)] -enum ChromaClass { +pub enum ChromaClass { C420, C422, C444, @@ -78,6 +78,16 @@ impl PipeFormat { } } +/// Chroma class and bit depth of an FFmpeg pixel format name. +/// +/// Public counterpart of [`classify`]. `VideoCodec::forced_pix_fmt` needs it to +/// decide whether the format the pipeline will hand a hardware encoder is one +/// that encoder can actually take — a question about the *output* of the graph, +/// where the rest of this module is about its input. +pub fn chroma_and_depth(name: &str) -> (ChromaClass, u32) { + classify(&name.to_ascii_lowercase()) +} + /// The raw pixel format to pipe for a source whose probed format is `probed`. pub fn decode_pixel_format(probed: Option<&str>) -> PipeFormat { let probed = probed.map(str::trim).filter(|s| !s.is_empty()); From 2e30b12664e1c70bba1f61e27175743553d61e89 Mon Sep 17 00:00:00 2001 From: Stuart Cameron Date: Wed, 19 Aug 2026 10:43:30 +1000 Subject: [PATCH 2/2] feat(ui): a 4:2:0 10-bit format, and say when one is being substituted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The worker half of #74 keeps the job running, but silently changing someone's output is only acceptable if they can see it coming and choose otherwise. A 4:2:0 10-bit output format. The only 10-bit layout NVENC, QSV and AMF can encode, so it is how a 10-bit source keeps its grading through a GPU encoder by choice rather than by the guard's fallback. Verified end to end: a 10-bit 4:2:2 source comes out yuv420p10le, profile High 10. A warning under the dropdown naming the substitute and the reason, before the job runs. It covers both routes into #74 — a 4:2:2 source at "Match source", and an explicitly chosen 4:2:2 output — and stays silent for anything encodable, for VideoToolbox and AMF, and until a file is loaded. It is a second implementation of the worker's decision, and if the two disagree the interface promises one thing while the encode does another, which is worse than either being wrong alone. Both sides are pinned to the same table of cases: "substitutions match the worker, case for case" against test_nvenc_cannot_be_handed_422 and its neighbours. pixelFormatChromaLayout is likewise a coarse twin of ChromaClass, following the precedent already set by pixelFormatBitDepth rather than inventing a new one. A second help dialog explaining what the app does to colour, as opposed to the existing one explaining what the formats are and which to pick: the pipe source normalising upward, filters converting down and back per pass, UI thresholds being in 8-bit units and rescaled to the clip depth, the output conversion dithering, the Y4M pipe stripping SAR and colour tags so they must be re-stamped, and the encoder having the last word. A distinct icon rather than a second info_outline, asserted, because two identical adjacent buttons read as one control repeated. --- CLAUDE.md | 83 +++++++- app/lib/models/encoding_settings.dart | 4 + app/lib/utils/pixel_format.dart | 159 +++++++++++++++ app/lib/views/settings/settings_dialog.dart | 167 ++++++++++++++++ .../hardware_encoder_chroma_warning_test.dart | 189 ++++++++++++++++++ ...tegration_high_bit_depth_filters_test.dart | 11 + app/test/pixel_format_test.dart | 2 + .../settings_colour_pipeline_help_test.dart | 124 ++++++++++++ 8 files changed, 738 insertions(+), 1 deletion(-) create mode 100644 app/test/hardware_encoder_chroma_warning_test.dart create mode 100644 app/test/settings_colour_pipeline_help_test.dart diff --git a/CLAUDE.md b/CLAUDE.md index 400ca92..ed5a833 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1657,8 +1657,89 @@ value here shows up as "this GPU doesn't work" rather than as an error: (a later `-pix_fmt` wins), which is the escape hatch for 10-bit HEVC on hardware that supports it. +- **A hardware encoder's declared pix_fmt list is not a statement about the + machine (issue #74).** The list ffmpeg negotiates against is compiled in; + NVENC's real capabilities are queried from the driver at `avcodec_open2`. A + recent ffmpeg built against NVENC SDK 13 advertises `yuv422p` on + `h264_nvenc` for Blackwell's 4:2:2 support, so negotiation picks it for any + 4:2:2 source — and every pre-Blackwell card then fails the job outright with + *"YUV422P not supported / No capable devices found"*, zero frames written. + Reported on an RTX 4070 Super with a CineForm `yuv422p10le` capture at the + default "Match source" colour format. **ffmpeg cannot negotiate its way out + of this**, so `forced_pix_fmt` picks the format instead. + +`VideoCodec::forced_pix_fmt` therefore takes the format the pipeline will +actually hand the encoder — `VideoJob::encoder_input_pix_fmt`, which is the +output conversion when one is selected and the pipe format otherwise. Three +things about it are load-bearing: + +- **The NVENC/QSV arm is conditional and the other two are not.** HuffYUV and + AMF take one format whatever the source was; NVENC does not. Pinning it + unconditionally the way AMF is pinned would flatten a 10-bit 4:2:0 source to + 8-bit for everyone it already serves correctly, so the guard fires only on + 4:2:2, 4:4:4, or >8-bit into an H.264 encoder (neither family has a 10-bit + H.264 mode). A 4:2:0 job emits no `-pix_fmt` at all, exactly as before. +- **HEVC keeps the depth, H.264 cannot.** 4:2:2 10-bit into `hevc_nvenc` + becomes `p010le`, not `yuv420p` — only the chroma has to go. NVENC gets the + planar `yuv420p` and QSV the semi-planar `nv12`, each family's native name. +- **VideoToolbox is deliberately excluded.** It never advertises a mode it + lacks, so its negotiation is trustworthy and forcing a format would only + throw away chroma it could have kept. QSV *is* included, preventively rather + than on a report: `hevc_qsv` advertises `y210le` on builds whose hardware may + not have it, which is the same trap. + 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. +hardware in the matrix — and the same is true of NVENC and QSV, so these rest +on unit tests over the emitted arguments plus reporter confirmation. Note the +functional probe in `HardwareEncoderDetector` cannot catch the #74 class at +all: it encodes one `yuv420p` frame from lavfi, so it correctly reports NVENC +as *available* — the device works, only the format doesn't. Don't try to fix a +format problem in the device probe; the format isn't known until a file loads. + +### The user-facing half of #74 + +The guard above keeps the job running, but silently changing someone's output +is only acceptable if they can see it coming and choose otherwise. Two things +shipped with it: + +**A 4:2:0 10-bit output format.** `ChromaSubsampling::Yuv420P10` / +`ChromaSubsampling.yuv420p10` — the only 10-bit layout NVENC, QSV and AMF can +encode, so it is how a 10-bit source keeps its grading through a GPU encoder by +the user's own choice rather than by the guard's fallback. Verified end to end +(`integration_high_bit_depth_filters_test`): a 10-bit 4:2:2 source comes out +`yuv420p10le`, profile **High 10**. Adding an option touches four places — +`vapoursynth_format`, `ffmpeg_pix_fmt`, the Dart enum with its `outputBitDepth`, +and `chromaFormatHelpSections`, which `settings_chroma_help_test` fails on if +the new label goes unmentioned. + +**A warning under the dropdown.** `hardwareEncoderChromaWarning` in +`app/lib/utils/pixel_format.dart` says which format will be substituted and +why, before the job runs. It covers both routes into #74 — a 4:2:2 *source* at +"Match source", and an explicitly chosen 4:2:2 *output* — and it stays silent +for everything encodable, for VideoToolbox and AMF, and until a file is loaded. + +> **It is a second implementation of the worker's decision, and that is the +> risk.** If the two disagree the interface promises one thing and the encode +> does another, which is worse than either being wrong alone. Both sides are +> therefore pinned to **the same table of cases** — `substitutions match the +> worker, case for case` in `hardware_encoder_chroma_warning_test.dart` against +> `test_nvenc_cannot_be_handed_422` and its neighbours in `video_job.rs`. Change +> one and change both. `pixelFormatChromaLayout` is likewise a coarse Dart twin +> of `ChromaClass`; the Dart side already reimplements this kind of pix_fmt +> parsing in `pixelFormatBitDepth`, so it follows that precedent rather than +> inventing a new one. + +**A second help dialog**, `ColourPipelineHelpIcon` / +`colourPipelineHelpSections`, beside the existing `ChromaFormatHelpIcon`. The +two answer different questions and both are worth having: the first is *what +are these formats and which do I pick*, the second is *what does the app do to +my colour* — the pipe source normalising upward on the way in, filters +converting down and back per pass, every UI threshold being in 8-bit units and +rescaled to the clip depth, the output conversion dithering, the Y4M pipe +stripping SAR and colour tags so they must be re-stamped, and the encoder +having the last word. A deliberately distinct icon (`schema_outlined`, not a +second `info_outline`), asserted, because two identical adjacent buttons read +as one control repeated. ## QTGMC Parameters Reference diff --git a/app/lib/models/encoding_settings.dart b/app/lib/models/encoding_settings.dart index 6895bb3..a151786 100644 --- a/app/lib/models/encoding_settings.dart +++ b/app/lib/models/encoding_settings.dart @@ -66,6 +66,10 @@ enum ChromaSubsampling { original('original', 'Match source', null, null), /// Convert to 8-bit YUV420 for maximum compatibility (smaller files). yuv420('yuv420', '4:2:0 8-bit', 'most compatible', 8), + /// Convert to 10-bit YUV420. The only 10-bit layout NVENC, QSV and AMF can + /// encode, so it is the way to keep a 10-bit source's grading on a GPU + /// encoder — 4:2:2 fails outright on most of them (issue #74). + yuv420p10('yuv420p10', '4:2:0 10-bit', 'best 10-bit for GPU encoders', 10), /// Convert to 8-bit YUV422 for higher chroma quality. yuv422('yuv422', '4:2:2 8-bit', 'more colour detail', 8), /// Convert to 10-bit YUV422: keeps a 10-bit source's precision while diff --git a/app/lib/utils/pixel_format.dart b/app/lib/utils/pixel_format.dart index 1f1eeb5..ea9a2ad 100644 --- a/app/lib/utils/pixel_format.dart +++ b/app/lib/utils/pixel_format.dart @@ -1,6 +1,9 @@ // Utilities for interpreting FFmpeg pixel-format strings (e.g. the `pix_fmt` // reported by ffprobe: "yuv420p", "yuv422p10le", "yuv420p16le"). +import '../models/encoding_settings.dart'; +import '../models/video_job.dart'; + /// Best-effort per-component bit depth for an FFmpeg pixel-format string. /// /// Returns 8 for the common 8-bit formats (yuv420p, yuv422p, nv12, rgb24, …), @@ -71,3 +74,159 @@ String? chromaConversionBitDepthWarning({ 'colour format. Choose "Match source" to keep the source\'s bit depth' '${targetBitDepth < 10 ? ', or 4:2:2 10-bit to keep more of it' : ''}.'; } + +/// Chroma layout of an FFmpeg pixel-format string, coarse enough for the one +/// question the UI asks of it: can a hardware encoder take this? +/// +/// Mirrors `ChromaClass` in `worker/src/pixel_format.rs`, which is the authority +/// — it decides what the worker actually does. This copy only decides what the +/// warning *says*, and `hardware_encoder_chroma_warning_test.dart` pins the two +/// to the same table so they cannot drift into disagreeing on screen. +enum ChromaLayout { + /// 4:2:0 — one colour sample per 2x2 block. + c420, + + /// 4:2:2 and anything hardware treats like it (4:4:0, 4:1:1, 4:1:0). + c422, + + /// 4:4:4, and RGB, which carries full colour resolution by construction. + c444, +} + +/// Best-effort chroma layout for an FFmpeg pixel-format string. +/// +/// Unknown and null formats fall back to [ChromaLayout.c420] — the conservative +/// choice here, because 4:2:0 is what every encoder accepts, so an unrecognized +/// format produces no spurious warning. (`pixelFormatBitDepth` falls back the +/// same way and for the same reason.) +ChromaLayout pixelFormatChromaLayout(String? pixFmt) { + if (pixFmt == null || pixFmt.trim().isEmpty) return ChromaLayout.c420; + final f = pixFmt.trim().toLowerCase(); + + // Planar YUV: the three digits after the family prefix are the subsampling. + final planar = RegExp(r'^yuv[aj]?(\d{3})').firstMatch(f); + if (planar != null) { + switch (planar.group(1)!) { + case '420': + return ChromaLayout.c420; + // 4:1:0 and 4:1:1 subsample more coarsely than 4:2:2 horizontally and + // 4:4:0 more coarsely vertically, but none of them is 4:2:0, and no + // hardware encoder takes any of them — so they warn alongside 4:2:2. + case '410': + case '411': + case '422': + case '440': + return ChromaLayout.c422; + default: + return ChromaLayout.c444; + } + } + + // Semi-planar: nv12/nv21 are 4:2:0, nv16 4:2:2, nv24/nv42 4:4:4; p010/p016 + // are 4:2:0, p210/p216 4:2:2, p410/p416 4:4:4 (the middle digit is the + // subsampling, as in `pixelFormatBitDepth`). + const semiPlanar = { + 'nv12': ChromaLayout.c420, + 'nv21': ChromaLayout.c420, + 'nv16': ChromaLayout.c422, + 'nv24': ChromaLayout.c444, + 'nv42': ChromaLayout.c444, + }; + final stem = f.replaceFirst(RegExp(r'(le|be)$'), ''); + final named = semiPlanar[stem]; + if (named != null) return named; + final p = RegExp(r'^p(\d)(?:10|12|16)$').firstMatch(stem); + if (p != null) { + switch (p.group(1)!) { + case '0': + return ChromaLayout.c420; + case '2': + return ChromaLayout.c422; + default: + return ChromaLayout.c444; + } + } + + // Gray has no chroma at all, so every encoder can hold it. + if (stem.startsWith('gray') || stem.startsWith('ya')) return ChromaLayout.c420; + + // RGB and planar GBR carry full colour resolution. + if (RegExp(r'^(a?rgb|a?bgr|gbra?p)').hasMatch(stem)) return ChromaLayout.c444; + + return ChromaLayout.c420; +} + +/// Warning message when the chosen output colour format is one the selected +/// hardware encoder cannot take, and the worker will therefore substitute +/// another — or null when there is nothing to say. +/// +/// This is the UI half of issue #74. An encoder's declared format list is +/// compiled into ffmpeg, but NVENC's real capabilities are queried from the +/// driver at open time: a recent ffmpeg advertises `yuv422p` on `h264_nvenc` +/// for Blackwell's 4:2:2 support, and every earlier card then failed the whole +/// job. The worker now substitutes an encodable format instead of failing, so +/// this exists to say so *before* the job runs rather than only in its log. +/// +/// **Keep in step with `VideoCodec::forced_pix_fmt` in +/// `worker/src/models/video_job.rs`**, which is what actually happens. The two +/// share a table of cases in their tests. +/// +/// [chromaSubsampling] decides the format outright unless it is +/// [ChromaSubsampling.original], in which case the source's [pixelFormat] does. +String? hardwareEncoderChromaWarning({ + required VideoCodec codec, + required ChromaSubsampling chromaSubsampling, + String? pixelFormat, +}) { + // Only NVENC and QSV advertise formats their hardware may not have. + // VideoToolbox never does, so its negotiation is trustworthy; AMF is pinned + // to nv12 unconditionally and has been since issue #51, so it converts + // whatever it is given and there is no surprise to warn about. + final isNvenc = codec.isNvenc; + final isQsv = codec == VideoCodec.h264Qsv || codec == VideoCodec.h265Qsv; + if (!isNvenc && !isQsv) return null; + + // What the encoder will actually be handed: the output conversion when one is + // selected, the source's own format otherwise. + final ChromaLayout layout; + final int depth; + final String describedAs; + switch (chromaSubsampling) { + case ChromaSubsampling.original: + // Nothing to warn about until a file is loaded and we know its format. + if (pixelFormat == null) return null; + layout = pixelFormatChromaLayout(pixelFormat); + depth = pixelFormatBitDepth(pixelFormat); + describedAs = 'Your source is $pixelFormat, and "Match source" keeps it'; + default: + layout = chromaSubsampling == ChromaSubsampling.yuv420 || + chromaSubsampling == ChromaSubsampling.yuv420p10 + ? ChromaLayout.c420 + : ChromaLayout.c422; + depth = chromaSubsampling.outputBitDepth ?? 8; + describedAs = '${chromaSubsampling.label} is selected'; + } + + final chromaUnsupported = layout != ChromaLayout.c420; + // Neither family has a 10-bit H.264 mode at all. + final depthUnsupported = depth > 8 && codec.isH264; + if (!chromaUnsupported && !depthUnsupported) return null; + + // Mirrors forced_pix_fmt: H.264 has only 8-bit 4:2:0; HEVC keeps the depth. + final substitute = codec.isH264 + ? '4:2:0 8-bit' + : (depth > 8 ? '4:2:0 10-bit' : '4:2:0 8-bit'); + + final reason = chromaUnsupported + ? '${codec.encoderFamily} cannot encode ${layout == ChromaLayout.c444 ? "4:4:4" : "4:2:2"} ' + 'on most GPUs' + : '${codec.encoderFamily} has no 10-bit H.264 mode'; + + final advice = codec.isH264 && depth > 8 + ? ' Choose an H.265 encoder to keep the 10-bit grading, or a software ' + 'encoder to keep the colour detail as well.' + : ' Choose a software encoder to keep it as it is.'; + + return '$describedAs, but $reason, so the output will be converted to ' + '$substitute.$advice'; +} diff --git a/app/lib/views/settings/settings_dialog.dart b/app/lib/views/settings/settings_dialog.dart index 28fd377..d5ef65d 100644 --- a/app/lib/views/settings/settings_dialog.dart +++ b/app/lib/views/settings/settings_dialog.dart @@ -55,6 +55,10 @@ const List<(String, String)> chromaFormatHelpSections = [ 'refuse to open.\n\n' '4:2:0 8-bit — plays on everything. Use it for anything going to ' "someone else's device, the web, or a TV.\n\n" + '4:2:0 10-bit — the same universal colour layout with the finer ' + 'grading. This is the one to pick for a 10-bit source on a GPU ' + 'encoder: NVENC, QSV and AMF can all encode it, and none of them can ' + 'encode 4:2:2 on most cards.\n\n' '4:2:2 8-bit — keeps the extra colour detail of a 4:2:2 or ' 'analogue-captured source, at 8-bit precision.\n\n' '4:2:2 10-bit — keeps the colour detail and the 10-bit grading. Best ' @@ -144,6 +148,148 @@ Future showChromaFormatHelp(BuildContext context) { ); } +/// How the colour of a frame is carried from the source file to the output — +/// the mechanics behind the dropdown above, for anyone who wants to know why +/// their file came out the way it did. +/// +/// Everything here is a real property of the pipeline, not a simplification. +/// If any of it changes, this text is wrong and +/// `settings_colour_pipeline_help_test.dart` cannot tell — it only checks the +/// sections are present and well-formed. +const List<(String, String)> colourPipelineHelpSections = [ + ( + 'Reading the source', + 'VapourBox does not open your file with a VapourSynth source filter. FFmpeg ' + 'decodes it and pipes raw planar frames in, which is what makes seeking ' + 'frame-accurate.\n\n' + 'The pipe can carry most planar YUV formats directly, so a 10-bit 4:2:2 ' + 'ProRes or CineForm arrives at full precision. A format it cannot carry ' + '(RGB, semi-planar, alpha, 4:1:1) is converted on the way in — always ' + 'upward, never to less chroma resolution or fewer bits, so reading a ' + 'file can never be the thing that degrades it.', + ), + ( + 'Processing', + 'Filters run in whatever format the frame arrived in. Where a plugin cannot ' + 'handle the depth — a few are 8-bit only, one caps at 10 — the pipeline ' + 'converts down for that pass alone and restores the format immediately ' + 'after, rather than dropping the whole clip to 8-bit.\n\n' + 'Every threshold, level and offset in the interface is expressed in ' + '8-bit terms (0–255), because that is the vocabulary the filters were ' + 'documented in. Those values are rescaled to the actual depth inside the ' + 'script, so a brightness or levels adjustment does the same thing to a ' + '10-bit source as to an 8-bit one.', + ), + ( + 'Converting for output', + 'The colour format you choose above is applied last, after every filter, so ' + 'the whole graph runs at the source precision even when the output is ' + '8-bit.\n\n' + 'Reducing depth dithers rather than rounds. Plain rounding turns a ' + 'shallow gradient — a sky, a fade, a VHS luma ramp — into visible bands; ' + 'error diffusion keeps it smooth.', + ), + ( + 'What the pipe loses, and what puts it back', + 'The Y4M pipe between VapourSynth and the encoder carries pixels and ' + 'nothing else. It strips the pixel aspect ratio and every colour tag — ' + 'matrix, primaries, transfer, range — so anything VapourSynth knew about ' + 'them is gone by the time the encoder sees the frames.\n\n' + 'They are therefore re-declared on the encoder from the values read out ' + 'of your source. Without that the output would be untagged, and an ' + 'untagged file is read as BT.601 limited by every player: a BT.709 or ' + 'full-range source would come out subtly wrong in colour with nothing to ' + 'indicate why. Tags are carried through, never invented — a source that ' + 'declares nothing stays undeclared.', + ), + ( + 'The encoder has the last word', + 'A hardware encoder advertises the formats its driver was built to support, ' + 'not the ones the card in your machine actually has. NVENC lists 4:2:2 ' + 'because the newest NVIDIA cards can do it; every earlier card rejects ' + 'it when the encode starts.\n\n' + 'So the format is chosen for hardware encoders rather than negotiated: ' + 'anything they cannot take is converted to 4:2:0 first, keeping 10-bit ' + 'precision where the codec allows it (H.265 does, H.264 has no 10-bit ' + 'mode at all). The job log names the substitution whenever one happens. ' + 'A later -pix_fmt in Custom FFmpeg Arguments overrides it, which is the ' + 'way to use a mode your card really does have.', + ), +]; + +/// The second help affordance beside the colour format dropdown: the same +/// click-to-open dialog pattern as [ChromaFormatHelpIcon], but answering a +/// different question. That one explains what the formats *are* and which to +/// choose; this one explains what the application *does* with them. +/// +/// A distinct icon rather than a second `info_outline`, so two adjacent buttons +/// don't look like the same thing twice. +class ColourPipelineHelpIcon extends StatelessWidget { + const ColourPipelineHelpIcon({super.key}); + + @override + Widget build(BuildContext context) { + return IconButton( + icon: const Icon( + Icons.schema_outlined, + size: 20, + semanticLabel: 'How VapourBox handles colour', + ), + color: Theme.of(context).colorScheme.primary, + visualDensity: VisualDensity.compact, + padding: const EdgeInsets.only(left: 4), + constraints: const BoxConstraints(), + onPressed: () => showColourPipelineHelp(context), + ); + } +} + +/// Open the colour pipeline explanation. Same shape as [showChromaFormatHelp] — +/// fixed width, scrollable, height-capped by the dialog itself. +Future showColourPipelineHelp(BuildContext context) { + return showDialog( + context: context, + builder: (ctx) { + final theme = Theme.of(ctx); + return AlertDialog( + title: const Text('How VapourBox handles colour'), + content: SizedBox( + width: 520, + child: SingleChildScrollView( + child: Column( + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + Text( + 'What happens to a frame between your source file and the ' + 'finished output.', + style: theme.textTheme.bodyMedium, + ), + for (final (heading, body) in colourPipelineHelpSections) ...[ + const SizedBox(height: 20), + Text( + heading, + style: theme.textTheme.titleSmall + ?.copyWith(fontWeight: FontWeight.bold), + ), + const SizedBox(height: 6), + Text(body, + style: theme.textTheme.bodyMedium?.copyWith(height: 1.4)), + ], + ], + ), + ), + ), + actions: [ + TextButton( + onPressed: () => Navigator.of(ctx).pop(), + child: const Text('Close'), + ), + ], + ); + }, + ); +} + class SettingsDialog extends StatefulWidget { const SettingsDialog({super.key}); @@ -850,6 +996,10 @@ class _OutputSettingsTabState extends State<_OutputSettingsTab> { // and which option to reach for. The short line below the // dropdown is the summary; this is the explanation. const ChromaFormatHelpIcon(), + // A second dialog answering the other question people + // ask here: not "which do I pick" but "what is the app + // actually doing to my colour". + const ColourPipelineHelpIcon(), ], ), const SizedBox(height: 8), @@ -867,6 +1017,7 @@ class _OutputSettingsTabState extends State<_OutputSettingsTab> { ), ), ..._buildChromaBitDepthWarning(viewModel, settings), + ..._buildChromaEncoderWarning(viewModel, settings), ], ), ), @@ -970,6 +1121,22 @@ class _OutputSettingsTabState extends State<_OutputSettingsTab> { return [const SizedBox(height: 12), WarningBanner(message: message)]; } + /// Issue #74: warn when the selected colour format is one the selected + /// hardware encoder cannot take, so the substitution the worker performs is + /// visible before the job runs rather than only in its log. + List _buildChromaEncoderWarning( + MainViewModel viewModel, + EncodingSettings settings, + ) { + final message = hardwareEncoderChromaWarning( + codec: settings.codec, + chromaSubsampling: settings.chromaSubsampling, + pixelFormat: viewModel.videoInfo?.pixelFormat, + ); + if (message == null) return const []; + return [const SizedBox(height: 12), WarningBanner(message: message)]; + } + /// Whether a hardware encoder is relevant to the current platform's GPU APIs: /// VideoToolbox is macOS-only; QSV/NVENC/AMF apply to Windows and Linux. /// (Software/ProRes/lossless codecs are platform-agnostic.) diff --git a/app/test/hardware_encoder_chroma_warning_test.dart b/app/test/hardware_encoder_chroma_warning_test.dart new file mode 100644 index 0000000..b109774 --- /dev/null +++ b/app/test/hardware_encoder_chroma_warning_test.dart @@ -0,0 +1,189 @@ +// The UI half of issue #74. +// +// A hardware encoder advertises the formats its driver was *built* against, not +// the ones the card actually has: a recent ffmpeg lists `yuv422p` on +// `h264_nvenc` for Blackwell, and every earlier NVIDIA card then failed the +// whole job at encoder open. The worker substitutes an encodable format instead +// of failing; this warning is what tells the user before the job runs. +// +// The substitution table below is the same one asserted in Rust by +// `test_nvenc_cannot_be_handed_422` and its neighbours in +// `worker/src/models/video_job.rs`. If the two disagree, the interface promises +// one thing and the encode does another — which is worse than either being +// wrong on its own, so both sides are pinned to the same cases. + +import 'package:flutter_test/flutter_test.dart'; + +import 'package:vapourbox/models/encoding_settings.dart'; +import 'package:vapourbox/models/video_job.dart'; +import 'package:vapourbox/utils/pixel_format.dart'; + +void main() { + group('pixelFormatChromaLayout', () { + test('reads planar YUV subsampling', () { + expect(pixelFormatChromaLayout('yuv420p'), ChromaLayout.c420); + expect(pixelFormatChromaLayout('yuv420p10le'), ChromaLayout.c420); + expect(pixelFormatChromaLayout('yuv422p'), ChromaLayout.c422); + expect(pixelFormatChromaLayout('yuv422p10le'), ChromaLayout.c422); + expect(pixelFormatChromaLayout('yuv444p12le'), ChromaLayout.c444); + expect(pixelFormatChromaLayout('yuvj420p'), ChromaLayout.c420); + expect(pixelFormatChromaLayout('yuva444p10le'), ChromaLayout.c444); + }); + + test('the coarse layouts warn alongside 4:2:2', () { + // No hardware encoder takes any of these either, and calling them 4:2:2 + // in a warning is close enough to be useful and never wrong about the + // conclusion. + expect(pixelFormatChromaLayout('yuv411p'), ChromaLayout.c422); + expect(pixelFormatChromaLayout('yuv410p'), ChromaLayout.c422); + expect(pixelFormatChromaLayout('yuv440p'), ChromaLayout.c422); + }); + + test('reads semi-planar and packed formats', () { + expect(pixelFormatChromaLayout('nv12'), ChromaLayout.c420); + expect(pixelFormatChromaLayout('nv16'), ChromaLayout.c422); + expect(pixelFormatChromaLayout('nv24'), ChromaLayout.c444); + expect(pixelFormatChromaLayout('p010le'), ChromaLayout.c420); + expect(pixelFormatChromaLayout('p210le'), ChromaLayout.c422); + expect(pixelFormatChromaLayout('p410le'), ChromaLayout.c444); + expect(pixelFormatChromaLayout('rgb24'), ChromaLayout.c444); + expect(pixelFormatChromaLayout('gbrp'), ChromaLayout.c444); + }); + + test('unknown and gray formats fall back to the encodable layout', () { + // The fallback must never invent a warning: 4:2:0 is what every encoder + // takes, so an unrecognised name stays silent. + expect(pixelFormatChromaLayout(null), ChromaLayout.c420); + expect(pixelFormatChromaLayout(''), ChromaLayout.c420); + expect(pixelFormatChromaLayout('something_new'), ChromaLayout.c420); + expect(pixelFormatChromaLayout('gray10le'), ChromaLayout.c420); + }); + }); + + group('hardwareEncoderChromaWarning', () { + String? warn(VideoCodec codec, String? pixFmt, + [ChromaSubsampling cs = ChromaSubsampling.original]) => + hardwareEncoderChromaWarning( + codec: codec, + chromaSubsampling: cs, + pixelFormat: pixFmt, + ); + + test('the reported case warns and names the substitute', () { + // CineForm yuv422p10le -> h264_nvenc at "Match source", on an RTX 4070 + // Super. This produced zero frames and "No capable devices found". + final msg = warn(VideoCodec.h264Nvenc, 'yuv422p10le'); + expect(msg, isNotNull); + expect(msg, contains('yuv422p10le')); + expect(msg, contains('4:2:0 8-bit')); + expect(msg, contains('NVIDIA NVENC')); + }); + + test('substitutions match the worker, case for case', () { + // MIRRORS the Rust table in worker/src/models/video_job.rs. The right + // column is the format `forced_pix_fmt` returns, spelled the way this + // warning says it. + const cases = <(VideoCodec, String, String?)>[ + // 4:2:2 in — H.264 loses the depth too, H.265 keeps it. + (VideoCodec.h264Nvenc, 'yuv422p10le', '4:2:0 8-bit'), // -> yuv420p + (VideoCodec.h265Nvenc, 'yuv422p10le', '4:2:0 10-bit'), // -> p010le + (VideoCodec.h265Nvenc, 'yuv422p', '4:2:0 8-bit'), // -> yuv420p + (VideoCodec.h264Nvenc, 'yuv444p', '4:2:0 8-bit'), // -> yuv420p + // 10-bit 4:2:0 in — only H.264 has a problem with it. + (VideoCodec.h264Nvenc, 'yuv420p10le', '4:2:0 8-bit'), // -> yuv420p + (VideoCodec.h265Nvenc, 'yuv420p10le', null), // -> None + // 8-bit 4:2:0 in — nothing to do on either. + (VideoCodec.h264Nvenc, 'yuv420p', null), // -> None + (VideoCodec.h265Nvenc, 'yuv420p', null), // -> None + // QSV takes the same decisions (nv12/p010le on the worker side). + (VideoCodec.h264Qsv, 'yuv422p', '4:2:0 8-bit'), // -> nv12 + (VideoCodec.h265Qsv, 'yuv422p10le', '4:2:0 10-bit'), // -> p010le + (VideoCodec.h264Qsv, 'yuv420p', null), // -> None + ]; + + for (final (codec, pixFmt, expected) in cases) { + final msg = warn(codec, pixFmt); + if (expected == null) { + expect(msg, isNull, + reason: '$pixFmt into ${codec.value} is encodable as-is'); + } else { + expect(msg, isNotNull, + reason: '$pixFmt into ${codec.value} must warn'); + expect(msg, contains(expected), + reason: '$pixFmt into ${codec.value} converts to $expected'); + } + } + }); + + test('encoders that negotiate correctly are never warned about', () { + // VideoToolbox never advertises a mode it lacks. AMF has been pinned to + // nv12 unconditionally since issue #51, so it always converts and there + // is no surprise. Software and ProRes take 4:2:2 natively. + for (final codec in [ + VideoCodec.h264, + VideoCodec.h265, + VideoCodec.h264Videotoolbox, + VideoCodec.h265Videotoolbox, + VideoCodec.h264Amf, + VideoCodec.h265Amf, + VideoCodec.prores422, + VideoCodec.ffv1, + ]) { + expect(warn(codec, 'yuv422p10le'), isNull, + reason: '${codec.value} should not warn'); + } + }); + + test('an explicit 4:2:2 choice warns whatever the source was', () { + // The conversion is done by the pipeline, so a 4:2:0 source reaches the + // encoder as 4:2:2 all the same — this is the second route into #74. + final msg = + warn(VideoCodec.h265Nvenc, 'yuv420p', ChromaSubsampling.yuv422p10); + expect(msg, isNotNull); + expect(msg, contains('4:2:2 10-bit')); + expect(msg, contains('4:2:0 10-bit')); + }); + + test('the new 4:2:0 10-bit option is the way out, and is silent', () { + // The whole point of adding it: a 10-bit source keeping its grading on a + // GPU encoder, with nothing to warn about. + expect( + warn(VideoCodec.h265Nvenc, 'yuv422p10le', + ChromaSubsampling.yuv420p10), + isNull); + // H.264 still cannot do 10-bit, so it still warns. + expect( + warn(VideoCodec.h264Nvenc, 'yuv422p10le', + ChromaSubsampling.yuv420p10), + contains('4:2:0 8-bit')); + }); + + test('4:2:0 8-bit is always safe', () { + for (final codec in [ + VideoCodec.h264Nvenc, + VideoCodec.h265Nvenc, + VideoCodec.h264Qsv, + VideoCodec.h265Qsv, + ]) { + expect(warn(codec, 'yuv422p10le', ChromaSubsampling.yuv420), isNull); + } + }); + + test('silent before a file is loaded', () { + // "Match source" cannot be judged without knowing the source. + expect(warn(VideoCodec.h264Nvenc, null), isNull); + // But an explicit choice can be, and still warns. + expect( + warn(VideoCodec.h264Nvenc, null, ChromaSubsampling.yuv422), isNotNull); + }); + + test('H.264 is told which way out actually helps', () { + // Switching to H.265 keeps the depth; only software keeps the chroma. + final msg = warn(VideoCodec.h264Nvenc, 'yuv422p10le'); + expect(msg, contains('H.265')); + final chromaOnly = warn(VideoCodec.h265Nvenc, 'yuv422p'); + expect(chromaOnly, contains('software')); + expect(chromaOnly, isNot(contains('H.265'))); + }); + }); +} diff --git a/app/test/integration_high_bit_depth_filters_test.dart b/app/test/integration_high_bit_depth_filters_test.dart index 53da129..fab2fc5 100644 --- a/app/test/integration_high_bit_depth_filters_test.dart +++ b/app/test/integration_high_bit_depth_filters_test.dart @@ -902,6 +902,17 @@ void main() { expect(out['profile'], 'High'); }, timeout: const Timeout(Duration(minutes: 5))); + test('4:2:0 10-bit keeps the precision and drops only the chroma', () async { + // The option added for issue #74: NVENC, QSV and AMF can all encode + // 10-bit 4:2:0 and none of them can encode 4:2:2 on most hardware, so + // this is how a 10-bit source keeps its grading through a GPU encoder. + // High 10 rather than High, and 4:2:0 rather than 4:2:2 — getting either + // half wrong would defeat the purpose. + final out = await encodeWith(ChromaSubsampling.yuv420p10, 'yuv420p10'); + expect(out['pix_fmt'], 'yuv420p10le'); + expect(out['profile'], 'High 10'); + }, timeout: const Timeout(Duration(minutes: 5))); + test('4:2:2 gives an 8-bit 4:2:2 file', () async { final out = await encodeWith(ChromaSubsampling.yuv422, 'yuv422'); expect(out['pix_fmt'], 'yuv422p'); diff --git a/app/test/pixel_format_test.dart b/app/test/pixel_format_test.dart index 23f9481..e3f21e4 100644 --- a/app/test/pixel_format_test.dart +++ b/app/test/pixel_format_test.dart @@ -158,6 +158,7 @@ void main() { // that forgets it would silently stop warning. expect(ChromaSubsampling.original.outputBitDepth, isNull); expect(ChromaSubsampling.yuv420.outputBitDepth, 8); + expect(ChromaSubsampling.yuv420p10.outputBitDepth, 10); expect(ChromaSubsampling.yuv422.outputBitDepth, 8); expect(ChromaSubsampling.yuv422p10.outputBitDepth, 10); }); @@ -168,6 +169,7 @@ void main() { // the wire format. A mismatch makes the worker reject the job config. expect(ChromaSubsampling.original.value, 'original'); expect(ChromaSubsampling.yuv420.value, 'yuv420'); + expect(ChromaSubsampling.yuv420p10.value, 'yuv420p10'); expect(ChromaSubsampling.yuv422.value, 'yuv422'); expect(ChromaSubsampling.yuv422p10.value, 'yuv422p10'); }); diff --git a/app/test/settings_colour_pipeline_help_test.dart b/app/test/settings_colour_pipeline_help_test.dart new file mode 100644 index 0000000..09aff23 --- /dev/null +++ b/app/test/settings_colour_pipeline_help_test.dart @@ -0,0 +1,124 @@ +// The second colour help dialog: not "which format do I pick" (that is +// settings_chroma_help_test.dart) but "what does VapourBox actually do to my +// colour". Added for people who want to reason about the output rather than +// just choose from a list. +// +// The same two failures the first dialog already hit are pinned here, because +// nothing about them was specific to that one: prose in a const drifts away +// from the code it describes, and a multi-paragraph explanation that cannot +// scroll runs off the bottom of a small window with no way to read the rest. + +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; + +import 'package:vapourbox/views/settings/settings_dialog.dart'; + +String get _helpText => + colourPipelineHelpSections.map((s) => '${s.$1}\n${s.$2}').join('\n\n'); + +Widget _harness(Brightness brightness) => MaterialApp( + theme: ThemeData( + colorScheme: + ColorScheme.fromSeed(seedColor: Colors.blue, brightness: brightness), + useMaterial3: true, + ), + home: const Scaffold(body: Center(child: ColourPipelineHelpIcon())), + ); + +void main() { + group('the pipeline explanation', () { + test('covers every stage a frame passes through', () { + // Each of these is a real, separately-observable stage. Dropping one + // leaves a gap exactly where someone would be trying to reason. + expect(_helpText.toLowerCase(), contains('ffmpeg')); + expect(_helpText.toLowerCase(), contains('pipe')); + expect(_helpText.toLowerCase(), contains('dither')); + expect(_helpText.toLowerCase(), contains('encoder')); + }); + + test('explains the two things that surprise people', () { + // The Y4M pipe silently dropping aspect and colour tags is why the output + // has to be re-stamped, and is invisible from outside. + expect(_helpText, contains('BT.601')); + expect(_helpText.toLowerCase(), contains('untagged')); + // And a hardware encoder advertising a mode its card lacks is issue #74. + expect(_helpText, contains('4:2:2')); + expect(_helpText, contains('NVENC')); + }); + + test('says values are rescaled rather than taken literally', () { + // The 8-bit-vocabulary rule is the single most useful thing here for + // anyone tuning a filter on a 10-bit source. + expect(_helpText, contains('0–255')); + expect(_helpText.toLowerCase(), contains('rescal')); + }); + + test('every section has a heading and a real body', () { + expect(colourPipelineHelpSections, isNotEmpty); + for (final (heading, body) in colourPipelineHelpSections) { + expect(heading.trim(), isNotEmpty); + expect(body.trim().length, greaterThan(80), + reason: '"$heading" has no real explanation under it'); + } + }); + }); + + group('the icon', () { + testWidgets('opens the dialog on click', (tester) async { + await tester.pumpWidget(_harness(Brightness.dark)); + await tester.tap(find.byType(IconButton)); + await tester.pumpAndSettle(); + + expect(find.text('How VapourBox handles colour'), findsOneWidget); + for (final (heading, _) in colourPipelineHelpSections) { + expect(find.text(heading), findsOneWidget); + } + }); + + testWidgets('is distinguishable from the format help beside it', ( + tester, + ) async { + // Two adjacent buttons drawn the same way read as one control repeated. + await tester.pumpWidget(_harness(Brightness.light)); + final icon = tester.widget(find.byType(Icon)); + expect(icon.icon, isNot(Icons.info_outline), + reason: 'ChromaFormatHelpIcon already uses info_outline'); + expect(icon.semanticLabel, isNotNull, + reason: 'a screen reader must be able to tell them apart too'); + }); + }); + + group('the dialog', () { + testWidgets('scrolls, and fits a small window', (tester) async { + tester.view.physicalSize = const Size(800, 600); + tester.view.devicePixelRatio = 1.0; + addTearDown(tester.view.reset); + + await tester.pumpWidget(_harness(Brightness.dark)); + await tester.tap(find.byType(IconButton)); + await tester.pumpAndSettle(); + expect(tester.takeException(), isNull); + + final scrollable = find.descendant( + of: find.byType(AlertDialog), + matching: find.byType(Scrollable), + ); + expect(scrollable, findsWidgets); + + final last = colourPipelineHelpSections.last.$1; + await tester.drag(scrollable.first, const Offset(0, -4000)); + await tester.pumpAndSettle(); + expect(find.text(last), findsOneWidget); + expect(tester.takeException(), isNull); + }); + + testWidgets('closes again', (tester) async { + await tester.pumpWidget(_harness(Brightness.light)); + await tester.tap(find.byType(IconButton)); + await tester.pumpAndSettle(); + await tester.tap(find.widgetWithText(TextButton, 'Close')); + await tester.pumpAndSettle(); + expect(find.byType(AlertDialog), findsNothing); + }); + }); +}