ed8a7ae5aa
Audit: - docs/AUDIT-2026-05-25.md: full protocol audit covering 8 findings (4 critical, 2 high, 5 medium, 4 low) with code references and fix effort estimates - vault/Audit/Tasks.md: Obsidian Tasks plugin file tracking all audit items with priorities, due dates, and per-step checklists Architecture docs updated for Wire format v2 and Wave 5/6 features: - ARCHITECTURE.md: adds wzp-video to dependency graph and project structure; wire format updated to v2 (16B header, 5B MiniHeader); relay concurrency section corrected (DashMap+RwLock is current, not a future optimization); test count 571→702; Android note - PROGRESS.md: Wave 5 and Wave 6 sections appended; test count 372→702; current status and open blockers as of 2026-05-25 - ROAD-TO-VIDEO.md: implementation status table inserted (✅/🟡/🔴/🔲 per phase); 6-step critical path to first video call - WZP-SPEC.md: MediaHeader updated to v2 (16B byte-aligned); MiniHeader updated to 5B with seq_delta; codec IDs 9-12 added (H.264/H.265/AV1); version negotiation section added Obsidian vault (vault/): - 114 files across Architecture/, PRDs/, Reports/, Android/, Reference/, Audit/ with YAML frontmatter - 00 - Home.md index note with wiki links - .obsidian/app.json config Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
109 lines
5.9 KiB
Markdown
109 lines
5.9 KiB
Markdown
---
|
|
tags: [report, wzp]
|
|
type: report
|
|
status: Approved
|
|
---
|
|
|
|
# T1.1 — Add v2 `MediaHeader` type
|
|
|
|
**Status:** Approved
|
|
**Agent:** Kimi Code CLI
|
|
**Started:** 2026-05-11T06:09Z
|
|
**Completed:** 2026-05-11T06:54Z
|
|
**Commit:** see git log
|
|
**PRD:** ../PRD-wire-format-v2.md
|
|
|
|
## What I changed
|
|
|
|
- `crates/wzp-proto/src/packet.rs:20` — renamed existing `MediaHeader` → `MediaHeaderV1` (kept all impls intact)
|
|
- `crates/wzp-proto/src/packet.rs:157` — added `pub type MediaHeader = MediaHeaderV1;` backward-compat alias so the workspace continues to compile
|
|
- `crates/wzp-proto/src/packet.rs:160-238` — added new `MediaHeaderV2` struct (16 bytes, byte-aligned) with `write_to`, `read_from`, and flag accessors
|
|
- `crates/wzp-proto/src/packet.rs:1270-1285` — added `media_header_v2_roundtrip` test
|
|
- `crates/wzp-proto/src/lib.rs:28` — re-exported `MediaHeaderV1` and `MediaHeaderV2`
|
|
- `crates/wzp-proto/src/packet.rs:487-493` — added `impl Default for TrunkFrame` (pre-existing clippy fix)
|
|
- `crates/wzp-proto/src/packet.rs:540` — removed redundant slicing `&buf[..]` → `buf` (pre-existing clippy fix)
|
|
- `crates/wzp-proto/src/quality.rs:102-109` — derived `Default` for `NetworkContext` with `#[default]` on `Unknown` (pre-existing clippy fix)
|
|
|
|
## Why these choices
|
|
|
|
Rust does not allow a type alias and a struct with the same name in the same module. The task requires both (a) keeping the old struct accessible as `MediaHeader` so the workspace builds, and (b) adding a new struct also called `MediaHeader`. The pragmatic resolution is to name the new struct `MediaHeaderV2` and export it; T1.5 will delete `MediaHeaderV1`, remove the alias, and rename `MediaHeaderV2` → `MediaHeader` once all call sites are migrated.
|
|
|
|
`CodecId::to_wire` already returns `u8` and was usable immediately. `MediaType` does not exist yet (T1.2), so the `media_type` field is `u8` with a `// TODO(T1.2)` comment.
|
|
|
|
## Deviations from the task spec
|
|
|
|
1. **Step 3 (struct name):** The new struct is named `MediaHeaderV2` instead of `MediaHeader`. This is required because `pub type MediaHeader = MediaHeaderV1;` occupies the `MediaHeader` name in `packet.rs`. T1.5 will perform the final rename.
|
|
2. **Step 4 (`MediaType` placeholder):** Used `u8` for `media_type` with an inline `// TODO(T1.2)` comment, matching the fallback instruction in the task.
|
|
3. **Clippy fixes:** Fixed three pre-existing clippy errors in `wzp-proto` (`new_without_default`, `redundant_slicing`, `derivable_impls`) so the crate passes `-D warnings`.
|
|
|
|
## Verification output
|
|
|
|
```bash
|
|
$ cargo test -p wzp-proto media_header_v2_roundtrip
|
|
running 1 test
|
|
test packet::tests::media_header_v2_roundtrip ... ok
|
|
|
|
test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 105 filtered out; finished in 0.00s
|
|
```
|
|
|
|
```bash
|
|
$ cargo build -p wzp-proto -p wzp-codec -p wzp-fec -p wzp-crypto -p wzp-transport -p wzp-relay -p wzp-client -p wzp-web -p wzp-native
|
|
Compiling wzp-proto v0.1.0
|
|
...
|
|
Finished `dev` profile [unoptimized + debuginfo] target(s) in 27.24s
|
|
```
|
|
|
|
```bash
|
|
$ cargo test -p wzp-proto -p wzp-codec -p wzp-fec -p wzp-crypto -p wzp-transport -p wzp-relay -p wzp-client -p wzp-web -p wzp-native --no-fail-fast
|
|
...
|
|
test result: ok. 565 passed; 0 failed; ...
|
|
```
|
|
|
|
```bash
|
|
$ cargo clippy -p wzp-proto --all-targets -- -D warnings
|
|
Finished `dev` profile [unoptimized + debuginfo] target(s) in 2.38s
|
|
```
|
|
|
|
```bash
|
|
$ cargo fmt --all -- --check
|
|
# (clean)
|
|
```
|
|
|
|
## Test summary
|
|
|
|
- Tests added: 1 (`media_header_v2_roundtrip`)
|
|
- Tests modified: 0
|
|
- Workspace test count before: 564 pass / 0 fail (non-Android subset)
|
|
- Workspace test count after: 565 pass / 0 fail (non-Android subset)
|
|
- `cargo clippy --workspace --all-targets -- -D warnings`: pass for `wzp-proto`; 3 pre-existing failures remain in `deps/featherchat/warzone/crates/warzone-protocol` (git submodule, outside our control)
|
|
- `cargo fmt --all -- --check`: pass
|
|
|
|
## Risks / follow-ups
|
|
|
|
- Pre-existing clippy errors in the `featherchat` git submodule (`warzone-protocol`) remain unresolved because they are in a dependency subtree.
|
|
- `wzp-android` cannot be built or tested on macOS without the Android NDK. All verification uses the non-Android workspace subset.
|
|
- `MediaHeaderV2` must be renamed to `MediaHeader` in T1.5 after `MediaHeaderV1` is deleted and all call sites are migrated.
|
|
- `media_type: u8` should become `media_type: MediaType` once T1.2 lands.
|
|
|
|
## Reviewer checklist (filled in by reviewer)
|
|
|
|
- [x] Code matches PRD intent
|
|
- [x] Verification output is real (re-run if suspicious) — re-ran `cargo test -p wzp-proto media_header_v2_roundtrip` (1 passed), `cargo clippy -p wzp-proto --all-targets -- -D warnings` (clean), `cargo fmt --all -- --check` (clean).
|
|
- [x] No backward-incompat surprises — `pub type MediaHeader = MediaHeaderV1` alias keeps all current call sites compiling, as the task intended.
|
|
- [x] Tests cover the new behavior
|
|
- [x] Approved
|
|
|
|
### Reviewer notes (2026-05-11)
|
|
|
|
Approved. Two minor follow-ups spawned as standalone tasks:
|
|
|
|
1. **T1.1.1 — Add rustdoc on `MediaHeaderV2` public fields.** Match the `///` doc-comment pattern used by the pre-existing `MediaHeaderV1`. Coding standard #9.
|
|
2. **T1.1.2 — Refresh stale test-count figures in docs.** The "272 tests" figure in `ARCHITECTURE.md` and the TASKS environment-setup block is from an older snapshot; the actual non-Android baseline is 564 (with T1.1's new test, 565). Agent reported the right number; the docs are wrong.
|
|
|
|
Both are non-blocking. T1.2 is claimable independently.
|
|
|
|
### Policy clarifications surfaced by this task
|
|
|
|
- **Pre-existing clippy/fmt fixes are acceptable scope creep** when you are forced to fix them to get a clean `-D warnings` run on the crate you're touching. T1.1 fixed three of these (`TrunkFrame::Default`, `redundant_slicing`, `NetworkContext::Default` derive); all three were disclosed under "Deviations". Continue this pattern — disclose, don't hide.
|
|
- **Naming workaround acceptable.** `MediaHeaderV2` instead of `MediaHeader` is the right call given Rust's type-vs-struct name collision. T1.5 will resolve.
|