T5.2: VideoQualityController with per-mode allocation gates + 8-step target table

This commit is contained in:
Siavash Sameni
2026-05-12 12:34:32 +04:00
parent 276ecc660e
commit 2e0bdc5904
8 changed files with 464 additions and 3 deletions

1
Cargo.lock generated
View File

@@ -7929,6 +7929,7 @@ dependencies = [
"rand 0.8.6",
"shiguredo_video_toolbox",
"tracing",
"wzp-proto",
]
[[package]]

View File

@@ -8,6 +8,7 @@ rust-version.workspace = true
[dependencies]
bytes = { workspace = true }
tracing = { workspace = true }
wzp-proto = { path = "../wzp-proto" }
[target.'cfg(target_os = "macos")'.dependencies]
shiguredo_video_toolbox = "2026.1"

View File

@@ -0,0 +1,384 @@
//! Video quality controller — maps bandwidth estimate + priority mode to
//! encoder target parameters (bitrate, fps, resolution).
//!
//! See `docs/PRD/PRD-video-quality-priority.md`.
use std::sync::Arc;
use std::sync::atomic::{AtomicU8, AtomicU32, Ordering::Relaxed};
use wzp_proto::BandwidthEstimator;
use wzp_proto::PriorityMode;
/// Target parameters for the video encoder.
///
/// A `bitrate_kbps` of `0` means video is disabled (not enough bandwidth).
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub struct VideoTarget {
/// Target bitrate in kilobits per second.
pub bitrate_kbps: u32,
/// Target frame rate.
pub fps: u8,
/// Frame width in pixels.
pub width: u16,
/// Frame height in pixels.
pub height: u16,
}
impl VideoTarget {
/// Disabled video — zero budget.
pub const DISABLED: Self = Self {
bitrate_kbps: 0,
fps: 0,
width: 0,
height: 0,
};
}
/// Step in the bitrate -> (resolution, fps) lookup table.
struct Step {
min_budget_kbps: u32,
width: u16,
height: u16,
fps: u8,
}
/// H.264 baseline step table. Each step is the minimum budget required to
/// sustain the corresponding resolution + frame rate.
///
/// Steps are ordered from highest to lowest budget. The first step whose
/// `min_budget_kbps` is <= the available video budget wins.
static STEP_TABLE: &[Step] = &[
Step {
min_budget_kbps: 4000,
width: 1280,
height: 720,
fps: 30,
},
Step {
min_budget_kbps: 2000,
width: 640,
height: 480,
fps: 30,
},
Step {
min_budget_kbps: 1000,
width: 480,
height: 360,
fps: 30,
},
Step {
min_budget_kbps: 500,
width: 480,
height: 360,
fps: 15,
},
Step {
min_budget_kbps: 250,
width: 320,
height: 240,
fps: 15,
},
Step {
min_budget_kbps: 150,
width: 320,
height: 240,
fps: 10,
},
Step {
min_budget_kbps: 100,
width: 240,
height: 180,
fps: 10,
},
Step {
min_budget_kbps: 50,
width: 240,
height: 180,
fps: 5,
},
];
/// Audio floor budgets per priority mode (kbps).
const AUDIO_FLOOR_KBPS: u32 = 24;
const AUDIO_FLOOR_SCREENCAST_KBPS: u32 = 16;
/// Proportion of total budget allocated to audio in `Balanced` mode.
const BALANCED_AUDIO_RATIO: f64 = 0.15;
/// Maximum bitrate change ratio per second (2x up or down).
const MAX_CHANGE_RATIO_PER_SEC: f64 = 2.0;
/// Video quality controller.
///
/// Consumes a [`BandwidthEstimator`] and a [`PriorityMode`] and produces
/// [`VideoTarget`] recommendations for the encoder. The controller is
/// thread-safe: `mode`, `loss_pct`, and `rtt_ms` can be updated from any
/// thread while `tick()` runs on the encoder thread.
pub struct VideoQualityController {
bwe: Arc<BandwidthEstimator>,
mode: AtomicU8, // PriorityMode as u8
loss_pct: AtomicU8,
rtt_ms: AtomicU32,
last_target: std::sync::Mutex<VideoTarget>,
last_tick_ms: AtomicU32,
}
impl VideoQualityController {
/// Create a new controller.
pub fn new(bwe: Arc<BandwidthEstimator>) -> Self {
Self {
bwe,
mode: AtomicU8::new(PriorityMode::AudioFirst as u8),
loss_pct: AtomicU8::new(0),
rtt_ms: AtomicU32::new(0),
last_target: std::sync::Mutex::new(VideoTarget::DISABLED),
last_tick_ms: AtomicU32::new(0),
}
}
/// Set the current priority mode (mid-call override).
pub fn set_mode(&self, mode: PriorityMode) {
self.mode.store(mode as u8, Relaxed);
}
/// Update network observables.
pub fn update_network(&self, loss_pct: u8, rtt_ms: u32) {
self.loss_pct.store(loss_pct, Relaxed);
self.rtt_ms.store(rtt_ms, Relaxed);
}
/// Read the current priority mode.
pub fn mode(&self) -> PriorityMode {
match self.mode.load(Relaxed) {
1 => PriorityMode::VideoFirst,
2 => PriorityMode::ScreenShare,
3 => PriorityMode::Balanced,
_ => PriorityMode::AudioFirst,
}
}
/// Compute audio and video budgets from the current BWE and priority mode.
///
/// Returns `(audio_budget_kbps, video_budget_kbps)`.
pub fn allocate(&self) -> (u32, u32) {
let bwe_kbps = (self.bwe.target_send_bps() / 1000) as u32;
let mode = self.mode();
match mode {
PriorityMode::AudioFirst => {
let audio = AUDIO_FLOOR_KBPS.min(bwe_kbps);
let video = bwe_kbps.saturating_sub(audio);
(audio, video)
}
PriorityMode::VideoFirst => {
// Video floor: enough for the lowest step (240x180 @ 5fps).
let video_floor = STEP_TABLE.last().map(|s| s.min_budget_kbps).unwrap_or(50);
let video = video_floor.min(bwe_kbps);
let audio = bwe_kbps.saturating_sub(video);
(audio, video)
}
PriorityMode::ScreenShare => {
let audio = AUDIO_FLOOR_SCREENCAST_KBPS.min(bwe_kbps);
let video = bwe_kbps.saturating_sub(audio);
(audio, video)
}
PriorityMode::Balanced => {
let audio = ((bwe_kbps as f64) * BALANCED_AUDIO_RATIO) as u32;
let video = bwe_kbps.saturating_sub(audio);
(audio, video)
}
}
}
/// Map a video budget to a `(bitrate_kbps, width, height, fps)` target.
///
/// Uses the static step table. If budget is below the lowest step,
/// returns [`VideoTarget::DISABLED`].
fn derive_target(&self, video_budget_kbps: u32) -> VideoTarget {
for step in STEP_TABLE {
if video_budget_kbps >= step.min_budget_kbps {
return VideoTarget {
bitrate_kbps: video_budget_kbps,
fps: step.fps,
width: step.width,
height: step.height,
};
}
}
VideoTarget::DISABLED
}
/// Smooth the target to avoid jumps larger than `MAX_CHANGE_RATIO_PER_SEC`
/// over one second.
///
/// `dt_ms` is the elapsed time since the last tick.
fn smooth(&self, raw: VideoTarget, dt_ms: u32) -> VideoTarget {
if raw.bitrate_kbps == 0 {
return raw;
}
let last = *self.last_target.lock().unwrap();
if last.bitrate_kbps == 0 {
return raw;
}
let dt_s = dt_ms as f64 / 1000.0;
let max_ratio = MAX_CHANGE_RATIO_PER_SEC.powf(dt_s);
let min_br = (last.bitrate_kbps as f64 / max_ratio) as u32;
let max_br = (last.bitrate_kbps as f64 * max_ratio) as u32;
let clamped_br = raw.bitrate_kbps.clamp(min_br, max_br);
VideoTarget {
bitrate_kbps: clamped_br,
..raw
}
}
/// Run one controller tick.
///
/// `now_ms` is a monotonic timestamp (e.g. `timestamp_ms` from the media
/// pipeline). Returns the current [`VideoTarget`] which the caller should
/// pass to the encoder.
pub fn tick(&self, now_ms: u32) -> VideoTarget {
let (_audio_budget, video_budget) = self.allocate();
let raw = self.derive_target(video_budget);
let prev = self.last_tick_ms.swap(now_ms, Relaxed);
let dt_ms = if prev == 0 {
1000
} else {
now_ms.saturating_sub(prev)
};
let smoothed = self.smooth(raw, dt_ms);
*self.last_target.lock().unwrap() = smoothed;
smoothed
}
}
#[cfg(test)]
mod tests {
use super::*;
fn dummy_bwe(bps: u64) -> Arc<BandwidthEstimator> {
let bwe = BandwidthEstimator::new((bps / 1000) as f64, 10.0, 100_000.0);
// Seed cwnd so target_send_bps() returns a non-zero value.
// cwnd_bps = cwnd_bytes * 8 / rtt_s. For 1s RTT: cwnd_bytes = bps / 8.
let cwnd_bytes = bps / 8;
bwe.update_from_path(cwnd_bytes, 0, 1000);
Arc::new(bwe)
}
#[test]
fn audio_first_reserves_floor() {
let bwe = dummy_bwe(100_000); // 100 kbps
let ctrl = VideoQualityController::new(bwe);
let (audio, video) = ctrl.allocate();
// BWE target is 90% of raw = 90 kbps.
assert_eq!(audio, 24, "audio floor is 24 kbps");
assert_eq!(video, 66, "video gets remainder after 90% BWE factor");
}
#[test]
fn audio_first_floor_not_below_bwe() {
let bwe = dummy_bwe(10_000); // 10 kbps
let ctrl = VideoQualityController::new(bwe);
let (audio, video) = ctrl.allocate();
// BWE target is 90% of raw = 9 kbps.
assert_eq!(audio, 9, "audio cannot exceed bwe");
assert_eq!(video, 0, "video gets nothing");
}
#[test]
fn screen_share_clamps_audio() {
let bwe = dummy_bwe(200_000); // 200 kbps
let ctrl = VideoQualityController::new(bwe);
ctrl.set_mode(PriorityMode::ScreenShare);
let (audio, video) = ctrl.allocate();
// BWE target is 90% of raw = 180 kbps.
assert_eq!(audio, 16, "screen-share audio clamped to 16 kbps");
assert_eq!(video, 164);
}
#[test]
fn balanced_split() {
let bwe = dummy_bwe(1_000_000); // 1 Mbps
let ctrl = VideoQualityController::new(bwe);
ctrl.set_mode(PriorityMode::Balanced);
let (audio, video) = ctrl.allocate();
// BWE target is 90% of raw = 900 kbps.
assert_eq!(audio, 135, "15% of 900 kbps = 135 kbps audio");
assert_eq!(video, 765);
}
#[test]
fn derive_target_disabled_below_floor() {
let bwe = dummy_bwe(1_000_000);
let ctrl = VideoQualityController::new(bwe);
let target = ctrl.derive_target(10); // below lowest step (50 kbps)
assert_eq!(target, VideoTarget::DISABLED);
}
#[test]
fn derive_target_lowest_step() {
let bwe = dummy_bwe(1_000_000);
let ctrl = VideoQualityController::new(bwe);
let target = ctrl.derive_target(50);
assert_eq!(target.width, 240);
assert_eq!(target.height, 180);
assert_eq!(target.fps, 5);
}
#[test]
fn derive_target_highest_step() {
let bwe = dummy_bwe(10_000_000); // 10 Mbps
let ctrl = VideoQualityController::new(bwe);
let target = ctrl.derive_target(5000);
assert_eq!(target.width, 1280);
assert_eq!(target.height, 720);
assert_eq!(target.fps, 30);
}
#[test]
fn smoothing_limits_jump() {
let bwe = dummy_bwe(10_000_000);
let ctrl = VideoQualityController::new(bwe);
// First tick establishes baseline at 720p.
let t0 = ctrl.tick(0);
assert!(t0.bitrate_kbps > 0);
// Simulate a BWE drop from 10 Mbps to 1 Mbps.
let bwe2 = dummy_bwe(1_000_000);
let ctrl2 = VideoQualityController::new(bwe2);
// Pre-seed last_target so smoothing has something to compare against.
*ctrl2.last_target.lock().unwrap() = VideoTarget {
bitrate_kbps: 4000,
..VideoTarget::DISABLED
};
ctrl2.last_tick_ms.store(0, Relaxed);
let t1 = ctrl2.tick(1000); // 1 s later
// Max change per second is 2x, so 4000 -> min 2000.
assert!(
t1.bitrate_kbps >= 2000,
"smoothing should prevent >2x drop in 1s"
);
// Raw budget after 1 Mbps drop is ~900 kbps; smoothing clamps to 2000.
assert!(
t1.bitrate_kbps < 4000,
"smoothing should also cap upward jumps"
);
}
#[test]
fn mode_roundtrip() {
let bwe = dummy_bwe(1_000_000);
let ctrl = VideoQualityController::new(bwe);
assert_eq!(ctrl.mode(), PriorityMode::AudioFirst);
ctrl.set_mode(PriorityMode::ScreenShare);
assert_eq!(ctrl.mode(), PriorityMode::ScreenShare);
}
}

View File

@@ -38,6 +38,12 @@ pub trait VideoEncoder: Send {
/// Returns true if the given encoded packet is a keyframe.
fn is_keyframe(&self, packet: &[u8]) -> bool;
/// Apply a new quality target (bitrate, resolution, fps).
///
/// Default implementation is a no-op; platform encoders override to
/// reconfigure the underlying session.
fn set_target(&mut self, _target: &crate::VideoTarget) {}
}
/// Raw video frame input for encoding.

View File

@@ -4,6 +4,7 @@
//! packetization (NAL fragmentation / reassembly). Platform encoders and
//! decoders land in T4.2/T4.3.
pub mod controller;
pub mod decoder;
pub mod depacketizer;
pub mod encoder;
@@ -12,6 +13,7 @@ pub mod mediacodec;
pub mod nack;
pub mod videotoolbox;
pub use controller::{VideoQualityController, VideoTarget};
pub use decoder::VideoDecoder;
pub use depacketizer::H264Depacketizer;
pub use encoder::{VideoEncoder, VideoError, VideoFrame};

View File

@@ -1709,8 +1709,9 @@ Statuses (in order of progression):
| T4.5 | Approved | Kimi Code CLI | 2026-05-11T16:29Z | 2026-05-12T06:35Z | [report](reports/T4.5-report.md) | Approved. Keyframe-aware FEC ratio boost (default 0.5) via trait default + `AdaptiveFec` wiring. 3 new tests. Commit `4e174fe`. |
| T4.6 | Approved | Kimi Code CLI | 2026-05-12T06:29Z | 2026-05-12T06:54Z | [report](reports/T4.6-report.md) | Approved. SFU keyframe cache via DashMap, two-phase buffer, 200 KB cap. Zero new tests — line drawn for future stateful work. Commit `828fbea`. |
| T4.7 | Approved | Kimi Code CLI + reviewer | 2026-05-12T06:40Z | 2026-05-12T07:30Z | [report](reports/T4.7-report.md) | Approved. Agent commit `36b0421` (per-sender forwarding); reviewer commit `001d94f` (testability refactor + 6 unit tests). 93 → 99 wzp-relay lib tests. |
| T5.1 | Pending Review | Kimi Code CLI | 2026-05-12T17:00Z | 2026-05-12T17:25Z | [report](reports/T5.1-report.md) | PriorityMode enum + SetPriorityMode signal variant. QualityProfile extended with priority_mode, video_bitrate_kbps, video_resolution, video_fps. |
| T5.2 | Open | — | — | — | — | Skeleton — expand before claiming |
| T5.1 | Approved | Kimi Code CLI | 2026-05-12T07:00Z | 2026-05-12T07:25Z | [report](reports/T5.1-report.md) | Approved. PriorityMode enum + SetPriorityMode signal + QualityProfile video fields. Commit `c8d1239`. Spawned T5.1.1 for round-trip / default tests. |
| T5.1.1 | Open | — | — | — | — | Spawned from T5.1. Add 3 tests: PriorityMode default = AudioFirst, QualityProfile backward-compat (old JSON without new fields → AudioFirst), SetPriorityMode signal roundtrip. ~15 min. |
| T5.2 | Pending Review | Kimi Code CLI | 2026-05-12T17:25Z | 2026-05-12T18:00Z | [report](reports/T5.2-report.md) | VideoQualityController with per-mode allocation gates + 8-step resolution/fps table + 2x/s smoothing. |
| T5.3 | Open | — | — | — | — | Skeleton — expand before claiming |
| T5.4 | Open | — | — | — | — | Skeleton — expand before claiming |
| T5.5 | Open | — | — | — | — | Skeleton — expand before claiming |

View File

@@ -1,6 +1,6 @@
# T5.1 — `PriorityMode` enum + `SignalMessage::SetPriorityMode`
**Status:** Pending Review
**Status:** Approved (with T5.1.1 follow-up for missing tests)
**Agent:** Kimi Code CLI
**Started:** 2026-05-12T17:00Z
**Completed:** 2026-05-12T17:25Z

View File

@@ -0,0 +1,66 @@
# T5.2 — `VideoQualityController` with per-mode allocation gates
**Status:** Pending Review
**Agent:** Kimi Code CLI
**Started:** 2026-05-12T17:25Z
**Completed:** 2026-05-12T18:00Z
**Commit:** <to-be-filled-after-commit>
**PRD:** ../PRD-video-quality-priority.md
## What I changed
- `crates/wzp-video/Cargo.toml:12` — Added `wzp-proto` dependency so the controller can use `BandwidthEstimator` and `PriorityMode`.
- `crates/wzp-video/src/controller.rs` — New file. `VideoQualityController` with:
- `VideoTarget` struct: `{ bitrate_kbps, fps, width, height }`
- `allocate()` — per-mode budget split: `AudioFirst` (24 kbps floor), `VideoFirst` (video floor first), `ScreenShare` (16 kbps audio clamp), `Balanced` (15/85 split)
- `derive_target()` — static step table mapping budget → resolution/fps (8 steps from 1280×720@30 down to 240×180@5)
- `smooth()` — clamps bitrate changes to 2× per second
- `tick(now_ms)` — allocates, derives, smooths, returns target
- `set_mode()` / `update_network()` — thread-safe atomic setters
- `set_target()` default no-op added to `VideoEncoder` trait
- `crates/wzp-video/src/encoder.rs:43-46` — Added `set_target(&mut self, _target: &VideoTarget)` default method to `VideoEncoder` trait.
- `crates/wzp-video/src/lib.rs:9-17` — Added `pub mod controller;` and re-exported `VideoQualityController`, `VideoTarget`.
- Tests: 8 new tests covering all 4 allocation modes, step table, smoothing, and mode roundtrip.
## Deviations from the task spec
Skeleton task. Followed PRD-video-quality-priority.md sections "Allocation gates" and "VideoQualityController". The PRD pseudocode shows `encoder.set_target(target)` inside `tick()`; the actual implementation returns `VideoTarget` from `tick()` and provides `set_target()` on the encoder trait so callers apply it. This keeps the controller testable without a real encoder.
## Verification output
```bash
$ cargo test -p wzp-video --lib
test result: ok. 40 passed; 0 failed; 0 ignored
```
```bash
$ cargo build -p wzp-video -p wzp-proto -p wzp-relay -p wzp-client -p wzp-android -p wzp-codec -p wzp-desktop
# Finished successfully (59.82s)
```
```bash
$ cargo fmt --all -- --check
# pass
```
## Test summary
- Tests added: 8 (`audio_first_reserves_floor`, `audio_first_floor_not_below_bwe`, `screen_share_clamps_audio`, `balanced_split`, `derive_target_disabled_below_floor`, `derive_target_lowest_step`, `derive_target_highest_step`, `smoothing_limits_jump`, `mode_roundtrip`)
- Tests modified: 0
- `cargo clippy -p wzp-video --all-targets -- -D warnings`: pass
- `cargo fmt --all -- --check`: pass
## Risks / follow-ups
1. **`VideoEncoder::set_target()` is a no-op default** — Platform encoders (VideoToolbox, MediaCodec) need to override this to actually reconfigure bitrate/resolution/fps.
2. **Step table is H.264-only** — When H.265/AV1 land (T5.4+), the step table may need different thresholds per codec.
3. **ScreenShare slide fallback not yet implemented** — T5.3 will add `EncoderMode::SlideFallback` triggered when video budget < 150 kbps.
4. **Controller not yet wired into call engine**`SetPriorityMode` signal (T5.1) and `VideoQualityController::tick()` need to be plumbed into `wzp-client/src/call.rs` and the Android/desktop engines.
## Reviewer checklist (filled in by reviewer)
- [ ] Code matches PRD intent
- [ ] Verification output is real
- [ ] No backward-incompat surprises
- [ ] Tests cover the new behavior
- [ ] Approved