fix(udpscope): bound accumulated-burst chaining so packet loss cannot displace the trace
Forward-chaining each accumulated burst onto the previous one suppresses arrival jitter, but an unchecked chain never recovers: one lost datagram, or a declared sampling rate that differs from the producer's real one, dates every later sample early for the rest of the run. The chain is now a prediction, compared each packet against the arrival anchor and abandoned beyond kBurstResyncThresholdS, which bounds the error instead of accumulating it. Plan amended so the hrt-fit fallback (unusable here: the fit needs 32 packets and is itself corrupted by bursty arrivals) cannot come back. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.6
parent
5a8479cda9
commit
892e3eae28
@@ -1609,6 +1609,42 @@ TEST(FrameDecoder, AccumulatedScalarSurvivesBurstyDelivery) {
|
||||
}
|
||||
}
|
||||
|
||||
// ADDED in Task 4 review. The counterweight to the test above: suppressing
|
||||
// arrival jitter by chaining bursts is only safe while the chain is CHECKED. On
|
||||
// UDP packets are lost, and an unchecked chain closes the hole silently and
|
||||
// dates every later sample early for the rest of the run.
|
||||
TEST(FrameDecoder, AccumulatedScalarResynchronisesAfterLostPackets) {
|
||||
FrameDecoder dec;
|
||||
SignalMeta m;
|
||||
m.name = "Acc";
|
||||
m.typeCode = 9;
|
||||
m.numRows = 1;
|
||||
m.samplingRate = 1000.0; /* 10 samples = 10 ms per packet */
|
||||
dec.setSignals({m});
|
||||
|
||||
std::vector<double> ts;
|
||||
for (int p = 0; p < 10; p++) {
|
||||
FrameBuilder fb;
|
||||
fb.addSignal(std::vector<double>(10, 1.0));
|
||||
const FrameView& f = fb.build(0, 500.0 + p * 0.010, 10);
|
||||
dec.beginFrame(f);
|
||||
ASSERT_TRUE(dec.timestamps(f, 0, ts));
|
||||
}
|
||||
EXPECT_NEAR(ts[9], 500.090, 1e-9);
|
||||
|
||||
/* A full second of packets never arrives. The next one lands at 501.100. */
|
||||
FrameBuilder fb;
|
||||
fb.addSignal(std::vector<double>(10, 1.0));
|
||||
const FrameView& f = fb.build(0, 501.100, 10);
|
||||
dec.beginFrame(f);
|
||||
ASSERT_TRUE(dec.timestamps(f, 0, ts));
|
||||
|
||||
/* Chaining blindly would put this burst at 500.091..500.100, as though no
|
||||
* data were missing. */
|
||||
EXPECT_NEAR(ts[0], 501.091, 1e-9);
|
||||
EXPECT_NEAR(ts[9], 501.100, 1e-9);
|
||||
}
|
||||
|
||||
TEST(FrameDecoder, AccumulatedScalarDerivesDtFromTheHrtGapWhenNoRateIsDeclared) {
|
||||
FrameDecoder dec;
|
||||
SignalMeta m;
|
||||
@@ -1882,8 +1918,44 @@ bool FrameDecoder::timestamps(const FrameView& f, uint32_t idx,
|
||||
return true;
|
||||
}
|
||||
|
||||
/* Rule 3: accumulated scalar, based on the producer's own hrt. */
|
||||
/* Rule 3: accumulated scalar.
|
||||
*
|
||||
* AMENDED after Task 4 review. The version below originally sent EVERY
|
||||
* accumulated scalar through the hrt fit, falling back to packetBurst until
|
||||
* the fit was ready. That cannot work when a declared samplingRate is
|
||||
* present: HrtRateFit needs 32 packets, bursty delivery can begin before
|
||||
* that, and packetBurst then crams a 10 ms burst into a 50 us arrival gap —
|
||||
* exactly the sawtooth this rule exists to prevent. Worse, HrtRateFit fits
|
||||
* hrt against ARRIVAL time, so a burst episode corrupts the very rate the
|
||||
* fallback is waiting on.
|
||||
*
|
||||
* With a declared rate none of that is needed: the intra-packet step is
|
||||
* exact, and bursts are contiguous, so the next burst is PREDICTED at
|
||||
* lastEmittedEnd + dt. The prediction must be checked, not trusted — a pure
|
||||
* chain silently closes the hole left by a lost datagram and dates every
|
||||
* later sample early for the rest of the run. So each packet compares the
|
||||
* prediction against the arrival anchor and abandons it beyond
|
||||
* kBurstResyncThresholdS. The hrt path below remains for samplingRate == 0. */
|
||||
if (d.numElements() == 1u && nElems > 1u) {
|
||||
const double dtDeclared = (d.samplingRate > 0.0) ? (1.0 / d.samplingRate) : 0.0;
|
||||
if (d.samplingRate > 0.0) {
|
||||
const double arrivalAnchor =
|
||||
wallNow - static_cast<double>(nElems - 1u) * dtDeclared;
|
||||
double base = arrivalAnchor;
|
||||
if (st.lastEmittedValid) {
|
||||
const double predicted = st.lastEmittedEnd + dtDeclared;
|
||||
if (std::fabs(predicted - arrivalAnchor) <= kBurstResyncThresholdS) {
|
||||
base = predicted;
|
||||
}
|
||||
}
|
||||
tsOut.resize(nElems);
|
||||
for (uint32_t e = 0; e < nElems; e++) {
|
||||
tsOut[e] = base + static_cast<double>(e) * dtDeclared;
|
||||
}
|
||||
st.lastEmittedEnd = tsOut[nElems - 1u];
|
||||
st.lastEmittedValid = true;
|
||||
return true;
|
||||
}
|
||||
if (!hrtFit_.ready()) {
|
||||
return packetBurst(idx, nElems, wallNow, tsOut);
|
||||
}
|
||||
@@ -1891,10 +1963,8 @@ bool FrameDecoder::timestamps(const FrameView& f, uint32_t idx,
|
||||
const double base = st.offset.map(hrtSec, wallNow);
|
||||
|
||||
double dt;
|
||||
if (d.samplingRate > 0.0) {
|
||||
dt = 1.0 / d.samplingRate;
|
||||
} else if (st.lastAccValid && st.prevAccCount > 0u &&
|
||||
hrtSec > st.lastAccHrtSec) {
|
||||
if (st.lastAccValid && st.prevAccCount > 0u &&
|
||||
hrtSec > st.lastAccHrtSec) {
|
||||
/* The flushes carry contiguous RT cycles, so the gap divided by the
|
||||
* previous packet's sample count is exactly one cycle period. */
|
||||
dt = (hrtSec - st.lastAccHrtSec) /
|
||||
@@ -1945,7 +2015,7 @@ cd Client/udpscope && cmake --build build -j && ./build/udpscope_tests --gtest_f
|
||||
|
||||
Expected: PASS, 9 tests.
|
||||
|
||||
If `AccumulatedScalarSurvivesBurstyDelivery` fails on the first few samples, check that `beginFrame()` is being called before `timestamps()` — the hrt fit needs 32 packets before rule 3 engages, and the packets before that legitimately go through rule 4.
|
||||
If `AccumulatedScalarSurvivesBurstyDelivery` fails, do NOT reach for the hrt fit: with a declared `samplingRate` rule 3 never consults it, precisely because the fit is not ready for the first 32 packets and is itself corrupted by bursty arrivals. Check instead that `lastEmittedEnd`/`lastEmittedValid` are being updated on every emitted burst. The only test that may legitimately fall through to rule 4 early is `AccumulatedScalarDerivesDtFromTheHrtGapWhenNoRateIsDeclared`, whose arrivals are uniform, so `packetBurst` is accurate there.
|
||||
|
||||
- [ ] **Step 8: Commit**
|
||||
|
||||
|
||||
Reference in New Issue
Block a user