Repository navigation
Video: probe the auto encoder in the background (NVENC then CPU) - #93
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f4648845ad
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } else { None }; | ||
| let input_size = resized.unwrap_or(input_size); | ||
| let asked = crate::capture_setting::requested_chroma(&rpc_fields).unwrap_or(settings.borrow().chroma); | ||
| let capture_selection = crate::video_select::selection_for_chroma(capture_selection, asked, capture_probe.as_ref()); |
There was a problem hiding this comment.
Expose the probed 4:4:4 fallback in device capabilities
When auto runs on a non-NVIDIA host and both VAAPI and CPU probes pass, video_selection.encoder is VAAPI while this line makes CPU the 4:4:4 fallback. However, the connected callback still passes full_chroma_for(encoder) to device_capability (src/webrtc.rs:187-190), which is false for VAAPI; src/capability.rs:37-39 documents that the official viewer locks chroma when 4:4:4 is omitted. Consequently the viewer never sends field 7, so this new fallback is unreachable and auto users lose the 真彩 option they had under legacy CPU selection; compute advertised chroma support from session_probe/encoder_for_chroma as well.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in f7acd7f: device_capability now gets advertise_full_chroma(selection, session_probe), which advertises 4:4:4 when the selected encoder supports it, or when the source is auto and a probed fallback that supports 4:4:4 passed. Explicit vaapi (setting or UU_VIDEO_ENCODER) still does not advertise it. Covered in video_select tests.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f7acd7f0b7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| virtual_audio::init(); | ||
| let _audio_cleanup=virtual_audio::Cleanup; | ||
| log::info!("Starting uu-remote-linux host v{}...", env!("CARGO_PKG_VERSION")); | ||
| video::warm_auto_probe(); |
There was a problem hiding this comment.
Probe auto before running desktop-video-probe
When the encoder source is auto on a non-NVIDIA host with working VAAPI, desktop-video-probe returns through the branch at main.rs:490-496 before this warm-up, so the selection() calls in video::probe and probe_input see an empty cache and use the legacy CPU choice. The server later probes and runs VAAPI, meaning the local encoding diagnostic benchmarks and validates a different encoder than production; synchronously complete the auto probe for that subcommand or pass its report into selection.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 762d230: video::probe and probe_input call ensure_auto_probed() before selection(). When the source is auto it runs the bounded probe synchronously (3 s per candidate, cached), so desktop-video-probe (including UU_PROBE_INPUT) encodes with the same encoder as the server. An explicit setting or UU_VIDEO_ENCODER skips it.
| field falls through to the environment, then to the auto probe (NVENC before CPU when an | ||
| NVIDIA driver is loaded, and VAAPI is not tried; otherwise VAAPI then CPU; codec stays H.265 | ||
| on hardware and H.264 on CPU). The probe runs in the background at startup and does not delay |
There was a problem hiding this comment.
Update the stale opt-in-only VAAPI sections
For users following the dedicated VAAPI instructions after reading this new auto-probe description, both README.en.md:441-444 and README.md:386-390 still say VAAPI is opt-in and is never selected automatically, and AGENTS.md:280-283 repeats it. That directly contradicts the new non-NVIDIA auto order here, so the setup guidance should distinguish explicit selection from the new automatic probe.
AGENTS.md reference: AGENTS.md:L275-L278
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 762d230: the VAAPI sections in README.md, README.en.md and AGENTS.md now separate explicit selection (set-encoder vaapi / UU_VIDEO_ENCODER=vaapi) from auto, where VAAPI is probed only when no NVIDIA driver is present.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 762d23068a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| /// Exercise the real capture/encoder chain without UU auth or a network peer. | ||
| pub async fn probe(size:(u32,u32),display:String,portal:Option<Arc<crate::portal::PortalSession>>,path:Option<String>)->Result<(),String> { | ||
| ensure_auto_probed().await; |
There was a problem hiding this comment.
Honor the 4:4:4 fallback in desktop-video-probe
When UU_PROBE_CHROMA=444 is used on a non-NVIDIA host where both VAAPI and CPU pass probing, this warm-up makes selection() resolve to VAAPI; probe_settings() therefore leaves chroma at 4:2:0 and probe() benchmarks VAAPI. Production handles the same viewer request by calling selection_for_chroma(..., report) and moving to CPU 4:4:4, so although the warm-up fixes the previous empty-cache mismatch, this diagnostic still validates a different encoder/chroma path; resolve the requested chroma against the report when choosing both probe settings and encoder.
AGENTS.md reference: AGENTS.md:L169-L169
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 8687ae8: probe_plan() passes UU_PROBE_CHROMA through selection_for_chroma with the cached probe report, the same function sessions use. Auto with both VAAPI and CPU probed encodes on CPU at 4:4:4; if VAAPI is the only passing candidate it stays VAAPI at 4:2:0. Test: probe_chroma_request_moves_auto_vaapi_to_cpu.
| pub(crate) async fn probe_encoder_idr(encoder: Encoder) -> crate::video_select::ProbeOutcome { | ||
| use crate::video_select::{ProbeFail, ProbeOutcome}; | ||
| let settings = VideoSettings { codec: VideoCodec::H264, ..Default::default() }; | ||
| let mut command = ffmpeg_command(); |
There was a problem hiding this comment.
Include native encoders in auto-probe decisions
On an X11 inproc-av build with the default native pipeline, external FFmpeg can lack h264_nvenc/h264_vaapi or fail its process implementation while the built-in native encoder remains usable, but this probe tests only ffmpeg_command(). Its cached failure then makes every later auto session select CPU even though read_encoder_with would run the native engine before the process fallback; on NVIDIA, an immediate pre-probe session can consequently use native NVENC and reconnect on CPU. Check the native engine before letting a process-only failure disqualify the hardware candidate.
AGENTS.md reference: AGENTS.md:L41-L45
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 8687ae8. The native engine needs a real display to open, so there is no synthetic native probe. On inproc-av, when the native engine would run for a candidate, a process-only failure is recorded as native_unprobed instead of disqualifying it, and auto keeps the legacy choice for that candidate (NVENC when NVIDIA is present). Default builds are unchanged. Tests: process_failure_keeps_native_candidate_unprobed, native_unprobed_hardware_uses_legacy_choice.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8687ae8a32
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| /// Session snapshot after a 真彩 request. Uses the probe captured with the session, not a later one. | ||
| pub fn selection_for_chroma(mut selection: Selection, chroma: u64, probed: Option<&ProbeReport>) -> Selection { | ||
| selection.encoder = encoder_for_chroma(selection, chroma, probed).0; |
There was a problem hiding this comment.
Re-select H.264 after the 4:4:4 CPU fallback
On a non-NVIDIA host where auto selected VAAPI and both sides support H.265, preferred_codec_for (src/capture_setting.rs:142-148) has already set VideoSettings.codec to H.265 because the base selection was hardware; this helper changes only the encoder when 真彩 is enabled. The restart therefore uses CPU libx265 4:4:4 even though the cached CPU outcome validated only H.264, forcing an avoidable failed encoder start when libx265 is unavailable (and failing an H.265-only offer) or incurring the documented software-HEVC slowdown instead of the H.264 default for CPU. When the fallback lands on CPU and the codec preference is auto, re-resolve to H.264 if that track was negotiated.
AGENTS.md reference: AGENTS.md:L125-L126
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 38d5848: when 4:4:4 moves an auto-VAAPI session to CPU and the codec preference is auto, the host switches to H.264 if that track was negotiated, and CaptureSettingSync field 13 reports the codec in use. A forced h264 / h265 is left as-is, and an H.265-only offer stays H.265.
|
谢谢 @ferstar。第②步的方向和 #85 里商定的一致:有 NVIDIA 时仍是 NVENC→CPU,探测放在后台,不阻塞第一路连接,显式
验证情况:我只读了 diff,本地没有编译。CI 在 8687ae8 上全部通过。3090 这类机器有 NVIDIA,顺序仍是 NVENC→CPU,行为不变。 🤖 Generated with Claude Code |
|
Thanks. Status of the points above:
|
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 38d58487cf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| put_varint(10, u64::from(size.1), &mut out); | ||
| // Same GvPb_CodecType as CaptureSettingRequest field 13. The viewer learns a host-side | ||
| // switch (真彩 moving auto-VAAPI onto CPU H.264) from this sync, the way field 13 informs it. | ||
| put_varint(13, settings.codec.wire(), &mut out); |
There was a problem hiding this comment.
Verify field 13 before emitting CaptureSettingSync
Whenever a settings RPC is acknowledged, this now serializes field 13 into Message 25, but the repository only identifies codec field 13 on CaptureSettingRequest and the comment transfers that number to CaptureSettingSync by analogy rather than citing an observed field or official log. Protobuf field meanings are message-specific, so the official viewer may ignore this intended codec update or interpret the tag differently; verify the actual Sync descriptor/trace before emitting it.
AGENTS.md reference: AGENTS.md:L275-L276
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
6097324 移除了 CaptureSettingSync field 13 的发送。auto 不再选 VAAPI 后,这条路径不再需要。
| if next.chroma == CHROMA_444 && forced.is_none() { | ||
| next.codec = pick_codec(h264, h265, None, false); |
There was a problem hiding this comment.
Preserve H.265 for NVENC 4:4:4
When codec preference is auto, any capture request that leaves next.chroma == CHROMA_444 enters this branch, not only an auto-VAAPI-to-CPU fallback. On an NVENC session with both codecs negotiated, turning on 真彩 leaves the encoder on NVENC but pick_codec(..., false) changes the working hardware H.265 stream to H.264 (even if field 13 reported H.265); base the codec choice on the resolved encoder or limit this re-selection to an actual CPU fallback.
AGENTS.md reference: AGENTS.md:L197-L200
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
6097324 移除了 CPU 回退时的 codec 重选,NVENC 4:4:4 保持原 codec(H.265 不会被改成 H.264)。
| /// One synthetic H.264 IDR. Failure reasons stay enum values: stderr and device paths are not logged. | ||
| pub(crate) async fn probe_encoder_idr(encoder: Encoder) -> crate::video_select::ProbeOutcome { | ||
| use crate::video_select::{ProbeFail, ProbeOutcome}; | ||
| let settings = VideoSettings { codec: VideoCodec::H264, ..Default::default() }; |
There was a problem hiding this comment.
Validate HEVC before auto-selecting VAAPI
On a non-NVIDIA host whose VAAPI device supports H.264 but not HEVC, while libx265 is installed, this H.264-only probe marks VAAPI as chosen. hevc_available(Vaapi) then succeeds on its CPU fallback, and preferred_codec_for treats the selected encoder as hardware and chooses H.265; read_encoder consequently fails hevc_vaapi and settles on slow CPU libx265 instead of the software H.264 default. Track which encoder passed the HEVC probe or avoid promoting a candidate for an unvalidated default codec.
AGENTS.md reference: AGENTS.md:L125-L126
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
6097324 起 auto 不再把 VAAPI 作为候选(有 NVIDIA 时 NVENC→CPU,否则 CPU),VAAPI 只能显式选择,行为与 main 相同。
VAAPI stays available through explicit selection only until VallierDev#33 is resolved. Removes the auto VAAPI 4:4:4 candidate move, the codec re-selection on CPU fallback and the CaptureSettingSync field 13 emission.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6097324191
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /// True when `read_encoder_with` would open the in-process engine for this candidate on X11. | ||
| fn native_engine_would_run(encoder: Encoder) -> bool { | ||
| #[cfg(feature = "inproc-av")] | ||
| { crate::inproc_encoder::selected(encoder) } |
There was a problem hiding this comment.
Do not preserve native NVENC on non-X11 backends
When a binary is built with inproc-av and uses Wayland, Portal, or KMS, and the FFmpeg NVENC probe fails, this still returns true because inproc_encoder::selected does not inspect the active backend. The fresh evidence after the earlier native-probe fix is that native_inproc in src/video.rs:645-646 explicitly rejects those displays, so the cached NativeUnprobed outcome keeps NVENC even though the session can only run the already-failed process; the stream then advertises hardware capabilities/codecs before falling back to CPU. Include actual backend eligibility in this decision or defer it until the session backend is known.
AGENTS.md reference: AGENTS.md:L41-L41
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
d6e2b32 中,只有原生引擎对当前显示后端可用时(X11 + inproc-av)才记为 native_unprobed 并保留 NVENC;Wayland/Portal/KMS/headless 下进程探测失败会回退 CPU。新增单测 native_unprobed_only_when_x11_inproc_is_eligible。
| // One candidate (CPU, no NVIDIA) has nothing to choose; skip the synthetic IDR. | ||
| let mut outcomes = Vec::with_capacity(order.len()); | ||
| if order.len() == 1 { | ||
| outcomes.push(crate::video_select::ProbeOutcome::Ok); |
There was a problem hiding this comment.
Distinguish an untested CPU from a successful probe
On every auto host without NVIDIA, this branch executes no FFmpeg command but stores ProbeOutcome::Ok, so both the startup log and doctor report cpu=ok. If FFmpeg/libx264 is missing or cannot initialize, the diagnostic therefore claims a successful IDR that never occurred, obscuring why desktop video fails; record a skipped/assumed outcome (or actually probe in diagnostic mode) instead.
AGENTS.md reference: AGENTS.md:L125-L125
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
d6e2b32 新增 ProbeOutcome::NotProbed。没有 NVIDIA 时只有 CPU 一个候选,日志和 doctor 显示 cpu=not probed (only candidate),选择结果仍为 CPU。
|
@codex review |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
谢谢 @ferstar,采用备选方案后范围清楚多了。上次提的第 1、3 点已经处理,第 2 点等 #33 解决后再另开 PR,同意。 我重新看了 d6e2b32 相对 main 的 diff,还有一点合并前想改: NVENC 探测失败的结果不要一直缓存到进程退出 现在 问题出在开机自启:这个探测正好在 user service 启动时跑,那时 NVIDIA 模块或 CUDA 可能还没就绪;GPU 也可能正被别的 CUDA 任务占着;消费级卡还有 NVENC 会话数上限。这些都是暂时性失败,但只要撞上一次,这台机器就会一直用 CPU 编码,可能持续几周,日志里也只有启动时那一行 建议:
其他部分没有问题:没有 NVIDIA 时只用 CPU、跳过探测、记为 验证情况:我只读了 diff,没有在本地编译。CI 在 d6e2b32 上已有 5 项通过, 🤖 Generated with Claude Code |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e0f31c59f2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| fn auto_encoder(nvidia: bool, probed: Option<&ProbeReport>) -> Encoder { | ||
| probed.map(|p| p.chosen).unwrap_or_else(|| legacy_auto(nvidia)) |
There was a problem hiding this comment.
Avoid reporting legacy NVENC from one-shot settings commands
When an already-running NVIDIA host has cached an NVENC probe failure and the user runs uuremote-server set-encoder auto, the settings command is a separate process and therefore has no ProbeReport; this fallback resolves to legacy NVENC, so set_video_pref prints encoder=Nvenc and says it applies to the next connection, while that connection in the running host reads its cached CPU choice. Have the one-shot command run the probe before printing or report auto as unresolved instead of claiming a concrete encoder.
AGENTS.md reference: AGENTS.md:L168-L168
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
一次性的 settings 命令是独立进程,拿不到运行中 host 的探测结果,所以打印的是探测前的规则。这一点已写入 PR 描述的 Known limitations;实际探测结果可以用 doctor 查看。本 PR 不改这一行为。
…in the background A transient NVENC failure at boot (module not ready, GPU busy, session limit) no longer pins auto to CPU until the host restarts. Successful and CPU-only results stay cached.
|
@codex review |
|
关于 NVENC 探测失败结果的缓存,已在 acbc0b2 处理:
|
|
acbc0b2 的 TTL 处理已审,逻辑没问题:成功和 CPU-only 跳过不过期,失败 10 分钟后由桌面连接触发一次后台重探,当次连接仍用缓存,并发由 flight 锁兜住;单测覆盖了失败→过期→重探成功回到 NVENC。阻塞项可以关闭。 CI 失败是与 main 的语义冲突,不是这次改动本身的问题:刚合并的 #95 新增了 小问题,不阻塞: rebase 后 CI 全绿、Codex 没有未决意见,我就 squash 合并。 🤖 Generated with Claude Code |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: acbc0b2e31
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /// does not expire. A second failure restarts the TTL. | ||
| #[test] | ||
| fn failure_then_reprobe_success_selects_nvenc() { | ||
| clear_probe(); |
There was a problem hiding this comment.
Serialize tests that mutate the global probe cache
When cargo test uses its default parallel harness (as the checked CI workflow does), this test and session_before_probe_uses_legacy_auto both call clear_probe/store_probe on the same process-global CACHE. Either test can clear or overwrite the report between the other's assertions (for example, clearing after the failed report is stored makes probe_needs_run(before) unexpectedly true), making the new suite nondeterministic; serialize the whole test bodies with a shared test lock or avoid the global cache.
AGENTS.md reference: AGENTS.md:L66-L66
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
9a6fdb3 新增 PROBE_TEST_LOCK,会改全局探测缓存的两个测试(session_before_probe_uses_legacy_auto、failure_then_reprobe_success_selects_nvenc)在整个测试体内持锁,串行执行。
| pub fn probe_failure_deadline(outcomes: &[ProbeOutcome], now: std::time::Instant) -> Option<std::time::Instant> { | ||
| let failed = outcomes.iter().any(|o| matches!(o, ProbeOutcome::Fail(_))); | ||
| failed.then_some(now + PROBE_FAILURE_TTL) |
There was a problem hiding this comment.
Do not expire successful NVENC results on a CPU miss
When NVENC emits an IDR but the following CPU candidate fails (for example, a custom FFmpeg build has h264_nvenc but not libx264), this any(Fail) gives the otherwise successful NVENC report an expiry. The warm_auto_probe calls then rerun both probes on the next desktop offer after each ten-minute interval, and doctor labels the result as a failure, despite the documented cache rule applying only when NVENC failed and CPU was chosen; derive expiry from the failed hardware candidate/selected fallback, not an unused later candidate.
AGENTS.md reference: AGENTS.md:L125-L125
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
9a6fdb3 中,只有在硬件候选失败且最终选了 CPU 时才设置过期时间;NVENC 成功而 CPU 失败的结果不过期,doctor 也不标为失败。新增单测 nvenc_ok_cpu_fail_does_not_expire。
Adapts video_wake_tests (from VallierDev#95) to the resolve_pair probe argument. Also: serialize the tests that touch the global probe cache, and expire a probe result only when a hardware candidate failed and CPU was chosen.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9a6fdb32cf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| fn auto_encoder(nvidia: bool, probed: Option<&ProbeReport>) -> Encoder { | ||
| probed.map(|p| p.chosen).unwrap_or_else(|| legacy_auto(nvidia)) |
There was a problem hiding this comment.
Invalidate probe reports when NVIDIA availability changes
When the process initially sees no NVIDIA device, it stores the CPU-only NotProbed report without an expiry; if the driver/module or an eGPU appears later, selection() recomputes nvidia == true but this line still returns the cached CPU choice, and warm_auto_probe() will never run again. The inverse leaves a cached NVENC success selected after the device disappears, so later sessions advertise hardware/H.265 before falling back. Treat a report whose order differs from auto_order(nvidia) as stale so the auto rule continues to track the currently available candidates.
AGENTS.md reference: AGENTS.md:L125-L125
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
65bbee2 中,探测结果会记录当时 NVIDIA 是否存在。新的桌面连接开始时,如果 NVIDIA 存在与否变了,就把结果视为过期,在后台重新探测(复用单飞路径),这一路连接先用缓存结果。新增单测 nvidia_presence_change_makes_probe_stale。
| /// Start the auto probe if it is not cached, or the cached hardware failure has expired. | ||
| /// Returns immediately. One probe runs at a time; an in-flight probe is left alone. | ||
| pub fn warm_auto_probe() { | ||
| if !crate::video_select::probe_needs_run(std::time::Instant::now()) { return; } |
There was a problem hiding this comment.
Skip probing when an explicit encoder is selected
When host_settings.json or UU_VIDEO_ENCODER explicitly selects CPU, VAAPI, or NVENC, the normal host still calls warm_auto_probe() unconditionally at startup, and this guard checks only the cache, so it launches the unrelated NVENC/CPU FFmpeg probes anyway. On a GPU already near its NVENC session limit, a first desktop connection can overlap this unused probe and fall back to CPU despite an explicit encoder choice; gate the spawn on selection().encoder_source == Source::Auto, as ensure_auto_probed() already does.
AGENTS.md reference: AGENTS.md:L125-L125
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
65bbee2 中,只有编码器来源是 auto 时 warm_auto_probe 才会探测。设置文件或 UU_VIDEO_ENCODER 显式指定编码器时,启动时和每次连接都不再探测。新增单测 warm_probe_only_when_encoder_source_is_auto。
…explicit encoders
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 65bbee236b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if let Some(cached) = crate::video_select::cached_probe() { | ||
| if !crate::video_select::probe_stale(&cached, now, nvidia) { return cached; } | ||
| } | ||
| let retry = crate::video_select::cached_probe().is_some(); |
There was a problem hiding this comment.
Reserve the failure-retry label for failed reports
When NVIDIA presence changes after a successful NVENC report or a CPU-only NotProbed report, this is still true merely because a cached report exists, so run_auto_probe later logs retry after failure even though no candidate failed. This misdiagnoses the documented presence-change re-probe path; derive the label from the previous report's failure state (such as expires_at) rather than cache existence.
AGENTS.md reference: AGENTS.md:L125-L125
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
3c4fb3c 中,日志标签改为按上一份结果判断:上一份带失败过期时间(expires_at)时记 retry after failure;没有失败、只是 NVIDIA 存在与否变了时记 NVIDIA availability changed;首次探测不带标签。探测行为不变。新增单测 reprobe_reason_follows_previous_state。
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3c4fb3cbf7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| field falls through to the environment, then to auto (NVENC then CPU when an NVIDIA driver | ||
| is loaded; CPU with no probe when it is not; codec stays H.265 on hardware and H.264 on CPU). | ||
| The probe runs in the background at startup and does not delay the first picture. A success | ||
| stays cached until the process exits; an NVENC failure is kept for 10 minutes, and the next |
There was a problem hiding this comment.
Qualify the successful probe cache lifetime
When an NVIDIA device or driver appears or disappears while the host remains running, this statement is false: probe_stale invalidates even a successful report when report.nvidia != nvidia, and the next desktop offer invokes warm_auto_probe. The environment-variable table immediately above already describes that availability-change re-probe, so saying the success lasts until process exit gives users contradictory diagnostics; qualify this sentence and the parallel Chinese paragraph with the NVIDIA-presence exception.
AGENTS.md reference: AGENTS.md:L125-L125
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
84c3119 已在 README.en.md 和 README.md 对应段落补上例外:NVIDIA 存在与否变化时,成功结果也会失效,下一次桌面连接时在后台重新探测。AGENTS.md 的 UU_VIDEO_ENCODER 一行已有这一说明,没有改动。只改文档,代码未变。
|
@codex review |
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
已 squash 合并,感谢 @ferstar 这一轮反复打磨!TTL 重探、NVIDIA 存在性变化失效、显式编码器不探测、测试串行化都已到位,Codex 最后一轮无意见,CI 六项全绿。目前只有单测和 CI 验证,NVIDIA 机器上的实机探测日志待后续版本发布后观察。#85 第③步(运行中切换)另行讨论。 🤖 Generated with Claude Code |
Summary
autoencoder selection probes in the background and does not block the first desktop connection. With an NVIDIA driver the order is NVENC then CPU. Without one the only candidate is CPU and the probe is skipped. VAAPI stays available throughset-encoder vaapiandUU_VIDEO_ENCODER=vaapi.What changed
cpu/vaapi/nvenc(setting file orUU_VIDEO_ENCODER) does not start it. A success and a CPU-only skip stay cached while NVIDIA presence matches the report. A later desktop connection whose presence differs (/proc/driver/nvidia/versionor/dev/nvidia0) treats the report as stale. A hardware-candidate failure (NVENC failed, chosen CPU) expires after 10 minutes (PROBE_FAILURE_TTL). The next desktop connection after either kind of staleness callswarm_auto_probe(non-blocking, one probe in flight) and keeps the cached encoder; a successful retry selects NVENC, another failure restarts the 10 minutes.native_unprobeddoes not expire by time. Failure reasons are an enum (timeout,no_idr,spawn,exit,native_unprobed). A retry logsEncoder probe: retry after failurewith those labels only.doctorappends(failure result, retried after 10 min)when the cached result has a failure deadline, and runs the probe only for auto.--features inproc-av, a process-only failure of NVENC keeps NVENC asnative_unprobedonly when the native engine is eligible for the active display (X11). Wayland, portal, and KMS fall back to CPU.not probed (only candidate); the selected encoder is still CPU.desktop-video-probewaits for the auto probe (ensure_auto_probed). HEVC availability is cached per encoder.doctorprints the probe line (encoder=outcome) viaformat_probe.Not included (VAAPI in auto pending #33, F4)
VAAPI is not an auto candidate in any build until #33 (the FFmpeg process VAAPI path ignores maxrate). There is no auto-VAAPI 4:4:4 fallback and no CaptureSettingSync field 13. F4 is out of scope.
Compatibility
Unset /
autoon a host with no NVIDIA driver selects CPU, the same default as main. A host with NVIDIA defaults to NVENC, the same default as main, and moves to CPU only when the NVENC probe fails. Explicitcpu/vaapi/nvencfromhost_settings.jsonorUU_VIDEO_ENCODERis unchanged and ignores the probe. Codec preference is unchanged: hardware prefers H.265, CPU prefers H.264, setting thenUU_VIDEO_CODECthen auto.Known limitations
The probe is one 256×256 H.264 IDR through the FFmpeg process. It does not open the in-process NVENC or VAAPI engines (those need a display). A process failure that becomes
native_unprobedkeeps the pre-probe NVENC choice on X11 and is not on the 10-minute retry; the stream still falls back to CPU later if that engine fails to start. Wayland, portal, and KMS do not keep NVENC after a failed process probe. The connection that notices an expired failure still encodes with the cached CPU choice until the background retry is stored. The probe does not cover H.265; the existing HEVC probe still runs per encoder.Tests
mbx test --locked --bin uuremote-server(237 passed, 0 failed, 5 ignored),mbx test --locked --features inproc-av --bin uuremote-server(241 passed, 0 failed, 5 ignored), andmbx test --locked --features wayland,portal --bin uuremote-server(244 passed, 0 failed, 7 ignored). New tests:success_and_not_probed_do_not_expire,hardware_failure_expires_after_ttl,failure_then_reprobe_success_selects_nvenc,nvidia_presence_change_makes_probe_stale,warm_probe_only_when_encoder_source_is_auto,reprobe_reason_follows_previous_state. Warnings are the existing webrtc-dtls, webrtc, x11_engine comment, anduuremote-servertest-bin set; no new warnings from this change.