summaryrefslogtreecommitdiff
path: root/internal/tag
diff options
context:
space:
mode:
authorPeter Stone <thepeterstone@gmail.com>2026-09-21 06:50:42 +0000
committerPeter Stone <thepeterstone@gmail.com>2026-09-21 06:50:42 +0000
commit293a627358553c9f61802caf9d4bc76af9c2e239 (patch)
treee4769ac949659953bd5e09cebea9d0697e82548f /internal/tag
podkit: playlist URL to private podcast feed
One Go binary with fetch / tag / feed / serve / all. The stages run separately on purpose: the download is the slow part and should not be repeated while adjusting titles or the base URL. Built from a shell + Python + Go sketch. The pieces that needed fixing on the way, all of which produce a feed that loads wrong or not at all: - Enclosure URLs were unescaped. Track titles routinely contain spaces and ampersands, which make both the URL and the XML invalid — the likeliest way for a feed to fail to load entirely. Only the final path segment is escaped; escaping the base would mangle scheme and host. - Ordering came from filename sort, which is correct only while zero-padding is applied consistently. One unpadded file puts lecture 10 after lecture 1. Order now comes from the parsed track number. - No pubDate and no itunes:type. Podcast clients default to newest-first, which starts a lecture course at the final lecture. Items now carry distinct ascending dates and the feed is marked serial. - Missing channel description and link, which several clients reject outright. - A struct field with no xml tag emitted a stray <EnclosureURL> element. - guid had no isPermaLink="false", so a reader may treat it as a fetchable URL. - Errors from ReadDir, Info and Marshal were discarded, so a missing directory produced an empty feed that looked like a working setup with nothing in it. - os.Args indexed without a length check. Beyond the sketch: a download archive so an interrupted series resumes, an up-front check for yt-dlp and ffmpeg before the long download rather than a post-processing failure after the bytes are on disk, non-audio files excluded by extension so cover art and .part files cannot become episodes, and a serve command with token gating that 404s rather than 401s. Found by the first live run against a LibriVox collection: sources whose own titles begin with a track number came out as "01 - 01 - April.mp3", because the output template prepends the playlist index regardless. Collapsed, but only when the two numbers agree — "01 - 1964 - Lecture" is a year and "05 - 12 Angry Men" is a title, and neither is a repeat. 47 tests. Verified end to end against public-domain audio: downloaded, converted, tagged, served, and the delivered file is a playable 128kbps MP3 with correct ID3 title/artist/album/track frames.
Diffstat (limited to 'internal/tag')
-rw-r--r--internal/tag/tag.go132
-rw-r--r--internal/tag/tag_test.go158
2 files changed, 290 insertions, 0 deletions
diff --git a/internal/tag/tag.go b/internal/tag/tag.go
new file mode 100644
index 0000000..653fac7
--- /dev/null
+++ b/internal/tag/tag.go
@@ -0,0 +1,132 @@
+// Package tag reads and writes ID3v2 tags on downloaded audio.
+package tag
+
+import (
+ "fmt"
+ "os"
+ "path/filepath"
+ "strconv"
+ "strings"
+
+ "github.com/bogem/id3v2/v2"
+
+ "podkit/internal/feed"
+)
+
+// Metadata is the subset of ID3 fields worth setting for a lecture series.
+type Metadata struct {
+ Album string
+ Artist string
+ AlbumArtist string
+ Genre string
+ Year string
+}
+
+// Result reports what happened to one file.
+type Result struct {
+ File string
+ Track int
+ Title string
+ Err error
+}
+
+// Read returns the title and track number stored in a file's tags.
+//
+// Signature matches feed.TagReader so the feed generator can prefer real tags
+// over a filename guess. A file with no tags is not an error — it is the
+// normal state of a fresh download — so it reports ok=false and lets the
+// caller fall back.
+func Read(path string) (title string, track int, ok bool) {
+ t, err := id3v2.Open(path, id3v2.Options{Parse: true})
+ if err != nil {
+ return "", 0, false
+ }
+ defer func() { _ = t.Close() }()
+
+ title = strings.TrimSpace(t.Title())
+ // A track frame can be "3" or "3/7"; only the part before the slash is the
+ // number of this track.
+ raw := strings.TrimSpace(t.GetTextFrame(t.CommonID("Track number/Position in set")).Text)
+ if i := strings.IndexByte(raw, '/'); i >= 0 {
+ raw = raw[:i]
+ }
+ if n, err := strconv.Atoi(strings.TrimSpace(raw)); err == nil {
+ track = n
+ }
+ return title, track, title != "" || track > 0
+}
+
+// WriteDir tags every audio file in dir, deriving the track number and title
+// from the filename.
+//
+// Only .mp3 is handled: ID3v2 is an MP3 container convention, and writing it
+// into an .opus or .m4a file produces something no player will read.
+func WriteDir(dir string, md Metadata) ([]Result, error) {
+ entries, err := os.ReadDir(dir)
+ if err != nil {
+ return nil, fmt.Errorf("reading %s: %w", dir, err)
+ }
+
+ var results []Result
+ total := 0
+ for _, e := range entries {
+ if !e.IsDir() && strings.EqualFold(filepath.Ext(e.Name()), ".mp3") {
+ total++
+ }
+ }
+
+ for _, e := range entries {
+ if e.IsDir() || !strings.EqualFold(filepath.Ext(e.Name()), ".mp3") {
+ continue
+ }
+ path := filepath.Join(dir, e.Name())
+ track, title := feed.ParseName(e.Name())
+ res := Result{File: e.Name(), Track: track, Title: title}
+ if err := writeOne(path, track, total, title, md); err != nil {
+ res.Err = err
+ }
+ results = append(results, res)
+ }
+ return results, nil
+}
+
+func writeOne(path string, track, total int, title string, md Metadata) error {
+ // Parse:true so an existing tag is preserved and updated rather than
+ // replaced wholesale, which would discard artwork yt-dlp may have embedded.
+ t, err := id3v2.Open(path, id3v2.Options{Parse: true})
+ if err != nil {
+ return fmt.Errorf("opening: %w", err)
+ }
+ defer func() { _ = t.Close() }()
+
+ t.SetTitle(title)
+ if md.Album != "" {
+ t.SetAlbum(md.Album)
+ }
+ if md.Artist != "" {
+ t.SetArtist(md.Artist)
+ }
+ if md.Genre != "" {
+ t.SetGenre(md.Genre)
+ }
+ if md.Year != "" {
+ t.SetYear(md.Year)
+ }
+ if md.AlbumArtist != "" {
+ t.AddTextFrame(t.CommonID("Band/Orchestra/Accompaniment"), t.DefaultEncoding(), md.AlbumArtist)
+ }
+ if track > 0 {
+ // "n/total" so a player can show "3 of 7" and, more importantly, sort
+ // the series correctly when the filenames are not available to it.
+ value := strconv.Itoa(track)
+ if total > 0 {
+ value = fmt.Sprintf("%d/%d", track, total)
+ }
+ t.AddTextFrame(t.CommonID("Track number/Position in set"), t.DefaultEncoding(), value)
+ }
+
+ if err := t.Save(); err != nil {
+ return fmt.Errorf("saving tags: %w", err)
+ }
+ return nil
+}
diff --git a/internal/tag/tag_test.go b/internal/tag/tag_test.go
new file mode 100644
index 0000000..f722cb6
--- /dev/null
+++ b/internal/tag/tag_test.go
@@ -0,0 +1,158 @@
+package tag
+
+import (
+ "os"
+ "path/filepath"
+ "strings"
+ "testing"
+)
+
+// A minimal but genuinely valid MP3: one silent MPEG-1 Layer III frame.
+// id3v2 needs a real file to append a tag to, and a zero-byte file is not one.
+func writeMP3(t *testing.T, dir, name string) string {
+ t.Helper()
+ // 0xFF 0xFB = MPEG-1 Layer III, no CRC; 0x90 = 128kbps 44.1kHz; then padding.
+ frame := []byte{0xFF, 0xFB, 0x90, 0x00}
+ body := append(frame, make([]byte, 413)...)
+ p := filepath.Join(dir, name)
+ if err := os.WriteFile(p, body, 0o644); err != nil {
+ t.Fatalf("writing %s: %v", name, err)
+ }
+ return p
+}
+
+func TestWriteDir_TagsEachFile(t *testing.T) {
+ dir := t.TempDir()
+ writeMP3(t, dir, "01 - The Law of Gravitation.mp3")
+ writeMP3(t, dir, "02 - The Relation of Mathematics to Physics.mp3")
+
+ results, err := WriteDir(dir, Metadata{
+ Album: "Character of Physical Law",
+ Artist: "Richard Feynman",
+ })
+ if err != nil {
+ t.Fatalf("WriteDir: %v", err)
+ }
+ if len(results) != 2 {
+ t.Fatalf("got %d results, want 2", len(results))
+ }
+ for _, r := range results {
+ if r.Err != nil {
+ t.Errorf("%s: %v", r.File, r.Err)
+ }
+ }
+
+ title, track, ok := Read(filepath.Join(dir, "01 - The Law of Gravitation.mp3"))
+ if !ok {
+ t.Fatal("tags were not readable after writing")
+ }
+ if title != "The Law of Gravitation" {
+ t.Errorf("title = %q", title)
+ }
+ if track != 1 {
+ t.Errorf("track = %d, want 1", track)
+ }
+}
+
+// Read is handed to the feed generator, which relies on ok=false to fall back
+// to the filename. An untagged file is the normal state of a fresh download,
+// not an error.
+func TestRead_UntaggedFileReportsNotOK(t *testing.T) {
+ dir := t.TempDir()
+ p := writeMP3(t, dir, "01 - Untagged.mp3")
+ if _, _, ok := Read(p); ok {
+ t.Error("an untagged file should report ok=false")
+ }
+}
+
+func TestRead_MissingFileReportsNotOK(t *testing.T) {
+ if _, _, ok := Read(filepath.Join(t.TempDir(), "nope.mp3")); ok {
+ t.Error("a missing file should report ok=false, not panic")
+ }
+}
+
+// Track frames are commonly "3/7". Taking the whole string would make the
+// number unparseable and lose the ordering.
+func TestRead_ParsesTrackOfTotal(t *testing.T) {
+ dir := t.TempDir()
+ writeMP3(t, dir, "03 - Third.mp3")
+ writeMP3(t, dir, "01 - First.mp3")
+ writeMP3(t, dir, "02 - Second.mp3")
+
+ if _, err := WriteDir(dir, Metadata{Album: "A"}); err != nil {
+ t.Fatalf("WriteDir: %v", err)
+ }
+ _, track, ok := Read(filepath.Join(dir, "03 - Third.mp3"))
+ if !ok {
+ t.Fatal("tags not readable")
+ }
+ if track != 3 {
+ t.Errorf("track = %d, want 3 (from a \"3/3\" frame)", track)
+ }
+}
+
+// ID3v2 is an MP3 convention; writing it into other containers produces
+// something no player reads, so other formats are skipped rather than corrupted.
+func TestWriteDir_SkipsNonMP3(t *testing.T) {
+ dir := t.TempDir()
+ writeMP3(t, dir, "01 - One.mp3")
+ if err := os.WriteFile(filepath.Join(dir, "02 - Two.opus"), []byte("not an mp3"), 0o644); err != nil {
+ t.Fatal(err)
+ }
+ if err := os.WriteFile(filepath.Join(dir, "cover.jpg"), []byte("jpeg"), 0o644); err != nil {
+ t.Fatal(err)
+ }
+
+ results, err := WriteDir(dir, Metadata{Album: "A"})
+ if err != nil {
+ t.Fatalf("WriteDir: %v", err)
+ }
+ if len(results) != 1 || !strings.HasSuffix(results[0].File, ".mp3") {
+ t.Errorf("got %+v, want only the mp3", results)
+ }
+}
+
+func TestWriteDir_MissingDirectoryIsAnError(t *testing.T) {
+ if _, err := WriteDir(filepath.Join(t.TempDir(), "nope"), Metadata{}); err == nil {
+ t.Error("expected an error for a missing directory")
+ }
+}
+
+// A file with no track prefix should still get its metadata; only the track
+// number is absent.
+func TestWriteDir_HandlesUnnumberedFiles(t *testing.T) {
+ dir := t.TempDir()
+ writeMP3(t, dir, "Introduction.mp3")
+
+ results, err := WriteDir(dir, Metadata{Album: "Course", Artist: "Someone"})
+ if err != nil {
+ t.Fatalf("WriteDir: %v", err)
+ }
+ if results[0].Err != nil {
+ t.Fatalf("tagging failed: %v", results[0].Err)
+ }
+ title, track, ok := Read(filepath.Join(dir, "Introduction.mp3"))
+ if !ok || title != "Introduction" {
+ t.Errorf("title = %q ok = %v, want the filename stem", title, ok)
+ }
+ if track != 0 {
+ t.Errorf("track = %d, want 0 for an unnumbered file", track)
+ }
+}
+
+// Re-running the tagger must be safe; the pipeline is meant to be resumable.
+func TestWriteDir_IsIdempotent(t *testing.T) {
+ dir := t.TempDir()
+ writeMP3(t, dir, "01 - One.mp3")
+ md := Metadata{Album: "A", Artist: "B"}
+
+ for i := 0; i < 3; i++ {
+ if _, err := WriteDir(dir, md); err != nil {
+ t.Fatalf("pass %d: %v", i, err)
+ }
+ }
+ title, track, ok := Read(filepath.Join(dir, "01 - One.mp3"))
+ if !ok || title != "One" || track != 1 {
+ t.Errorf("after 3 passes: title=%q track=%d ok=%v", title, track, ok)
+ }
+}