fix(stats): wire PipeWire drops, expand Display, purge dead residue
Oracle-driven P1 fix plan. Resolves the StatsSnapshot 'computed but never
consumed' debt that was silently zeroing two real diagnostic fields and
leaving a dozen more unreported.
Bug fix (Oracle step 2):
- state_portal.rs: set_pipewire_dropped(0, 0) and set_queue_depths(0, 0)
were hardcoded, silently discarding real PipeWire diagnostics. Now wires
to self.cap.dropped_count() (with pw_dropped_prev delta tracking) and
self.cap.capture_queue_depth(). The encoded side stays 0 because the
encoder thread exposes no queue-depth API.
Display expansion (Oracle step 1):
- stats.rs: StatsSnapshot::Display now reports 12 previously-silent fields
paired with their existing p95/max counterparts — capture/encoded/sent
frame counts, elapsed_secs, *_avg_ms gap timing, frame_age_avg_ms,
per-stage import/sws/encode/total avg_ms, output_frame_bytes_p95. Each
line of the format string maps to one operational question (cadence,
drops, queue pressure, latency, bandwidth); layout note added.
Dead residue purge (Oracle steps 5 + 6):
- stats.rs: removed record_over_budget method + over_budget_count field
(no caller; total_p95_ms answers the useful question without an
arbitrary budget threshold).
- state.rs: removed InFlightSurface::Allocd variant (never constructed)
and CaptureSource::alloc_frame trait method (prototype leftover; the
sole impl in cap_wlr_screencopy.rs returned None unconditionally).
- cap_wlr_screencopy.rs: removed the alloc_frame stub; updated the
unit-type Frame doc to reference the asynchronicity rationale without
the deleted method.
- cap_portal.rs: removed redundant 'let dropped = dropped;' shadowing
flagged by clippy::redundant_locals (line 849).
Deferred (Oracle step 4 — needs product decision):
- scale_avg/scale_p95/transfer_avg/transfer_p95/send_wait_p95 fields
still appear in Display but producers in the live encode path don't
record them, so they often show misleading zeros. Either add real
EncState timing for scale/transfer stages, or remove the fields from
Display until then.
All 79 unit tests + 3 integration tests still pass. clippy: 0 errors.
Warning count: multiple_fields_never_read on StatsSnapshot,
method_never_used on record_over_budget/dropped_count/capture_queue_depth/
alloc_frame, variant_never_constructed on Allocd, redundant_locals on
dropped — all gone.
This commit is contained in:
@@ -846,7 +846,6 @@ fn pipewire_thread(ctx: PwThreadCtx) {
|
||||
.process({
|
||||
let format_info = format_info.clone();
|
||||
let frame_tx = frame_tx.clone();
|
||||
let dropped = dropped;
|
||||
move |stream, _| {
|
||||
// SAFETY: raw_buf ownership invariant — PipeWire's process callback
|
||||
// contract requires that every buffer acquired via `dequeue_raw_buffer`
|
||||
|
||||
@@ -21,9 +21,9 @@ pub struct CapWlrScreencopy {
|
||||
}
|
||||
|
||||
impl CaptureSource for CapWlrScreencopy {
|
||||
/// Unit type: wlr-screencopy is fully asynchronous — `alloc_frame()`
|
||||
/// always returns `None`. The frame object is created by Dispatch
|
||||
/// impls calling `manager.capture_output()`, not by this method.
|
||||
/// Unit type: wlr-screencopy is fully asynchronous — frame allocation is
|
||||
/// driven by Dispatch impls calling `manager.capture_output()`, so there
|
||||
/// is no synchronous `alloc_frame`-style API on this trait.
|
||||
type Frame = ();
|
||||
|
||||
fn new(
|
||||
@@ -40,14 +40,6 @@ impl CaptureSource for CapWlrScreencopy {
|
||||
})
|
||||
}
|
||||
|
||||
fn alloc_frame(&mut self) -> Option<Self::Frame> {
|
||||
// wlr-screencopy is asynchronous: the Dispatch impl creates a new
|
||||
// ZwlrScreencopyFrameV1 which triggers the buffer allocation flow
|
||||
// (buffer event → negotiate format → create DMA-BUF). This method
|
||||
// always returns None.
|
||||
None
|
||||
}
|
||||
|
||||
fn queue_copy(&mut self, buffer: &WlBuffer, _qh: &QueueHandle<State<Self>>) {
|
||||
if let Some(frame) = &self.current_frame {
|
||||
frame.copy(buffer);
|
||||
|
||||
@@ -65,8 +65,6 @@ pub trait CaptureSource: Sized + 'static {
|
||||
qh: &QueueHandle<State<Self>>,
|
||||
) -> Result<Self>;
|
||||
|
||||
fn alloc_frame(&mut self) -> Option<Self::Frame>;
|
||||
|
||||
fn queue_copy(&mut self, buffer: &WlBuffer, qh: &QueueHandle<State<Self>>);
|
||||
|
||||
fn on_done_with_frame(&mut self, frame: Self::Frame);
|
||||
@@ -193,7 +191,6 @@ pub(crate) enum EncConstructionStage<S: CaptureSource> {
|
||||
pub enum InFlightSurface<S: CaptureSource> {
|
||||
None,
|
||||
AllocQueued,
|
||||
Allocd(S::Frame),
|
||||
CopyQueued {
|
||||
surface: ff::frame::Video,
|
||||
drm_map: ff::ffi::AVDRMFrameDescriptor,
|
||||
|
||||
+9
-2
@@ -338,8 +338,15 @@ impl StatePortal {
|
||||
|
||||
// 每秒输出一次结构化管道统计(仅 --stats 启用时记录日志)
|
||||
if self.args.stats && self.stats.should_snapshot() {
|
||||
self.stats.set_pipewire_dropped(0, 0);
|
||||
self.stats.set_queue_depths(0, 0);
|
||||
// Wire PipeWire drop counter (delta-tracked via pw_dropped_prev) and
|
||||
// capture channel depth. Oracle audit 2026-06-28: previously hardcoded
|
||||
// (0, 0), which silently zeroed two real diagnostic fields.
|
||||
let total_dropped = self.cap.dropped_count();
|
||||
self.stats.set_pipewire_dropped(total_dropped, self.pw_dropped_prev);
|
||||
self.pw_dropped_prev = total_dropped;
|
||||
// capture queue depth is real; encoded side has no exposed depth — the
|
||||
// encoder thread publishes timings only, not a frame queue length.
|
||||
self.stats.set_queue_depths(self.cap.capture_queue_depth(), 0);
|
||||
if let Some(ref enc_thread) = self.enc_thread {
|
||||
while let Ok(timing) = enc_thread.timing_rx.try_recv() {
|
||||
self.stats.record_encode_thread(
|
||||
|
||||
+41
-22
@@ -38,7 +38,6 @@ pub struct PipelineStats {
|
||||
encoded_frames: u64,
|
||||
sent_frames: u64,
|
||||
pipewire_dropped: u64,
|
||||
over_budget_count: u64,
|
||||
/// Count of frames dropped by encode thread due to Y-plane hash dedup
|
||||
/// (EncodeOutcome::SkippedDuplicate). Read from atomic counter set by
|
||||
/// encode thread, computed as delta since previous snapshot.
|
||||
@@ -86,7 +85,6 @@ impl PipelineStats {
|
||||
encoded_frames: 0,
|
||||
sent_frames: 0,
|
||||
pipewire_dropped: 0,
|
||||
over_budget_count: 0,
|
||||
duplicate_frames_skipped: 0,
|
||||
prev_duplicate_frames_skipped: 0,
|
||||
capture_queue_depth: 0,
|
||||
@@ -213,11 +211,6 @@ impl PipelineStats {
|
||||
self.encoded_queue_depth = encoded;
|
||||
}
|
||||
|
||||
/// Record that a frame exceeded its time budget.
|
||||
pub fn record_over_budget(&mut self) {
|
||||
self.over_budget_count += 1;
|
||||
}
|
||||
|
||||
/// Returns true if at least 1 second has elapsed since the last snapshot
|
||||
/// (or since creation). If true, call `snapshot_and_reset` to get the stats.
|
||||
pub fn should_snapshot(&self) -> bool {
|
||||
@@ -236,7 +229,6 @@ impl PipelineStats {
|
||||
encoded_frames: self.encoded_frames,
|
||||
sent_frames: self.sent_frames,
|
||||
pipewire_dropped: self.pipewire_dropped,
|
||||
over_budget_count: self.over_budget_count,
|
||||
duplicate_frames_skipped: self.duplicate_frames_skipped,
|
||||
capture_queue_depth: self.capture_queue_depth,
|
||||
encoded_queue_depth: self.encoded_queue_depth,
|
||||
@@ -275,7 +267,6 @@ impl PipelineStats {
|
||||
self.encoded_frames = 0;
|
||||
self.sent_frames = 0;
|
||||
self.pipewire_dropped = 0;
|
||||
self.over_budget_count = 0;
|
||||
self.duplicate_frames_skipped = 0;
|
||||
self.capture_queue_depth = 0;
|
||||
self.encoded_queue_depth = 0;
|
||||
@@ -310,7 +301,6 @@ pub struct StatsSnapshot {
|
||||
pub encoded_frames: u64,
|
||||
pub sent_frames: u64,
|
||||
pub pipewire_dropped: u64,
|
||||
pub over_budget_count: u64,
|
||||
pub duplicate_frames_skipped: u64,
|
||||
// Queue depths
|
||||
pub capture_queue_depth: usize,
|
||||
@@ -352,43 +342,72 @@ pub struct StatsSnapshot {
|
||||
|
||||
impl std::fmt::Display for StatsSnapshot {
|
||||
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||
// Layout note: each line answers one operational question.
|
||||
// - line 1: throughput (fps + frame counts + window length)
|
||||
// - line 2: drops (PipeWire backlog, encoder over-budget, frame-hash dedup)
|
||||
// - line 3: queue back-pressure (capture + encoded)
|
||||
// - line 4-6: gap timing (avg + p95 + max) — answers "is cadence stable?"
|
||||
// - line 7: capture-to-send age (avg + p95 + max) — answers "how stale?"
|
||||
// - line 8: per-stage encode timing (avg + p95) — answers "where is latency?"
|
||||
// - line 9: output bandwidth (bytes/sec + per-frame p95/max)
|
||||
//
|
||||
// The avg counterparts were computed but never displayed before Oracle
|
||||
// audit 2026-06-28; they pair with the existing p95/max to surface both
|
||||
// central tendency and tail behaviour in the same glance.
|
||||
write!(
|
||||
f,
|
||||
"capture_fps={:.1} encoded_fps={:.1} sent_fps={:.1} \
|
||||
pw_dropped={} over_budget={} duplicate_frames_skipped={} \
|
||||
cap_q={} enc_q={} \
|
||||
cap_gap_p95={:.1}ms cap_gap_max={:.1}ms \
|
||||
enc_gap_p95={:.1}ms enc_gap_max={:.1}ms \
|
||||
sent_gap_p95={:.1}ms sent_gap_max={:.1}ms \
|
||||
frame_age_p95={:.1}ms frame_age_max={:.1}ms \
|
||||
send_wait_p95={:.1}ms \
|
||||
import_p95={:.1}ms scale_p95={:.1}ms transfer_p95={:.1}ms \
|
||||
sws_p95={:.1}ms encode_p95={:.1}ms total_p95={:.1}ms \
|
||||
output_bps={:.0} frame_bytes_max={}",
|
||||
"elapsed={:.1}s capture_fps={:.1} encoded_fps={:.1} sent_fps={:.1} \
|
||||
capture_frames={} encoded_frames={} sent_frames={} \
|
||||
pw_dropped={} duplicate_frames_skipped={} \
|
||||
cap_q={} enc_q={} \
|
||||
cap_gap_avg={:.1}ms cap_gap_p95={:.1}ms cap_gap_max={:.1}ms \
|
||||
enc_gap_avg={:.1}ms enc_gap_p95={:.1}ms enc_gap_max={:.1}ms \
|
||||
sent_gap_avg={:.1}ms sent_gap_p95={:.1}ms sent_gap_max={:.1}ms \
|
||||
frame_age_avg={:.1}ms frame_age_p95={:.1}ms frame_age_max={:.1}ms \
|
||||
send_wait_p95={:.1}ms \
|
||||
import_avg={:.1}ms import_p95={:.1}ms \
|
||||
scale_avg={:.1}ms scale_p95={:.1}ms transfer_avg={:.1}ms transfer_p95={:.1}ms \
|
||||
sws_avg={:.1}ms sws_p95={:.1}ms \
|
||||
encode_avg={:.1}ms encode_p95={:.1}ms total_avg={:.1}ms total_p95={:.1}ms \
|
||||
output_bps={:.0} frame_bytes_p95={} frame_bytes_max={}",
|
||||
self.elapsed_secs,
|
||||
self.capture_fps,
|
||||
self.encoded_fps,
|
||||
self.sent_fps,
|
||||
self.capture_frames,
|
||||
self.encoded_frames,
|
||||
self.sent_frames,
|
||||
self.pipewire_dropped,
|
||||
self.over_budget_count,
|
||||
self.duplicate_frames_skipped,
|
||||
self.capture_queue_depth,
|
||||
self.encoded_queue_depth,
|
||||
self.capture_gap_avg_ms,
|
||||
self.capture_gap_p95_ms,
|
||||
self.capture_gap_max_ms,
|
||||
self.encoded_gap_avg_ms,
|
||||
self.encoded_gap_p95_ms,
|
||||
self.encoded_gap_max_ms,
|
||||
self.sent_gap_avg_ms,
|
||||
self.sent_gap_p95_ms,
|
||||
self.sent_gap_max_ms,
|
||||
self.frame_age_avg_ms,
|
||||
self.frame_age_p95_ms,
|
||||
self.frame_age_max_ms,
|
||||
self.send_wait_p95_ms,
|
||||
self.import_avg_ms,
|
||||
self.import_p95_ms,
|
||||
self.scale_avg_ms,
|
||||
self.scale_p95_ms,
|
||||
self.transfer_avg_ms,
|
||||
self.transfer_p95_ms,
|
||||
self.sws_avg_ms,
|
||||
self.sws_p95_ms,
|
||||
self.encode_avg_ms,
|
||||
self.encode_p95_ms,
|
||||
self.total_avg_ms,
|
||||
self.total_p95_ms,
|
||||
self.output_bytes_per_sec,
|
||||
self.output_frame_bytes_p95,
|
||||
self.output_frame_bytes_max,
|
||||
)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user