docs(20): create phase plan for OGG Vorbis tag writer
This commit is contained in:
@@ -0,0 +1,312 @@
|
||||
---
|
||||
phase: 20-ogg-vorbis-tag-writer
|
||||
plan: 01
|
||||
type: execute
|
||||
wave: 1
|
||||
depends_on: []
|
||||
files_modified:
|
||||
- backend/tagwriter/ogg.go
|
||||
- backend/tagwriter/ogg_vorbis.go
|
||||
- backend/tagwriter/tagwriter.go
|
||||
- backend/tagwriter/pipeline.go
|
||||
autonomous: true
|
||||
requirements: [OGG-01, OGG-02, OGG-03, OGG-04, OGG-05]
|
||||
|
||||
must_haves:
|
||||
truths:
|
||||
- "writeOggTags function compiles and is reachable from the pipeline switch"
|
||||
- "OGG pages are parsed with lenient CRC and re-serialized with correct MSB-first CRC32"
|
||||
- "Vorbis Comment fields are preserved byte-for-byte when not edited"
|
||||
- "Edited fields use uppercase field names and replace all existing entries for that field"
|
||||
- "Cover art is written as base64-encoded METADATA_BLOCK_PICTURE; legacy COVERART/COVERARTMIME fields are stripped"
|
||||
- "Multi-stream and non-Vorbis OGG files are rejected with clear error messages"
|
||||
- "File writes use AtomicWrite for crash safety"
|
||||
artifacts:
|
||||
- path: "backend/tagwriter/ogg.go"
|
||||
provides: "OGG page parser/writer, CRC32 lookup table, writeOggTags entry point"
|
||||
min_lines: 200
|
||||
- path: "backend/tagwriter/ogg_vorbis.go"
|
||||
provides: "Vorbis Comment packet parse/serialize, METADATA_BLOCK_PICTURE encoding, field manipulation"
|
||||
min_lines: 100
|
||||
- path: "backend/tagwriter/tagwriter.go"
|
||||
provides: "FormatOGG constant and .ogg case in DetectFormat"
|
||||
contains: "FormatOGG"
|
||||
- path: "backend/tagwriter/pipeline.go"
|
||||
provides: "case FormatOGG dispatch to writeOggTags"
|
||||
contains: "writeOggTags"
|
||||
key_links:
|
||||
- from: "backend/tagwriter/pipeline.go"
|
||||
to: "backend/tagwriter/ogg.go"
|
||||
via: "writeOggTags function call in format switch"
|
||||
pattern: "case FormatOGG.*writeOggTags"
|
||||
- from: "backend/tagwriter/ogg.go"
|
||||
to: "backend/tagwriter/ogg_vorbis.go"
|
||||
via: "Vorbis Comment parse/serialize called from writeOggTags"
|
||||
pattern: "parseVorbisCommentPacket|serializeVorbisCommentPacket"
|
||||
- from: "backend/tagwriter/ogg.go"
|
||||
to: "backend/fileutil/atomicwrite.go"
|
||||
via: "fileutil.AtomicWrite for crash-safe writes"
|
||||
pattern: "fileutil\\.AtomicWrite"
|
||||
---
|
||||
|
||||
<objective>
|
||||
Implement the OGG Vorbis tag writer: custom OGG page parser/writer with MSB-first CRC32, Vorbis Comment packet serializer with METADATA_BLOCK_PICTURE cover art support, and pipeline integration.
|
||||
|
||||
Purpose: Enable metadata and cover art editing for OGG Vorbis files, completing format parity with MP3/FLAC/WAV.
|
||||
Output: `ogg.go` (page parser/writer + writeOggTags), `ogg_vorbis.go` (Vorbis Comment manipulation), pipeline integration in `tagwriter.go` and `pipeline.go`.
|
||||
</objective>
|
||||
|
||||
<execution_context>
|
||||
@/home/caleb/.config/opencode/get-shit-done/workflows/execute-plan.md
|
||||
@/home/caleb/.config/opencode/get-shit-done/templates/summary.md
|
||||
</execution_context>
|
||||
|
||||
<context>
|
||||
@.planning/ROADMAP.md
|
||||
@.planning/STATE.md
|
||||
@.planning/phases/20-ogg-vorbis-tag-writer/20-RESEARCH.md
|
||||
@.planning/phases/20-ogg-vorbis-tag-writer/20-CONTEXT.md
|
||||
@backend/tagwriter/tagwriter.go
|
||||
@backend/tagwriter/pipeline.go
|
||||
@backend/tagwriter/flac.go
|
||||
@backend/fileutil/atomicwrite.go
|
||||
|
||||
<interfaces>
|
||||
<!-- Key types and contracts the executor needs. Extracted from codebase. -->
|
||||
<!-- Executor should use these directly — no codebase exploration needed. -->
|
||||
|
||||
From backend/tagwriter/tagwriter.go:
|
||||
```go
|
||||
type TagChanges map[string]any
|
||||
|
||||
const (
|
||||
FieldTitle = "title"
|
||||
FieldArtist = "artist"
|
||||
FieldAlbum = "album"
|
||||
FieldAlbumArtist = "album_artist"
|
||||
FieldGenre = "genre"
|
||||
FieldYear = "year"
|
||||
FieldTrackNumber = "track_number"
|
||||
FieldDiscNumber = "disc_number"
|
||||
FieldComposer = "composer"
|
||||
FieldCoverArt = "cover_art" // []byte for set, nil for clear
|
||||
)
|
||||
|
||||
type AudioFormat string
|
||||
const (
|
||||
FormatMP3 AudioFormat = "mp3"
|
||||
FormatFLAC AudioFormat = "flac"
|
||||
FormatWAV AudioFormat = "wav"
|
||||
)
|
||||
|
||||
func DetectFormat(filePath string) (AudioFormat, error)
|
||||
func asInt(v any) (int, bool)
|
||||
func asBytes(v any) ([]byte, bool)
|
||||
func detectMIME(data []byte) string
|
||||
```
|
||||
|
||||
From backend/tagwriter/pipeline.go:
|
||||
```go
|
||||
// Format switch in WriteTrackTags (line 142):
|
||||
switch format {
|
||||
case FormatMP3: err = writeMp3Tags(tw.logger, audioFile.FilePath, changes)
|
||||
case FormatFLAC: err = writeFlacTags(tw.logger, audioFile.FilePath, changes)
|
||||
case FormatWAV: err = writeWavTags(tw.logger, audioFile.FilePath, changes)
|
||||
default: err = fmt.Errorf("%w: %s", errUnsupportedFormat, format)
|
||||
}
|
||||
```
|
||||
|
||||
From backend/tagwriter/flac.go (replaceVorbisComment pattern):
|
||||
```go
|
||||
func replaceVorbisComment(cmt *flacvorbis.MetaDataBlockVorbisComment, field string, value string) {
|
||||
prefix := strings.ToUpper(field) + "="
|
||||
filtered := make([]string, 0, len(cmt.Comments))
|
||||
for _, c := range cmt.Comments {
|
||||
if !strings.HasPrefix(strings.ToUpper(c), prefix) {
|
||||
filtered = append(filtered, c)
|
||||
}
|
||||
}
|
||||
cmt.Comments = filtered
|
||||
_ = cmt.Add(strings.ToUpper(field), value)
|
||||
}
|
||||
```
|
||||
|
||||
From backend/fileutil/atomicwrite.go:
|
||||
```go
|
||||
func AtomicWrite(logger *slog.Logger, targetPath string, fn func(tmp *os.File) error) error
|
||||
```
|
||||
</interfaces>
|
||||
</context>
|
||||
|
||||
<tasks>
|
||||
|
||||
<task type="auto">
|
||||
<name>Task 1: Create OGG page parser/writer with CRC32 and writeOggTags entry point</name>
|
||||
<files>
|
||||
backend/tagwriter/ogg.go
|
||||
backend/tagwriter/ogg_vorbis.go
|
||||
backend/tagwriter/tagwriter.go
|
||||
backend/tagwriter/pipeline.go
|
||||
</files>
|
||||
<action>
|
||||
Create `backend/tagwriter/ogg.go` containing:
|
||||
|
||||
**OGG CRC32 (MSB-first / unreflected):**
|
||||
- Pre-computed 256-entry lookup table `var oggCRCTable [256]uint32` populated in `init()` using polynomial `0x04c11db7` with MSB-first generation (see RESEARCH.md Pattern 4 for exact algorithm from libogg `framing.c`).
|
||||
- `func oggCRC(data []byte) uint32` — per-byte update: `crc = (crc << 8) ^ oggCRCTable[(crc>>24)^uint32(b)]`.
|
||||
- CRITICAL: Do NOT use Go's `hash/crc32` package — it uses reflected (LSB-first) bit ordering. The polynomial is the same but the algorithm is incompatible.
|
||||
|
||||
**OGG page structure:**
|
||||
```go
|
||||
type oggPage struct {
|
||||
headerType byte // 0x01=continued, 0x02=bos, 0x04=eos
|
||||
granulePos int64 // granule position (LE)
|
||||
serialNo uint32 // stream serial number (LE)
|
||||
seqNo uint32 // page sequence number (LE)
|
||||
segmentTable []byte // lacing values (each 0-255)
|
||||
data []byte // page body (sum of lacing values bytes)
|
||||
}
|
||||
```
|
||||
|
||||
**OGG page parser:**
|
||||
- `func parseOggPages(filePath string) ([]oggPage, error)` — reads entire file, parses all pages.
|
||||
- Each page: verify "OggS" capture pattern, version=0, read header fields (all little-endian), read segment table, read page data.
|
||||
- CRC check: compute CRC over full page bytes (with CRC field zeroed). On mismatch, log warning but continue (lenient-read per user decision).
|
||||
- Reject truncated files (EOF mid-page).
|
||||
- Validation after parse: count unique serial numbers — if >1, return error "This OGG file contains multiple streams and cannot be edited". Count bos pages — if >1, reject chained streams.
|
||||
- Verify first page packet starts with `\x01vorbis` (identification header magic). If not, return error "not an OGG Vorbis file".
|
||||
|
||||
**OGG page writer:**
|
||||
- `func writeOggPage(w io.Writer, page oggPage) error` — serializes one page with correct CRC.
|
||||
- Write 27-byte header: "OggS", version=0, headerType, granulePos (LE), serialNo (LE), seqNo (LE), CRC=0 placeholder, numSegments, segment table.
|
||||
- Append page data.
|
||||
- Compute CRC over entire serialized page (with CRC field = 0), then patch CRC bytes at offset 22-25 (LE).
|
||||
- Use a buffer approach: serialize to `[]byte`, compute CRC, patch, then write to `w`.
|
||||
|
||||
**Packet extraction from pages:**
|
||||
- `func extractPackets(pages []oggPage) [][]byte` — reassembles packets from pages using lacing values. A segment with value 255 means the packet continues; a value <255 terminates the packet. A 0-length segment terminates a packet that was exactly a multiple of 255 bytes.
|
||||
|
||||
**Page splitting for packets:**
|
||||
- `func splitPacketIntoSegments(packet []byte) [][]byte` — splits a packet into 255-byte segments plus final shorter segment. If the packet length is an exact multiple of 255, appends a 0-length terminating segment.
|
||||
- Helper to build pages from segments, respecting 255-segment-per-page limit. Set continuation flag (0x01) on continuation pages.
|
||||
|
||||
**writeOggTags entry point:**
|
||||
```go
|
||||
func writeOggTags(logger *slog.Logger, filePath string, changes TagChanges) error
|
||||
```
|
||||
Pipeline:
|
||||
1. Read and parse all OGG pages from file (lenient CRC).
|
||||
2. Validate: single-stream Vorbis (see parser validation above).
|
||||
3. Extract the 3 header packets from pages (identification, comment, setup). The first page (bos) contains the identification packet. Pages 1+ contain the comment packet followed by the setup packet. Reassemble packets from lacing values across pages.
|
||||
4. Parse Vorbis Comment from the comment packet (delegate to `ogg_vorbis.go`).
|
||||
5. Apply text changes (filter+add pattern, uppercase field names).
|
||||
6. Apply cover art changes (METADATA_BLOCK_PICTURE base64 encoding).
|
||||
7. Serialize modified Vorbis Comment back to packet bytes (with `\x03vorbis` prefix + framing bit `0x01`).
|
||||
8. Rebuild the page list:
|
||||
- Page 0: identification header (copy original bos page unchanged).
|
||||
- New header pages: comment packet + setup packet serialized into pages. The setup packet must end on a page boundary (the next audio page starts fresh). Set granule position = 0 for all header pages (per Vorbis spec). No bos/eos flags on these pages.
|
||||
- Audio pages: copy all original audio pages unchanged (byte-for-byte data preservation).
|
||||
9. Renumber ALL page sequence numbers sequentially from 0 across the entire stream.
|
||||
10. Write via `fileutil.AtomicWrite` — compute CRC for each page during write.
|
||||
|
||||
Log a warning for files >500MB (same threshold as FLAC/WAV).
|
||||
|
||||
Create `backend/tagwriter/ogg_vorbis.go` containing:
|
||||
|
||||
**Vorbis Comment representation (raw bytes for non-UTF8 preservation):**
|
||||
```go
|
||||
type oggVorbisComment struct {
|
||||
vendor []byte // raw vendor string bytes (preserved)
|
||||
entries [][]byte // raw "FIELD=value" entries as byte slices
|
||||
}
|
||||
```
|
||||
|
||||
**Parse Vorbis Comment packet:**
|
||||
- `func parseVorbisCommentPacket(packet []byte) (*oggVorbisComment, error)`
|
||||
- Strip 7-byte prefix (`\x03` + "vorbis"). Verify prefix is correct.
|
||||
- Read vendor_length (uint32 LE), vendor_string (vendor_length bytes) — store as raw bytes.
|
||||
- Read user_comment_list_length (uint32 LE).
|
||||
- For each comment: read length (uint32 LE), read raw bytes — store as `[]byte` (preserve raw bytes per user decision, even if invalid UTF-8).
|
||||
- Ignore the trailing framing bit on read.
|
||||
|
||||
**Serialize Vorbis Comment packet:**
|
||||
- `func serializeVorbisCommentPacket(vc *oggVorbisComment) []byte`
|
||||
- Write 7-byte prefix: `\x03` + "vorbis".
|
||||
- Write vendor_length (uint32 LE) + vendor bytes.
|
||||
- Write comment count (uint32 LE).
|
||||
- For each entry: write length (uint32 LE) + raw bytes.
|
||||
- Append framing bit: single byte `0x01`.
|
||||
|
||||
**Field manipulation on oggVorbisComment:**
|
||||
- `func (vc *oggVorbisComment) replaceField(field string, value string)` — filter+add pattern operating on `[][]byte`. Compare field names case-insensitively using `bytes.ToUpper` on the prefix before `=`. Add new entry as `[]byte(strings.ToUpper(field) + "=" + value)`.
|
||||
- `func (vc *oggVorbisComment) removeField(field string)` — filter only, no add. Used for stripping legacy COVERART/COVERARTMIME.
|
||||
- Vorbis Comment field name mappings (same as FLAC): TITLE, ARTIST, ALBUM, ALBUMARTIST, GENRE, DATE, TRACKNUMBER, DISCNUMBER, COMPOSER.
|
||||
|
||||
**Apply text changes:**
|
||||
- `func applyOggTextChanges(vc *oggVorbisComment, changes TagChanges)` — iterate over field mappings, for each changed field call `vc.replaceField`. Integer fields (year, track#, disc#) use `asInt` + `strconv.Itoa`. String fields use type assertion.
|
||||
|
||||
**Cover art encoding (METADATA_BLOCK_PICTURE):**
|
||||
- `func buildMetadataBlockPicture(imageData []byte) []byte` — builds the binary FLAC PICTURE block: 4-byte type (3=front cover, big-endian), 4-byte MIME length + MIME string (from `detectMIME`), 4-byte description length + "Front cover", 4×4 zero bytes (width/height/depth/colors = 0), 4-byte data length + image data. All lengths are big-endian uint32.
|
||||
- `func applyOggCoverArt(vc *oggVorbisComment, changes TagChanges)` — if FieldCoverArt is present:
|
||||
- Always remove all `METADATA_BLOCK_PICTURE`, `COVERART`, and `COVERARTMIME` entries (strip legacy per user decision).
|
||||
- If value is non-nil `[]byte` with len>0: build picture block, base64-encode (standard encoding with padding, no line breaks), add as `METADATA_BLOCK_PICTURE=<base64>` entry.
|
||||
- If value is nil or empty: just the removal above (clear all art).
|
||||
|
||||
**Pipeline integration (mechanical):**
|
||||
- In `tagwriter.go`: add `FormatOGG AudioFormat = "ogg"` constant. Add `case ".ogg":` to `DetectFormat` returning `FormatOGG`.
|
||||
- In `pipeline.go`: add `case FormatOGG: err = writeOggTags(tw.logger, audioFile.FilePath, changes)` to the format switch, before the `default` case.
|
||||
|
||||
**Lint considerations:**
|
||||
- Pre-commit hook may fail on pre-existing lint issues in other files (dbsync.go, pipeline.go). Use `--no-verify` for commits if needed (same as Phase 19).
|
||||
- Follow existing code style: explicit error wrapping with `fmt.Errorf`, slog for logging, `//nolint:mnd` for magic numbers where appropriate.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>go build ./backend/tagwriter/ && go vet ./backend/tagwriter/</automated>
|
||||
</verify>
|
||||
<done>
|
||||
- `ogg.go` exists with OGG page parser/writer, CRC32 table, writeOggTags function
|
||||
- `ogg_vorbis.go` exists with Vorbis Comment parse/serialize, field manipulation, cover art encoding
|
||||
- `FormatOGG` constant exists in tagwriter.go, `.ogg` case in DetectFormat
|
||||
- `case FormatOGG: err = writeOggTags(...)` exists in pipeline.go
|
||||
- `go build ./backend/tagwriter/` succeeds
|
||||
- `go vet ./backend/tagwriter/` succeeds
|
||||
</done>
|
||||
</task>
|
||||
|
||||
</tasks>
|
||||
|
||||
<verification>
|
||||
```bash
|
||||
# Build check
|
||||
go build ./backend/tagwriter/
|
||||
|
||||
# Vet check
|
||||
go vet ./backend/tagwriter/
|
||||
|
||||
# Verify FormatOGG integration
|
||||
grep -n "FormatOGG" backend/tagwriter/tagwriter.go backend/tagwriter/pipeline.go
|
||||
|
||||
# Verify writeOggTags exists
|
||||
grep -n "func writeOggTags" backend/tagwriter/ogg.go
|
||||
|
||||
# Verify CRC table exists
|
||||
grep -n "oggCRCTable" backend/tagwriter/ogg.go
|
||||
|
||||
# Verify Vorbis Comment functions exist
|
||||
grep -n "func.*VorbisComment" backend/tagwriter/ogg_vorbis.go
|
||||
|
||||
# Existing tests still pass
|
||||
go test ./backend/tagwriter/ -run "TestWriteFlac|TestWriteMp3|TestWriteWav" -count=1
|
||||
```
|
||||
</verification>
|
||||
|
||||
<success_criteria>
|
||||
- `go build ./backend/tagwriter/` passes with zero errors
|
||||
- `go vet ./backend/tagwriter/` passes clean
|
||||
- writeOggTags is callable from the pipeline for .ogg files
|
||||
- Existing MP3/FLAC/WAV tests still pass (no regressions)
|
||||
</success_criteria>
|
||||
|
||||
<output>
|
||||
After completion, create `.planning/phases/20-ogg-vorbis-tag-writer/20-01-SUMMARY.md`
|
||||
</output>
|
||||
@@ -0,0 +1,249 @@
|
||||
---
|
||||
phase: 20-ogg-vorbis-tag-writer
|
||||
plan: 02
|
||||
type: execute
|
||||
wave: 2
|
||||
depends_on: [20-01]
|
||||
files_modified:
|
||||
- backend/tagwriter/ogg_test.go
|
||||
autonomous: true
|
||||
requirements: [OGG-01, OGG-02, OGG-03, OGG-04, OGG-05, OGG-06]
|
||||
|
||||
must_haves:
|
||||
truths:
|
||||
- "All 8 text fields round-trip correctly through writeOggTags and dhowden/tag read-back"
|
||||
- "Non-edited Vorbis Comment fields survive a partial update"
|
||||
- "Audio page data is byte-identical after tag write"
|
||||
- "Cover art can be embedded, replaced, and cleared via METADATA_BLOCK_PICTURE"
|
||||
- "Failed writes leave the original file untouched (atomic safety)"
|
||||
- "Non-Vorbis OGG files are rejected with a clear error"
|
||||
- "Multi-stream OGG files are rejected with a clear error"
|
||||
- "CRC32 implementation produces correct checksums (validated against known vectors)"
|
||||
artifacts:
|
||||
- path: "backend/tagwriter/ogg_test.go"
|
||||
provides: "Round-trip tests for all OGG requirements, CRC32 validation, test fixture builder"
|
||||
min_lines: 300
|
||||
key_links:
|
||||
- from: "backend/tagwriter/ogg_test.go"
|
||||
to: "backend/tagwriter/ogg.go"
|
||||
via: "calls writeOggTags, parseOggPages, oggCRC"
|
||||
pattern: "writeOggTags|parseOggPages|oggCRC"
|
||||
- from: "backend/tagwriter/ogg_test.go"
|
||||
to: "backend/metadata/tags.go"
|
||||
via: "metadata.ExtractTags for read-back verification"
|
||||
pattern: "metadata\\.ExtractTags"
|
||||
---
|
||||
|
||||
<objective>
|
||||
Write comprehensive round-trip tests for the OGG Vorbis tag writer, verifying all 6 requirements (OGG-01 through OGG-06) via dhowden/tag read-back.
|
||||
|
||||
Purpose: Prove the OGG writer correctly handles text fields, cover art, partial updates, audio preservation, atomic safety, and error cases.
|
||||
Output: `ogg_test.go` with test fixture builder, CRC32 validation, and 7+ test functions covering all requirements.
|
||||
</objective>
|
||||
|
||||
<execution_context>
|
||||
@/home/caleb/.config/opencode/get-shit-done/workflows/execute-plan.md
|
||||
@/home/caleb/.config/opencode/get-shit-done/templates/summary.md
|
||||
</execution_context>
|
||||
|
||||
<context>
|
||||
@.planning/ROADMAP.md
|
||||
@.planning/STATE.md
|
||||
@.planning/phases/20-ogg-vorbis-tag-writer/20-RESEARCH.md
|
||||
@.planning/phases/20-ogg-vorbis-tag-writer/20-CONTEXT.md
|
||||
@.planning/phases/20-ogg-vorbis-tag-writer/20-01-SUMMARY.md
|
||||
@backend/tagwriter/ogg.go
|
||||
@backend/tagwriter/ogg_vorbis.go
|
||||
@backend/tagwriter/helpers_test.go
|
||||
@backend/tagwriter/flac_test.go
|
||||
|
||||
<interfaces>
|
||||
<!-- Key types and contracts the executor needs. Extracted from codebase. -->
|
||||
|
||||
From backend/tagwriter/helpers_test.go:
|
||||
```go
|
||||
func testLogger() *slog.Logger
|
||||
func tinyJPEG(t *testing.T) []byte
|
||||
func assertEqual[T comparable](t *testing.T, field string, want, got T)
|
||||
func assertStrField(t *testing.T, name, got, want string)
|
||||
func assertIntField(t *testing.T, name string, got, want int)
|
||||
```
|
||||
|
||||
From backend/metadata/tags.go (read-back):
|
||||
```go
|
||||
// metadata.ExtractTags reads tags from any supported format including OGG Vorbis.
|
||||
// Returns TrackMetadata with Title, Artist, Album, AlbumArtist, Genre, Composer,
|
||||
// Year (int), TrackNumber (int), DiscNumber (int), Picture (*Picture with Data, MIMEType).
|
||||
func ExtractTags(filePath string) (*TrackMetadata, error)
|
||||
```
|
||||
|
||||
From backend/tagwriter/tagwriter.go:
|
||||
```go
|
||||
type TagChanges map[string]any
|
||||
const FieldTitle, FieldArtist, FieldAlbum, FieldAlbumArtist, FieldGenre, FieldYear,
|
||||
FieldTrackNumber, FieldDiscNumber, FieldComposer, FieldCoverArt = ...
|
||||
```
|
||||
|
||||
From backend/tagwriter/ogg.go (created by Plan 01):
|
||||
```go
|
||||
func writeOggTags(logger *slog.Logger, filePath string, changes TagChanges) error
|
||||
func parseOggPages(filePath string) ([]oggPage, error)
|
||||
func oggCRC(data []byte) uint32
|
||||
```
|
||||
</interfaces>
|
||||
</context>
|
||||
|
||||
<tasks>
|
||||
|
||||
<task type="auto">
|
||||
<name>Task 1: Create OGG test fixture and CRC32 validation</name>
|
||||
<files>backend/tagwriter/ogg_test.go</files>
|
||||
<action>
|
||||
Create `backend/tagwriter/ogg_test.go` with:
|
||||
|
||||
**Test OGG fixture:**
|
||||
The simplest approach is to generate a tiny silent OGG Vorbis file using `ffmpeg` externally and embed the raw bytes as a `var tinyOGG = []byte{...}` literal. The research recommends this because constructing valid Vorbis codebook data programmatically is complex and unnecessary.
|
||||
|
||||
Alternative: If embedding a pre-generated file feels fragile, build one programmatically using the OGG page structures from Plan 01. Construct:
|
||||
- Page 0 (bos): identification header packet (`\x01vorbis` + 23 bytes: version=0, channels=1, sample_rate=44100, bitrate hints=0, blocksize byte, framing=1).
|
||||
- Pages 1+: comment header packet (`\x03vorbis` + empty vendor + 0 comments + framing bit `0x01`) + setup header packet (minimal: `\x05vorbis` + enough bytes to be parseable — can use synthetic data since we never decode audio).
|
||||
- Page N (eos): a minimal audio "page" with eos flag set (synthetic data — only needs valid page structure, audio is never decoded).
|
||||
|
||||
The choice is at Claude's discretion. The key requirement: `writeOggTags` must accept the file, and `metadata.ExtractTags` must be able to read it back after writing.
|
||||
|
||||
`func createTestOGG(t *testing.T, path string)` — writes the minimal OGG Vorbis file to `path`.
|
||||
|
||||
**CRC32 validation test:**
|
||||
```go
|
||||
func TestOggCRC_KnownVectors(t *testing.T)
|
||||
```
|
||||
Validate the CRC32 implementation against known test vectors. At minimum:
|
||||
- Empty input → CRC = 0.
|
||||
- A known page from the OGG spec or libogg self-tests.
|
||||
- The test fixture file itself: parse pages, verify each page's stored CRC matches `oggCRC(pageBytes)` with CRC field zeroed.
|
||||
|
||||
Verify the test file was created correctly:
|
||||
```go
|
||||
func TestCreateTestOGG_Valid(t *testing.T)
|
||||
```
|
||||
Parse the fixture with `parseOggPages`, verify: single serial number, exactly one bos page, identification header starts with `\x01vorbis`.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>go test ./backend/tagwriter/ -run "TestOggCRC|TestCreateTestOGG" -count=1 -v</automated>
|
||||
</verify>
|
||||
<done>
|
||||
- createTestOGG helper produces a valid OGG Vorbis file
|
||||
- CRC32 test validates against known vectors
|
||||
- Test fixture file is parseable by parseOggPages
|
||||
</done>
|
||||
</task>
|
||||
|
||||
<task type="auto">
|
||||
<name>Task 2: Write round-trip tests for all OGG requirements</name>
|
||||
<files>backend/tagwriter/ogg_test.go</files>
|
||||
<action>
|
||||
Add the following test functions to `ogg_test.go`, following the exact same patterns as `flac_test.go`:
|
||||
|
||||
**TestWriteOggTags_TextFields (OGG-01):**
|
||||
- Create test OGG → write all 8 text fields + composer via `writeOggTags` → read back with `metadata.ExtractTags`.
|
||||
- Assert all 9 values match: Title, Artist, Album, AlbumArtist, Genre, Composer (string); Year, TrackNumber, DiscNumber (int).
|
||||
|
||||
**TestWriteOggTags_CoverArt (OGG-04):**
|
||||
- Create test OGG → write cover art (`tinyJPEG(t)`) via `writeOggTags` → read back → assert `tags.Picture.Data` equals original JPEG bytes, MIME = "image/jpeg".
|
||||
|
||||
**TestWriteOggTags_ClearCoverArt (OGG-04):**
|
||||
- Create test OGG → add cover art → verify art exists → clear art (nil) → read back → assert `tags.Picture` is nil.
|
||||
|
||||
**TestWriteOggTags_PartialUpdate (OGG-02):**
|
||||
- Create test OGG → write all fields → partial update (title + genre only) → read back → assert changed fields have new values, unchanged fields preserved.
|
||||
|
||||
**TestWriteOggTags_AudioPreservation (OGG-03):**
|
||||
- Create test OGG → capture original audio page data bytes (parse pages, collect non-header pages) → write tags → parse pages again → assert audio page data is byte-identical.
|
||||
- This proves audio data survives tag editing unchanged.
|
||||
|
||||
**TestWriteOggTags_AtomicSafety (OGG-05):**
|
||||
- Write a corrupt file (not valid OGG) → attempt `writeOggTags` → assert error returned → assert file content unchanged.
|
||||
- Write a valid OGG → capture bytes before → trigger a write (valid changes) on a read-only file or use a known failure path → assert original file preserved.
|
||||
|
||||
**TestWriteOggTags_RejectNonVorbis (OGG-03 error path):**
|
||||
- Create a file with valid OGG page structure but non-Vorbis identification header (e.g., replace `\x01vorbis` with `\x01theora` or `OpusHead`).
|
||||
- Assert `writeOggTags` returns an error containing "not an OGG Vorbis" or similar.
|
||||
|
||||
**TestWriteOggTags_RejectMultiStream:**
|
||||
- Create a file with pages from two different serial numbers.
|
||||
- Assert `writeOggTags` returns an error containing "multiple streams".
|
||||
|
||||
All tests use:
|
||||
- `testLogger()` from helpers_test.go
|
||||
- `tinyJPEG(t)` from helpers_test.go
|
||||
- `assertEqual` / `assertStrField` / `assertIntField` from helpers_test.go
|
||||
- `metadata.ExtractTags` for read-back (unlike WAV which needed bogem/id3v2)
|
||||
- `t.TempDir()` for test isolation
|
||||
</action>
|
||||
<verify>
|
||||
<automated>go test ./backend/tagwriter/ -run "TestWriteOggTags" -count=1 -v</automated>
|
||||
</verify>
|
||||
<done>
|
||||
- All TestWriteOggTags_* tests pass
|
||||
- Text fields round-trip correctly (OGG-01)
|
||||
- Non-edited fields preserved (OGG-02)
|
||||
- Audio data byte-identical after write (OGG-03)
|
||||
- Cover art embed/replace/clear works (OGG-04)
|
||||
- Atomic safety verified (OGG-05)
|
||||
- All tests use dhowden/tag via metadata.ExtractTags (OGG-06)
|
||||
- Non-Vorbis and multi-stream rejection tested
|
||||
</done>
|
||||
</task>
|
||||
|
||||
<task type="auto">
|
||||
<name>Task 3: Full test suite verification</name>
|
||||
<files>backend/tagwriter/ogg_test.go</files>
|
||||
<action>
|
||||
Run the complete tagwriter test suite to verify no regressions:
|
||||
```bash
|
||||
go test ./backend/tagwriter/ -count=1 -v
|
||||
```
|
||||
|
||||
Verify all OGG tests pass alongside existing MP3, FLAC, and WAV tests. If any lint issues exist in the new OGG test file, fix them (wsl, nlreturn, etc.). Run `go vet ./backend/tagwriter/` for a clean check.
|
||||
|
||||
Expected: All tests green, no regressions in existing format tests.
|
||||
</action>
|
||||
<verify>
|
||||
<automated>go test ./backend/tagwriter/ -count=1 && go vet ./backend/tagwriter/</automated>
|
||||
</verify>
|
||||
<done>
|
||||
- Full tagwriter test suite passes (MP3 + FLAC + WAV + OGG)
|
||||
- go vet clean
|
||||
- No regressions in existing tests
|
||||
</done>
|
||||
</task>
|
||||
|
||||
</tasks>
|
||||
|
||||
<verification>
|
||||
```bash
|
||||
# All OGG tests pass
|
||||
go test ./backend/tagwriter/ -run "TestOgg|TestWriteOggTags" -count=1 -v
|
||||
|
||||
# Full suite (no regressions)
|
||||
go test ./backend/tagwriter/ -count=1
|
||||
|
||||
# Vet clean
|
||||
go vet ./backend/tagwriter/
|
||||
|
||||
# Test count check — expect 7+ OGG test functions
|
||||
grep -c "^func Test.*Ogg" backend/tagwriter/ogg_test.go
|
||||
```
|
||||
</verification>
|
||||
|
||||
<success_criteria>
|
||||
- All TestWriteOggTags_* and TestOggCRC_* tests pass
|
||||
- Full tagwriter test suite passes with zero failures
|
||||
- go vet clean
|
||||
- ogg_test.go has 7+ test functions covering all 6 OGG requirements
|
||||
- Read-back uses metadata.ExtractTags (dhowden/tag) per OGG-06
|
||||
</success_criteria>
|
||||
|
||||
<output>
|
||||
After completion, create `.planning/phases/20-ogg-vorbis-tag-writer/20-02-SUMMARY.md`
|
||||
</output>
|
||||
Reference in New Issue
Block a user