DEV-1030 Incorrect recording duration after SD card import (CASE:8952)

[MAzalya]

No description provided.

[marknolan]

Superseded by #300, which adopts the half of this that I had missed. Closing here, but
the reorder/duplicate detection is yours and it is carried across with credit on the
commit.

What this PR got right, and #300 did not

My branch rejected an exact 0x000000 and nothing else, so a corrupted non-zero value, a
duplicated packet and a reordered one all still added 2²⁴ ticks. My own test
testNonZeroBackwardStepIsStillAWrap asserted that as correct behaviour. It isn't, and
this PR is what showed it — detecting the general case by comparing the backward step
against a window derived from the sampling rate. #300 now does that.

The six-device table in DEV-1030 is also better evidence of the mechanism than anything I
had: every device's error an integer multiple of 512 s, against one clean device as the
control. I had one file. That table is going onto DEV-1023.

Why the polarity is inverted in #300

The rule here makes "corrupt" the default and calls something a roll-over only when the
backward step clears maxTicks − 10 · expectedTicksPerSample. #300 makes forward motion
the default and carves out reorder and invalid-zero as the narrow cases. Same input, and
the failure modes swap places.

Three things I found simulating this rule against the bytes of the affected recording,
each of which the inverted form avoids:

1. At the rate the affected trial actually ran, the fix takes over the whole file.
expectedTicksPerSample = getRtcClockFreq() / getSamplingRateShimmer().

rate returned	period	of the 201 samples after the bad record
32768/65 = 504.1230769…	65.000000	1 classified corrupt — exact, works
504.12 (rounded)	65.000397	201 — all of them

At the rounded rate the interpolated value overshoots each following real sample by four
ten-thousandths of a tick, so every one is reclassified corrupt and replaced. From the
first bad record onward the file is on a synthesised nominal-rate ramp rather than the
device's clock, and any genuine jitter or gap after that point is erased.

The duration still comes out right, which is exactly why the manual check looked clean. An
end-to-end duration comparison cannot see this; a unit test catches it immediately.

2. An unset rate silently restores the original defect. getSamplingRateShimmer()
returns maxSetRate, initialised to 0.0, over a map that can be empty. Java evaluates
32768.0 / 0.0 to Infinity without throwing, so maxTicks − 10 · Infinity is -Infinity
and -delta > -Infinity is always true — every backward step becomes a roll-over. The
zero record yields 16777216, the exact +2²⁴ the fix exists to prevent, with the
corruption counter reading 0. #300 returns a window of zero for every shape of unknown
rate, and has a test for infinity, NaN and negatives specifically because a division is
what produces them.

3. A roll-over preceded by more than ten lost samples is misread as corruption. This is
the polarity itself: with "corrupt" as the default, packet loss before a wrap flips the
classification. It self-heals on the following sample, so the cost is small — but it is the
case that decides which way round the rule should be, and wrap-after-heavy-loss-24bit in
the shared vectors pins it.

What else changed on the way

Reviewing the merge turned up two problems in my rule, not this one:

• A window sized as a fraction of the modulo — which is what my web SDK copy used —
  misreads every 1.75–2.0 s dropout on the 2-byte counter as a reorder and silently loses
  the wrap. An ordinary Bluetooth gap. Your rate-derived sizing is what fixes it; DEV-1023: one timestamp-unwrap rule — invalid zeros, reorders, duplicates and wraps #300
  uses eight sample periods, which shrinks that band to about 16 ms.
• Stating the rule on "is the candidate below the last unwrapped value" misses a packet
  arriving late from before a wrap boundary — it costs 512 s twice. DEV-1023: one timestamp-unwrap rule — invalid zeros, reorders, duplicates and wraps #300 compares
  modular distances instead.

Where it is now

The rule is specified once in log-and-stream-common
(#136), with 26
machine-readable conformance vectors that the Java, C#, pyshimmer and web SDK suites all
run. Four implementations of one wire format drifted apart precisely because nobody could
check them against each other; that is now a test rather than a review habit.

wrap-after-heavy-loss-24bit, wrap-spanning-dropout-1p8s-16bit and
reorder-across-wrap-boundary-24bit are each the case that one of the superseded rules
gets wrong — including two of mine.

Worth a look at #300 when you have a moment, and thanks for catching the gap.

[marknolan]

Closing as superseded by #300 — see the comment above. The reorder/duplicate detection from here is carried across with a Co-Authored-By on the commit; reopen if you would rather take it forward on this branch instead.