Skip to content

Commit b8d6459

Browse files
fix: rewrite Ogg parser for multi-page packets (#9256)
The previous implementation used naive regex matching on binary data converted to strings. This approach failed entirely when a large prompt caused the `OpusTags` packet to span multiple Ogg pages. This commit replaces the fragile string search with a robust binary parser that strictly follows the Ogg logical bitstream and Vorbis Comments specifications. It correctly handles Ogg lacing and reassembles segmented packets, ensuring that large JSON metadata is successfully extracted. Also adds unit tests (ogg.test.ts) to verify the correct parsing of both standard and multi-page Ogg files.
1 parent 59c3215 commit b8d6459

2 files changed

Lines changed: 423 additions & 24 deletions

File tree

src/scripts/metadata/ogg.test.ts

Lines changed: 305 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,305 @@
1+
import { describe, expect, it } from 'vitest'
2+
import { getOggMetadata } from './ogg'
3+
4+
function createOggWithOpusTags(comments: {
5+
[key: string]: string
6+
}): ArrayBuffer {
7+
const OGG_HEADER_SIZE = 27
8+
const vendor = 'ComfyUI'
9+
const vendorBytes = new TextEncoder().encode(vendor)
10+
const commentKeys = Object.keys(comments)
11+
12+
// Construct OpusTags Packet
13+
let totalPacketSize = 8 + 4 + vendorBytes.length + 4
14+
const commentEntries: Uint8Array[] = []
15+
for (const key of commentKeys) {
16+
const entry = `${key}=${comments[key]}`
17+
const entryBytes = new TextEncoder().encode(entry)
18+
commentEntries.push(entryBytes)
19+
totalPacketSize += 4 + entryBytes.length
20+
}
21+
22+
const packetData = new Uint8Array(totalPacketSize)
23+
const packetView = new DataView(packetData.buffer)
24+
let pos = 0
25+
packetData.set(new TextEncoder().encode('OpusTags'), pos)
26+
pos += 8
27+
packetView.setUint32(pos, vendorBytes.length, true)
28+
pos += 4
29+
packetData.set(vendorBytes, pos)
30+
pos += vendorBytes.length
31+
packetView.setUint32(pos, commentKeys.length, true)
32+
pos += 4
33+
for (const entryBytes of commentEntries) {
34+
packetView.setUint32(pos, entryBytes.length, true)
35+
pos += 4
36+
packetData.set(entryBytes, pos)
37+
pos += entryBytes.length
38+
}
39+
40+
// Construct Ogg Pages for Metadata
41+
const pages: Uint8Array[] = []
42+
let packetPos = 0
43+
let packetEnded = false
44+
45+
while (!packetEnded) {
46+
const remainingPacketSize = packetData.length - packetPos
47+
let dataSizeInThisPage = 0
48+
const lacingValues: number[] = []
49+
50+
for (let i = 0; i < 255; i++) {
51+
const remainingForThisPacket = remainingPacketSize - dataSizeInThisPage
52+
if (remainingForThisPacket >= 255) {
53+
lacingValues.push(255)
54+
dataSizeInThisPage += 255
55+
} else {
56+
lacingValues.push(remainingForThisPacket)
57+
dataSizeInThisPage += remainingForThisPacket
58+
packetEnded = true
59+
break
60+
}
61+
}
62+
63+
const numSegments = lacingValues.length
64+
const pageSize = OGG_HEADER_SIZE + numSegments + dataSizeInThisPage
65+
const page = new Uint8Array(pageSize)
66+
67+
page.set(new TextEncoder().encode('OggS'), 0)
68+
page[5] = packetPos > 0 ? 0x01 : 0x00 // 0x01 indicates continued packet
69+
page[26] = numSegments
70+
71+
page.set(lacingValues, OGG_HEADER_SIZE)
72+
73+
if (dataSizeInThisPage > 0) {
74+
page.set(
75+
packetData.subarray(packetPos, packetPos + dataSizeInThisPage),
76+
OGG_HEADER_SIZE + numSegments
77+
)
78+
}
79+
80+
pages.push(page)
81+
packetPos += dataSizeInThisPage
82+
}
83+
84+
// Append Dummy Audio Ogg Pages
85+
let currentTotalSize = pages.reduce((sum, p) => sum + p.length, 0)
86+
const TARGET_SIZE = 65536 // 64KB: ensures the file spans multiple Ogg pages for multi-page parsing tests
87+
88+
while (currentTotalSize < TARGET_SIZE) {
89+
// Create a dummy audio page (approx 65KB per page)
90+
const numSegments = 255
91+
const segmentDataSize = 255
92+
const dataSize = numSegments * segmentDataSize
93+
const pageSize = OGG_HEADER_SIZE + numSegments + dataSize
94+
const audioPage = new Uint8Array(pageSize)
95+
96+
// Ogg Header for dummy audio
97+
audioPage.set(new TextEncoder().encode('OggS'), 0)
98+
audioPage[5] = 0x00 // Normal page
99+
audioPage[26] = numSegments
100+
101+
// Segment Table (all max size)
102+
audioPage.fill(
103+
segmentDataSize,
104+
OGG_HEADER_SIZE,
105+
OGG_HEADER_SIZE + numSegments
106+
)
107+
108+
// The rest of the array remains 0x00, acting as valid empty dummy audio data
109+
pages.push(audioPage)
110+
currentTotalSize += audioPage.length
111+
}
112+
113+
// Combine all pages into one Buffer
114+
const combined = new Uint8Array(currentTotalSize)
115+
let offset = 0
116+
for (const p of pages) {
117+
combined.set(p, offset)
118+
offset += p.length
119+
}
120+
121+
return combined.buffer
122+
}
123+
124+
describe('getOggMetadata', () => {
125+
it('should extract prompt and workflow from a valid Ogg file', async () => {
126+
const prompt = { '1': { class_type: 'KSampler' } }
127+
const workflow = { nodes: [{ id: 1 }] }
128+
129+
const buffer = createOggWithOpusTags({
130+
prompt: JSON.stringify(prompt),
131+
workflow: JSON.stringify(workflow)
132+
})
133+
134+
const file = new File([buffer], 'test.ogg', { type: 'audio/ogg' })
135+
const result = await getOggMetadata(file)
136+
137+
expect(result.prompt).toEqual(prompt)
138+
expect(result.workflow).toEqual(workflow)
139+
})
140+
141+
it('should handle large metadata spanning multiple Ogg pages (over 64KB)', async () => {
142+
// Generate a 70,000 character string (exceeds 255 * 255 = 65,025 bytes to ensure it spans multiple pages)
143+
const hugeString = 'b'.repeat(70000)
144+
const prompt = { data: hugeString }
145+
const workflow = { nodes: [{ id: 'large-node', type: 'AnyType' }] }
146+
147+
const buffer = createOggWithOpusTags({
148+
prompt: JSON.stringify(prompt),
149+
workflow: JSON.stringify(workflow)
150+
})
151+
152+
// Verify: Check if the helper function actually generated a file with multiple pages (2 or more 'OggS' headers)
153+
const view = new Uint8Array(buffer)
154+
const oggMagic = new TextEncoder().encode('OggS')
155+
let oggSCount = 0
156+
for (let i = 0; i < view.length - 4; i++) {
157+
if (
158+
view[i] === oggMagic[0] &&
159+
view[i + 1] === oggMagic[1] &&
160+
view[i + 2] === oggMagic[2] &&
161+
view[i + 3] === oggMagic[3]
162+
) {
163+
oggSCount++
164+
}
165+
}
166+
expect(oggSCount).toBeGreaterThan(1)
167+
168+
// Execute and Assert: Verify that data spanning multiple pages is correctly reconstructed into a single JSON
169+
const file = new File([buffer], 'huge_multi_page.ogg', {
170+
type: 'audio/ogg'
171+
})
172+
const result = await getOggMetadata(file)
173+
174+
expect(result.prompt).toEqual(prompt)
175+
expect(result.workflow).toEqual(workflow)
176+
})
177+
178+
describe('structural corruptions and truncations', () => {
179+
// Generate truncated/invalid buffer scenarios
180+
const scenarios: { name: string; createBuffer: () => ArrayBuffer }[] = [
181+
{
182+
name: 'abruptly truncated segment table',
183+
createBuffer: () => {
184+
const buffer = new Uint8Array(28)
185+
buffer.set(new TextEncoder().encode('OggS'), 0)
186+
buffer[26] = 50 // Declaring 50 segments while only 28 bytes exist
187+
return buffer.buffer
188+
}
189+
},
190+
{
191+
name: 'truncated user comment in OpusTags',
192+
createBuffer: () => {
193+
const packet = new Uint8Array(20)
194+
packet.set(new TextEncoder().encode('OpusTags'), 0)
195+
new DataView(packet.buffer).setUint32(8, 0, true)
196+
new DataView(packet.buffer).setUint32(12, 1, true)
197+
new DataView(packet.buffer).setUint32(16, 500, true) // Declared length 500
198+
const buffer = new Uint8Array(27 + 1 + packet.length)
199+
buffer.set(new TextEncoder().encode('OggS'), 0)
200+
buffer[26] = 1
201+
buffer[27] = packet.length
202+
buffer.set(packet, 28)
203+
return buffer.buffer
204+
}
205+
},
206+
{
207+
name: 'file smaller than the Ogg magic number',
208+
createBuffer: () => new Uint8Array([1, 2]).buffer
209+
},
210+
211+
{
212+
name: 'page has an invalid magic number (stop parsing)',
213+
createBuffer: () => {
214+
const page1 = new Uint8Array(29)
215+
page1.set(new TextEncoder().encode('OggS'), 0)
216+
page1[26] = 1
217+
page1[27] = 1
218+
const page2 = new Uint8Array(30)
219+
page2.set(new TextEncoder().encode('FAIL'), 0)
220+
const combined = new Uint8Array(page1.length + page2.length)
221+
combined.set(page1, 0)
222+
combined.set(page2, page1.length)
223+
return combined.buffer
224+
}
225+
}
226+
]
227+
228+
for (const { name, createBuffer } of scenarios) {
229+
it(`should safely handle: ${name}`, async () => {
230+
const file = new File(
231+
[createBuffer()],
232+
`test_${name.replace(/\s+/g, '_')}.ogg`
233+
)
234+
try {
235+
const result = await getOggMetadata(file)
236+
expect(result.prompt).toBeUndefined()
237+
expect(result.workflow).toBeUndefined()
238+
} catch (e) {
239+
// If it throws (e.g. RangeError from DataView out of bounds), it's successfully handled gracefully by the caller
240+
expect(e).toBeInstanceOf(Error)
241+
}
242+
})
243+
}
244+
})
245+
246+
describe('invalid or unrelated comment contents', () => {
247+
const scenarios: {
248+
name: string
249+
createBuffer: () => ArrayBuffer
250+
expectedPrompt?: any
251+
}[] = [
252+
{
253+
name: 'unrelated Vorbis comments',
254+
createBuffer: () =>
255+
createOggWithOpusTags({
256+
unrelated_key: 'unrelated_value',
257+
prompt: '{"valid": true}'
258+
}),
259+
expectedPrompt: { valid: true } // Workflow is undefined, Prompt is valid
260+
},
261+
{
262+
name: 'user comments without an equal sign',
263+
createBuffer: () => {
264+
const commentBytes = new TextEncoder().encode(
265+
'invalid_comment_no_equal_sign'
266+
)
267+
const packet = new Uint8Array(16 + 4 + commentBytes.length)
268+
packet.set(new TextEncoder().encode('OpusTags'), 0)
269+
new DataView(packet.buffer).setUint32(8, 0, true)
270+
new DataView(packet.buffer).setUint32(12, 1, true)
271+
new DataView(packet.buffer).setUint32(16, commentBytes.length, true)
272+
packet.set(commentBytes, 20)
273+
274+
const buffer = new Uint8Array(27 + 1 + packet.length)
275+
buffer.set(new TextEncoder().encode('OggS'), 0)
276+
buffer[26] = 1
277+
buffer[27] = packet.length
278+
buffer.set(packet, 28)
279+
return buffer.buffer
280+
}
281+
}
282+
]
283+
284+
for (const { name, createBuffer, expectedPrompt } of scenarios) {
285+
it(`should appropriately handle: ${name}`, async () => {
286+
const file = new File(
287+
[createBuffer()],
288+
`test_${name.replace(/\s+/g, '_')}.ogg`
289+
)
290+
try {
291+
const result = await getOggMetadata(file)
292+
if (expectedPrompt) {
293+
expect(result.prompt).toEqual(expectedPrompt) // either undefined or { valid: true }
294+
} else {
295+
expect(result.prompt).toBeUndefined()
296+
}
297+
expect(result.workflow).toBeUndefined()
298+
} catch (e) {
299+
// Handled via thrown exception
300+
expect(e).toBeInstanceOf(Error)
301+
}
302+
})
303+
}
304+
})
305+
})

0 commit comments

Comments
 (0)