diff --git a/docs/superpowers/plans/2026-05-01-groupref-ssrc-discovery.md b/docs/superpowers/plans/2026-05-01-groupref-ssrc-discovery.md new file mode 100644 index 0000000000..39fb15075b --- /dev/null +++ b/docs/superpowers/plans/2026-05-01-groupref-ssrc-discovery.md @@ -0,0 +1,1121 @@ +# GroupInstanceReferenceImpl Reactive SSRC Discovery — Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Replace `GroupInstanceReferenceImpl`'s test-SFU-only `ActiveAudioSsrcs` discovery path with a per-receiver `webrtc::FrameTransformerInterface` that observes incoming SSRCs at the depacketizer-to-decoder boundary, buffers their first ≤1 s of frames, drives an audio recvonly transceiver to be added via the existing `_requestMediaChannelDescriptions` callback, and flushes the buffer once renegotiation completes — restoring audio in real Telegram group calls without a startup gap. + +**Architecture:** A single `GRAudioFrameTransformer` instance is installed on every audio receiver in this `GroupInstanceReferenceInternal` (mid=0 sendrecv outgoing, plus each recvonly transceiver as it's added). It maintains a per-SSRC state machine (`kBuffering` → `kDrained`) and exposes `releaseSsrc(ssrc)` for the discovery flow to call after `onRenegotiationComplete`. A 250 ms debounce coalesces bursts into one renegotiation. The `ActiveAudioSsrcs` data-channel mechanism (test-SFU only) is removed from both the Go SFU and the C++ ReferenceImpl so the tap is the single discovery path in test and production. + +**Tech Stack:** C++17, WebRTC `FrameTransformerInterface` API, Bazel 8.4.2, the existing tgcalls test bench (Go/Pion SFU + `tgcalls_cli` integration tests). + +--- + +## Reference: Spec & key call-sites + +- **Spec:** `docs/superpowers/specs/2026-05-01-groupref-ssrc-discovery-design.md` +- **WebRTC plumbing reference:** `third-party/webrtc/webrtc/api/frame_transformer_interface.h` (the `FrameTransformerInterface` / `TransformableFrameInterface` / `TransformedFrameCallback` contract); `third-party/webrtc/webrtc/media/engine/webrtc_voice_engine.cc:2240-2266` (the unsignaled→signaled stream promotion path that lets buffered frames flow into the same receive stream). +- **C++ files we modify:** `submodules/TgVoipWebrtc/tgcalls/tgcalls/group/GroupInstanceReferenceImpl.cpp` (only `.cpp`; the `.h` doesn't change). +- **Go file we modify:** `submodules/TgVoipWebrtc/tgcalls/tools/go_sfu/sfu.go`. +- **Docs to refresh:** `submodules/TgVoipWebrtc/CLAUDE.md` (ReferenceImpl section), `submodules/TgVoipWebrtc/tgcalls/tools/cli/CLAUDE.md` (group-mode SSRC discovery flow), `submodules/TgVoipWebrtc/tgcalls/tools/go_sfu/CLAUDE.md` (drop the `ActiveAudioSsrcs` mention). + +## File structure + +| File | Change | Responsibility | +|---|---|---| +| `submodules/TgVoipWebrtc/tgcalls/tgcalls/group/GroupInstanceReferenceImpl.cpp` | modify | Add `GRAudioFrameTransformer` (anonymous-namespace), wire into `start()` and `renegotiate()`, add `handleDiscoveredAudioSsrc` / `scheduleDiscoveryRenegotiation`, delete `handleActiveAudioSsrcs` and its dispatch, call `releaseSsrc` in `onRenegotiationComplete`, add `_audioFrameTransformer` and `_discoveryRenegotiationScheduled` members. | +| `submodules/TgVoipWebrtc/tgcalls/tools/go_sfu/sfu.go` | modify | Delete `buildActiveSSRCsMessage`, `broadcastActiveSSRCs`, both call-sites (post-Join goroutine line 330, Leave line 1061). The `participantSSRCs` audio-collection helper used by `broadcastActiveSSRCs` is otherwise unused; remove it. | +| `submodules/TgVoipWebrtc/CLAUDE.md` | modify | Update the "GroupInstanceReferenceImpl" section: SSRC discovery now via tap; remove "ActiveAudioSsrcs" reference; add note that `_audioFrameTransformer` is the seam where e2e decrypt will land. | +| `submodules/TgVoipWebrtc/tgcalls/tools/cli/CLAUDE.md` | modify | Remove the `ActiveAudioSsrcs` line from the group-mode flow description. | +| `submodules/TgVoipWebrtc/tgcalls/tools/go_sfu/CLAUDE.md` | modify | Remove "ActiveAudioSsrcs" from the `sfu.go` bullet. | + +No new files. No iOS code touched. No CLI test-tool code touched (we ride on the existing `--mute-participants` and per-SSRC level invariants from the previous fix). + +--- + +## Test bench reference + +The existing CLI test (built earlier in this branch) is the integration harness. After every code change, the regression sweep is: + +```bash +/Users/isaac/build/telegram/telegram-ios/build-input/bazel-8.4.2-darwin-arm64 \ + build //submodules/TgVoipWebrtc/tgcalls/tools/cli:tgcalls_cli 2>&1 | tail -3 + +# T1: All-Reference, no mute — must SUCCESS +/Users/isaac/build/telegram/telegram-ios/bazel-bin/submodules/TgVoipWebrtc/tgcalls/tools/cli/tgcalls_cli \ + --mode group --participants 0 --reference-participants 3 --duration 6 --quiet +# Expected: "Result: SUCCESS", exit code 0 + +# T2: Mixed (2C+2R) — must SUCCESS +/Users/isaac/build/telegram/telegram-ios/bazel-bin/submodules/TgVoipWebrtc/tgcalls/tools/cli/tgcalls_cli \ + --mode group --participants 2 --reference-participants 2 --duration 6 --quiet + +# T3: All-Reference with P0 muted — must SUCCESS (muted-peer invariant) +/Users/isaac/build/telegram/telegram-ios/bazel-bin/submodules/TgVoipWebrtc/tgcalls/tools/cli/tgcalls_cli \ + --mode group --participants 0 --reference-participants 3 --mute-participants 0 --duration 6 --quiet + +# T4: Mixed muted (custom side muted) — must SUCCESS +/Users/isaac/build/telegram/telegram-ios/bazel-bin/submodules/TgVoipWebrtc/tgcalls/tools/cli/tgcalls_cli \ + --mode group --participants 1 --reference-participants 2 --mute-participants 0 --duration 6 --quiet + +# T5: Mixed muted (reference side muted) — must SUCCESS +/Users/isaac/build/telegram/telegram-ios/bazel-bin/submodules/TgVoipWebrtc/tgcalls/tools/cli/tgcalls_cli \ + --mode group --participants 2 --reference-participants 1 --mute-participants 2 --duration 6 --quiet +``` + +Tasks below name a specific subset of T1–T5 to run as the verification step. Always run **all five** before the final commit of each task — the muted-peer invariant from the audio-level fix is the strongest detector for routing regressions. + +--- + +### Task 1: Delete `ActiveAudioSsrcs` from the test SFU + +**Files:** +- Modify: `submodules/TgVoipWebrtc/tgcalls/tools/go_sfu/sfu.go` + +**Why first:** Forces every subsequent test run to exercise the discovery path we're about to build. Until the tap is in, all-Reference tests will fail (expected); mixed tests will still pass (CustomImpl discovers via raw RTP). This intentional regression is the visible TDD-red for the rest of the plan. + +- [ ] **Step 1: Read the current state** + +Confirm the three locations with: + +```bash +grep -n "broadcastActiveSSRCs\|buildActiveSSRCsMessage" \ + /Users/isaac/build/telegram/telegram-ios/submodules/TgVoipWebrtc/tgcalls/tools/go_sfu/sfu.go +``` + +Expected output: +``` +330: s.broadcastActiveSSRCs() +886:// broadcastActiveSSRCs sends the current set of active audio SSRCs to all connected participants. +888:func (s *SFU) broadcastActiveSSRCs() { +911: msg := buildActiveSSRCsMessage(ssrcs) +986:func buildActiveSSRCsMessage(ssrcs []int32) string { +1061: s.broadcastActiveSSRCs() +``` + +- [ ] **Step 2: Delete the two call-sites** + +Remove the line `s.broadcastActiveSSRCs()` at line 330 (inside the post-Join goroutine, immediately above `s.broadcastActiveVideoSSRCs()`). Remove the line `s.broadcastActiveSSRCs()` at line 1061 (inside `Leave`, immediately above `s.broadcastActiveVideoSSRCs()`). + +- [ ] **Step 3: Delete `broadcastActiveSSRCs` and `buildActiveSSRCsMessage`** + +Delete the entire function `broadcastActiveSSRCs` (the doc-comment at line 886 through the closing `}` of the function around line 916, including the `participantSSRCs` map it builds locally). + +Delete the entire function `buildActiveSSRCsMessage` (line 986 through line 996). + +- [ ] **Step 4: Verify nothing else references them** + +```bash +grep -n "broadcastActiveSSRCs\|buildActiveSSRCsMessage\|ActiveAudioSsrcs" \ + /Users/isaac/build/telegram/telegram-ios/submodules/TgVoipWebrtc/tgcalls/tools/go_sfu/sfu.go +``` + +Expected output: empty. + +- [ ] **Step 5: Build** + +```bash +/Users/isaac/build/telegram/telegram-ios/build-input/bazel-8.4.2-darwin-arm64 \ + build //submodules/TgVoipWebrtc/tgcalls/tools/cli:tgcalls_cli 2>&1 | tail -3 +``` + +Expected: `INFO: Build completed successfully`. + +- [ ] **Step 6: Run T1 (all-Reference) — confirm RED** + +```bash +/Users/isaac/build/telegram/telegram-ios/bazel-bin/submodules/TgVoipWebrtc/tgcalls/tools/cli/tgcalls_cli \ + --mode group --participants 0 --reference-participants 3 --duration 6 --quiet +echo "EXIT=$?" +``` + +Expected: `Result: FAILED`, `Audio received: 0/3`, `EXIT=1`. (No discovery → no recvonly transceivers → no audio.) + +- [ ] **Step 7: Run T2 (mixed) — confirm partial regression** + +```bash +/Users/isaac/build/telegram/telegram-ios/bazel-bin/submodules/TgVoipWebrtc/tgcalls/tools/cli/tgcalls_cli \ + --mode group --participants 2 --reference-participants 2 --duration 6 --quiet +echo "EXIT=$?" +``` + +Expected: `Result: FAILED`. CustomImpl peers will still discover ReferenceImpl audio via raw RTP (so they receive ReferenceImpl audio), but ReferenceImpl peers cannot discover anyone (so they receive nothing). `Audio received` will report something less than 4/4. + +- [ ] **Step 8: Commit the intentional regression** + +```bash +cd /Users/isaac/build/telegram/telegram-ios +git add submodules/TgVoipWebrtc/tgcalls/tools/go_sfu/sfu.go +git commit -m "$(cat <<'EOF' +test sfu: remove ActiveAudioSsrcs broadcast + +The test-only ActiveAudioSsrcs colibri message is being replaced by +ReferenceImpl's per-receiver FrameTransformer-based discovery (next +commits). Removing the message first forces every subsequent test +run to exercise the new code path — keeping the message in place +would let test passes mask production-real bugs. + +T1/T2 currently FAIL as expected; T3-T5 likewise — the muted-peer +invariant cannot hold without working discovery. All restored by the +end of this PR. + +Co-Authored-By: Claude Opus 4.7 (1M context) +EOF +)" +``` + +--- + +### Task 2: Delete `handleActiveAudioSsrcs` from `GroupInstanceReferenceImpl` + +**Files:** +- Modify: `submodules/TgVoipWebrtc/tgcalls/tgcalls/group/GroupInstanceReferenceImpl.cpp` + +**Why now:** The handler is now dead code (no SFU sends `ActiveAudioSsrcs` anymore). Removing it before adding the new path means we don't temporarily have two SSRC-add paths that could both insert the same entry into `_remoteSsrcs`. + +- [ ] **Step 1: Confirm the call sites** + +```bash +grep -n "handleActiveAudioSsrcs\|colibriClass.*ActiveAudio" \ + /Users/isaac/build/telegram/telegram-ios/submodules/TgVoipWebrtc/tgcalls/tgcalls/group/GroupInstanceReferenceImpl.cpp +``` + +Expected: +``` +1060: if (colibriClass == "ActiveAudioSsrcs") { +1061: handleActiveAudioSsrcs(json); +1063: void handleActiveAudioSsrcs(json11::Json const &json) { +``` + +- [ ] **Step 2: Read the dispatch site** + +Read lines 1048–1067 of `GroupInstanceReferenceImpl.cpp` to confirm the surrounding context of `onDataChannelMessage`. + +- [ ] **Step 3: Remove the dispatch (lines 1056–1067)** + +Inside `onDataChannelMessage`, locate the `colibriClass == "ActiveAudioSsrcs"` branch and the comment immediately above (`// ActiveVideoSsrcs and SenderVideoConstraints are handled by the` ... `// setRequestedVideoChannels() in response.`). Delete the JSON parse block, the colibriClass extraction, and the if-branch that calls `handleActiveAudioSsrcs(json)`. + +The remaining body of `onDataChannelMessage` should consist of the `_dataChannelMessageReceived` forwarding only: + +```cpp + void onDataChannelMessage(std::string const &msg) { + // Forward all data channel messages to the application. + // Audio SSRCs are now discovered by the per-receiver frame + // transformer (see GRAudioFrameTransformer); video channel + // requests are app-driven via setRequestedVideoChannels. + if (_dataChannelMessageReceived) { + _dataChannelMessageReceived(msg); + } + } +``` + +- [ ] **Step 4: Delete `handleActiveAudioSsrcs`** + +Delete the entire function `handleActiveAudioSsrcs(json11::Json const &json)` (starts at the line that previously matched `1063: void handleActiveAudioSsrcs(json11::Json const &json) {`, ends at the matching `}`). It's roughly 60 lines including the diff loop and the `_requestMediaChannelDescriptions` call. + +- [ ] **Step 5: Verify nothing else references it** + +```bash +grep -n "handleActiveAudioSsrcs\|ActiveAudioSsrcs" \ + /Users/isaac/build/telegram/telegram-ios/submodules/TgVoipWebrtc/tgcalls/tgcalls/group/GroupInstanceReferenceImpl.cpp +``` + +Expected: empty. + +- [ ] **Step 6: Build** + +```bash +/Users/isaac/build/telegram/telegram-ios/build-input/bazel-8.4.2-darwin-arm64 \ + build //submodules/TgVoipWebrtc/tgcalls/tools/cli:tgcalls_cli 2>&1 | tail -3 +``` + +Expected: `INFO: Build completed successfully`. + +- [ ] **Step 7: Run T1 — still RED, same reason** + +```bash +/Users/isaac/build/telegram/telegram-ios/bazel-bin/submodules/TgVoipWebrtc/tgcalls/tools/cli/tgcalls_cli \ + --mode group --participants 0 --reference-participants 3 --duration 6 --quiet +echo "EXIT=$?" +``` + +Expected: `Result: FAILED`, `EXIT=1`. (No regression added beyond Task 1; we just deleted dead code.) + +- [ ] **Step 8: Commit** + +```bash +cd /Users/isaac/build/telegram/telegram-ios +git add submodules/TgVoipWebrtc/tgcalls/tgcalls/group/GroupInstanceReferenceImpl.cpp +git commit -m "$(cat <<'EOF' +GroupInstanceReferenceImpl: remove handleActiveAudioSsrcs + +ActiveAudioSsrcs was a test-SFU-only colibri message. With the test +SFU no longer sending it, the handler is dead code. Removing now +before introducing the new discovery path so we don't briefly have +two paths inserting into _remoteSsrcs. + +Co-Authored-By: Claude Opus 4.7 (1M context) +EOF +)" +``` + +--- + +### Task 3: Add `GRAudioFrameTransformer` skeleton (no behavior yet) + +**Files:** +- Modify: `submodules/TgVoipWebrtc/tgcalls/tgcalls/group/GroupInstanceReferenceImpl.cpp` + +**Why staged:** Adding the class first as a do-nothing pass-through transformer lets us verify the WebRTC plumbing works (the catch-all transformer never breaks audio, OnTransformedFrame is reachable) before we layer on the per-SSRC state machine. Initially we also install nothing — the class just compiles. + +- [ ] **Step 1: Locate the anonymous namespace insertion point** + +Read lines 109–148 of `GroupInstanceReferenceImpl.cpp` to confirm where the anonymous namespace ends (the `} // anonymous namespace` line). + +- [ ] **Step 2: Add the class right before `} // anonymous namespace`** + +Insert the following code immediately before the closing `} // anonymous namespace` (after `GRCreateSDPObserver`): + +```cpp +// --- Per-receiver audio frame transformer --- +// +// One instance is installed (via `RtpReceiverInterface::SetDepacketizerToDecoderFrameTransformer`) +// on every audio receiver in this GroupInstanceReferenceInternal: +// - mid=0 sendrecv outgoing audio (its receive side acts as the catch-all +// for unsignaled SSRCs); +// - each recvonly audio transceiver added by the SSRC-discovery flow. +// +// Behavior per SSRC: +// - First-sight (no entry yet): notify discovery; buffer the frame. +// - kBuffering (waiting for renegotiation): append to per-SSRC FIFO. +// - kDrained (releaseSsrc has fired): pass through immediately. +// +// `releaseSsrc(ssrc)` is invoked by ReferenceImpl on the media thread +// from `onRenegotiationComplete` once a recvonly transceiver owns the +// SSRC. The transformer drains the per-SSRC FIFO via OnTransformedFrame +// and transitions the entry to kDrained. +// +// `decryptHook` is reserved for the e2e fix (separate PR). Today it is +// nullptr and the transformer just forwards frames unchanged. +class GRAudioFrameTransformer : public webrtc::FrameTransformerInterface { +public: + using SsrcCallback = std::function; + using DecryptHook = std::function; + + GRAudioFrameTransformer(SsrcCallback onNewSsrc, + DecryptHook decrypt, + rtc::Thread* mediaThread) + : _onNewSsrc(std::move(onNewSsrc)), + _decrypt(std::move(decrypt)), + _mediaThread(mediaThread) {} + + // Stub — to be filled in Task 4. + void Transform(std::unique_ptr frame) override { + // Pass-through for now (verifies plumbing). + webrtc::MutexLock lock(&_mu); + const uint32_t ssrc = frame->GetSsrc(); + rtc::scoped_refptr sink; + auto it = _perSsrcSinks.find(ssrc); + if (it != _perSsrcSinks.end()) { + sink = it->second; + } else if (_broadcastSink) { + sink = _broadcastSink; + } + if (!sink) return; + sink->OnTransformedFrame(std::move(frame)); + } + + // Stub — to be filled in Task 4. + void releaseSsrc(uint32_t /*ssrc*/) {} + + void RegisterTransformedFrameCallback( + rtc::scoped_refptr cb) override { + webrtc::MutexLock lock(&_mu); + _broadcastSink = std::move(cb); + } + void RegisterTransformedFrameSinkCallback( + rtc::scoped_refptr cb, + uint32_t ssrc) override { + webrtc::MutexLock lock(&_mu); + _perSsrcSinks[ssrc] = std::move(cb); + } + void UnregisterTransformedFrameCallback() override { + webrtc::MutexLock lock(&_mu); + _broadcastSink = nullptr; + } + void UnregisterTransformedFrameSinkCallback(uint32_t ssrc) override { + webrtc::MutexLock lock(&_mu); + _perSsrcSinks.erase(ssrc); + } + +private: + SsrcCallback _onNewSsrc; + DecryptHook _decrypt; + rtc::Thread* _mediaThread; // identity only; not used for thread-affine asserts in this skeleton + + webrtc::Mutex _mu; + rtc::scoped_refptr _broadcastSink RTC_GUARDED_BY(_mu); + std::map> _perSsrcSinks RTC_GUARDED_BY(_mu); +}; +``` + +- [ ] **Step 3: Add a `webrtc::Mutex` include if missing** + +```bash +grep -n "rtc_base/synchronization/mutex.h\|webrtc::Mutex\b" \ + /Users/isaac/build/telegram/telegram-ios/submodules/TgVoipWebrtc/tgcalls/tgcalls/group/GroupInstanceReferenceImpl.cpp | head -5 +``` + +If the include is missing, add to the includes near the top of the file (after the existing webrtc includes): + +```cpp +#include "rtc_base/synchronization/mutex.h" +``` + +- [ ] **Step 4: Build** + +```bash +/Users/isaac/build/telegram/telegram-ios/build-input/bazel-8.4.2-darwin-arm64 \ + build //submodules/TgVoipWebrtc/tgcalls/tools/cli:tgcalls_cli 2>&1 | tail -10 +``` + +Expected: `INFO: Build completed successfully`. + +If the build fails on `webrtc::Mutex` — try `#include "api/sequence_checker.h"` and use `webrtc::Mutex` from there, or fall back to `std::mutex` (the rest of the file already uses `` via `_audioLevelsMutex` in `tools/cli/group_participant.cpp`; check what's available in `tgcalls/tgcalls/group/`). + +- [ ] **Step 5: Run T1 — still expected to fail (we haven't installed the transformer yet)** + +```bash +/Users/isaac/build/telegram/telegram-ios/bazel-bin/submodules/TgVoipWebrtc/tgcalls/tools/cli/tgcalls_cli \ + --mode group --participants 0 --reference-participants 3 --duration 6 --quiet +echo "EXIT=$?" +``` + +Expected: `Result: FAILED`, `EXIT=1`. The class is defined but unused — same red as Tasks 1–2. + +- [ ] **Step 6: Commit** + +```bash +cd /Users/isaac/build/telegram/telegram-ios +git add submodules/TgVoipWebrtc/tgcalls/tgcalls/group/GroupInstanceReferenceImpl.cpp +git commit -m "$(cat <<'EOF' +GroupInstanceReferenceImpl: add GRAudioFrameTransformer skeleton + +Pass-through frame transformer scaffolding. Behavior (per-SSRC +buffering, releaseSsrc, decrypt hook) lands in the next commit. + +Co-Authored-By: Claude Opus 4.7 (1M context) +EOF +)" +``` + +--- + +### Task 4: Implement the per-SSRC state machine in `GRAudioFrameTransformer` + +**Files:** +- Modify: `submodules/TgVoipWebrtc/tgcalls/tgcalls/group/GroupInstanceReferenceImpl.cpp` (the class added in Task 3) + +**Why staged:** Once installed (Task 5), this class becomes part of every audio frame's path. Splitting "skeleton" from "behavior" makes the diff small enough to review carefully — bugs here silently drop audio. + +- [ ] **Step 1: Add the `Entry` struct, constants, and `_entries` map to the class** + +Inside `class GRAudioFrameTransformer` (after `_perSsrcSinks` declaration), add: + +```cpp +private: + enum class SsrcState { kBuffering, kDrained }; + + struct Entry { + SsrcState state = SsrcState::kBuffering; + std::deque> buffer; + int64_t firstFrameTimeMs = 0; + }; + + static constexpr int64_t kSsrcDiscoveryTimeoutMs = 1000; + static constexpr size_t kMaxBufferedFramesPerSsrc = 60; + static constexpr size_t kMaxConcurrentBufferedSsrcs = 64; + + void evictExpired_n() RTC_EXCLUSIVE_LOCKS_REQUIRED(_mu); + + std::map _entries RTC_GUARDED_BY(_mu); +``` + +Add `` to the includes if not already present (usually pulled in transitively): + +```bash +grep -n "" /Users/isaac/build/telegram/telegram-ios/submodules/TgVoipWebrtc/tgcalls/tgcalls/group/GroupInstanceReferenceImpl.cpp +``` + +If missing, add `#include ` near the top with the other STL includes. + +- [ ] **Step 2: Replace `Transform` with the real implementation** + +Replace the stub `Transform` body with: + +```cpp + void Transform(std::unique_ptr frame) override { + if (!frame) return; + const uint32_t ssrc = frame->GetSsrc(); + + // Path A: kDrained — pass through. + // Path B: kBuffering — buffer. + // Path C: new SSRC — insert + post discovery + buffer. + // Path D: at the SSRC cap — drop silently. + rtc::scoped_refptr liveSink; + bool notifyDiscovery = false; + std::unique_ptr liveFrame; + { + webrtc::MutexLock lock(&_mu); + evictExpired_n(); + + auto it = _entries.find(ssrc); + if (it == _entries.end()) { + if (_entries.size() >= kMaxConcurrentBufferedSsrcs) { + // Path D: drop without notify, no allocation. + return; + } + Entry entry; + entry.firstFrameTimeMs = rtc::TimeMillis(); + entry.buffer.push_back(std::move(frame)); + _entries.emplace(ssrc, std::move(entry)); + notifyDiscovery = true; + } else if (it->second.state == SsrcState::kBuffering) { + if (it->second.buffer.size() >= kMaxBufferedFramesPerSsrc) { + it->second.buffer.pop_front(); + } + it->second.buffer.push_back(std::move(frame)); + } else { + // kDrained: pick the live sink and emit outside the lock. + auto sinkIt = _perSsrcSinks.find(ssrc); + if (sinkIt != _perSsrcSinks.end()) { + liveSink = sinkIt->second; + } else { + liveSink = _broadcastSink; + } + liveFrame = std::move(frame); + } + } + + if (liveSink && liveFrame) { + liveSink->OnTransformedFrame(std::move(liveFrame)); + } + if (notifyDiscovery && _onNewSsrc) { + _onNewSsrc(ssrc); + } + } +``` + +- [ ] **Step 3: Replace `releaseSsrc` with the real implementation** + +Replace the stub `releaseSsrc` body with: + +```cpp + void releaseSsrc(uint32_t ssrc) { + std::deque> toFlush; + rtc::scoped_refptr sink; + { + webrtc::MutexLock lock(&_mu); + auto it = _entries.find(ssrc); + if (it == _entries.end() || it->second.state == SsrcState::kDrained) { + // Either the SSRC was unknown, or already drained. Mark drained + // so future frames take the live-passthrough path. + if (it == _entries.end()) { + Entry entry; + entry.state = SsrcState::kDrained; + entry.firstFrameTimeMs = rtc::TimeMillis(); + _entries.emplace(ssrc, std::move(entry)); + } + return; + } + it->second.state = SsrcState::kDrained; + toFlush = std::move(it->second.buffer); + + auto sinkIt = _perSsrcSinks.find(ssrc); + if (sinkIt != _perSsrcSinks.end()) { + sink = sinkIt->second; + } else { + sink = _broadcastSink; + } + } + + if (!sink) return; + while (!toFlush.empty()) { + auto f = std::move(toFlush.front()); + toFlush.pop_front(); + sink->OnTransformedFrame(std::move(f)); + } + } +``` + +- [ ] **Step 4: Implement `evictExpired_n`** + +Add this private method definition inside the class (e.g., after `releaseSsrc`): + +```cpp + void evictExpired_n() RTC_EXCLUSIVE_LOCKS_REQUIRED(_mu) { + const int64_t now = rtc::TimeMillis(); + for (auto& [ssrc, entry] : _entries) { + if (entry.state == SsrcState::kBuffering && + !entry.buffer.empty() && + (now - entry.firstFrameTimeMs) > kSsrcDiscoveryTimeoutMs) { + entry.buffer.clear(); + // Leave the entry in kBuffering with empty buffer so we don't + // re-notify discovery. If the application ever recovers + // (renegotiation completes belatedly), releaseSsrc will still + // mark kDrained and live frames will flow through. + } + } + } +``` + +- [ ] **Step 5: Add `rtc::TimeMillis` include if missing** + +```bash +grep -n "rtc::TimeMillis\b\|rtc_base/time_utils.h" \ + /Users/isaac/build/telegram/telegram-ios/submodules/TgVoipWebrtc/tgcalls/tgcalls/group/GroupInstanceReferenceImpl.cpp | head -5 +``` + +If only the call shows up but not the header, add: + +```cpp +#include "rtc_base/time_utils.h" +``` + +near the other rtc_base includes. + +- [ ] **Step 6: Build** + +```bash +/Users/isaac/build/telegram/telegram-ios/build-input/bazel-8.4.2-darwin-arm64 \ + build //submodules/TgVoipWebrtc/tgcalls/tools/cli:tgcalls_cli 2>&1 | tail -10 +``` + +Expected: `INFO: Build completed successfully`. + +- [ ] **Step 7: Run T1 — still expected to fail (transformer still not installed)** + +```bash +/Users/isaac/build/telegram/telegram-ios/bazel-bin/submodules/TgVoipWebrtc/tgcalls/tools/cli/tgcalls_cli \ + --mode group --participants 0 --reference-participants 3 --duration 6 --quiet +echo "EXIT=$?" +``` + +Expected: `Result: FAILED`, `EXIT=1`. Code is built but unused. + +- [ ] **Step 8: Commit** + +```bash +cd /Users/isaac/build/telegram/telegram-ios +git add submodules/TgVoipWebrtc/tgcalls/tgcalls/group/GroupInstanceReferenceImpl.cpp +git commit -m "$(cat <<'EOF' +GroupInstanceReferenceImpl: implement GRAudioFrameTransformer state machine + +Per-SSRC buffer / drain / passthrough. releaseSsrc transitions +kBuffering→kDrained atomically (mark under lock, drain outside lock) +to avoid races. evictExpired_n bounds buffer lifetime to 1s. + +Caps: kMaxConcurrentBufferedSsrcs=64, kMaxBufferedFramesPerSsrc=60. +Worst-case memory ≈ 308 KB. + +Co-Authored-By: Claude Opus 4.7 (1M context) +EOF +)" +``` + +--- + +### Task 5: Wire the transformer into `start()` (catch-all only) and observe discovery callback fires + +**Files:** +- Modify: `submodules/TgVoipWebrtc/tgcalls/tgcalls/group/GroupInstanceReferenceImpl.cpp` + +**Why staged:** Install on the catch-all and prove `Transform` fires (via the `_onNewSsrc` callback). Discovery doesn't yet drive a renegotiation — that's the next task. After this task, T1 should still FAIL but with the discovery callback observably firing. + +- [ ] **Step 1: Add the member field** + +Locate the `Audio.` block in the private member section (around line 1556 — `_outgoingAudioTrack`, `_outgoingAudioTransceiver`). Add immediately after: + +```cpp + // Per-receiver audio frame transformer (catch-all + every recvonly). + rtc::scoped_refptr _audioFrameTransformer; +``` + +- [ ] **Step 2: Add a forward declaration for `handleDiscoveredAudioSsrc` (function defined in Task 6)** + +Inside the class, near the other private method declarations / definitions for `handleActiveAudioSsrcs`-replacement work — for now, declare a stub: + +```cpp + void handleDiscoveredAudioSsrc(uint32_t ssrc) { + // To be implemented in Task 6. + RTC_LOG(LS_INFO) << "GroupRef: discovered audio SSRC " << ssrc; + } +``` + +Place this near the existing media-thread methods (e.g., near `wireRemoteAudioLevelSinks`). + +- [ ] **Step 3: Construct + install the transformer in `start()`** + +Locate the block after `_outgoingAudioTransceiver` is created (around line 386, immediately after the `params.encodings[0].max_bitrate_bps = ...; _outgoingAudioTransceiver->sender()->SetParameters(params);` block). Insert before `_outgoingAudioTrack->set_enabled(false); // Muted by default.`: + +```cpp + // Install the audio frame transformer on mid=0's receiver. The + // receive side of this sendrecv transceiver is PeerConnection's + // catch-all for unsignaled SSRCs, so the transformer captures + // every previously-unseen remote SSRC. The same instance is + // attached to each recvonly receiver in renegotiate() so that + // live audio passes through the transformer consistently + // (and so the e2e PR has a single attachment point). + _audioFrameTransformer = rtc::make_ref_counted( + /*onNewSsrc=*/[weak, threads = _threads](uint32_t ssrc) { + threads->getMediaThread()->PostTask([weak, ssrc]() { + if (auto strong = weak.lock()) { + strong->handleDiscoveredAudioSsrc(ssrc); + } + }); + }, + /*decrypt=*/nullptr, // wired by the e2e PR + /*mediaThread=*/_threads->getMediaThread().get()); + _outgoingAudioTransceiver->receiver() + ->SetDepacketizerToDecoderFrameTransformer(_audioFrameTransformer); +``` + +Note: `weak` is the `weak_ptr` already in scope earlier in `start()`. Verify by re-reading the start of `start()`: + +```bash +sed -n '173,195p' /Users/isaac/build/telegram/telegram-ios/submodules/TgVoipWebrtc/tgcalls/tgcalls/group/GroupInstanceReferenceImpl.cpp +``` + +If `weak` is not in scope at the insertion point, declare locally: + +```cpp + const auto weak = std::weak_ptr(shared_from_this()); +``` + +- [ ] **Step 4: Build** + +```bash +/Users/isaac/build/telegram/telegram-ios/build-input/bazel-8.4.2-darwin-arm64 \ + build //submodules/TgVoipWebrtc/tgcalls/tools/cli:tgcalls_cli 2>&1 | tail -10 +``` + +Expected: `INFO: Build completed successfully`. + +- [ ] **Step 5: Run T1 with verbose output — confirm discovery callback fires** + +```bash +/Users/isaac/build/telegram/telegram-ios/bazel-bin/submodules/TgVoipWebrtc/tgcalls/tools/cli/tgcalls_cli \ + --mode group --participants 0 --reference-participants 3 --duration 6 2>&1 \ + | grep "discovered audio SSRC" | head -10 +``` + +Expected: at least one `GroupRef: discovered audio SSRC ` line per remote peer per participant. (Logs go through `LogSinkImpl` which writes to a per-participant log file then deletes; if the grep returns nothing, briefly add `fprintf(stderr, "...")` mirroring the RTC_LOG to verify, then revert before commit.) + +- [ ] **Step 6: Run T1 (full) — still expected to FAIL** + +```bash +/Users/isaac/build/telegram/telegram-ios/bazel-bin/submodules/TgVoipWebrtc/tgcalls/tools/cli/tgcalls_cli \ + --mode group --participants 0 --reference-participants 3 --duration 6 --quiet +echo "EXIT=$?" +``` + +Expected: `Result: FAILED`. The handler is a logging stub; no recvonly transceiver is added; audio is dropped (no `OnTransformedFrame` call yet because the entry is `kBuffering` indefinitely). + +- [ ] **Step 7: Commit** + +```bash +cd /Users/isaac/build/telegram/telegram-ios +git add submodules/TgVoipWebrtc/tgcalls/tgcalls/group/GroupInstanceReferenceImpl.cpp +git commit -m "$(cat <<'EOF' +GroupInstanceReferenceImpl: install GRAudioFrameTransformer on mid=0 + +The transformer is now in the depacketizer path of mid=0's receiver +(PeerConnection's catch-all for unsignaled audio). Every previously- +unseen SSRC fires the discovery callback, which currently just logs. +The next commit drives renegotiation and flushes buffered audio. + +Co-Authored-By: Claude Opus 4.7 (1M context) +EOF +)" +``` + +--- + +### Task 6: Implement `handleDiscoveredAudioSsrc` + `scheduleDiscoveryRenegotiation` and call `releaseSsrc` from `onRenegotiationComplete` + +**Files:** +- Modify: `submodules/TgVoipWebrtc/tgcalls/tgcalls/group/GroupInstanceReferenceImpl.cpp` + +**Why staged:** This is the GREEN step — the test harness should pass for the first time after this task. Wiring discovery → renegotiation → release closes the loop, but until we also install the transformer on each recvonly receiver (Task 7), live frames after the flush continue to take the catch-all path. That's tolerable today (catch-all is the same instance) but Task 7 makes it explicit and future-proof. + +- [ ] **Step 1: Add `_discoveryRenegotiationScheduled` member** + +In the private member section, near `_isRenegotiating`: + +```cpp + // Discovery-renegotiation debounce. + bool _discoveryRenegotiationScheduled = false; +``` + +- [ ] **Step 2: Replace the `handleDiscoveredAudioSsrc` stub with the real implementation** + +```cpp + // Single entry point for adding a remote audio SSRC. Runs on the + // media thread (posted to from the worker-thread frame transformer + // callback). + void handleDiscoveredAudioSsrc(uint32_t ssrc) { + if (ssrc == 0) return; + if (ssrc == _outgoingSsrc) return; + if (_remoteSsrcs.count(ssrc) > 0) return; + + std::string mid = std::to_string(_nextMid++); + RemoteSsrcInfo info; + info.mid = mid; + _remoteSsrcs.emplace(ssrc, std::move(info)); + + if (_requestMediaChannelDescriptions) { + _requestMediaChannelDescriptions({ssrc}, + [](std::vector&&) {}); + } + scheduleDiscoveryRenegotiation(); + + RTC_LOG(LS_INFO) << "GroupRef: queued discovered audio SSRC " << ssrc + << " (mid=" << mid << ")"; + } +``` + +- [ ] **Step 3: Add `scheduleDiscoveryRenegotiation`** + +Place near the existing `renegotiate()` method: + +```cpp + static constexpr int kDiscoveryRenegotiationDelayMs = 250; + + void scheduleDiscoveryRenegotiation() { + if (_discoveryRenegotiationScheduled) return; + _discoveryRenegotiationScheduled = true; + + const auto weak = std::weak_ptr(shared_from_this()); + _threads->getMediaThread()->PostDelayedTask( + [weak]() { + auto strong = weak.lock(); + if (!strong) return; + strong->_discoveryRenegotiationScheduled = false; + strong->renegotiate(); + }, + webrtc::TimeDelta::Millis(kDiscoveryRenegotiationDelayMs)); + } +``` + +- [ ] **Step 4: Add the `releaseSsrc` loop in `onRenegotiationComplete`** + +Locate `onRenegotiationComplete()` (around line 1314). Insert after `wireRemoteAudioLevelSinks();` and before `_isRenegotiating = false;`: + +```cpp + if (_audioFrameTransformer) { + for (auto& [ssrc, info] : _remoteSsrcs) { + if (info.transceiver && info.transceiver->mid().has_value()) { + _audioFrameTransformer->releaseSsrc(ssrc); + } + } + } +``` + +- [ ] **Step 5: Build** + +```bash +/Users/isaac/build/telegram/telegram-ios/build-input/bazel-8.4.2-darwin-arm64 \ + build //submodules/TgVoipWebrtc/tgcalls/tools/cli:tgcalls_cli 2>&1 | tail -10 +``` + +Expected: `INFO: Build completed successfully`. + +- [ ] **Step 6: Run T1 — should now SUCCESS** + +```bash +/Users/isaac/build/telegram/telegram-ios/bazel-bin/submodules/TgVoipWebrtc/tgcalls/tools/cli/tgcalls_cli \ + --mode group --participants 0 --reference-participants 3 --duration 6 --quiet +echo "EXIT=$?" +``` + +Expected: `Result: SUCCESS`, `Audio received: 3/3`, `EXIT=0`. + +If FAIL: re-read the spec's "Where flushed frames actually go" section. The most likely issue is that the per-SSRC sink callback registered when the unsignaled stream was created is no longer the one routing to the (now-promoted) audio receive stream's decoder. Add a one-shot debug print inside `releaseSsrc` to confirm `sink != nullptr` and `toFlush.size() > 0`. + +- [ ] **Step 7: Run T2-T5 — verify no regressions** + +Run each scenario from the "Test bench reference" block above (T1 through T5). All five must report `Result: SUCCESS` and `EXIT=0`. + +If T3-T5 (muted-peer scenarios) pass: the muted-peer invariant from the previous fix still holds, so the per-receiver audio-level sinks are correctly wired and reading PCM from the post-promotion stream. + +- [ ] **Step 8: Commit** + +```bash +cd /Users/isaac/build/telegram/telegram-ios +git add submodules/TgVoipWebrtc/tgcalls/tgcalls/group/GroupInstanceReferenceImpl.cpp +git commit -m "$(cat <<'EOF' +GroupInstanceReferenceImpl: drive discovery renegotiation, flush buffer + +- handleDiscoveredAudioSsrc inserts into _remoteSsrcs, fires + _requestMediaChannelDescriptions per SSRC, schedules a debounced + renegotiation. +- scheduleDiscoveryRenegotiation coalesces bursts: at most one + renegotiation per 250ms, regardless of how many SSRCs land in + the window. +- onRenegotiationComplete iterates _remoteSsrcs and calls + releaseSsrc(ssrc) for every entry whose transceiver now has a + mid, draining the per-SSRC FIFO into OnTransformedFrame so the + buffered audio plays without a startup gap. + +T1-T5 all pass. + +Co-Authored-By: Claude Opus 4.7 (1M context) +EOF +)" +``` + +--- + +### Task 7: Install `_audioFrameTransformer` on every recvonly transceiver as it's added + +**Files:** +- Modify: `submodules/TgVoipWebrtc/tgcalls/tgcalls/group/GroupInstanceReferenceImpl.cpp` + +**Why now (not in Task 6):** Today the catch-all transformer covers live audio for promoted streams (it's already on the stream from the unsignaled-stream creation path). Explicitly installing it on each recvonly receiver removes our reliance on internal WebRTC stream-promotion behavior carrying the transformer along, and matches the exact attachment pattern the e2e PR will use. + +- [ ] **Step 1: Find the AddTransceiver call in `renegotiate()`** + +```bash +grep -n "AddTransceiver(cricket::MEDIA_TYPE_AUDIO" \ + /Users/isaac/build/telegram/telegram-ios/submodules/TgVoipWebrtc/tgcalls/tgcalls/group/GroupInstanceReferenceImpl.cpp +``` + +Expected: a single hit inside `renegotiate()` (around line 1142). + +- [ ] **Step 2: Attach the transformer right after the transceiver is created** + +Modify the block that handles the success branch of `AddTransceiver`. Original: + +```cpp + auto result = _peerConnection->AddTransceiver(cricket::MEDIA_TYPE_AUDIO, init); + if (result.ok()) { + info.transceiver = result.value(); + RTC_LOG(LS_INFO) << "GroupRef: Added recvonly transceiver for SSRC " << ssrc; + } +``` + +Replace with: + +```cpp + auto result = _peerConnection->AddTransceiver(cricket::MEDIA_TYPE_AUDIO, init); + if (result.ok()) { + info.transceiver = result.value(); + if (_audioFrameTransformer) { + info.transceiver->receiver() + ->SetDepacketizerToDecoderFrameTransformer(_audioFrameTransformer); + } + RTC_LOG(LS_INFO) << "GroupRef: Added recvonly transceiver for SSRC " << ssrc; + } +``` + +- [ ] **Step 3: Build** + +```bash +/Users/isaac/build/telegram/telegram-ios/build-input/bazel-8.4.2-darwin-arm64 \ + build //submodules/TgVoipWebrtc/tgcalls/tools/cli:tgcalls_cli 2>&1 | tail -3 +``` + +Expected: `INFO: Build completed successfully`. + +- [ ] **Step 4: Run T1-T5 — must all SUCCESS** + +```bash +for cfg in \ + "0 3 --duration 6" \ + "2 2 --duration 6" \ + "0 3 --mute-participants 0 --duration 6" \ + "1 2 --mute-participants 0 --duration 6" \ + "2 1 --mute-participants 2 --duration 6"; do + /Users/isaac/build/telegram/telegram-ios/bazel-bin/submodules/TgVoipWebrtc/tgcalls/tools/cli/tgcalls_cli \ + --mode group --participants $cfg --quiet 2>&1 | tail -3 + echo "EXIT=$?"; echo "---" +done +``` + +Each block must end with `Result: SUCCESS` and `EXIT=0`. + +- [ ] **Step 5: Commit** + +```bash +cd /Users/isaac/build/telegram/telegram-ios +git add submodules/TgVoipWebrtc/tgcalls/tgcalls/group/GroupInstanceReferenceImpl.cpp +git commit -m "$(cat <<'EOF' +GroupInstanceReferenceImpl: attach frame transformer to each recvonly receiver + +Live audio for a promoted SSRC was already flowing through our +transformer (carried over from the unsignaled stream creation +path), but we depended on internal WebRTC stream-promotion +behavior to make that work. Explicit per-receiver attachment +removes that dependency and matches what the e2e PR will need. + +Co-Authored-By: Claude Opus 4.7 (1M context) +EOF +)" +``` + +--- + +### Task 8: Documentation refresh + +**Files:** +- Modify: `submodules/TgVoipWebrtc/CLAUDE.md` +- Modify: `submodules/TgVoipWebrtc/tgcalls/tools/cli/CLAUDE.md` +- Modify: `submodules/TgVoipWebrtc/tgcalls/tools/go_sfu/CLAUDE.md` + +- [ ] **Step 1: Update `submodules/TgVoipWebrtc/CLAUDE.md` — ReferenceImpl section** + +Find the "GroupInstanceReferenceImpl" section. Update the "Dynamic Participant Handling → Audio" sub-bullet: + +Before (or similar): +> 1. SFU sends `{"colibriClass":"ActiveAudioSsrcs","ssrcs":[54321,98765]}` over data channel +> 2. Client diffs against known SSRCs +> 3. New SSRCs: add recvonly audio transceiver → renegotiate (new offer + constructed answer mirroring offer mids) +> 4. Removed SSRCs: clean up from tracking map + +After: +> 1. `GRAudioFrameTransformer` (installed on mid=0's receiver and on every recvonly audio receiver) sees a frame with an unknown SSRC at the depacketizer→decoder boundary, buffers it, and notifies the media thread. +> 2. `handleDiscoveredAudioSsrc` inserts the SSRC into `_remoteSsrcs`, fires `_requestMediaChannelDescriptions({ssrc}, ...)` (matches CustomImpl's contract), and schedules a 250 ms-debounced renegotiation. +> 3. Renegotiation adds a recvonly transceiver bound to the new SSRC; `buildRemoteAnswer` includes the SSRC on the new m-line; `WebRtcVoiceReceiveChannel::AddRecvStream` promotes the existing unsignaled stream in place. +> 4. `onRenegotiationComplete` calls `_audioFrameTransformer->releaseSsrc(ssrc)`, which drains the buffered FIFO via `OnTransformedFrame` so the user hears the participant's first ~250–500 ms of audio without a gap. +> 5. Subsequent live frames for the SSRC pass through the transformer's `kDrained` branch directly to the decoder. +> +> The `colibriClass=ActiveAudioSsrcs` data-channel mechanism (test-SFU only) was removed; the tap is the single discovery path. Removed-SSRC handling is the same as CustomImpl's: stale recvonly transceivers stay in the SDP indefinitely; participant departures are tracked at the application layer (MTProto). + +- [ ] **Step 2: Update `submodules/TgVoipWebrtc/tgcalls/tools/cli/CLAUDE.md` — group flow** + +Find the "Architecture (Group)" section. Remove the bullet about `ActiveAudioSsrcs` ("SSRC discovery: SFU broadcasts `ActiveAudioSsrcs` and `ActiveVideoSsrcs` ...") and replace with: + +> SSRC discovery: video SSRCs are broadcast via `ActiveVideoSsrcs` (used by the test app's `dataChannelMessageReceived` callback to call `setRequestedVideoChannels`). Audio SSRCs are discovered by `GroupInstanceReferenceImpl`'s per-receiver `GRAudioFrameTransformer` directly from incoming RTP — same shape CustomImpl uses (`receiveUnknownSsrcPacket` → `_requestMediaChannelDescriptions`). + +- [ ] **Step 3: Update `submodules/TgVoipWebrtc/tgcalls/tools/go_sfu/CLAUDE.md`** + +In the bullet that lists `sfu.go`'s responsibilities, remove the phrase `'ActiveAudioSsrcs'/`ActiveVideoSsrcs' broadcasting` and replace with `ActiveVideoSsrcs broadcasting` (audio is no longer broadcast). + +- [ ] **Step 4: Verify builds still work after the doc changes (defensive)** + +```bash +/Users/isaac/build/telegram/telegram-ios/build-input/bazel-8.4.2-darwin-arm64 \ + build //submodules/TgVoipWebrtc/tgcalls/tools/cli:tgcalls_cli 2>&1 | tail -3 +``` + +Expected: `INFO: Build completed successfully` (no source files touched, but a no-op build verifies the docs don't accidentally break a generated file). + +- [ ] **Step 5: Commit** + +```bash +cd /Users/isaac/build/telegram/telegram-ios +git add submodules/TgVoipWebrtc/CLAUDE.md \ + submodules/TgVoipWebrtc/tgcalls/tools/cli/CLAUDE.md \ + submodules/TgVoipWebrtc/tgcalls/tools/go_sfu/CLAUDE.md +git commit -m "$(cat <<'EOF' +docs: ReferenceImpl SSRC discovery via per-receiver frame transformer + +Document the new GRAudioFrameTransformer-based audio SSRC discovery +flow in both the tgcalls library overview and the test-bench notes. +Drop ActiveAudioSsrcs references — the message no longer exists. + +Co-Authored-By: Claude Opus 4.7 (1M context) +EOF +)" +``` + +--- + +### Task 9: Final regression sweep + +**Files:** none changed. + +- [ ] **Step 1: Run the full sweep** + +```bash +echo "=== T1: All-Reference no mute ===" +/Users/isaac/build/telegram/telegram-ios/bazel-bin/submodules/TgVoipWebrtc/tgcalls/tools/cli/tgcalls_cli \ + --mode group --participants 0 --reference-participants 3 --duration 6 --quiet | tail -3 +echo "EXIT=$?" + +echo "=== T2: Mixed (2C+2R) no mute ===" +/Users/isaac/build/telegram/telegram-ios/bazel-bin/submodules/TgVoipWebrtc/tgcalls/tools/cli/tgcalls_cli \ + --mode group --participants 2 --reference-participants 2 --duration 6 --quiet | tail -3 +echo "EXIT=$?" + +echo "=== T3: All-Reference, P0 muted ===" +/Users/isaac/build/telegram/telegram-ios/bazel-bin/submodules/TgVoipWebrtc/tgcalls/tools/cli/tgcalls_cli \ + --mode group --participants 0 --reference-participants 3 --mute-participants 0 --duration 6 --quiet | tail -3 +echo "EXIT=$?" + +echo "=== T4: Mixed (1C+2R), custom muted ===" +/Users/isaac/build/telegram/telegram-ios/bazel-bin/submodules/TgVoipWebrtc/tgcalls/tools/cli/tgcalls_cli \ + --mode group --participants 1 --reference-participants 2 --mute-participants 0 --duration 6 --quiet | tail -3 +echo "EXIT=$?" + +echo "=== T5: Mixed (2C+1R), reference muted ===" +/Users/isaac/build/telegram/telegram-ios/bazel-bin/submodules/TgVoipWebrtc/tgcalls/tools/cli/tgcalls_cli \ + --mode group --participants 2 --reference-participants 1 --mute-participants 2 --duration 6 --quiet | tail -3 +echo "EXIT=$?" +``` + +Expected: every block ends with `Result: SUCCESS` and `EXIT=0`. + +- [ ] **Step 2: Confirm muted-peer level invariant explicitly (T3 verbose)** + +```bash +/Users/isaac/build/telegram/telegram-ios/bazel-bin/submodules/TgVoipWebrtc/tgcalls/tools/cli/tgcalls_cli \ + --mode group --participants 0 --reference-participants 3 --mute-participants 0 --duration 6 2>&1 \ + | grep -E "Validate" | head -5 +``` + +Expected: +``` +Validate: OK: P1 (ref) reported muted P0 (ssrc=...) at max level 0.000 +Validate: OK: P2 (ref) reported muted P0 (ssrc=...) at max level 0.000 +``` + +- [ ] **Step 3: Confirm no leftover `ActiveAudioSsrcs` references** + +```bash +grep -rn "ActiveAudioSsrcs\|handleActiveAudioSsrcs\|broadcastActiveSSRCs\|buildActiveSSRCsMessage" \ + /Users/isaac/build/telegram/telegram-ios/submodules/TgVoipWebrtc/ 2>/dev/null | grep -v ".log\|.o\|index/\|bazel-" +``` + +Expected: empty (or only matches inside `.git/`). + +- [ ] **Step 4: Confirm git log structure** + +```bash +cd /Users/isaac/build/telegram/telegram-ios +git log --oneline -10 +``` + +Expected to see 8 new commits from this PR (Tasks 1, 2, 3, 4, 5, 6, 7, 8) since the previous baseline. + +- [ ] **Step 5: This task has no commit** + +The sweep is verification-only. + +--- + +## Out of scope (do NOT touch in this PR) + +- `descriptor.e2eEncryptDecrypt` wiring. The seam (`DecryptHook` constructor parameter on `GRAudioFrameTransformer`) is in place; the actual decrypt callback is the next PR. +- Outgoing-audio SSRC>int31 masking. Separate fix. +- Push-style discovery via a new `GroupInstanceInterface::addIncomingAudioSsrcs(...)` method. +- Removed-SSRC cleanup (recvonly transceivers stay in the SDP indefinitely — same as CustomImpl). +- iOS-side code changes — the existing `_requestMediaChannelDescriptions` callback already serves the discovery path. + +--- + +## Self-review notes + +- **Spec coverage:** Every section of the spec has at least one task. Frame transformer (Tasks 3–4), every-receiver install (Tasks 5, 7), discovery + debounce + release (Task 6), `ActiveAudioSsrcs` removal (Tasks 1–2), docs (Task 8), regression sweep (Task 9). +- **Type / name consistency:** `GRAudioFrameTransformer` (not `GRSsrcTapTransformer`) used everywhere. Member field `_audioFrameTransformer` everywhere. Constants `kSsrcDiscoveryTimeoutMs`, `kMaxBufferedFramesPerSsrc`, `kMaxConcurrentBufferedSsrcs`, `kDiscoveryRenegotiationDelayMs` defined once and referenced once. +- **Placeholder scan:** No "TBD"/"TODO"/"similar to N" — every code-touching step has the actual code. +- **Test verification:** The test bench T1–T5 are referenced by exact CLI invocations, with the muted-peer invariant called out as the strongest signal for routing regressions. Task 1's intentional regression is explicitly framed as TDD-red so the engineer doesn't think they broke something. diff --git a/docs/superpowers/specs/2026-05-01-groupref-ssrc-discovery-design.md b/docs/superpowers/specs/2026-05-01-groupref-ssrc-discovery-design.md new file mode 100644 index 0000000000..1ee706a42a --- /dev/null +++ b/docs/superpowers/specs/2026-05-01-groupref-ssrc-discovery-design.md @@ -0,0 +1,354 @@ +# GroupInstanceReferenceImpl: Reactive Remote-Audio-SSRC Discovery + +**Date:** 2026-05-01 +**Status:** Approved (design only — no implementation yet) +**Scope:** `submodules/TgVoipWebrtc/tgcalls/tgcalls/group/GroupInstanceReferenceImpl.cpp` and adjacent test wiring. + +## Problem + +`GroupInstanceReferenceImpl` (PeerConnection-based group call client) currently learns about remote audio SSRCs **only** from a `colibriClass=ActiveAudioSsrcs` data-channel message broadcast by the test-bench Pion SFU (`tgcalls/tools/go_sfu/sfu.go`). The real Telegram SFU does not send that message — `GroupInstanceCustomImpl::receiveDataChannelMessage` only handles `SenderVideoConstraints` and `DebugMessage`. CustomImpl discovers SSRCs reactively from raw RTP via `GroupNetworkManager` → `receiveUnknownSsrcPacket` → `maybeRequestUnknownSsrc` → `_requestMediaChannelDescriptions`. ReferenceImpl has no equivalent path; in real calls every remote audio packet is silently dropped (or routed to mid=0's unsignaled handler with no application visibility). + +The fix must: +- Surface every previously-unseen remote audio SSRC to ReferenceImpl's internal logic, ideally on the first frame. +- Drive the addition of a recvonly audio transceiver for that SSRC. +- Match CustomImpl's app-facing contract — the application sees the same `_requestMediaChannelDescriptions(ssrcs, completion)` callback it already implements. +- Use a single discovery mechanism in both real and test environments (the test SFU's `ActiveAudioSsrcs` broadcast becomes obsolete and is removed; see "Removing `ActiveAudioSsrcs`" below). + +## Approach: one `GRAudioFrameTransformer` installed on every audio receiver + +WebRTC exposes `RtpReceiverInterface::SetDepacketizerToDecoderFrameTransformer(FrameTransformerInterface*)`. The transformer's `Transform(frame)` takes ownership of a `std::unique_ptr` and there is **no requirement that it call `OnTransformedFrame` synchronously** — the design is explicitly async. The transformer can hold the frame for arbitrarily long; the audio pipeline simply waits. + +We install **the same `GRAudioFrameTransformer` instance on every audio receiver** in this `GroupInstanceReferenceInternal`: + +- **mid=0 (sendrecv outgoing audio)** — explicitly via `_outgoingAudioTransceiver->receiver()->SetDepacketizerToDecoderFrameTransformer(_audioFrameTransformer)` in `start()`. This lands as both the per-receiver transformer and (because mid=0's receive side has `signaled_ssrc=nullopt`) the channel's `unsignaled_frame_transformer_` — meaning any unsignaled stream created later for an unknown SSRC also gets it. +- **Each recvonly audio transceiver** (added by the discovery flow) — explicitly via the same call when the transceiver is added in `renegotiate()`. This is structurally required for the upcoming e2e-decrypt fix (every receiver needs the decrypt hook); attaching it now too is free and removes our reliance on stream-promotion implicitly carrying the transformer along. + +The transformer carries the per-SSRC state machine (described below). It also carries a `decryptHook` callable (initially null) that the e2e PR will wire to `descriptor.e2eEncryptDecrypt`. Today the hook just passes the frame through unchanged; tomorrow it decrypts before `OnTransformedFrame`. + +### Why install on every receiver explicitly + +If we relied solely on `unsignaled_frame_transformer_`, the transformer would be carried onto a recvonly transceiver only via the stream-promotion path (`webrtc_voice_engine.cc:2258-2266`: `MaybeDeregisterUnsignaledRecvStream` keeps the stream object intact, transformer attached). That's correct *today* but it's an internal-WebRTC behavior we'd be pinning to. Once we explicitly attach to each recvonly receiver we own the lifecycle: the transformer is on the stream because we put it there, regardless of how WebRTC's voice engine handles the unsignaled→signaled transition. + +For the buffer flush specifically: the buffered frames were captured while the SSRC was on mid=0's unsignaled-fallback path. When we later install the transformer on the recvonly receiver R' for the same SSRC, the channel's stream (now promoted in place) keeps the same transformer reference (same instance, same pointer) — so `releaseSsrc` flushes through `_perSsrcSinks[ssrc]` (the sink callback WebRTC registered when the stream first appeared) and frames land in that one stream's decoder. + +### Tap behavior + +For each SSRC the transformer maintains an `Entry { state, buffer, firstFrameTimeMs }` with `state ∈ { kBuffering, kDrained }`. + +`Transform(frame)` (worker thread): +1. Acquire the mutex. +2. Look up the entry for `frame->GetSsrc()`. +3. If absent and we're below `kMaxConcurrentBufferedSsrcs`: insert with `kBuffering` state, post `_onNewSsrc(ssrc)` to the media thread (outside the lock), buffer the frame. +4. If `kBuffering`: append to buffer (drop oldest if `buffer.size() >= kMaxBufferedFramesPerSsrc`). +5. If `kDrained`: release the lock, then call `OnTransformedFrame(std::move(frame))`. Live audio flows through. + +`releaseSsrc(uint32_t ssrc)` (media thread, called from `onRenegotiationComplete`): +1. Acquire the mutex. +2. Locate the entry; if absent or already `kDrained`, return. +3. Move the deque out, mark `state = kDrained`, release the mutex. +4. Iterate the moved deque calling `OnTransformedFrame(std::move(frame))` on each. Performed outside the lock to avoid re-entrant deadlocks. + +The order of operations matters: marking `kDrained` *before* releasing the lock guarantees that any concurrent `Transform()` either (a) sees `kBuffering` and buffers — but its frame is lost because we already drained the FIFO, or (b) sees `kDrained` and passes through. Case (a) is a real one-frame-loss race window. We accept it: at 20 ms Opus that's a single packet of audio, inaudible, and overwhelmingly unlikely (the window is the few microseconds between unlocking and starting the drain loop). + +### Failure mode + +If `releaseSsrc(X)` is not called within `kSsrcDiscoveryTimeoutMs = 1000` ms (renegotiation failed or app declined the SSRC), an in-line eviction inside `Transform()` drops the buffer and **leaves the entry in `kBuffering` with empty FIFO**. Subsequent frames continue to attempt to buffer (and immediately drop on the FIFO cap = 0 / re-eviction), so the participant remains silent until the entry is cleared. Acceptable — the same outcome as the pure-drop alternative would have produced. + +### Net behavior + +- **Audible continuity for new participants.** Buffered frames flush into the same stream that carries future audio. NetEQ sees the buffered burst as one large jitter-buffer fill followed by normal-paced packets — same shape as recovering from a network glitch. May produce a brief audible artifact during the burst (NetEQ may accelerate or reorder) but no silence. +- **No orphaned `AudioReceiveStream`.** Stream is promoted in place; one stream per SSRC. +- **Tap stays in the live path.** Per-frame: one mutex, one map lookup, one `OnTransformedFrame`. Hot but cheap. + +CustomImpl already uses `FrameTransformer` (for E2E encryption) — the pattern is established in this codebase. Pass-through transformers also work in WebRTC (the `OnTransformedFrame` callback is the only contract). + +### Why not the alternatives + +- **`OnTrack` for unsignaled audio:** PeerConnection's "default" handler creates one default track. Multi-SSRC behavior is murky; fragile. +- **`PeerConnection::GetStats()` polling:** Works but adds 100–250 ms of discovery latency per new participant (audio drops during the window) and the stats walk is non-trivial. +- **Tap on a custom socket factory:** Requires re-implementing SRTP decrypt and RTP parsing. Too invasive. + +## Components + +``` + ┌────────────────────────────┐ + incoming RTP (any SSRC) ───▶ │ PeerConnection BUNDLE │ + │ demuxer (SSRC-keyed) │ + └─────────────┬──────────────┘ + │ + ┌───────────────────┼─────────────────────┐ + │ │ │ + signaled SSRC X signaled SSRC Y unknown / catch-all + (recvonly mid=N1) (recvonly mid=N2) (sendrecv mid=0) + │ │ │ + ▼ ▼ ▼ + ┌──────────────┐ ┌──────────────┐ ┌──────────────────────┐ + │ AudioTrack │ │ AudioTrack │ │ GRSsrcTapTransformer │ + │ + level sink │ │ + level sink │ │ (Transform → notify │ + └──────────────┘ └──────────────┘ │ + OnTransformedFrame)│ + └──────────┬───────────┘ + │ (worker thread) + │ + ▼ + PostTask to media thread + │ + ▼ + handleDiscoveredAudioSsrc(ssrc) + │ + ▼ + (existing) renegotiate() + + _requestMediaChannelDescriptions +``` + +### `GRAudioFrameTransformer` (new, anonymous-namespace class in `GroupInstanceReferenceImpl.cpp`) + +```cpp +class GRAudioFrameTransformer : public webrtc::FrameTransformerInterface { +public: + using SsrcCallback = std::function; + // Hook for the future e2e-decrypt fix. Called per frame on the worker + // thread before OnTransformedFrame. Today: identity (passes the frame + // through unchanged). The e2e PR will assign it to a closure that + // unwraps the descriptor.e2eEncryptDecrypt envelope. + using DecryptHook = std::function; + + GRAudioFrameTransformer(SsrcCallback onNewSsrc, + DecryptHook decrypt, // may be nullptr + rtc::Thread* mediaThread); + + // Called from ReferenceImpl on the media thread after onRenegotiationComplete + // confirms a recvonly transceiver now owns `ssrc`. Drains the per-SSRC + // FIFO into OnTransformedFrame in arrival order; subsequent frames for + // `ssrc` flow through unchanged (kDrained = live passthrough). + void releaseSsrc(uint32_t ssrc); + + // FrameTransformerInterface + void Transform(std::unique_ptr frame) override; + void RegisterTransformedFrameCallback(rtc::scoped_refptr) override; + void RegisterTransformedFrameSinkCallback(rtc::scoped_refptr, uint32_t ssrc) override; + void UnregisterTransformedFrameCallback() override; + void UnregisterTransformedFrameSinkCallback(uint32_t ssrc) override; + +private: + enum class SsrcState { kBuffering, kDrained }; + + struct Entry { + SsrcState state = SsrcState::kBuffering; + std::deque> buffer; + int64_t firstFrameTimeMs = 0; // for timeout eviction + }; + + void evictExpired_n() RTC_EXCLUSIVE_LOCKS_REQUIRED(_mu); // called inside Transform + + SsrcCallback _onNewSsrc; + rtc::Thread* _mediaThread; // used only as identity for assertions + + webrtc::Mutex _mu; + rtc::scoped_refptr _broadcastSink RTC_GUARDED_BY(_mu); + std::map> _perSsrcSinks RTC_GUARDED_BY(_mu); + std::map _entries RTC_GUARDED_BY(_mu); + + static constexpr int64_t kSsrcDiscoveryTimeoutMs = 1000; + static constexpr size_t kMaxBufferedFramesPerSsrc = 60; // ~1.2s at 20ms Opus + static constexpr size_t kMaxConcurrentBufferedSsrcs = 64; // upper bound on memory +}; +``` + +#### `Transform` (worker thread) + +1. `ssrc = frame->GetSsrc()`. Acquire `_mu`. +2. `evictExpired_n()` — walk `_entries`; for any whose `firstFrameTimeMs` is older than `kSsrcDiscoveryTimeoutMs` and still `kBuffering`, clear the buffer (entry stays so we don't re-notify; subsequent frames re-evict and stay silent until the entry is removed by an explicit application action — acceptable failure mode). +3. Look up the entry for `ssrc`: + - **Not present and `_entries.size() >= kMaxConcurrentBufferedSsrcs`** → drop frame, no notify (overflow protection against pathological SFU behavior). + - **Not present otherwise** → insert `Entry{ kBuffering, {}, now() }`, push frame, **release lock**, invoke `_onNewSsrc(ssrc)` (which posts to media thread → `handleDiscoveredAudioSsrc(ssrc)`). + - **Present and `kBuffering`**: + - If `entry.buffer.size() >= kMaxBufferedFramesPerSsrc` → drop the oldest buffered frame (FIFO bounded). + - Push `std::move(frame)` to the back of `entry.buffer`. + - **Present and `kDrained`** → take a local copy of the appropriate sink callback (per-SSRC if registered, else broadcast), **release lock**, call `sink->OnTransformedFrame(std::move(frame))`. This is the live-audio path after `releaseSsrc` has fired. + +The mutex is held only across the lookup and entry/buffer mutation. Both branches that emit through `OnTransformedFrame` (`kDrained` in `Transform`, and `releaseSsrc`) drop the lock before the call to avoid re-entrant deadlocks — WebRTC may synchronously schedule decoder work in `OnTransformedFrame` that calls back into related machinery. + +#### `releaseSsrc(uint32_t ssrc)` (media thread) + +1. Acquire `_mu`. Locate the entry; if missing or already `kDrained`, return. +2. Find the appropriate sink callback: per-SSRC if registered, else broadcast. +3. Mark `entry.state = kDrained` and `std::move` the deque out. **Marking `kDrained` before releasing the lock** is what guarantees no concurrent `Transform()` can buffer a frame that we then fail to flush. +4. Release `_mu`. Iterate the moved deque calling `sink->OnTransformedFrame(std::move(frame))` on each. + +#### `Register*` / `Unregister*` + +Store the callbacks under `_mu`. The per-SSRC `RegisterTransformedFrameSinkCallback` is invoked by WebRTC the first time a new SSRC arrives at this transformer; we hold it so `releaseSsrc` can dispatch through it. `Unregister*` clears. + +### Hook points in `GroupInstanceReferenceInternal` + +In `start()`, after `_outgoingAudioTransceiver` is created (mid=0): + +```cpp +auto weak = std::weak_ptr(shared_from_this()); +auto threads = _threads; +_audioFrameTransformer = rtc::make_ref_counted( + /*onNewSsrc=*/[weak, threads](uint32_t ssrc) { + threads->getMediaThread()->PostTask([weak, ssrc]() { + if (auto strong = weak.lock()) { + strong->handleDiscoveredAudioSsrc(ssrc); + } + }); + }, + /*decrypt=*/nullptr, // wired in the e2e PR + /*mediaThread=*/_threads->getMediaThread()); +_outgoingAudioTransceiver->receiver()->SetDepacketizerToDecoderFrameTransformer( + _audioFrameTransformer); +``` + +In `renegotiate()`, immediately after `_peerConnection->AddTransceiver(MEDIA_TYPE_AUDIO, recvonly)` succeeds for an SSRC discovered through the tap, attach the same transformer to the new receiver: + +```cpp +auto result = _peerConnection->AddTransceiver(cricket::MEDIA_TYPE_AUDIO, init); +if (result.ok()) { + info.transceiver = result.value(); + info.transceiver->receiver() + ->SetDepacketizerToDecoderFrameTransformer(_audioFrameTransformer); +} +``` + +In `onRenegotiationComplete()`, after `wireRemoteAudioLevelSinks()` runs, release any SSRCs whose recvonly transceiver just became active: + +```cpp +if (_audioFrameTransformer) { + for (auto& [ssrc, info] : _remoteSsrcs) { + if (info.transceiver && info.transceiver->mid().has_value()) { + // Idempotent: releaseSsrc no-ops on entries already drained. + _audioFrameTransformer->releaseSsrc(ssrc); + } + } +} +``` + +### `handleDiscoveredAudioSsrc(uint32_t ssrc)` (new, on media thread) + +This is the **only** entry point for adding a remote audio SSRC. It accumulates the SSRC into `_remoteSsrcs` and **schedules** a single coalesced renegotiation rather than firing one immediately: + +```cpp +void handleDiscoveredAudioSsrc(uint32_t ssrc) { + if (ssrc == 0) return; + if (ssrc == _outgoingSsrc) return; // our own + if (_remoteSsrcs.count(ssrc) > 0) return; // already known + + std::string mid = std::to_string(_nextMid++); + RemoteSsrcInfo info; + info.mid = mid; + _remoteSsrcs.emplace(ssrc, std::move(info)); + + if (_requestMediaChannelDescriptions) { + _requestMediaChannelDescriptions({ssrc}, [](auto&&) { /* fire-and-forget */ }); + } + scheduleDiscoveryRenegotiation(); +} +``` + +### Removing `ActiveAudioSsrcs` + +With the tap as the canonical discovery path, the test SFU's `ActiveAudioSsrcs` broadcast becomes redundant — and continuing to ship it would mean test runs exercise a code path that doesn't exist in production. Three deletions: + +1. `tools/go_sfu/sfu.go` — remove the `colibriClass=ActiveAudioSsrcs` broadcast (the message construction at `sfu.go:987` and any per-participant-join trigger that emits it). Remove the `ColibriClass`/`Ssrcs` JSON struct used solely for this purpose. +2. `GroupInstanceReferenceImpl.cpp` — delete `handleActiveAudioSsrcs(json)` and the `if (colibriClass == "ActiveAudioSsrcs") { ... }` dispatch in `onDataChannelMessage`. The dispatch becomes "forward to app callback if set" only (currently forwards regardless, so just drop the colibri branch). +3. `tools/cli/group_participant.cpp` — no change required. The CLI already only reacts to `ActiveVideoSsrcs` in its `dataChannelMessageReceived`; `ActiveAudioSsrcs` was never observed by the test app, only consumed internally by ReferenceImpl. + +Verification that removal is safe: `grep -r ActiveAudioSsrcs` across the repo currently returns hits only in (1) the SFU emitter, (2) the ReferenceImpl handler we're deleting, and (3) documentation/CLAUDE.md files (which we update as part of the change). CustomImpl never references it; iOS app code never references it; the test CLI never references it. + +Removed-SSRC handling: the deleted `handleActiveAudioSsrcs` also processed *removals* (SFU told us a participant left). After deletion, ReferenceImpl no longer reacts to participant departures via the data channel. This matches CustomImpl's behavior — CustomImpl also has no remove path; SSRCs simply go silent and the application removes them from the participant list via MTProto. Recvonly transceivers stay in the SDP indefinitely, which is a small per-call leak but not a correctness issue. (If this proves to be a problem in long-running calls, a future change can add a "remove if no audio for N seconds" sweep.) + +### `scheduleDiscoveryRenegotiation()` — debounce window + +A 250 ms delayed task on the media thread coalesces a burst of discoveries into one renegotiation. The existing `renegotiate()` already iterates `_remoteSsrcs` and adds a recvonly transceiver for any entry that doesn't have one yet, so all SSRCs accumulated during the delay window are picked up in a single offer/answer cycle. + +```cpp +static constexpr int kDiscoveryRenegotiationDelayMs = 250; + +void scheduleDiscoveryRenegotiation() { + if (_discoveryRenegotiationScheduled) return; + _discoveryRenegotiationScheduled = true; + auto weak = std::weak_ptr(shared_from_this()); + _threads->getMediaThread()->PostDelayedTask( + [weak]() { + auto strong = weak.lock(); + if (!strong) return; + strong->_discoveryRenegotiationScheduled = false; + strong->renegotiate(); + }, + webrtc::TimeDelta::Millis(kDiscoveryRenegotiationDelayMs)); +} +``` + +**Layering with existing serialization.** The debounce sits on top of `renegotiate()`'s existing `_isRenegotiating` / `_pendingRenegotiation` guard. Three regimes: + +1. **No renegotiation in flight when the timer fires:** `renegotiate()` runs immediately, picks up all queued SSRCs. +2. **A renegotiation is already in flight (e.g., from `setRequestedVideoChannels`):** the queued `renegotiate()` sets `_pendingRenegotiation`, runs after the in-flight cycle completes — picks up everything including the new SSRCs. +3. **More SSRCs discovered while the timer is pending:** `_discoveryRenegotiationScheduled == true` → no new task scheduled, the SSRC just lands in `_remoteSsrcs` and joins the upcoming batch. + +Result: at most one discovery-sourced renegotiation per 250 ms, regardless of arrival burst size. + +**Audible-gap implication.** New joiners are silent during the debounce window because their packets land in mid=0's catch-all (which still decodes them and feeds the AudioMixer for playback) but their per-receiver `GRAudioLevelSink` doesn't exist yet, so the speaking-indicator UI shows no level until the renegotiation completes (~250 ms + offer/answer round-trip). Audio is heard, the indicator just lags. Acceptable. + +**Stop semantics.** On `stop()`, the queued task may still fire. The `weak_ptr` guard makes the lambda a no-op if the internal has been destroyed. The `_isRenegotiating` flag inside `renegotiate()` also bails if `_peerConnection` has been closed. + +**Behavior change in test mode:** the existing `handleActiveAudioSsrcs` calls `_requestMediaChannelDescriptions` once per batch with all new SSRCs. After refactor, it issues N single-SSRC calls. This is acceptable: requests are local fire-and-forget callbacks into the app and bear no network cost in the CLI test bench. If the iOS app's implementation later turns out to be sensitive to call frequency, `handleActiveAudioSsrcs` can re-aggregate by collecting the SSRCs first and issuing one batched request after the per-SSRC `handleDiscoveredAudioSsrc` calls — but the simpler version is the starting point. + +## Data flow + +1. SFU starts forwarding remote audio for SSRC X (no signaling messages). +2. PeerConnection demuxes the first packet for SSRC X — no recvonly transceiver matches → routed to mid=0's catch-all receiver. The voice channel creates an unsignaled `WebRtcAudioReceiveStream` for X with our tap as the depacketizer-to-decoder transformer. `Transform(frame1)` is called. +3. Tap finds no entry for X → inserts `{ kBuffering, [frame1], now() }` → posts `_onNewSsrc(X)` to the media thread → `handleDiscoveredAudioSsrc(X)`. +4. `handleDiscoveredAudioSsrc` adds X to `_remoteSsrcs`, fires `_requestMediaChannelDescriptions({X}, ...)`, calls `scheduleDiscoveryRenegotiation()` (debounce 250 ms). +5. Subsequent packets for X arrive at the tap (state still `kBuffering`) → appended to the same FIFO (oldest dropped if `>= kMaxBufferedFramesPerSsrc`). +6. After 250 ms the debounce timer fires `renegotiate()`, which adds a recvonly transceiver bound to mid=`_nextMid++` for every entry in `_remoteSsrcs` that lacks one. `SetRemoteDescription` propagates SSRC X into the recvonly m-line; `WebRtcVoiceReceiveChannel::AddRecvStream(X)` finds X in `unsignaled_recv_ssrcs_` and **promotes** the existing stream in place (no new stream created; tap transformer remains attached). +7. `onRenegotiationComplete` runs: `wireRemoteAudioLevelSinks()` attaches a `GRAudioLevelSink` to the new recvonly receiver's track; for each SSRC whose transceiver now has a mid, ReferenceImpl calls `_ssrcTapTransformer->releaseSsrc(ssrc)`. +8. `releaseSsrc(X)` marks the entry `kDrained` under the lock, moves the FIFO out, releases the lock, and calls `OnTransformedFrame` for each buffered frame in arrival order. The promoted stream's decoder receives the burst; NetEQ buffers and plays out at natural rate. +9. Subsequent packets for X reach `Transform()`; the entry is `kDrained` → tap calls `OnTransformedFrame` directly. Live audio flows. The per-receiver `GRAudioLevelSink` (attached in step 7) reads real levels from the post-decode PCM stream. + +**Failure mode (timeout).** If `releaseSsrc(X)` is not called within `kSsrcDiscoveryTimeoutMs = 1000` ms (renegotiation failed or app declined the SSRC), the in-line eviction in `Transform` clears the buffer for X. The entry stays in `kBuffering` so we don't re-notify, but no future frames are forwarded — the participant goes silent. Same outcome as the pure-drop alternative. + +## Threading + +- `Transform` runs on PeerConnection's worker thread. Hot path — must be cheap. Per call: one mutex acquisition, a deque push (or drop), an O(N_active_SSRCs) eviction walk that is cheap (typically 0–2 items per call). On first-sight SSRC the worker thread also posts to the media thread. +- `releaseSsrc` runs on the media thread (called from `onRenegotiationComplete`). Acquires the same mutex, moves the FIFO out under lock, then releases the lock and calls `OnTransformedFrame` outside the lock to avoid re-entrant deadlocks (WebRTC's `OnTransformedFrame` may call back into the transformer infrastructure). +- `OnTransformedFrame` itself: WebRTC's contract does not pin it to a specific thread; calling from the media thread is legal. WebRTC dispatches the actual depacketize/decode onto the worker thread internally. +- `handleDiscoveredAudioSsrc` runs on the media thread. Existing renegotiation machinery is media-thread-safe. +- Lifetime: the transformer is owned by `_ssrcTapTransformer` (member, `scoped_refptr`). `weak_ptr` capture in the SSRC callback prevents use-after-free during teardown. WebRTC clears the transformer when the receiver is destroyed (PeerConnection close); any frames still in the FIFO at that point are released by the deque destructor (the underlying `TransformableFrameInterface` instances are owned `unique_ptr`s and clean up automatically). + +## Testing + +After removing `ActiveAudioSsrcs`, the tap is the only discovery path in every mode — test and real. The existing CLI test bench validates it without modification: + +- All-Reference, no mute: each ReferenceImpl peer must discover the others via the tap, add recvonly transceivers, and report `level ≥ 0.05` (current invariant in `validateGroupState`). +- Mixed (CustomImpl + ReferenceImpl), no mute: same invariant from both sides. +- Mute scenarios: muted peers must still be discovered (their packets carry encoded silence; the tap fires on first packet) and their `level` must read 0 (existing muted-peer invariant from the previous fix). + +If any of these regress, the tap is broken — which is exactly the coverage we want. + +No new CLI flags or SFU exports needed; the previous draft's `--suppress-active-audio-ssrcs` is moot. + +## Out of scope (this design) + +- **E2E `e2eEncryptDecrypt` wiring** is a separate fix, but this design pre-installs the surface it needs: `GRAudioFrameTransformer::DecryptHook` is a constructor parameter (nullptr today). The e2e PR captures `descriptor.e2eEncryptDecrypt` and passes a closure that decrypts the frame's `GetData()` and writes back via `SetData()` before the transformer calls `OnTransformedFrame`. No further structural changes — the transformer is already attached to every audio receiver. +- SSRC>int31 join-payload masking (separate fix). +- Push-style discovery via a new `GroupInstanceInterface::addIncomingAudioSsrcs(...)` method. May be added later but is not needed to fix the immediate problem. +- Video SSRC discovery — handled by the existing app-facing `dataChannelMessageReceived` callback for `ActiveVideoSsrcs` (and via `setRequestedVideoChannels` from MTProto data on iOS). The same per-receiver transformer pattern would extend to incoming video for video e2e, but that's outside the audio scope here. + +## Risks + +- **Buffer-flush correctness.** The tap holds frames until `releaseSsrc` fires. If the call is missed (bug in `onRenegotiationComplete`, race with `stop()`), the timeout clears the buffer at 1 s and the participant goes silent. The CLI integration tests catch this end-to-end: with `ActiveAudioSsrcs` removed, the tap is the *only* discovery path, so the existing `receivedAudio ≥ 0.05` invariant validates the full chain. +- **Tap-passthrough correctness.** Because the tap transformer remains attached after stream promotion, *every* live frame for an SSRC also passes through `Transform()` → `kDrained` branch → `OnTransformedFrame`. If the `kDrained` branch is broken or skipped, every audio frame for every promoted SSRC is silently dropped. Same CLI test coverage applies: the moment passthrough breaks, no peer hears anyone. +- **NetEQ jitter-buffer burst on flush.** `releaseSsrc` flushes up to ~1 s of audio (`kMaxBufferedFramesPerSsrc` × 20 ms = 1.2 s) into the receive stream in one tight loop. NetEQ sees this as a jitter-buffer fill spike. It will normally play out at the natural 50 fps rate, but the burst may exceed `audio_jitter_buffer_max_packets_` and trigger acceleration, deletion, or PLC artifacts. Worst case: a brief audio glitch when a new participant first speaks. Acceptable; mitigated by setting `kMaxBufferedFramesPerSsrc` aggressively low (e.g., 30 frames = 600 ms) if the artifact proves audible. +- **Race window during release.** Marking `kDrained` while still holding the lock prevents concurrent `Transform()` from buffering a doomed frame. There is still a microsecond between unlock and the start of the drain loop where a `Transform()` could beat `releaseSsrc` to the live-passthrough path — but the result is just the new frame arriving at the decoder *before* the buffered backlog. NetEQ reorders by RTP timestamp; harmless. +- **Renegotiation storm.** Mitigated by the 250 ms debounce in `scheduleDiscoveryRenegotiation()`: a burst of N SSRCs in the window collapses to one renegotiation. The existing `_isRenegotiating` / `_pendingRenegotiation` flags handle the case where another renegotiation source (e.g., `setRequestedVideoChannels`) is concurrently in flight. The first-sight check in the tap prevents duplicate per-SSRC scheduling. +- **Memory-pressure cap.** Worst-case buffered audio: `kMaxConcurrentBufferedSsrcs` × `kMaxBufferedFramesPerSsrc` × ~80 bytes/frame ≈ 64 × 60 × 80 = 308 KB. Both bounds are deliberately conservative — a misbehaving SFU sending unique SSRCs per packet hits the SSRC cap and drops further new ones rather than allocating unbounded memory. +- **Stream promotion is still load-bearing for buffered audio.** Live frames flow correctly because we explicitly install the transformer on each recvonly receiver — that path is no longer dependent on internal WebRTC behavior. *Buffered frames* still rely on the unsignaled→signaled stream promotion: the per-SSRC `TransformedFrameCallback` we got at first-sight is the one we replay through. If a future WebRTC update breaks promotion (constructs a new signaled stream and orphans the unsignaled one), `releaseSsrc` would dispatch frames into the orphan and they'd never decode. The CLI tests catch this — buffered audio would be silent during the first 250–500 ms of every new participant, which the audio-level invariant will flag in mute-style tests if extended to assert on first-second levels. + +## Files touched (anticipated) + +- `submodules/TgVoipWebrtc/tgcalls/tgcalls/group/GroupInstanceReferenceImpl.cpp` — add `GRAudioFrameTransformer` (per-SSRC FIFO + `releaseSsrc` + `decryptHook` callable initialized to nullptr); construct + install on mid=0's receiver in `start()`; install on each new recvonly receiver in `renegotiate()`; add `handleDiscoveredAudioSsrc` and `scheduleDiscoveryRenegotiation`; **delete `handleActiveAudioSsrcs` and its dispatch in `onDataChannelMessage`**; call `releaseSsrc` from `onRenegotiationComplete` for each newly-mid-assigned SSRC; add `_audioFrameTransformer` (`scoped_refptr`) and `_discoveryRenegotiationScheduled` members. +- `submodules/TgVoipWebrtc/tgcalls/tools/go_sfu/sfu.go` — **delete `ActiveAudioSsrcs` broadcast** (message construction at `sfu.go:987` and any per-join trigger). +- `submodules/TgVoipWebrtc/CLAUDE.md` and `submodules/TgVoipWebrtc/tgcalls/tools/cli/CLAUDE.md` — update to reflect that ReferenceImpl discovers SSRCs via a `FrameTransformer` tap on mid=0; remove `ActiveAudioSsrcs`/`SetDefaultRawAudioSink` references. +- No iOS-side changes (`OngoingCallThreadLocalContext.mm` etc.) — the existing `requestMediaChannelDescriptions` callback already supports the discovery path. +- No CLI changes (`tools/cli/main.cpp`, `group_mode.cpp/.h`) — the existing tests validate the tap directly. diff --git a/submodules/TgVoipWebrtc/CLAUDE.md b/submodules/TgVoipWebrtc/CLAUDE.md index 7766eb1d60..27994a67c2 100644 --- a/submodules/TgVoipWebrtc/CLAUDE.md +++ b/submodules/TgVoipWebrtc/CLAUDE.md @@ -158,7 +158,7 @@ GroupInstanceReferenceImpl ├── sendonly video transceiver (outgoing H264 simulcast, SDP-munged SSRCs) ├── recvonly audio transceivers (one per remote SSRC, added dynamically) ├── recvonly video transceivers (one per remote endpoint, added dynamically) - └── data channel ("data", for ActiveAudioSsrcs / ActiveVideoSsrcs) + └── data channel ("data", for ActiveVideoSsrcs and Colibri video constraints) ``` ### How It Differs from CustomImpl @@ -167,9 +167,9 @@ GroupInstanceReferenceImpl |--------|-----------|---------------| | Transport | Manual ICE/DTLS/SRTP via GroupNetworkManager | WebRTC PeerConnection | | SDP | None (custom JSON protocol) | Local SDP construction, translates to/from JSON | -| SSRC discovery | `unknownSsrcPacketReceived` on raw RTP | `ActiveAudioSsrcs`/`ActiveVideoSsrcs` data channel messages from SFU | +| SSRC discovery | `unknownSsrcPacketReceived` on raw RTP | Audio: `GRAudioFrameTransformer` on mid=0's unsignaled receiver. Video: `ActiveVideoSsrcs` data channel message from SFU | | Audio channels | Manual `IncomingAudioChannel` per SSRC | PeerConnection recvonly transceivers | -| Audio levels | RTP header extension parsing | Synthetic levels based on known SSRCs | +| Audio levels | RTP header extension parsing | Per-receiver `GRAudioLevelSink` reading real PCM levels | | Video outgoing | Manual `cricket::VideoChannel` with direct SSRC control | PeerConnection sendonly transceiver + SDP munging for simulcast SSRCs | | Video incoming | Manual `IncomingVideoChannel` per endpoint | PeerConnection recvonly transceivers with SSRCs in answer | | Video decode | Manual decoder lifecycle | PeerConnection handles internally | @@ -187,11 +187,17 @@ GroupInstanceReferenceImpl ### Dynamic Participant Handling -**Audio:** -1. SFU sends `{"colibriClass":"ActiveAudioSsrcs","ssrcs":[54321,98765]}` over data channel -2. Client diffs against known SSRCs -3. New SSRCs: add recvonly audio transceiver → renegotiate (new offer + constructed answer mirroring offer mids) -4. Removed SSRCs: clean up from tracking map +**Audio (per-receiver frame transformer):** +1. The first packet for an unknown SSRC X reaches mid=0's receiver — PeerConnection's catch-all for unsignaled audio. The voice channel creates a `WebRtcAudioReceiveStream` for X and attaches `GRAudioFrameTransformer` (registered as `unsignaled_frame_transformer_` on mid=0). +2. The transformer's `Transform(frame)` reads `frame->GetSsrc() = X`, sees X for the first time, **buffers the frame** in a per-SSRC FIFO, and posts the SSRC to the media thread. +3. `handleDiscoveredAudioSsrc(X)` inserts X into `_remoteSsrcs` with a fresh mid, fires `_requestMediaChannelDescriptions({X}, ...)` (matches CustomImpl's contract), and calls `scheduleDiscoveryRenegotiation()` (250 ms debounce). +4. After the debounce, `renegotiate()` adds a recvonly audio transceiver bound to mid=`_nextMid++` for every entry in `_remoteSsrcs` that doesn't have one. `buildRemoteAnswer` includes X on the new m-line; `WebRtcVoiceReceiveChannel::AddRecvStream(X)` PROMOTES the existing unsignaled stream in place (`webrtc_voice_engine.cc:2258-2266`) — the transformer stays attached. +5. `onRenegotiationComplete` runs `wireRemoteAudioLevelSinks()` (attaches `GRAudioLevelSink` per receiver), then calls `_audioFrameTransformer->releaseSsrc(ssrc)` for every SSRC whose transceiver now has a mid. The transformer drains the per-SSRC FIFO via `OnTransformedFrame` so the buffered audio plays without a startup gap. +6. Subsequent live frames for X take the transformer's `kDrained` branch (immediate passthrough). The per-receiver `GRAudioLevelSink` reads real PCM levels. + +The `colibriClass=ActiveAudioSsrcs` data-channel mechanism (test-SFU only) was removed; the tap is the single audio-discovery path. Removed-SSRC handling is the same as CustomImpl: stale recvonly transceivers stay in the SDP indefinitely; participant departures are tracked at the application layer (MTProto). + +The transformer is installed once on mid=0's receiver only — re-installing on the recvonly receivers triggers `Register{Sink,}TransformedFrameCallback` re-runs that overwrite valid registrations and misroute frames. Stream promotion keeps the single instance valid for every signaled SSRC. **Video:** 1. SFU sends `ActiveVideoSsrcs` over data channel → forwarded to app via `dataChannelMessageReceived` diff --git a/submodules/TgVoipWebrtc/tgcalls b/submodules/TgVoipWebrtc/tgcalls index 454718339e..c502af7a2e 160000 --- a/submodules/TgVoipWebrtc/tgcalls +++ b/submodules/TgVoipWebrtc/tgcalls @@ -1 +1 @@ -Subproject commit 454718339e9b8c9dad7effcd94a3f5e534043537 +Subproject commit c502af7a2e87595f1fbb8b98a4ff6942691f85cb