fix(encode/vulkan): PUNKTFUNK_VULKAN_RGB_DIRECT read "0 " as force-ON
The knob tested `v == "0"` for the disable case and treated everything else that was set as a force-enable. So a trailing space — the kind a `.env` line or a shell script leaves behind — made `PUNKTFUNK_VULKAN_RGB_DIRECT=0 ` mean the exact opposite of what the operator wrote, forcing the RGB-direct encode source on. On a cursor-blend session that is not a subtle difference: the EFC front-end cannot blend, so the pointer disappears from the stream. An empty value and any typo did the same thing. Now parsed like every sibling knob in the crate — `matches!(v.trim(), "1"|"true"|"yes"|"on")` and the matching false spellings — so unrecognised input falls back to the default instead of being read as a force-on. `=0` and `=1` keep working exactly as documented. Splits the parse into a pure function so the accepted spellings are covered by tests. That matters more than it looks: the env-var read itself is untestable in a parallel test binary (the process environment is shared), which is precisely why this knob's behaviour had no coverage and the inverted case survived. Verified with the canonical Linux gate (docker linux/amd64): fmt, clippy --all-targets at default and at nvenc,vulkan-encode,pyrowave, and the pf-encode test leg (37 passed).
This commit is contained in:
@@ -76,11 +76,22 @@ fn quality_request() -> u32 {
|
||||
/// default: ON wherever the probe passes, EXCEPT sessions that may need the CSC's cursor
|
||||
/// blend (see [`VulkanVideoEncoder::open`]). `=0` disables outright; `=1` forces it even on
|
||||
/// cursor-blend sessions (the pointer will be missing from the stream); unset = the default.
|
||||
///
|
||||
/// Parses like every sibling knob (`matches!(v.trim(), …)`): anything unrecognised — including
|
||||
/// an empty value and a value that is only whitespace — falls back to the default rather than
|
||||
/// being read as a force-on. The old `v == "0"` test made `"0 "` mean the *opposite* of what the
|
||||
/// operator wrote, and turned a typo into a forced RGB-direct session with no cursor.
|
||||
fn rgb_request() -> Option<bool> {
|
||||
match std::env::var("PUNKTFUNK_VULKAN_RGB_DIRECT") {
|
||||
Ok(v) if v == "0" => Some(false),
|
||||
Ok(_) => Some(true),
|
||||
Err(_) => None,
|
||||
parse_rgb_request(std::env::var("PUNKTFUNK_VULKAN_RGB_DIRECT").ok().as_deref())
|
||||
}
|
||||
|
||||
/// The pure half of [`rgb_request`], so the accepted spellings are testable without mutating the
|
||||
/// process environment (which no parallel test can do safely).
|
||||
fn parse_rgb_request(raw: Option<&str>) -> Option<bool> {
|
||||
match raw?.trim() {
|
||||
"1" | "true" | "yes" | "on" => Some(true),
|
||||
"0" | "false" | "no" | "off" => Some(false),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -4454,7 +4465,7 @@ unsafe fn build_parameters_av1(
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::{build_h265_rps_s0, pick_recovery_slot, VulkanVideoEncoder};
|
||||
use super::{build_h265_rps_s0, parse_rgb_request, pick_recovery_slot, VulkanVideoEncoder};
|
||||
use crate::{Codec, Encoder};
|
||||
use pf_frame::{CapturedFrame, FramePayload, PixelFormat};
|
||||
|
||||
@@ -4743,4 +4754,26 @@ mod tests {
|
||||
dump_smoke(&aus, "rgb.obu");
|
||||
}
|
||||
}
|
||||
|
||||
/// `PUNKTFUNK_VULKAN_RGB_DIRECT` accepts the same spellings as every sibling knob, trimmed.
|
||||
#[test]
|
||||
fn rgb_direct_knob_accepts_the_house_spellings() {
|
||||
for on in ["1", "true", "yes", "on", " 1", "1 ", "\ton\n"] {
|
||||
assert_eq!(parse_rgb_request(Some(on)), Some(true), "{on:?}");
|
||||
}
|
||||
for off in ["0", "false", "no", "off", " 0", "0 ", "\toff\n"] {
|
||||
assert_eq!(parse_rgb_request(Some(off)), Some(false), "{off:?}");
|
||||
}
|
||||
}
|
||||
|
||||
/// Unset, empty and unrecognised values all mean "use the default" — never a force-on. The
|
||||
/// pre-fix parse read anything-but-`"0"` as a force, so a trailing space on `PUNKTFUNK_...=0 `
|
||||
/// silently enabled the very path the operator was disabling, cursor and all.
|
||||
#[test]
|
||||
fn rgb_direct_knob_never_force_enables_on_an_unrecognised_value() {
|
||||
assert_eq!(parse_rgb_request(None), None);
|
||||
for junk in ["", " ", "2", "maybe", "0x0", "enabled"] {
|
||||
assert_eq!(parse_rgb_request(Some(junk)), None, "{junk:?}");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user