nick/amethyst
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 1 Wiki Activity

test+fix(nests): pin stripLegacyTimestampPrefix; primaryAudio filters to legacy

Browse Source 
Two audit-pass gaps:

G1 — `MoqLiteNestsListener.Companion.stripLegacyTimestampPrefix`
is the entire receive-side wire-format converter for audio frames
(every web speaker's Opus packet flows through it) and had zero
unit tests. A regression here is invisible to users — silent decode
failure of every web speaker — and to interop tests, because both
sides agree on the same bug. New StripLegacyTimestampPrefixTest pins:

  - all four QUIC varint-length tiers (1 / 2 / 4 / 8 bytes), one
    test per tier with a representative timestamp value plus a
    tag-byte assertion so the test fails loudly if Varint.encode's
    tier-selection changes.
  - empty-payload and malformed-payload fast paths (returns
    payload unchanged, same instance — no allocation).
  - round-trip against `NestMoqLiteBroadcaster`'s exact wire
    shape (`varint(timestamp_us) + raw_opus_packet`) across five
    timestamp values spanning all four tiers.

G6 — `RoomSpeakerCatalog.primaryAudio()` previously returned
`audio.renditions.values.firstOrNull()` with no container filter.
A future publisher that emits CMAF before legacy in iteration
order would surface the CMAF rendition, and our decoder pipeline
would then feed CMAF MOOF/MDAT bytes to its legacy
`varint(ts)+opus` parser and decode garbage. Filter to
`container.kind == Container.LEGACY_KIND` so:

  - mixed CMAF+legacy publishers always surface the legacy entry
    regardless of map iteration order
  - CMAF-only publishers correctly return null (caller falls
    back to "unknown codec / no audio")
  - publishers that omit `container` entirely also return null —
    treating "no container declared" as "legacy by default" would
    silently mis-decode a CMAF-only publisher that just forgot
    to spell out `container.kind`

LEGACY_KIND const lives on `Container.Companion` so call sites
share one canonical spelling. Three new test cases pin the new
filter behaviour (mixed iteration order, CMAF-only, missing
container). The pre-existing `toleratesUnknownKeys` test is
updated to declare a legacy container — the unknown-key
tolerance we're checking is about unrecognised siblings, not
container-presence.

https://claude.ai/code/session_014JfZJHSTvyYYWJbC9VbB47
This commit is contained in:
Claude
2026-05-06 19:09:24 +00:00
parent a94d12638f
commit 75f572ba3d
3 changed files with 238 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files
commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/RoomSpeakerCatalog.kt
+29 -6
View File
@@ -106,15 +106,38 @@ data class RoomSpeakerCatalog(
@Serializable
data class Container(
val kind: String? = null,
)
) {
companion object {
/**
* The only `container.kind` value the Amethyst decoder
* pipeline currently understands: each moq-lite frame is
* `varint(timestamp_us) + raw_codec_payload`. Source:
* `kixelated/moq/rs/hang/src/container/legacy.rs`.
*
* Other documented kinds — `"cmaf"` (MOOF/MDAT-fragmented
* MP4) and a handful of in-flight experimental shapes —
* would require a different decoder path and are
* intentionally rejected by [primaryAudio].
*/
const val LEGACY_KIND: String = "legacy"
}
}
/**
* First audio rendition, if any. The current single-Opus reality.
* Map iteration order is the JSON insertion order (kotlinx.serialization
* uses LinkedHashMap), so this picks the first rendition the
* publisher declared rather than an arbitrary one.
* First audio rendition with a [Container.LEGACY_KIND] container,
* which is the only container layout the Amethyst decoder pipeline
* understands today (`varint(timestamp_us) + opus_packet` per
* frame). Returns null when no legacy rendition exists — the UI
* surfaces "no audio info" rather than a CMAF rendition we'd try
* to feed to our legacy decoder.
*
* Picks the first match in JSON iteration order
* (kotlinx.serialization uses LinkedHashMap), so the publisher's
* preferred legacy rendition wins. A future publisher that emits
* CMAF-first then legacy still surfaces the legacy entry; CMAF-
* only publishers correctly return null.
*/
fun primaryAudio(): AudioConfig? = audio?.renditions?.values?.firstOrNull()
fun primaryAudio(): AudioConfig? = audio?.renditions?.values?.firstOrNull { it.container?.kind == Container.LEGACY_KIND }
companion object {
/**
commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/viewmodels/RoomSpeakerCatalogTest.kt
+64 -2
View File
@@ -90,9 +90,17 @@ class RoomSpeakerCatalogTest {
@Test
fun toleratesUnknownKeys() {
// Forward-compat: future hang catalog revisions can add
// fields without breaking older clients.
// fields without breaking older clients. Container.kind is
// declared as `legacy` because `primaryAudio()` filters to
// that kind — the unknown-key tolerance we're testing here
// is about unrecognized siblings, not about the container
// requirement itself.
val json =
"""{"audio":{"renditions":{"audio/data":{"codec":"opus","extra":"future-only"}}},"video":{},"newTopLevel":true}"""
"""{"audio":{"renditions":{"audio/data":{
| "codec":"opus","container":{"kind":"legacy"},
| "extra":"future-only"
|}}},"video":{},"newTopLevel":true}
""".trimMargin()
val catalog = RoomSpeakerCatalog.parseOrNull(json.encodeToByteArray())
assertNotNull(catalog)
assertEquals("opus", catalog.primaryAudio()?.codec)
@@ -116,6 +124,60 @@ class RoomSpeakerCatalogTest {
assertNull(catalog.primaryAudio())
}
@Test
fun primaryAudioPicksLegacyEvenWhenCmafComesFirst() {
// A future publisher may emit CMAF-first then legacy in the
// renditions map. We only know how to decode the legacy
// container today (varint(ts)+opus); CMAF (MOOF/MDAT) would
// be fed bytes-as-Opus and decode to garbage. The filter
// skips non-legacy renditions so the chosen entry is one we
// can actually play.
val mixed =
"""{"audio":{"renditions":{
| "video/cmaf":{"codec":"opus","container":{"kind":"cmaf"},"sampleRate":48000,"numberOfChannels":1},
| "audio/data":{"codec":"opus","container":{"kind":"legacy"},"sampleRate":48000,"numberOfChannels":1}
|}}}
""".trimMargin()
val catalog = RoomSpeakerCatalog.parseOrNull(mixed.encodeToByteArray())
assertNotNull(catalog)
val rendition = catalog.primaryAudio()
assertNotNull(rendition, "expected the legacy rendition, not CMAF")
assertEquals("legacy", rendition.container?.kind)
}
@Test
fun primaryAudioReturnsNullForCmafOnlyPublisher() {
// No legacy rendition → no decoder path. Returning null is
// the right contract: the caller falls back to "unknown
// codec / no audio" rather than feeding CMAF bytes to the
// legacy decoder.
val cmafOnly =
"""{"audio":{"renditions":{"audio/cmaf":{
| "codec":"opus","container":{"kind":"cmaf"},
| "sampleRate":48000,"numberOfChannels":1
|}}}}
""".trimMargin()
val catalog = RoomSpeakerCatalog.parseOrNull(cmafOnly.encodeToByteArray())
assertNotNull(catalog)
assertNull(catalog.primaryAudio())
}
@Test
fun primaryAudioReturnsNullWhenContainerKindIsMissing() {
// Defensive: a malformed publisher that omits `container`
// entirely must NOT be treated as legacy by default — we'd
// be guessing the wire shape. Same null fallback as the
// CMAF-only case.
val noContainer =
"""{"audio":{"renditions":{"audio/data":{
| "codec":"opus","sampleRate":48000,"numberOfChannels":1
|}}}}
""".trimMargin()
val catalog = RoomSpeakerCatalog.parseOrNull(noContainer.encodeToByteArray())
assertNotNull(catalog)
assertNull(catalog.primaryAudio())
}
@Test
fun missingAudioReturnsNullPrimary() {
// hang catalogs declaring only video MUST still parse without
nestsClient/src/commonTest/kotlin/com/vitorpamplona/nestsclient/StripLegacyTimestampPrefixTest.kt
+145
View File
@@ -0,0 +1,145 @@
/*
* Copyright (c) 2025 Vitor Pamplona
*
* Permission is hereby granted, free of charge, to any person obtaining a copy of
* this software and associated documentation files (the "Software"), to deal in
* the Software without restriction, including without limitation the rights to use,
* copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the
* Software, and to permit persons to whom the Software is furnished to do so,
* subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in all
* copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
* FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
* COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN
* AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
* WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
*/
package com.vitorpamplona.nestsclient
import com.vitorpamplona.quic.Varint
import kotlin.test.Test
import kotlin.test.assertContentEquals
import kotlin.test.assertSame
import kotlin.test.assertTrue
/**
* Unit tests for [MoqLiteNestsListener.Companion.stripLegacyTimestampPrefix].
*
* This function is the entire receive-side wire-format converter for
* audio frames — every web speaker's Opus packet flows through it. A
* regression here is invisible to users (silent decode failure of every
* web speaker) and to interop tests (both sides agree on the same bug).
* Pin the four QUIC varint-length tiers, the malformed paths, and a
* round-trip against `NestMoqLiteBroadcaster`'s wire format.
*
* Varint length tags (RFC 9000 §16, top 2 bits of the first byte):
* 00 → 1 byte total (6-bit value, 0..63)
* 01 → 2 bytes total (14-bit, 0..16383)
* 10 → 4 bytes total (30-bit, 0..2^30-1)
* 11 → 8 bytes total (62-bit, 0..2^62-1)
*/
class StripLegacyTimestampPrefixTest {
@Test
fun strips_one_byte_varint_for_small_timestamp() {
// `Varint.encode(0L)` returns a single byte with tag 00.
val frame = Varint.encode(0L) + OPUS
val stripped = MoqLiteNestsListener.stripLegacyTimestampPrefix(frame)
assertContentEquals(OPUS, stripped)
}
@Test
fun strips_one_byte_varint_at_max_6bit_value() {
// 63 (0x3F) is the largest value still encoded in 1 byte.
val frame = Varint.encode(63L) + OPUS
assertContentEquals(byteArrayOf(0x3F) + OPUS, frame, "varint(63) is a single 0x3F byte")
val stripped = MoqLiteNestsListener.stripLegacyTimestampPrefix(frame)
assertContentEquals(OPUS, stripped)
}
@Test
fun strips_two_byte_varint() {
// 64 (just past the 1-byte boundary) → tag 01, 2 bytes.
val frame = Varint.encode(64L) + OPUS
val tag = (frame[0].toInt() ushr 6) and 0x3
assertTrue(tag == 0x1, "varint(64) should use 2-byte form, got tag=$tag")
val stripped = MoqLiteNestsListener.stripLegacyTimestampPrefix(frame)
assertContentEquals(OPUS, stripped)
}
@Test
fun strips_four_byte_varint() {
// ~4.2 million µs ≈ 4.2 s — comfortably past the 2-byte 16383 µs
// limit and well within any real broadcast lifetime.
val frame = Varint.encode(4_200_000L) + OPUS
val tag = (frame[0].toInt() ushr 6) and 0x3
assertTrue(tag == 0x2, "varint(4_200_000) should use 4-byte form, got tag=$tag")
val stripped = MoqLiteNestsListener.stripLegacyTimestampPrefix(frame)
assertContentEquals(OPUS, stripped)
}
@Test
fun strips_eight_byte_varint() {
// > 2^30 µs (~17.9 minutes) forces the 8-byte form. Models a
// broadcast that's been running long enough for the timestamp
// counter to overflow into the widest tier — perfectly valid
// Opus stream timestamps but not exercised by the smaller
// tiers.
val frame = Varint.encode(2_000_000_000L) + OPUS
val tag = (frame[0].toInt() ushr 6) and 0x3
assertTrue(tag == 0x3, "varint(2_000_000_000) should use 8-byte form, got tag=$tag")
val stripped = MoqLiteNestsListener.stripLegacyTimestampPrefix(frame)
assertContentEquals(OPUS, stripped)
}
@Test
fun empty_payload_returns_empty() {
val empty = ByteArray(0)
// Same instance — no allocation on the empty path.
assertSame(empty, MoqLiteNestsListener.stripLegacyTimestampPrefix(empty))
}
@Test
fun malformed_payload_smaller_than_varint_returns_payload_unchanged() {
// A frame body that's just the high tag byte of an 8-byte
// varint with no follow-up bytes is malformed. The current
// contract is "return the payload unchanged so upstream
// surfaces the corruption rather than silently masking it."
// This pins that contract — if it ever changes (e.g. to
// throw or return empty), this test fails and forces an
// explicit decision.
val malformed = byteArrayOf(0xC0.toByte()) // tag=11, expects 8 bytes total
val result = MoqLiteNestsListener.stripLegacyTimestampPrefix(malformed)
assertContentEquals(malformed, result, "malformed frame returned unchanged")
assertSame(malformed, result, "no allocation on the unchanged-malformed path")
}
@Test
fun round_trip_against_broadcaster_wire_shape() {
// Mirror the byte sequence
// [com.vitorpamplona.nestsclient.audio.NestMoqLiteBroadcaster] writes:
// varint(timestamp_us) + raw_opus_packet
// Stripping MUST recover the exact opus packet byte-for-byte
// for every frame the watcher receives. Any drift here means
// every decoded frame loses or gains bytes at the front.
val timestamps = longArrayOf(0L, 20_000L, 40_000L, 1_000_000L, 2_000_000_000L)
for (ts in timestamps) {
val tsLen = Varint.size(ts)
val frame = ByteArray(tsLen + OPUS.size)
Varint.writeTo(ts, frame, 0)
OPUS.copyInto(frame, tsLen)
val stripped = MoqLiteNestsListener.stripLegacyTimestampPrefix(frame)
assertContentEquals(OPUS, stripped, "round-trip failed for timestamp $ts")
}
}
private companion object {
// 8 bytes of plausible-looking Opus packet bytes — the function
// doesn't care about the codec content, only the length /
// content equality after the strip.
private val OPUS = byteArrayOf(0x78, 0x12, 0x34, 0x56, 0x78.toByte(), 0x9A.toByte(), 0xBC.toByte(), 0xDE.toByte())
}
}