refactor(engine): one RIFF chunk walk, not two

`readWavChunks` and `parseWavLayout` each carried the same word-aligned
walk. The reason this was left alone before is real — the two are *not*
the same function, and unifying them means picking one behaviour for each
of four differences in the parser every render's audio goes through: one
breaks at the first `data` and returns a slice, the other scans every
chunk and returns offsets; one lets the decoder judge the format, the
other refuses anything but 16-bit PCM.

So only the walk moves. `riffChunks` yields `{ id, body, size }` and holds
no policy at all, both readers keep every one of those four behaviours,
and there is no cycle to route around because it belongs to neither of
them. It lives in engine/services beside both.

It also had no test, in either copy. A real WAV out of the mixer has an
even-sized `fmt ` first and `data` last, so neither of the two things the
walk exists for ever came up: the pad byte after an odd chunk, and not
assuming an ordering. Both mutations survived a full engine run before
`wavChunks.test.ts`; both fail now, as does removing the guard against a
header that runs past a truncated file.

engine services 739 passing / 3 skipped (736 + the 3 new).

Author: vanceingalls

packages/engine/src/services/audioFxRender.ts

@@ -21,6 +21,7 @@ import { enabledAudioFxNodes, type HfAudioFxChain } from "@hyperframes/core/audi
 import { serializeAutomation, type HfAutomation } from "@hyperframes/core/audio-automation";
 import { acquireBrowser } from "./browserManager.js";
 import { createEnvelopeWalker } from "./audioVolumeEnvelope.js";
+import { riffChunks } from "./wavChunks.js";
 import type { AudioVolumeKeyframe } from "./audioMixer.types.js";
 
 export class AudioFxRenderError extends Error {
@@ -49,22 +50,22 @@ function readWavChunks(buf: Buffer): {
   bits: number;
   data?: Buffer;
 } {
-  let offset = 12;
   const head = { format: 1, channels: 1, sampleRate: 48000, bits: 16 };
   let data: Buffer | undefined;
-  while (offset + 8 <= buf.length) {
-    const id = buf.toString("ascii", offset, offset + 4);
-    const size = buf.readUInt32LE(offset + 4);
+  for (const { id, body, size } of riffChunks(buf)) {
     if (id === "fmt ") {
-      head.format = buf.readUInt16LE(offset + 8);
-      head.channels = buf.readUInt16LE(offset + 10);
-      head.sampleRate = buf.readUInt32LE(offset + 12);
-      head.bits = buf.readUInt16LE(offset + 22);
+      head.format = buf.readUInt16LE(body);
+      head.channels = buf.readUInt16LE(body + 2);
+      head.sampleRate = buf.readUInt32LE(body + 4);
+      head.bits = buf.readUInt16LE(body + 14);
     } else if (id === "data") {
-      data = buf.subarray(offset + 8, Math.min(buf.length, offset + 8 + size));
+      data = buf.subarray(body, Math.min(buf.length, body + size));
+      // The payload is the rest of the file for anything the mixer writes, and
+      // reading past it buys nothing: `fmt ` precedes `data` in every WAV these
+      // steps produce, and the alternative is walking a several-hundred-megabyte
+      // tail chunk by chunk.
       break;
     }
-    offset += 8 + size + (size % 2);
   }
   return { ...head, data };
 }

packages/engine/src/services/audioVolumeEnvelope.ts

@@ -19,6 +19,7 @@ import { readFileSync, renameSync, writeFileSync } from "fs";
 import { randomBytes } from "crypto";
 import type { AudioVolumeKeyframe } from "./audioMixer.types.js";
 import { normaliseEnvelope } from "@hyperframes/core/media-volume-envelope";
+import { riffChunks } from "./wavChunks.js";
 
 const PCM_FORMAT = 1; // WAVE_FORMAT_PCM
 const SUPPORTED_BITS = 16;
@@ -42,26 +43,20 @@ function parseWavLayout(buffer: Buffer): WavLayout | null {
   if (buffer.length < 12 || buffer.toString("ascii", 0, 4) !== "RIFF") return null;
   if (buffer.toString("ascii", 8, 12) !== "WAVE") return null;
 
-  let offset = 12;
   let fmt: { numChannels: number; sampleRate: number; bitsPerSample: number } | null = null;
   let data: { offset: number; size: number } | null = null;
 
-  while (offset + 8 <= buffer.length) {
-    const chunkId = buffer.toString("ascii", offset, offset + 4);
-    const chunkSize = buffer.readUInt32LE(offset + 4);
-    const body = offset + 8;
-    if (chunkId === "fmt " && body + 16 <= buffer.length) {
+  for (const { id, body, size } of riffChunks(buffer)) {
+    if (id === "fmt " && body + 16 <= buffer.length) {
       if (buffer.readUInt16LE(body) !== PCM_FORMAT) return null;
       fmt = {
         numChannels: buffer.readUInt16LE(body + 2),
         sampleRate: buffer.readUInt32LE(body + 4),
         bitsPerSample: buffer.readUInt16LE(body + 14),
       };
-    } else if (chunkId === "data") {
-      data = { offset: body, size: Math.min(chunkSize, buffer.length - body) };
+    } else if (id === "data") {
+      data = { offset: body, size: Math.min(size, buffer.length - body) };
     }
-    // Chunks are word-aligned: an odd size carries a trailing pad byte.
-    offset = body + chunkSize + (chunkSize % 2);
   }
 
   if (!fmt || !data) return null;

packages/engine/src/services/wavChunks.test.ts

@@ -0,0 +1,56 @@
+import { describe, expect, it } from "vitest";
+import { riffChunks } from "./wavChunks.js";
+
+/**
+ * The two WAV readers that share this walk both had their own copy, and neither
+ * had a test for the walk itself — a real WAV out of the mixer has an even-sized
+ * `fmt ` first and `data` last, so the two things the walk exists to handle
+ * (ordering, and the pad byte after an odd chunk) never came up in either suite.
+ */
+function riff(chunks: { id: string; body: Buffer }[]): Buffer {
+  const parts: Buffer[] = [Buffer.from("RIFF"), Buffer.alloc(4), Buffer.from("WAVE")];
+  for (const { id, body } of chunks) {
+    const header = Buffer.alloc(8);
+    header.write(id, 0, "ascii");
+    header.writeUInt32LE(body.length, 4);
+    parts.push(header, body);
+    // Word alignment: an odd body is followed by a pad byte the size excludes.
+    if (body.length % 2) parts.push(Buffer.alloc(1));
+  }
+  const buf = Buffer.concat(parts);
+  buf.writeUInt32LE(buf.length - 8, 4);
+  return buf;
+}
+
+describe("riffChunks", () => {
+  it("steps over the pad byte after an odd-sized chunk", () => {
+    // An odd LIST is what ffmpeg writes for a metadata string of odd length. Read
+    // without the pad, every chunk after it is one byte out and reads as garbage.
+    const buf = riff([
+      { id: "LIST", body: Buffer.from("INFOodd") },
+      { id: "data", body: Buffer.from([1, 2, 3, 4]) },
+    ]);
+    const found = [...riffChunks(buf)];
+    expect(found.map((c) => c.id)).toEqual(["LIST", "data"]);
+    const data = found[1];
+    if (!data) throw new Error("no data chunk");
+    expect(buf.subarray(data.body, data.body + data.size)).toEqual(Buffer.from([1, 2, 3, 4]));
+  });
+
+  it("yields chunks in file order, whatever that order is", () => {
+    // `data` before `fmt ` is legal and the reason the walk advances by declared
+    // size rather than assuming a layout.
+    const buf = riff([
+      { id: "data", body: Buffer.alloc(6) },
+      { id: "fmt ", body: Buffer.alloc(16) },
+    ]);
+    expect([...riffChunks(buf)].map((c) => c.id)).toEqual(["data", "fmt "]);
+  });
+
+  it("stops at a chunk header that runs past the end of the file", () => {
+    // Truncated downloads and interrupted writes both land here; the walk must
+    // end rather than read off the buffer.
+    const buf = Buffer.concat([riff([{ id: "data", body: Buffer.alloc(4) }]), Buffer.from("da")]);
+    expect([...riffChunks(buf)].map((c) => c.id)).toEqual(["data"]);
+  });
+});

packages/engine/src/services/wavChunks.ts

@@ -0,0 +1,38 @@
+/**
+ * The RIFF chunk walk, which two WAV readers in this directory each had a copy
+ * of: `audioFxRender`'s `readWavChunks` and `audioVolumeEnvelope`'s
+ * `parseWavLayout`.
+ *
+ * Only the walk is shared. What the two do with the chunks is genuinely
+ * different — one wants a slice of the payload and lets the decoder judge the
+ * format, the other wants offsets to edit in place and refuses anything that is
+ * not 16-bit PCM — and folding those together would mean picking one behaviour
+ * for each difference, in the parser every render's audio passes through. So
+ * this yields chunks and holds no policy at all.
+ */
+
+export interface RiffChunk {
+  /** Four ASCII characters: `fmt `, `data`, `LIST`, `fact`, … */
+  id: string;
+  /** Byte offset of the chunk's body, past the 8-byte header. */
+  body: number;
+  /** The size the chunk declares. May run past the end of a truncated file. */
+  size: number;
+}
+
+/**
+ * Every chunk after the 12-byte RIFF header, in the order they sit.
+ *
+ * Advances by each chunk's declared size, so ordering is not assumed — `data`
+ * may precede `fmt `, and trailing LIST/fact chunks are walked past rather than
+ * tripped over. Chunks are word-aligned, so an odd size carries a pad byte.
+ */
+export function* riffChunks(buffer: Buffer): Generator<RiffChunk> {
+  let offset = 12;
+  while (offset + 8 <= buffer.length) {
+    const id = buffer.toString("ascii", offset, offset + 4);
+    const size = buffer.readUInt32LE(offset + 4);
+    yield { id, body: offset + 8, size };
+    offset += 8 + size + (size % 2);
+  }
+}