daniel/deepdrft
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 13 Wiki Activity

fix: Wave 18.1 review — pre-skip subtraction, t=0 anchor, PreSkip in sidecar, stderr on cancel

Browse Source
This commit is contained in:
daniel-c-harvey
2026-06-23 06:55:31 -04:00
parent 33d6f34d8a
commit 6add30a4ff
6 changed files with 251 additions and 45 deletions
Download Patch File Download Diff File Expand all files Collapse all files
DeepDrftTests/OggOpusParserTests.cs
+157 -24
View File
@@ -7,13 +7,19 @@ namespace DeepDrftTests;
/// <summary>
/// Coverage for the Phase 18.1 seek-index + setup-header extraction (§3.4a). These exercise the pure
/// Ogg-Opus walker and the sidecar codec over hand-built Ogg streams — no ffmpeg dependency — so the
/// granule→byte mapping, page-boundary snapping, 0.5 s bucketing, clamp totals, and setup-header capture
/// are asserted deterministically. The byte layout mirrors a real Opus stream: an OpusHead page, an
/// OpusTags page, then audio pages each carrying an end granule position at 48 kHz.
/// granule→byte mapping, pre-skip correction (RFC 7845 §4.3), page-boundary snapping, 0.5 s bucketing,
/// t=0 anchor, clamp totals, and setup-header capture are asserted deterministically. The byte layout
/// mirrors a real Opus stream: an OpusHead page, an OpusTags page, then audio pages each carrying an
/// end granule position at 48 kHz.
/// </summary>
[TestFixture]
public class OggOpusParserTests
{
// libopus default pre-skip: 312 samples at 48 kHz (≈ 6.5 ms). FFmpeg may use 3840 (~80 ms).
// Using 312 here as a realistic non-zero value that is small enough not to affect the test
// granules (which start at 48000), while still exercising the pre-skip subtraction path.
private const ushort TestPreSkip = 312;
[Test]
public void Walk_CapturesSetupHeader_AsLeadingOpusHeadAndOpusTagsPagesVerbatim()
{
@@ -32,9 +38,11 @@ public class OggOpusParserTests
}
[Test]
public void Walk_FirstSeekPoint_IsTheFirstAudioPageAtItsExactByteOffset()
public void Walk_FirstSeekPoint_IsAnchoredAtTimeZero_PointingAtFirstAudioPage()
{
var head = OggPage(granule: 0, OpusHeadPacket());
// Use a non-zero pre-skip to verify that the first seek point is explicitly anchored at
// corrected time = 0, not at the raw granule time.
var head = OggPage(granule: 0, OpusHeadPacket(preSkip: TestPreSkip));
var tags = OggPage(granule: 0, OpusTagsPacket());
var audio = OggPage(granule: 48000, AudioPacket(64));
@@ -45,9 +53,87 @@ public class OggOpusParserTests
Assert.That(walk, Is.Not.Null);
var first = walk!.SeekIndex.Points[0];
// The first point's byte offset must be the first audio page start (exact page boundary).
Assert.That(first.ByteOffset, Is.EqualTo(firstAudioOffset),
"The first seek point must land on the first audio page's start offset (an exact page boundary)");
Assert.That(first.GranulePosition, Is.EqualTo(48000UL));
// The first point's stored granule is clamped to preSkip so corrected presentation time = 0.
// This guarantees a binary search for t=0 always resolves to the first audio page.
Assert.That(walk.SeekIndex.PreSkip, Is.EqualTo(TestPreSkip),
"PreSkip must be parsed from OpusHead and carried into the seek index");
Assert.That(walk.SeekIndex.PresentationTimeSeconds(first), Is.EqualTo(0.0).Within(1e-12),
"First seek point must have corrected presentation time = 0 so a seek to t=0 always resolves");
}
[Test]
public void Walk_PreSkip_IsSubtractedFromGranuleInTimeCalculations()
{
// Pre-skip of 3840 samples (≈ 80 ms, the libopus typical value used by ffmpeg).
// Without the fix, pageTime = 48000 / 48000 = 1.0 s; with fix, (48000 - 3840) / 48000 = 0.92 s.
const ushort preSkip = 3840;
var head = OggPage(granule: 0, OpusHeadPacket(preSkip: preSkip));
var tags = OggPage(granule: 0, OpusTagsPacket());
// First audio page at granule 48000 (1.0 s raw; 0.92 s corrected)
var a1 = OggPage(granule: 48000, AudioPacket(64));
// Second audio page at granule 96000 (2.0 s raw; 1.92 s corrected)
var a2 = OggPage(granule: 96000, AudioPacket(64));
// Third audio page at granule 144000 (3.0 s raw; 2.92 s corrected)
var a3 = OggPage(granule: 144000, AudioPacket(64));
var walk = OggOpusParser.Walk(Concat(head, tags, a1, a2, a3));
Assert.That(walk, Is.Not.Null);
var index = walk!.SeekIndex;
Assert.That(index.PreSkip, Is.EqualTo(preSkip), "PreSkip must be parsed from OpusHead");
// TotalDurationSeconds must be pre-skip-corrected: (144000 - 3840) / 48000 = 2.92 s
var expectedDuration = (144000.0 - preSkip) / 48000.0;
Assert.That(index.TotalDurationSeconds, Is.EqualTo(expectedDuration).Within(1e-9),
"TotalDurationSeconds must subtract preSkip (RFC 7845 §4.3), not use raw lastGranule / 48000");
// The second indexed point (first real bucket) must have corrected time, not raw time.
// With correctedTime(a2) = 1.92 s and bucket = 0.5 s, it should fall in the 1.5 s bucket.
if (index.Points.Count > 1)
{
var secondPoint = index.Points[1];
var corrected = index.PresentationTimeSeconds(secondPoint);
Assert.That(corrected, Is.GreaterThan(0.0),
"Non-first indexed points must have positive corrected presentation times");
Assert.That(secondPoint.RawTimeSeconds, Is.GreaterThan(corrected),
"Raw time must be greater than corrected time when pre-skip > 0");
}
}
[Test]
public void Walk_SeekToZero_ResolvesToFirstAudioPageOffset_WithNonZeroPreSkip()
{
// This is the AC9 / Critical-2 regression test: a seek to t=0 must resolve to the first audio
// page's byte offset, not produce "no entry found". With the old code (no t=0 anchor and no
// pre-skip correction), the first indexed point had correctedTime ≈ 0.92 s (for preSkip=3840),
// so a binary search for t=0 would find no entry with time ≤ 0 and fail.
const ushort preSkip = 3840;
var head = OggPage(granule: 0, OpusHeadPacket(preSkip: preSkip));
var tags = OggPage(granule: 0, OpusTagsPacket());
var a1 = OggPage(granule: 48000, AudioPacket(64));
var a2 = OggPage(granule: 96000, AudioPacket(64));
var stream = Concat(head, tags, a1, a2);
var firstAudioByteOffset = (ulong)(head.Length + tags.Length);
var walk = OggOpusParser.Walk(stream);
Assert.That(walk, Is.Not.Null);
var index = walk!.SeekIndex;
var firstPoint = index.Points[0];
// Simulate the binary search: find the largest entry with PresentationTimeSeconds ≤ 0.
// With the fix, the first point has corrected time = 0.0, so it IS found.
Assert.That(index.PresentationTimeSeconds(firstPoint), Is.EqualTo(0.0).Within(1e-12),
"First point corrected time must be exactly 0.0 so binary search for t=0 resolves it");
Assert.That(firstPoint.ByteOffset, Is.EqualTo(firstAudioByteOffset),
"The t=0 anchor must point at the first audio page's byte offset, not the stream start");
}
[Test]
@@ -82,8 +168,8 @@ public class OggOpusParserTests
[Test]
public void Walk_Bucketing_EmitsRoughlyOneEntryPerHalfSecond()
{
// Twenty audio pages of 0.25 s each = 5 s total. At 0.5 s buckets we expect ~10 entries
// (the first audio page is always taken, then one per crossed half-second boundary).
// Twenty audio pages of 0.25 s each = 5 s total (zero pre-skip). At 0.5 s buckets:
// first point (anchored at t=0) + one per 0.5 s boundary = 1 + 10 = 11 entries expected.
var head = OggPage(granule: 0, OpusHeadPacket());
var tags = OggPage(granule: 0, OpusTagsPacket());
@@ -99,9 +185,9 @@ public class OggOpusParserTests
var walk = OggOpusParser.Walk(stream);
Assert.That(walk, Is.Not.Null);
// 5 s of audio → first point + one per 0.5 s boundary up to 5.0 s. Allow a small tolerance for
// boundary rounding, but it must be far below "one per page" (20) and at least the ~10 buckets.
Assert.That(walk!.SeekIndex.Points.Count, Is.InRange(9, 12),
// 5 s of audio with 0.5 s buckets: 1 anchor + 10 bucket crossings = 11 entries.
// Accept 10–12 for floating-point boundary tolerance, but must be far below 20 (one per page).
Assert.That(walk!.SeekIndex.Points.Count, Is.InRange(10, 12),
"Bucketing must coalesce ~0.25 s pages into ~0.5 s index entries, not one per page");
}
@@ -131,12 +217,13 @@ public class OggOpusParserTests
}
[Test]
public void Walk_ClampValues_ReflectFinalGranuleAndTotalByteLength()
public void Walk_ClampValues_ReflectPreSkipCorrectedDurationAndTotalByteLength()
{
var head = OggPage(granule: 0, OpusHeadPacket());
const ushort preSkip = 312;
var head = OggPage(granule: 0, OpusHeadPacket(preSkip: preSkip));
var tags = OggPage(granule: 0, OpusTagsPacket());
var a1 = OggPage(granule: 48000, AudioPacket(64)); // 1.0 s
var a2 = OggPage(granule: 144000, AudioPacket(64)); // 3.0 s (final)
var a1 = OggPage(granule: 48000, AudioPacket(64)); // 1.0 s raw; ~0.9935 s corrected
var a2 = OggPage(granule: 144000, AudioPacket(64)); // 3.0 s raw; ~2.9935 s corrected (final)
var stream = Concat(head, tags, a1, a2);
@@ -145,8 +232,13 @@ public class OggOpusParserTests
Assert.That(walk!.SeekIndex.TotalByteLength, Is.EqualTo((ulong)stream.Length),
"Total byte length must equal the full stream length for end-of-stream clamping");
Assert.That(walk.SeekIndex.TotalDurationSeconds, Is.EqualTo(3.0).Within(1e-9),
"Total duration must derive from the final page's granule position (144000 / 48000 = 3.0 s)");
var expectedDuration = (144000.0 - preSkip) / 48000.0;
Assert.That(walk.SeekIndex.TotalDurationSeconds, Is.EqualTo(expectedDuration).Within(1e-9),
"TotalDurationSeconds must be pre-skip-corrected: (lastGranule - preSkip) / 48000");
Assert.That(walk.SeekIndex.PreSkip, Is.EqualTo(preSkip),
"PreSkip must round-trip through the seek index");
}
[Test]
@@ -165,27 +257,58 @@ public class OggOpusParserTests
{
var points = new[]
{
new OpusSeekPoint(48000, 200),
new OpusSeekPoint(312, 200), // first point anchored at preSkip
new OpusSeekPoint(72000, 512),
new OpusSeekPoint(96000, 900),
};
var index = new OggOpusSeekIndex(points, TotalDurationSeconds: 2.0, TotalByteLength: 1024);
var index = new OggOpusSeekIndex(points, TotalDurationSeconds: 2.0, TotalByteLength: 1024,
PreSkip: 312);
var restored = OggOpusSeekIndex.FromBytes(index.ToBytes());
Assert.That(restored, Is.Not.Null);
Assert.That(restored!.TotalByteLength, Is.EqualTo(1024UL));
Assert.That(restored.TotalDurationSeconds, Is.EqualTo(2.0));
Assert.That(restored.PreSkip, Is.EqualTo((ushort)312),
"PreSkip must survive the binary round-trip");
Assert.That(restored.Points, Is.EqualTo(points));
}
[Test]
public void SeekIndex_PresentationTimeSeconds_SubtractsPreSkip()
{
const ushort preSkip = 3840;
var point = new OpusSeekPoint(GranulePosition: 48000, ByteOffset: 200);
var index = new OggOpusSeekIndex(
new[] { point }, TotalDurationSeconds: 0.92, TotalByteLength: 500, PreSkip: preSkip);
var corrected = index.PresentationTimeSeconds(point);
var expected = (48000.0 - preSkip) / 48000.0; // ≈ 0.92 s
Assert.That(corrected, Is.EqualTo(expected).Within(1e-9),
"PresentationTimeSeconds must return (granule - preSkip) / 48000, not raw granule / 48000");
}
[Test]
public void SeekIndex_PresentationTimeSeconds_ClampsToZeroForFirstAnchorPoint()
{
const ushort preSkip = 3840;
// First anchor point: granule stored as preSkip, so corrected time = 0.
var firstPoint = new OpusSeekPoint(GranulePosition: preSkip, ByteOffset: 150);
var index = new OggOpusSeekIndex(
new[] { firstPoint }, TotalDurationSeconds: 2.0, TotalByteLength: 500, PreSkip: preSkip);
Assert.That(index.PresentationTimeSeconds(firstPoint), Is.EqualTo(0.0).Within(1e-12),
"The t=0 anchor point (granule == preSkip) must yield corrected time = 0.0 exactly");
}
[Test]
public void Sidecar_RoundTrips_PreservingSetupHeaderAndIndex()
{
var setup = Encoding.ASCII.GetBytes("OpusHead-and-OpusTags-bytes-go-here");
var index = new OggOpusSeekIndex(
new[] { new OpusSeekPoint(48000, 200), new OpusSeekPoint(96000, 700) },
TotalDurationSeconds: 2.0, TotalByteLength: 800);
new[] { new OpusSeekPoint(312, 200), new OpusSeekPoint(96000, 700) },
TotalDurationSeconds: 2.0, TotalByteLength: 800, PreSkip: 312);
var sidecar = new OpusSidecar(setup, index);
var restored = OpusSidecar.FromBytes(sidecar.ToBytes());
@@ -195,6 +318,8 @@ public class OggOpusParserTests
"The sidecar must preserve the setup header so the client can prepend it to mid-stream slices");
Assert.That(restored.SeekIndex.Points, Is.EqualTo(index.Points));
Assert.That(restored.SeekIndex.TotalByteLength, Is.EqualTo(800UL));
Assert.That(restored.SeekIndex.PreSkip, Is.EqualTo((ushort)312),
"PreSkip must survive the sidecar binary round-trip");
}
[Test]
@@ -240,10 +365,18 @@ public class OggOpusParserTests
return Concat(header, packet);
}
private static byte[] OpusHeadPacket()
private static byte[] OpusHeadPacket(ushort preSkip = 0)
{
// "OpusHead" + a minimal valid-ish identification header tail (version, channels, pre-skip, etc.).
var tail = new byte[] { 1, 2, 0, 0, 0x80, 0xBB, 0, 0, 0, 0, 0 };
// "OpusHead" + RFC 7845 §5.1 identification header:
// [0] version = 1, [1] channel count = 2,
// [2-3] pre_skip (little-endian uint16), [4-7] input sample rate = 0xBB80 = 48000,
// [8-9] output gain = 0, [10] channel mapping family = 0.
var tail = new byte[11];
tail[0] = 1; // version
tail[1] = 2; // channels
BinaryPrimitives.WriteUInt16LittleEndian(tail.AsSpan(2, 2), preSkip); // pre_skip
BinaryPrimitives.WriteUInt32LittleEndian(tail.AsSpan(4, 4), 48000); // input sample rate
tail[10] = 0; // channel mapping family
return Concat(OggOpusConstants.OpusHeadSignature.ToArray(), tail);
}