diff --git a/.github/workflows/builder.yml b/.github/workflows/builder.yml index 3dd32d2..5149936 100644 --- a/.github/workflows/builder.yml +++ b/.github/workflows/builder.yml @@ -32,11 +32,11 @@ jobs: - run: go vet ./... - name: Check cyclomatic complexity run: | - go install github.com/fzipp/gocyclo/cmd/gocyclo@latest + go install github.com/fzipp/gocyclo/cmd/gocyclo@v0.6.0 gocyclo -top 20 -ignore '_test\.go$' -avg . - name: Check ineffectual assignments run: | - go install github.com/gordonklaus/ineffassign@latest + go install github.com/gordonklaus/ineffassign@v0.2.0 ineffassign ./... - uses: golangci/golangci-lint-action@v9 diff --git a/.gitignore b/.gitignore index 5f22320..6423cd8 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,10 @@ .direnv /jive-encoder -testdata/ +testdata/* +!testdata/0.md +!testdata/LMP0.flac +!testdata/linuxmatters-3000x3000.png *.mp3 *.opus *.m4a +coverage.out diff --git a/.harper-dictionary.txt b/.harper-dictionary.txt deleted file mode 100644 index 015bbcb..0000000 --- a/.harper-dictionary.txt +++ /dev/null @@ -1,2 +0,0 @@ -Jivedrop -jive-encoder diff --git a/AGENTS.md b/AGENTS.md index 0da9afe..10979c3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -19,7 +19,7 @@ nix develop # Enter NixOS development shell (ffmpeg, lame, mediainfo, just, go) just build # Build binary with version from git tags (CGO_ENABLED=1) just test # Run all Go tests just test-encoder # Integration test: encode testdata/LMP67.flac -just clean # Remove build artifacts and test outputs (*.mp3) +just clean # Remove build artifacts and test outputs (*.mp3/*.m4a/*.opus) ``` ## Architecture @@ -33,12 +33,11 @@ cmd/jive-encoder/ internal/ encoder/ # FFmpeg-based MP3/AAC/Opus encoding via ffmpeg-statigo encoder.go # Core encode pipeline: decode → filter → encode → muxer-native tag - preset.go # Per-format preset table (codec, bitrate, sample fmt/rate, muxer, extension, lowpass, cover) + preset.go # Per-format preset table (codec, bitrate, sample fmt/rate, extension, lowpass, cover) metadata.go # Hugo frontmatter parsing (YAML between --- delimiters) + muxer tag assembly stats.go # Duration/filesize extraction from the encoded file - id3/ # Cover-art scaling and tag-field carrier (no ID3 writer; FFmpeg muxers write tags) + artwork/ # Cover-art scaling (no tag writer; FFmpeg muxers write tags) artwork.go # Cover art scaling (1400-3000px range for Apple Podcasts) - taginfo.go # TagInfo carrier for episode metadata fields ui/ # Bubbletea TUI for encoding progress encode.go # Progress model with realtime speed calculation cli/ # Lipgloss-styled output @@ -67,11 +66,11 @@ third_party/ffmpeg-statigo/ # Git submodule: FFmpeg 8.1 static bindings ### Encoding Settings -`internal/encoder/preset.go` holds the per-format preset table (the single source of truth for codec, bitrate, sample format, sample rate, muxer, extension, lowpass, cover capability). Mono is the default; `--stereo` selects the stereo bitrate. +`internal/encoder/preset.go` holds the per-format preset table (the single source of truth for codec, bitrate, sample format, sample rate, extension, lowpass, cover capability). Mono is the default; `--stereo` selects the stereo bitrate. -- **MP3 (default)**: CBR 112/192kbps, 44.1kHz, sample fmt `s16p`, LAME quality 3, 20.5kHz lowpass; `mp3` muxer → `.mp3` -- **AAC-LC (`--format aac`)**: CBR 64/128kbps, 44.1kHz, sample fmt `fltp`, no lowpass; `ipod` muxer → `.m4a` -- **Opus (`--format opus`)**: VBR ~32/~48kbps, 48kHz (libopus rejects 44.1kHz), sample fmt `flt` (libopus rejects `fltp`), `vbr=on`, compression_level 10, no lowpass; `opus` muxer → `.opus` +- **MP3 (default)**: CBR 112/192kbps, 44.1kHz, sample fmt `s16p`, LAME quality 3, 20.5kHz lowpass; emits `.mp3` +- **AAC-LC (`--format aac`)**: CBR 64/128kbps, 44.1kHz, sample fmt `fltp`, no lowpass; emits `.m4a` +- **Opus (`--format opus`)**: VBR ~32/~48kbps, 48kHz (libopus rejects 44.1kHz), sample fmt `flt` (libopus rejects `fltp`), `vbr=on`, compression_level 10, no lowpass; emits `.opus` ### FFmpeg Integration @@ -85,7 +84,7 @@ third_party/ffmpeg-statigo/ # Git submodule: FFmpeg 8.1 static bindings - Tagging is FFmpeg muxer-native: standard keys (`title`/`artist`/`album`/`date`/`comment`/`track`) go into an `AVDictionary` on the output format context before `AVFormatWriteHeader`, so each muxer writes its own format: ID3v2.4 (MP3, via the `id3v2_version=4` WriteHeader muxer option), iTunes MP4 atoms (M4A), Vorbis comments (Opus) - Title renders `"{episode}: {title}"`; track maps to the episode number; empty fields are skipped - Cover is an attached-picture stream (`AVDispositionAttachedPic`) written right after the header, for cover-capable formats (MP3, AAC) only. **Opus has no embedded cover** (text tags only) -- `bogem/id3v2` is removed. `internal/id3/` holds only `artwork.go` (cover scaling) and `taginfo.go` (the `TagInfo` carrier) +- `bogem/id3v2` is removed. `internal/artwork/` holds cover scaling; `encoder.Metadata` is the single tag-field carrier from the CLI workflows to the encoder ## Code Conventions diff --git a/README.md b/README.md index b3cc60c..38f8821 100644 --- a/README.md +++ b/README.md @@ -35,7 +35,7 @@ Jive Encoder takes your mixed podcast audio (WAV/FLAC) and outputs RSS-ready pod ### Hugo Mode (Integrated Workflow) -For podcasts using Hugo static site generator and the something like [Castanet](https://github.com/mattstratton/castanet), Jive Encoder reads metadata from episode markdown: +For podcasts using the Hugo static site generator and a theme like [Castanet](https://github.com/mattstratton/castanet), Jive Encoder reads metadata from episode markdown: **Hugo mode automatically:** - Reads episode title and number from frontmatter @@ -115,7 +115,7 @@ Flags: --comment Comment URL (defaults to 'https://linuxmatters.sh' in Hugo mode) --cover Cover art path (required in standalone mode) --output-path Output file or directory path - --format Output format: mp3, aac, or opus (default: "mp3") + --format Output format: mp3, aac, or opus (default: mp3) --stereo Encode as stereo at 192kbps (default: mono at 112kbps) --version Show version information ``` @@ -126,6 +126,8 @@ Flags: Where `{ext}` is `.mp3`, `.m4a`, or `.opus` depending on `--format`. +If `--output-path` names a file, its extension must match `--format`. + ### Encoding settings | Format | Mono | Stereo | Sample rate | Notes | diff --git a/cmd/jive-encoder/help_test.go b/cmd/jive-encoder/help_test.go new file mode 100644 index 0000000..6e8eb1e --- /dev/null +++ b/cmd/jive-encoder/help_test.go @@ -0,0 +1,79 @@ +package main + +import ( + "bytes" + "strings" + "testing" + + "github.com/alecthomas/kong" + "github.com/linuxmatters/jive-encoder/internal/cli" +) + +// TestStyledHelpOutput renders --help against the real CLI struct through +// StyledHelpPrinter. It checks the default annotations and colour handling. +// The parser mirrors run(): same kong.Name, kong.Description, and kong.Help. +// Writers go to a buffer and exit is stubbed so help does not end the test. +func TestStyledHelpOutput(t *testing.T) { + // CLI is package-level mutable state shared with other tests. Parsing + // applies defaults to it, so snapshot and restore it around the parse. + saved := CLI + defer func() { CLI = saved }() + + var buf bytes.Buffer + parser, err := kong.New(&CLI, + kong.Name("jive-encoder"), + kong.Description("Drop the mix, ship the show—metadata, cover art, and all."), + kong.Help(cli.StyledHelpPrinter), + kong.Writers(&buf, &buf), + kong.Exit(func(int) {}), + ) + if err != nil { + t.Fatalf("failed to build parser: %v", err) + } + + // --help triggers the help printer. The stubbed exit means Parse returns + // normally afterwards; any residual parse error does not matter here. + _, _ = parser.Parse([]string{"--help"}) + + out := buf.String() + if out == "" { + t.Fatal("expected help output, got empty buffer") + } + + // Kong type names must never leak into the rendered defaults. + for _, leaked := range []string{"(default: STRING)", "(default: BOOL)"} { + if strings.Contains(out, leaked) { + t.Errorf("help output contains leaked type default %q", leaked) + } + } + + // The buffer is not a TTY, so colorprofile must degrade to plain text. + if strings.Contains(out, "\x1b[") { + t.Error("help output contains ANSI escape sequences on a non-TTY writer") + } + + stereoLine := findFlagLine(t, out, "--stereo") + // --stereo has no Kong default. Its single "(default: mono)" comes from + // the flag's help text, so a second occurrence means a rendered default. + if got := strings.Count(stereoLine, "(default:"); got != 1 { + t.Errorf("--stereo line has %d \"(default:\" occurrences, want 1: %q", got, stereoLine) + } + + formatLine := findFlagLine(t, out, "--format") + if !strings.Contains(formatLine, "(default: mp3)") { + t.Errorf("--format line missing \"(default: mp3)\": %q", formatLine) + } +} + +// findFlagLine returns the help output line containing the given flag. It +// fails the test if the flag is absent. +func findFlagLine(t *testing.T, out, flag string) string { + t.Helper() + for line := range strings.SplitSeq(out, "\n") { + if strings.Contains(line, flag) { + return line + } + } + t.Fatalf("help output has no line containing %q", flag) + return "" +} diff --git a/cmd/jive-encoder/hugo.go b/cmd/jive-encoder/hugo.go index a3b9a5b..3dc8249 100644 --- a/cmd/jive-encoder/hugo.go +++ b/cmd/jive-encoder/hugo.go @@ -7,7 +7,6 @@ import ( "github.com/linuxmatters/jive-encoder/internal/cli" "github.com/linuxmatters/jive-encoder/internal/encoder" - "github.com/linuxmatters/jive-encoder/internal/id3" ) // Hugo mode metadata defaults for the Linux Matters podcast. @@ -22,7 +21,7 @@ const ( type HugoWorkflow struct { // opts carries the parsed CLI fields, populated at construction. opts CLIOptions - // hugoMetadata is set during CollectMetadata and read during PostEncode + // hugoMetadata is set during CollectMetadata and read during PostEncode. hugoMetadata *encoder.EpisodeMetadata } @@ -32,7 +31,7 @@ func (h *HugoWorkflow) Validate() error { return fmt.Errorf("hugo mode requires episode markdown file as second argument") } - if !strings.HasSuffix(strings.ToLower(h.opts.EpisodeMD), ".md") { + if !isMarkdownPath(h.opts.EpisodeMD) { return fmt.Errorf("episode markdown file must have .md extension: %s", h.opts.EpisodeMD) } @@ -51,10 +50,10 @@ func (h *HugoWorkflow) Validate() error { // CollectMetadata parses Hugo frontmatter, applies defaults and flag overrides, // and resolves the cover art path. -func (h *HugoWorkflow) CollectMetadata() (id3.TagInfo, string, error) { +func (h *HugoWorkflow) CollectMetadata() (encoder.Metadata, string, error) { metadata, err := encoder.ParseEpisodeMetadata(h.opts.EpisodeMD) if err != nil { - return id3.TagInfo{}, "", fmt.Errorf("failed to parse episode metadata: %w", err) + return encoder.Metadata{}, "", fmt.Errorf("failed to parse episode metadata: %w", err) } h.hugoMetadata = metadata @@ -79,7 +78,7 @@ func (h *HugoWorkflow) CollectMetadata() (id3.TagInfo, string, error) { episodeNum = h.opts.Num } if _, err := encoder.ParseEpisodeNumber(episodeNum); err != nil { - return id3.TagInfo{}, "", fmt.Errorf("invalid episode number: %w", err) + return encoder.Metadata{}, "", fmt.Errorf("invalid episode number: %w", err) } if h.opts.Date != "" { date = h.opts.Date @@ -91,11 +90,11 @@ func (h *HugoWorkflow) CollectMetadata() (id3.TagInfo, string, error) { } else { coverArtPath, err = encoder.ResolveCoverArtPath(h.opts.EpisodeMD, metadata.EpisodeImage) if err != nil { - return id3.TagInfo{}, "", fmt.Errorf("failed to resolve cover art: %w", err) + return encoder.Metadata{}, "", fmt.Errorf("failed to resolve cover art: %w", err) } } - tagInfo := id3.TagInfo{ + tags := encoder.Metadata{ EpisodeNumber: episodeNum, Title: episodeTitle, Artist: artist, @@ -104,7 +103,7 @@ func (h *HugoWorkflow) CollectMetadata() (id3.TagInfo, string, error) { Comment: comment, } - return tagInfo, coverArtPath, nil + return tags, coverArtPath, nil } // PostEncode displays podcast statistics and handles frontmatter comparison and update prompting. @@ -127,7 +126,8 @@ func (h *HugoWorkflow) PostEncode(stats *encoder.FileStats) error { needsUpdate = true } - // Prompt user to update frontmatter if values differ or are missing + // A mismatch and a missing value both need confirmation, but each gets its + // own prompt wording so the user knows which case they are approving. if needsUpdate { promptAndUpdateFrontmatter(h.opts.EpisodeMD, "\nUpdate frontmatter with new values? [y/N]: ", stats.DurationString, stats.FileSizeBytes) } else if h.hugoMetadata.PodcastDuration == "" || h.hugoMetadata.PodcastBytes == 0 { diff --git a/cmd/jive-encoder/hugo_test.go b/cmd/jive-encoder/hugo_test.go index 23fb2b2..2aa8dac 100644 --- a/cmd/jive-encoder/hugo_test.go +++ b/cmd/jive-encoder/hugo_test.go @@ -1,6 +1,8 @@ package main import ( + "os" + "path/filepath" "strings" "testing" ) @@ -156,17 +158,19 @@ func TestHugoWorkflowValidate(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - // Construct the workflow with the test inputs threaded through - // CLIOptions. A dummy audio file keeps file-existence checks from - // masking the argument validation errors we are testing for. + episodeMD := tt.episodeMD + if !tt.wantErr { + episodeMD = existingMarkdownArgument(t, tt.episodeMD) + } + wf := &HugoWorkflow{opts: CLIOptions{ - EpisodeMD: tt.episodeMD, + EpisodeMD: episodeMD, }} err := wf.Validate() if tt.wantErr { if err == nil { - t.Errorf("HugoWorkflow.Validate() expected error, got nil (EpisodeMD=%q)", tt.episodeMD) + t.Errorf("HugoWorkflow.Validate() expected error, got nil (EpisodeMD=%q)", episodeMD) return } if tt.errMatch != "" && !strings.Contains(err.Error(), tt.errMatch) { @@ -175,10 +179,8 @@ func TestHugoWorkflowValidate(t *testing.T) { return } - // For valid cases we only check argument validation, not file existence. - // File-not-found errors are acceptable here since the files do not exist on disk. - if err != nil && !strings.Contains(err.Error(), "not found") && !strings.Contains(err.Error(), "not accessible") { - t.Errorf("HugoWorkflow.Validate() unexpected error: %v (EpisodeMD=%q)", err, tt.episodeMD) + if err != nil { + t.Errorf("HugoWorkflow.Validate() unexpected error: %v (EpisodeMD=%q)", err, episodeMD) } }) } @@ -238,22 +240,53 @@ func TestHugoWorkflowValidate_Integration(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - // Construct the workflow with the test inputs threaded through - // CLIOptions. A dummy audio file keeps file-existence checks from - // masking the argument validation errors we are testing for. + episodeMD := tt.episodeMD + if !tt.wantErr { + episodeMD = existingMarkdownArgument(t, tt.episodeMD) + } + wf := &HugoWorkflow{opts: CLIOptions{ - EpisodeMD: tt.episodeMD, + EpisodeMD: episodeMD, }} err := wf.Validate() if tt.wantErr && err == nil { t.Errorf("HugoWorkflow.Validate() expected error but got nil\n Description: %s\n EpisodeMD=%q", - tt.description, tt.episodeMD) + tt.description, episodeMD) } - if !tt.wantErr && err != nil && !strings.Contains(err.Error(), "not found") && !strings.Contains(err.Error(), "not accessible") { + if !tt.wantErr && err != nil { t.Errorf("HugoWorkflow.Validate() unexpected error: %v\n Description: %s\n EpisodeMD=%q", - err, tt.description, tt.episodeMD) + err, tt.description, episodeMD) } }) } } + +func existingMarkdownArgument(t *testing.T, path string) string { + t.Helper() + + root := t.TempDir() + workDir := filepath.Join(root, "work") + if err := os.MkdirAll(workDir, 0o755); err != nil { + t.Fatalf("create test work dir: %v", err) + } + t.Chdir(workDir) + + arg := path + if filepath.IsAbs(path) { + arg = filepath.Join(root, strings.TrimPrefix(path, string(filepath.Separator))) + } + + target := arg + if !filepath.IsAbs(target) { + target = filepath.Join(workDir, target) + } + if err := os.MkdirAll(filepath.Dir(target), 0o755); err != nil { + t.Fatalf("create markdown fixture dir: %v", err) + } + if err := os.WriteFile(target, []byte("---\n---\n"), 0o644); err != nil { + t.Fatalf("create markdown fixture: %v", err) + } + + return arg +} diff --git a/cmd/jive-encoder/main.go b/cmd/jive-encoder/main.go index 95e2d6c..b5468de 100644 --- a/cmd/jive-encoder/main.go +++ b/cmd/jive-encoder/main.go @@ -1,6 +1,7 @@ package main import ( + "errors" "fmt" "os" "path/filepath" @@ -9,9 +10,9 @@ import ( tea "charm.land/bubbletea/v2" "github.com/alecthomas/kong" "github.com/charmbracelet/x/term" + "github.com/linuxmatters/jive-encoder/internal/artwork" "github.com/linuxmatters/jive-encoder/internal/cli" "github.com/linuxmatters/jive-encoder/internal/encoder" - "github.com/linuxmatters/jive-encoder/internal/id3" "github.com/linuxmatters/jive-encoder/internal/ui" ) @@ -19,13 +20,6 @@ import ( // tag (e.g. "v0.1.0") for releases. var version = "dev" -// coverArtResult carries the outcome of concurrent cover art processing back -// to the encode pipeline. -type coverArtResult struct { - data []byte - err error -} - // WorkflowMode selects how metadata is sourced: from Hugo frontmatter or from // CLI flags alone. type WorkflowMode int @@ -57,21 +51,19 @@ var CLI struct { Version bool `help:"Show version information"` } -// detectMode determines if this is Hugo or Standalone workflow -func detectMode(audioFile, episodeMD string) WorkflowMode { - // With no audio file the mode is irrelevant; run() shows help and exits. - if audioFile == "" { - return HugoMode - } - - // A .md second argument signals Hugo mode. - if episodeMD != "" && strings.HasSuffix(strings.ToLower(episodeMD), ".md") { +// detectMode reports whether the invocation is a Hugo or Standalone workflow. +func detectMode(episodeMD string) WorkflowMode { + if isMarkdownPath(episodeMD) { return HugoMode } return StandaloneMode } +func isMarkdownPath(p string) bool { + return p != "" && strings.HasSuffix(strings.ToLower(p), ".md") +} + // sanitiseForFilename lowercases the string, replaces spaces with hyphens, and // strips anything that is not alphanumeric, hyphen, underscore, or dot, so the // result is safe to use as a filename. @@ -150,7 +142,7 @@ func resolveOutputPath(mode WorkflowMode, num, artist, cliArtist, ext, outputPat // CLI flags by the caller so encode itself reads no package-level state. type EncodeRequest struct { Mode WorkflowMode - TagInfo id3.TagInfo + Metadata encoder.Metadata CoverArtPath string OutputPath string AudioFile string @@ -164,7 +156,7 @@ type EncodeRequest struct { // the input line reads enc.GetInputInfo(). func printEncodePlan(req EncodeRequest, enc *encoder.Encoder) { cli.PrintSuccessLabel("Ready to encode:", fmt.Sprintf("%s -> %s", req.AudioFile, strings.ToUpper(req.Format))) - cli.PrintLabelValue("• Episode:", fmt.Sprintf("%s - %s", req.TagInfo.EpisodeNumber, req.TagInfo.Title)) + cli.PrintLabelValue("• Episode:", fmt.Sprintf("%s - %s", req.Metadata.EpisodeNumber, req.Metadata.Title)) if req.Mode == HugoMode { cli.PrintLabelValue("• Episode markdown:", req.EpisodeMD) } @@ -216,7 +208,7 @@ func runEncodeUI(enc *encoder.Encoder, outputMode string, outputBitrate int) enc // User interrupted with Ctrl+C. Encode has already returned (the model // quits only after EncodingCompleteMsg), so the caller's deferred Close // is safe. Report the interrupt; the caller discards the truncated file. - return encodeOutcome{err: fmt.Errorf("encoding cancelled"), partialFile: true} + return encodeOutcome{err: errors.New("encoding cancelled"), partialFile: true} } if encModel.Error() != nil { return encodeOutcome{err: fmt.Errorf("encoding failed: %w", encModel.Error()), partialFile: true} @@ -226,23 +218,22 @@ func runEncodeUI(enc *encoder.Encoder, outputMode string, outputBitrate int) enc // tea.Printf/Println no-op under WithoutRenderer, so emit the encode-stage // line directly from here when running without a TTY. This mirrors the TTY // completeView (which reports the encode finishing, not the whole job); - // cover-art and ID3 work still follow, and the final-artefact line marks - // success. + // stats extraction still follows, and the final-artefact line marks success. if !isTTY { - fmt.Println("Audio encoded, embedding metadata...") + fmt.Println("Audio encoded, extracting statistics...") } return encodeOutcome{} } -// embedMetadata finishes the job after a successful encode: tags and cover art +// extractStats finishes the job after a successful encode: tags and cover art // are written by the encoder during Initialize/Encode, so this only extracts // file statistics. The returned partial flag is true when the output file was written // successfully but stats extraction failed; in that case stats is nil. -func embedMetadata(req EncodeRequest, enc *encoder.Encoder) (stats *encoder.FileStats, partial bool) { +func extractStats(req EncodeRequest, enc *encoder.Encoder) (stats *encoder.FileStats, partial bool) { cli.PrintSuccessLabel("Complete:", req.OutputPath) - // Extract file statistics using duration from encoder (avoids re-opening file) + // Reuse the encoder's duration so stats do not re-open the output file. durationSecs := enc.GetDurationSecs() stats, err := encoder.GetFileStats(req.OutputPath, durationSecs) if err != nil { @@ -253,30 +244,20 @@ func embedMetadata(req EncodeRequest, enc *encoder.Encoder) (stats *encoder.File return stats, false } -// encode orchestrates the full encoding pipeline: print the plan, create and -// initialise the encoder, scale cover art concurrently, run the Bubbletea UI, -// handle the outcome, then embed metadata and extract statistics. The returned -// partial flag is true when the output file was written successfully but stats -// extraction failed; in that case stats is nil and error is nil. +// encode orchestrates the full encoding pipeline: scale cover art, create and +// initialise the encoder, print the plan, run the Bubbletea UI, handle the +// outcome, then extract file statistics. The returned partial flag is true +// when the output file was written successfully but stats extraction failed; +// in that case stats is nil and error is nil. func encode(req EncodeRequest) (stats *encoder.FileStats, partial bool, err error) { - // The encoder now embeds the cover as an attached-picture stream during - // Initialize/Encode, so the scaled bytes must exist before Initialize. Scale - // up front rather than overlapping the encode; this drops the old - // scale/encode concurrency, but scaling is fast so the cost is negligible. - coverArtChan := make(chan coverArtResult, 1) - go func() { - if req.CoverArtPath == "" { - coverArtChan <- coverArtResult{data: nil, err: nil} - return + // The encoder embeds the cover as an attached-picture stream during + // Initialize/Encode, so the scaled bytes must exist before Initialize. + var coverArt []byte + if req.CoverArtPath != "" { + coverArt, err = artwork.ScaleCoverArt(req.CoverArtPath) + if err != nil { + return nil, false, fmt.Errorf("failed to process cover art: %w", err) } - - artwork, artErr := id3.ScaleCoverArt(req.CoverArtPath) - coverArtChan <- coverArtResult{data: artwork, err: artErr} - }() - - coverResult := <-coverArtChan - if coverResult.err != nil { - return nil, false, fmt.Errorf("failed to process cover art: %w", coverResult.err) } enc, err := encoder.New(encoder.Config{ @@ -284,15 +265,8 @@ func encode(req EncodeRequest) (stats *encoder.FileStats, partial bool, err erro OutputPath: req.OutputPath, Format: req.Format, Stereo: req.Stereo, - CoverArt: coverResult.data, - Metadata: encoder.Metadata{ - EpisodeNumber: req.TagInfo.EpisodeNumber, - Title: req.TagInfo.Title, - Artist: req.TagInfo.Artist, - Album: req.TagInfo.Album, - Date: req.TagInfo.Date, - Comment: req.TagInfo.Comment, - }, + CoverArt: coverArt, + Metadata: req.Metadata, }) if err != nil { return nil, false, fmt.Errorf("failed to create encoder: %w", err) @@ -315,7 +289,7 @@ func encode(req EncodeRequest) (stats *encoder.FileStats, partial bool, err erro return nil, false, outcome.err } - stats, partial = embedMetadata(req, enc) + stats, partial = extractStats(req, enc) return stats, partial, nil } @@ -329,7 +303,7 @@ func run() int { kong.Description("Drop the mix, ship the show—metadata, cover art, and all."), kong.Vars{"version": version}, kong.UsageOnError(), - kong.Help(cli.StyledHelpPrinter(kong.HelpOptions{Compact: true})), + kong.Help(cli.StyledHelpPrinter), ) if CLI.Version { @@ -342,7 +316,7 @@ func run() int { return 0 } - mode := detectMode(CLI.AudioFile, CLI.EpisodeMD) + mode := detectMode(CLI.EpisodeMD) opts := CLIOptions{ EpisodeMD: CLI.EpisodeMD, Num: CLI.Num, @@ -367,13 +341,13 @@ func run() int { return 1 } - tagInfo, coverArtPath, err := wf.CollectMetadata() + metadata, coverArtPath, err := wf.CollectMetadata() if err != nil { cli.PrintError(err.Error()) return 1 } - outputPath, err := resolveOutputPath(mode, tagInfo.EpisodeNumber, tagInfo.Artist, CLI.Artist, encoder.ExtensionFor(CLI.Format), CLI.OutputPath) + outputPath, err := resolveOutputPath(mode, metadata.EpisodeNumber, metadata.Artist, CLI.Artist, encoder.ExtensionFor(CLI.Format), CLI.OutputPath) if err != nil { cli.PrintError(fmt.Sprintf("Failed to resolve output path: %v", err)) return 1 @@ -381,7 +355,7 @@ func run() int { stats, partial, err := encode(EncodeRequest{ Mode: mode, - TagInfo: tagInfo, + Metadata: metadata, CoverArtPath: coverArtPath, OutputPath: outputPath, AudioFile: CLI.AudioFile, diff --git a/cmd/jive-encoder/main_test.go b/cmd/jive-encoder/main_test.go index f4958dc..54e8c34 100644 --- a/cmd/jive-encoder/main_test.go +++ b/cmd/jive-encoder/main_test.go @@ -367,15 +367,17 @@ func TestGenerateFilename(t *testing.T) { // TestResolveOutputPath tests output path resolution with directories and files func TestResolveOutputPath(t *testing.T) { tests := []struct { - name string - outputPath string - mode WorkflowMode - num string - artist string - cliArtist string - ext string - wantErr bool - wantPath string // Substring check for path validation + name string + outputPath string + mode WorkflowMode + num string + artist string + cliArtist string + ext string + wantErr bool + wantPath string // Substring check for path validation + useTempDir bool // Replace outputPath with a fresh temp directory + wantInTempDir bool // Prefix wantPath with the temp directory }{ // Empty output path - use current directory with generated filename { @@ -413,15 +415,17 @@ func TestResolveOutputPath(t *testing.T) { }, // Existing directory - generate filename within it { - name: "existing directory", - outputPath: "", // Will be set to temp dir in test - mode: StandaloneMode, - num: "1", - artist: "Show", - cliArtist: "Show", - ext: ".mp3", - wantErr: false, - wantPath: "show-1.mp3", + name: "existing directory", + outputPath: "", // Replaced with temp dir via useTempDir + mode: StandaloneMode, + num: "1", + artist: "Show", + cliArtist: "Show", + ext: ".mp3", + wantErr: false, + wantPath: "show-1.mp3", + useTempDir: true, + wantInTempDir: true, }, // Explicit file path - use as-is { @@ -438,7 +442,7 @@ func TestResolveOutputPath(t *testing.T) { // File path in existing directory { name: "file path in existing temp directory", - outputPath: "", // Will be set in test + outputPath: "", // Replaced with temp dir via useTempDir mode: HugoMode, num: "99", artist: "", @@ -446,6 +450,7 @@ func TestResolveOutputPath(t *testing.T) { ext: ".m4a", wantErr: false, wantPath: "LMP99.m4a", + useTempDir: true, }, // Error cases: non-existent directory { @@ -476,11 +481,11 @@ func TestResolveOutputPath(t *testing.T) { t.Run(tt.name, func(t *testing.T) { // Handle dynamic temp directory paths testOutputPath := tt.outputPath - if tt.name == "existing directory" || tt.name == "file path in existing temp directory" { - tmpDir := t.TempDir() - testOutputPath = tmpDir - if tt.name == "existing directory" { - tt.wantPath = filepath.Join(testOutputPath, tt.wantPath) + wantPath := tt.wantPath + if tt.useTempDir { + testOutputPath = t.TempDir() + if tt.wantInTempDir { + wantPath = filepath.Join(testOutputPath, wantPath) } } @@ -498,8 +503,8 @@ func TestResolveOutputPath(t *testing.T) { return } - if tt.wantPath != "" && !isPathMatch(result, tt.wantPath) { - t.Errorf("resolveOutputPath() = %q; want path containing %q", result, tt.wantPath) + if wantPath != "" && !isPathMatch(result, wantPath) { + t.Errorf("resolveOutputPath() = %q; want path containing %q", result, wantPath) } }) } @@ -558,52 +563,37 @@ func isPathMatch(fullPath, expected string) bool { func TestDetectMode(t *testing.T) { tests := []struct { name string - audioFile string episodeMD string expected WorkflowMode }{ - // Empty audio file - no arguments provided - { - name: "empty audio file", - audioFile: "", - episodeMD: "", - expected: HugoMode, // Return value doesn't matter, exit will handle it - }, - // Hugo mode: second argument is .md file { name: "hugo mode with lowercase .md", - audioFile: "podcast.flac", episodeMD: "episode.md", expected: HugoMode, }, { name: "hugo mode with uppercase .MD", - audioFile: "podcast.flac", episodeMD: "episode.MD", expected: HugoMode, }, { name: "hugo mode with mixed case .Md", - audioFile: "podcast.flac", episodeMD: "episode.Md", expected: HugoMode, }, { name: "hugo mode with path containing .md", - audioFile: "podcast.flac", episodeMD: "content/episodes/67.md", expected: HugoMode, }, { name: "hugo mode with nested path and uppercase .MD", - audioFile: "audio.wav", episodeMD: "posts/episode/post.MD", expected: HugoMode, }, { name: "hugo mode with only filename .md", - audioFile: "LMP67.flac", episodeMD: "67.md", expected: HugoMode, }, @@ -611,37 +601,26 @@ func TestDetectMode(t *testing.T) { // Standalone mode: second argument is NOT a .md file { name: "standalone mode with .txt file", - audioFile: "podcast.flac", episodeMD: "readme.txt", expected: StandaloneMode, }, { name: "standalone mode with .md in middle of filename", - audioFile: "podcast.flac", episodeMD: "markdown_file.mp3", expected: StandaloneMode, }, { name: "standalone mode with .md not at end", - audioFile: "podcast.flac", episodeMD: "file.md.txt", expected: StandaloneMode, }, { name: "standalone mode with empty episodeMD string", - audioFile: "podcast.flac", - episodeMD: "", - expected: StandaloneMode, - }, - { - name: "standalone mode with audio file only", - audioFile: "LMP67.flac", episodeMD: "", expected: StandaloneMode, }, { name: "standalone mode with non-md extension", - audioFile: "audio.wav", episodeMD: "episode.yaml", expected: StandaloneMode, }, @@ -649,94 +628,60 @@ func TestDetectMode(t *testing.T) { // Edge cases { name: "just .md (no filename before extension)", - audioFile: "podcast.flac", episodeMD: ".md", expected: HugoMode, }, { name: "filename with multiple dots ending in .md", - audioFile: "podcast.flac", episodeMD: "my.episode.v2.md", expected: HugoMode, }, { name: "filename with multiple dots not ending in .md", - audioFile: "podcast.flac", episodeMD: "my.episode.v2.md.backup", expected: StandaloneMode, }, { name: "markdown file with spaces", - audioFile: "podcast.flac", episodeMD: "my episode file.md", expected: HugoMode, }, { name: "episode md with special characters", - audioFile: "podcast.flac", episodeMD: "episode-67_final.md", expected: HugoMode, }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result := detectMode(tt.audioFile, tt.episodeMD) - - if result != tt.expected { - t.Errorf("detectMode() = %v; want %v (AudioFile=%q, EpisodeMD=%q)", - result, tt.expected, tt.audioFile, tt.episodeMD) - } - }) - } -} -// TestDetectMode_Integration tests detectMode in realistic scenarios -func TestDetectMode_Integration(t *testing.T) { - tests := []struct { - name string - audioFile string - episodeMD string - expected WorkflowMode - description string - }{ + // Realistic CLI invocations { - name: "real hugo workflow", - audioFile: "LMP67.flac", - episodeMD: "content/episodes/67.md", - expected: HugoMode, - description: "User runs: jive-encoder LMP67.flac content/episodes/67.md", + name: "real hugo workflow: LMP67.flac content/episodes/67.md", + episodeMD: "content/episodes/67.md", + expected: HugoMode, }, { - name: "real standalone workflow", - audioFile: "podcast.wav", - episodeMD: "", - expected: StandaloneMode, - description: "User runs: jive-encoder podcast.wav --title 'Ep 1' --num 1 --cover art.png", + name: "real standalone workflow: podcast.wav with flags only", + episodeMD: "", + expected: StandaloneMode, }, { - name: "common mistake: user passes non-md file in hugo mode", - audioFile: "episode.flac", - episodeMD: "episode.txt", - expected: StandaloneMode, - description: "User runs: jive-encoder episode.flac episode.txt (should be .md not .txt)", + name: "common mistake: non-md episode file falls back to standalone", + episodeMD: "episode.txt", + expected: StandaloneMode, }, { - name: "edge: .md file with uppercase extension", - audioFile: "LMP99.flac", - episodeMD: "99.MD", - expected: HugoMode, - description: "Handles .MD (uppercase) correctly for cross-platform", + name: "uppercase .MD extension for cross-platform", + episodeMD: "99.MD", + expected: HugoMode, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - result := detectMode(tt.audioFile, tt.episodeMD) + result := detectMode(tt.episodeMD) if result != tt.expected { - t.Errorf("detectMode() = %v; want %v\n Description: %s\n AudioFile=%q, EpisodeMD=%q", - result, tt.expected, tt.description, tt.audioFile, tt.episodeMD) + t.Errorf("detectMode() = %v; want %v (EpisodeMD=%q)", + result, tt.expected, tt.episodeMD) } }) } diff --git a/cmd/jive-encoder/standalone.go b/cmd/jive-encoder/standalone.go index e25c201..60a073f 100644 --- a/cmd/jive-encoder/standalone.go +++ b/cmd/jive-encoder/standalone.go @@ -5,7 +5,6 @@ import ( "os" "github.com/linuxmatters/jive-encoder/internal/encoder" - "github.com/linuxmatters/jive-encoder/internal/id3" ) // StandaloneWorkflow implements the Workflow interface for standalone mode. @@ -40,11 +39,11 @@ func (s *StandaloneWorkflow) Validate() error { return nil } -// CollectMetadata builds TagInfo from CLI flags. -func (s *StandaloneWorkflow) CollectMetadata() (id3.TagInfo, string, error) { +// CollectMetadata builds the episode tag metadata from CLI flags. +func (s *StandaloneWorkflow) CollectMetadata() (encoder.Metadata, string, error) { album := resolveAlbum(s.opts.Album, s.opts.Artist) - tagInfo := id3.TagInfo{ + tags := encoder.Metadata{ EpisodeNumber: s.opts.Num, Title: s.opts.Title, Artist: s.opts.Artist, @@ -53,7 +52,7 @@ func (s *StandaloneWorkflow) CollectMetadata() (id3.TagInfo, string, error) { Comment: s.opts.Comment, } - return tagInfo, s.opts.Cover, nil + return tags, s.opts.Cover, nil } // PostEncode displays podcast statistics. Standalone mode has no frontmatter to update. diff --git a/cmd/jive-encoder/standalone_test.go b/cmd/jive-encoder/standalone_test.go index 72767f3..6945364 100644 --- a/cmd/jive-encoder/standalone_test.go +++ b/cmd/jive-encoder/standalone_test.go @@ -1,6 +1,8 @@ package main import ( + "os" + "path/filepath" "strings" "testing" ) @@ -184,11 +186,12 @@ func TestStandaloneWorkflowValidate(t *testing.T) { // Cover art path variations { - name: "cover as url", - title: "Episode", - num: "1", - cover: "https://example.com/cover.png", - wantErr: false, + name: "cover as url", + title: "Episode", + num: "1", + cover: "https://example.com/cover.png", + wantErr: true, + errMatch: "cover art not accessible", }, { name: "cover as relative path", @@ -215,20 +218,22 @@ func TestStandaloneWorkflowValidate(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - // Construct the workflow with the test inputs threaded through - // CLIOptions. A dummy audio file keeps file-existence checks from - // masking the argument validation errors we are testing for. + cover := tt.cover + if !tt.wantErr { + cover = existingCoverArgument(t, tt.cover) + } + wf := &StandaloneWorkflow{opts: CLIOptions{ Title: tt.title, Num: tt.num, - Cover: tt.cover, + Cover: cover, }} err := wf.Validate() if tt.wantErr { if err == nil { t.Errorf("StandaloneWorkflow.Validate() expected error, got nil\n Title=%q, Num=%q, Cover=%q", - tt.title, tt.num, tt.cover) + tt.title, tt.num, cover) return } if tt.errMatch != "" && !strings.Contains(err.Error(), tt.errMatch) { @@ -237,11 +242,9 @@ func TestStandaloneWorkflowValidate(t *testing.T) { return } - // For valid cases we only check argument validation, not file existence. - // File-not-found errors are acceptable here since the cover files do not exist on disk. - if err != nil && !strings.Contains(err.Error(), "not found") && !strings.Contains(err.Error(), "not accessible") { + if err != nil { t.Errorf("StandaloneWorkflow.Validate() unexpected error: %v\n Title=%q, Num=%q, Cover=%q", - err, tt.title, tt.num, tt.cover) + err, tt.title, tt.num, cover) } }) } @@ -325,24 +328,55 @@ func TestStandaloneWorkflowValidate_Integration(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - // Construct the workflow with the test inputs threaded through - // CLIOptions. A dummy audio file keeps file-existence checks from - // masking the argument validation errors we are testing for. + cover := tt.cover + if !tt.wantErr { + cover = existingCoverArgument(t, tt.cover) + } + wf := &StandaloneWorkflow{opts: CLIOptions{ Title: tt.title, Num: tt.num, - Cover: tt.cover, + Cover: cover, }} err := wf.Validate() if tt.wantErr && err == nil { t.Errorf("StandaloneWorkflow.Validate() expected error but got nil\n Description: %s\n Title=%q, Num=%q, Cover=%q", - tt.description, tt.title, tt.num, tt.cover) + tt.description, tt.title, tt.num, cover) } - if !tt.wantErr && err != nil && !strings.Contains(err.Error(), "not found") && !strings.Contains(err.Error(), "not accessible") { + if !tt.wantErr && err != nil { t.Errorf("StandaloneWorkflow.Validate() unexpected error: %v\n Description: %s\n Title=%q, Num=%q, Cover=%q", - err, tt.description, tt.title, tt.num, tt.cover) + err, tt.description, tt.title, tt.num, cover) } }) } } + +func existingCoverArgument(t *testing.T, path string) string { + t.Helper() + + root := t.TempDir() + workDir := filepath.Join(root, "work") + if err := os.MkdirAll(workDir, 0o755); err != nil { + t.Fatalf("create test work dir: %v", err) + } + t.Chdir(workDir) + + arg := path + if filepath.IsAbs(path) { + arg = filepath.Join(root, strings.TrimPrefix(path, string(filepath.Separator))) + } + + target := arg + if !filepath.IsAbs(target) { + target = filepath.Join(workDir, target) + } + if err := os.MkdirAll(filepath.Dir(target), 0o755); err != nil { + t.Fatalf("create cover fixture dir: %v", err) + } + if err := os.WriteFile(target, []byte("cover"), 0o644); err != nil { + t.Fatalf("create cover fixture: %v", err) + } + + return arg +} diff --git a/cmd/jive-encoder/workflow.go b/cmd/jive-encoder/workflow.go index a2f563e..ad8b8ef 100644 --- a/cmd/jive-encoder/workflow.go +++ b/cmd/jive-encoder/workflow.go @@ -5,7 +5,6 @@ import ( "github.com/linuxmatters/jive-encoder/internal/cli" "github.com/linuxmatters/jive-encoder/internal/encoder" - "github.com/linuxmatters/jive-encoder/internal/id3" ) // Workflow defines the mode-specific operations for Hugo and Standalone workflows. @@ -14,10 +13,10 @@ type Workflow interface { // Validate checks mode-specific arguments and file existence. Validate() error - // CollectMetadata gathers ID3 tag info and cover art path for the current mode. - // The cover art path is returned separately because it feeds the concurrent - // cover art goroutine, not TagInfo directly. - CollectMetadata() (id3.TagInfo, string, error) + // CollectMetadata gathers episode tag metadata and the cover art path for the + // current mode. The cover art path is returned separately because it feeds + // artwork.ScaleCoverArt, not the metadata tags directly. + CollectMetadata() (encoder.Metadata, string, error) // PostEncode handles post-encoding operations: stats display and, // in Hugo mode, frontmatter comparison and update prompting. diff --git a/flake.nix b/flake.nix index c0282c9..2ef5550 100644 --- a/flake.nix +++ b/flake.nix @@ -1,5 +1,5 @@ { - description = "MP3 encoder for linuxmatters.sh"; + description = "Podcast encoder (MP3, AAC, Opus) for linuxmatters.sh"; inputs = { nixpkgs.url = "github:nixos/nixpkgs/nixos-unstable"; diff --git a/go.mod b/go.mod index 8afdef0..a0e8c39 100644 --- a/go.mod +++ b/go.mod @@ -7,6 +7,7 @@ require ( charm.land/bubbletea/v2 v2.0.7 charm.land/lipgloss/v2 v2.0.4 github.com/alecthomas/kong v1.15.0 + github.com/charmbracelet/colorprofile v0.4.3 github.com/charmbracelet/harmonica v0.2.0 github.com/charmbracelet/x/term v0.2.2 github.com/linuxmatters/ffmpeg-statigo v0.0.0-00010101000000-000000000000 @@ -15,7 +16,6 @@ require ( ) require ( - github.com/charmbracelet/colorprofile v0.4.3 // indirect github.com/charmbracelet/ultraviolet v0.0.0-20260525132238-948f4557a654 // indirect github.com/charmbracelet/x/ansi v0.11.7 // indirect github.com/charmbracelet/x/termios v0.1.1 // indirect diff --git a/internal/id3/artwork.go b/internal/artwork/artwork.go similarity index 68% rename from internal/id3/artwork.go rename to internal/artwork/artwork.go index ff85645..40271ee 100644 --- a/internal/id3/artwork.go +++ b/internal/artwork/artwork.go @@ -1,9 +1,14 @@ -package id3 +// Package artwork scales podcast cover art to Apple Podcasts specifications. +// The encoder embeds the scaled bytes as an attached-picture stream for +// cover-capable formats. +package artwork import ( "bytes" "fmt" "image" + _ "image/gif" // register GIF decoder for image.Decode + _ "image/jpeg" // register JPEG decoder for image.Decode "image/png" "os" @@ -12,12 +17,12 @@ import ( // ScaleCoverArt scales cover art according to Apple Podcasts specifications: // - Images < 1400x1400: upscale to 1400x1400 -// - Images 1400x1400 to 3000x3000: use as-is (no scaling artifacts) +// - Images 1400x1400 to 3000x3000: use as-is (no scaling artefacts) // - Images > 3000x3000: downscale to 3000x3000 // -// To avoid needless recompression it returns the original PNG bytes untouched -// when no scaling is required, and only re-encodes scaled images or non-PNG -// inputs. +// It accepts PNG, JPEG, and GIF input. To avoid needless recompression an +// in-spec PNG returns its original bytes untouched; scaled images and non-PNG +// inputs re-encode to PNG. func ScaleCoverArt(inputPath string) ([]byte, error) { data, err := os.ReadFile(inputPath) if err != nil { @@ -55,23 +60,20 @@ func ScaleCoverArt(inputPath string) ([]byte, error) { return data, nil } - var finalImg image.Image + src := img if needsScaling { dst := image.NewRGBA(image.Rect(0, 0, targetSize, targetSize)) // Bilinear matches the scaler used by Jivefire thumbnail generation. draw.BiLinear.Scale(dst, dst.Bounds(), img, img.Bounds(), draw.Over, nil) - finalImg = dst - } else { - // Reaches here only for an in-spec non-PNG, re-encoded below. - finalImg = img + src = dst } - // Normalise every re-encoded path to PNG for a consistent APIC payload. + // Normalise every re-encoded path to PNG for a consistent attached-picture payload. var buf bytes.Buffer - err = png.Encode(&buf, finalImg) + err = png.Encode(&buf, src) if err != nil { return nil, fmt.Errorf("failed to encode scaled image: %w", err) } diff --git a/internal/id3/artwork_test.go b/internal/artwork/artwork_test.go similarity index 82% rename from internal/id3/artwork_test.go rename to internal/artwork/artwork_test.go index 070a0e6..0bd7824 100644 --- a/internal/id3/artwork_test.go +++ b/internal/artwork/artwork_test.go @@ -1,16 +1,22 @@ -package id3 +package artwork import ( "bytes" "image" "image/color" + "image/jpeg" "image/png" + "io" "os" "path/filepath" + "strconv" "strings" "testing" ) +// pngMagic is the eight-byte PNG file signature. +var pngMagic = []byte("\x89PNG\r\n\x1a\n") + // TestScaleCoverArt_ValidSquareImage tests scaling of valid square images func TestScaleCoverArt_ValidSquareImage(t *testing.T) { tests := []struct { @@ -288,8 +294,8 @@ func TestScaleCoverArt_OutputIsPNG(t *testing.T) { } } -// TestScaleCoverArt_SkipPNGReencoding tests that PNG images in acceptable range -// are not re-encoded, preserving the original PNG data for performance +// TestScaleCoverArt_SkipPNGReencoding tests that an in-spec PNG passes through +// with its original bytes untouched, avoiding a needless re-encode func TestScaleCoverArt_SkipPNGReencoding(t *testing.T) { tmpDir := t.TempDir() testImagePath := filepath.Join(tmpDir, "test_3000.png") @@ -299,11 +305,10 @@ func TestScaleCoverArt_SkipPNGReencoding(t *testing.T) { t.Fatalf("Failed to create test PNG: %v", err) } - originalInfo, err := os.Stat(testImagePath) + originalData, err := os.ReadFile(testImagePath) if err != nil { - t.Fatalf("Failed to stat original file: %v", err) + t.Fatalf("Failed to read original file: %v", err) } - originalSize := originalInfo.Size() scaledData, err := ScaleCoverArt(testImagePath) if err != nil { @@ -314,26 +319,60 @@ func TestScaleCoverArt_SkipPNGReencoding(t *testing.T) { t.Fatal("ScaleCoverArt returned nil data") } - scaledSize := int64(len(scaledData)) + // An in-spec PNG must return the original bytes byte-for-byte + if !bytes.Equal(scaledData, originalData) { + t.Errorf("Expected original PNG bytes to pass through untouched: got %d bytes, original %d bytes", len(scaledData), len(originalData)) + } +} + +// TestScaleCoverArt_InSpecJPEG tests that an in-spec JPEG re-encodes to PNG +// even though no scaling is needed +func TestScaleCoverArt_InSpecJPEG(t *testing.T) { + tmpDir := t.TempDir() + testImagePath := filepath.Join(tmpDir, "test.jpg") + + if err := createTestJPEG(testImagePath, 1500, 1500); err != nil { + t.Fatalf("Failed to create test JPEG: %v", err) + } + + scaledData, err := ScaleCoverArt(testImagePath) + if err != nil { + t.Fatalf("ScaleCoverArt failed: %v", err) + } + + // The non-PNG path must re-encode to PNG + if !bytes.HasPrefix(scaledData, pngMagic) { + t.Errorf("Expected output to start with PNG magic header, got % x", scaledData[:min(len(scaledData), 8)]) + } +} + +// TestScaleCoverArt_OutOfSpecJPEG tests that an undersized JPEG scales up +// and re-encodes to PNG +func TestScaleCoverArt_OutOfSpecJPEG(t *testing.T) { + tmpDir := t.TempDir() + testImagePath := filepath.Join(tmpDir, "test.jpg") + + if err := createTestJPEG(testImagePath, 800, 800); err != nil { + t.Fatalf("Failed to create test JPEG: %v", err) + } + + scaledData, err := ScaleCoverArt(testImagePath) + if err != nil { + t.Fatalf("ScaleCoverArt failed: %v", err) + } - // For PNG images in acceptable range with no scaling, the output should - // be very close in size to the original (may not be identical due to - // re-encoding, but should preserve the PNG efficiently) - // We allow up to 10% deviation to account for PNG compression variations - maxDeviation := originalSize / 10 - if scaledSize > originalSize+maxDeviation || scaledSize < originalSize-maxDeviation { - t.Logf("Warning: output size %d differs from original %d by more than 10%%", scaledSize, originalSize) + if !bytes.HasPrefix(scaledData, pngMagic) { + t.Errorf("Expected output to start with PNG magic header, got % x", scaledData[:min(len(scaledData), 8)]) } - // Verify output is valid PNG with correct dimensions decodedImg, err := png.Decode(bytes.NewReader(scaledData)) if err != nil { t.Fatalf("Failed to decode scaled image: %v", err) } bounds := decodedImg.Bounds() - if bounds.Dx() != 3000 || bounds.Dy() != 3000 { - t.Errorf("Expected 3000x3000, got %dx%d", bounds.Dx(), bounds.Dy()) + if bounds.Dx() != 1400 || bounds.Dy() != 1400 { + t.Errorf("Expected 1400x1400, got %dx%d", bounds.Dx(), bounds.Dy()) } } @@ -443,7 +482,7 @@ func TestScaleCoverArt_MultipleScalings(t *testing.T) { } for i, tt := range sizes { - testImagePath := filepath.Join(tmpDir, "test_"+string(rune(48+i))+".png") + testImagePath := filepath.Join(tmpDir, "test_"+strconv.Itoa(i)+".png") if err := createTestPNG(testImagePath, tt.inputSize, tt.inputSize); err != nil { t.Fatalf("Failed to create test PNG: %v", err) @@ -470,6 +509,17 @@ func TestScaleCoverArt_MultipleScalings(t *testing.T) { // Helper function to create test PNG images func createTestPNG(path string, width, height int) error { + return createTestImage(path, width, height, png.Encode) +} + +// Helper function to create test JPEG images +func createTestJPEG(path string, width, height int) error { + return createTestImage(path, width, height, func(w io.Writer, img image.Image) error { + return jpeg.Encode(w, img, &jpeg.Options{Quality: 90}) + }) +} + +func createTestImage(path string, width, height int, encode func(io.Writer, image.Image) error) error { img := image.NewRGBA(image.Rect(0, 0, width, height)) // Fill with a gradient pattern for visual distinctiveness @@ -488,5 +538,5 @@ func createTestPNG(path string, width, height int) error { } defer file.Close() - return png.Encode(file, img) + return encode(file, img) } diff --git a/internal/cli/help.go b/internal/cli/help.go index 86628a6..ca7089f 100644 --- a/internal/cli/help.go +++ b/internal/cli/help.go @@ -37,80 +37,82 @@ var ( Italic(true) ) -// StyledHelpPrinter creates a custom help printer with Lipgloss styling -func StyledHelpPrinter(options kong.HelpOptions) kong.HelpPrinter { - return kong.HelpPrinter(func(options kong.HelpOptions, ctx *kong.Context) error { - var sb strings.Builder - - // Title and description - sb.WriteString(TitleStyle.Render("Jive Encoder 🪩")) - sb.WriteString("\n") - sb.WriteString(helpDescStyle.Render("Drop your podcast .wav into a shiny MP3, AAC, or Opus with metadata, cover art, and all.")) +// StyledHelpPrinter is a kong.HelpPrinter that builds the help text with +// Lipgloss styling, then writes it through a colour-profile writer so colour +// degrades for non-TTY output. +func StyledHelpPrinter(options kong.HelpOptions, ctx *kong.Context) error { + var sb strings.Builder + + // Title and description + sb.WriteString(TitleStyle.Render(AppTitle)) + sb.WriteString("\n") + sb.WriteString(helpDescStyle.Render(ctx.Model.Help)) + sb.WriteString("\n") + + // Usage + sb.WriteString(helpSectionStyle.Render("Usage:")) + sb.WriteString("\n ") + sb.WriteString(helpModeStyle.Render("Hugo mode:")) + sb.WriteString("\n ") + fmt.Fprintf(&sb, "%s [flags]", ctx.Model.Name) + sb.WriteString("\n ") + sb.WriteString(helpModeStyle.Render("Standalone mode:")) + sb.WriteString("\n ") + fmt.Fprintf(&sb, "%s --title TEXT --num NUMBER --cover PATH [flags]", ctx.Model.Name) + sb.WriteString("\n") + + // Arguments section + args := getArguments(ctx) + if len(args) > 0 { sb.WriteString("\n") - - // Usage - sb.WriteString(helpSectionStyle.Render("Usage:")) - sb.WriteString("\n ") - sb.WriteString(helpModeStyle.Render("Hugo mode:")) - sb.WriteString("\n ") - fmt.Fprintf(&sb, "%s [flags]", ctx.Model.Name) - sb.WriteString("\n ") - sb.WriteString(helpModeStyle.Render("Standalone mode:")) - sb.WriteString("\n ") - fmt.Fprintf(&sb, "%s --title TEXT --num NUMBER --cover PATH [flags]", ctx.Model.Name) + sb.WriteString(helpSectionStyle.Render("Arguments:")) sb.WriteString("\n") - - // Arguments section - args := getArguments(ctx) - if len(args) > 0 { - sb.WriteString("\n") - sb.WriteString(helpSectionStyle.Render("Arguments:")) - sb.WriteString("\n") - for _, arg := range args { + for _, arg := range args { + sb.WriteString(" ") + sb.WriteString(helpArgStyle.Render(arg.name)) + if arg.help != "" { sb.WriteString(" ") - sb.WriteString(helpArgStyle.Render(arg.name)) - if arg.help != "" { - sb.WriteString(" ") - sb.WriteString(arg.help) - } - sb.WriteString("\n") + sb.WriteString(arg.help) } + sb.WriteString("\n") } + } - // Flags section - flags := getFlags(ctx) - if len(flags) > 0 { - sb.WriteString("\n") - sb.WriteString(helpSectionStyle.Render("Flags:")) - sb.WriteString("\n") - for _, flag := range flags { + // Flags section + flags := getFlags(ctx) + if len(flags) > 0 { + sb.WriteString("\n") + sb.WriteString(helpSectionStyle.Render("Flags:")) + sb.WriteString("\n") + for _, flag := range flags { + sb.WriteString(" ") + sb.WriteString(helpFlagStyle.Render(flag.flags)) + if flag.help != "" { sb.WriteString(" ") - sb.WriteString(helpFlagStyle.Render(flag.flags)) - if flag.help != "" { - sb.WriteString(" ") - sb.WriteString(flag.help) - } - if flag.defaultVal != "" { - sb.WriteString(" ") - sb.WriteString(helpDefaultStyle.Render("(default: " + flag.defaultVal + ")")) - } - sb.WriteString("\n") + sb.WriteString(flag.help) + } + if flag.defaultVal != "" { + sb.WriteString(" ") + sb.WriteString(helpDefaultStyle.Render("(default: " + flag.defaultVal + ")")) } + sb.WriteString("\n") } + } - sb.WriteString("\n") - fmt.Fprint(ctx.Stdout, sb.String()) - return nil - }) + sb.WriteString("\n") + // Degrade colour for non-TTY output, honouring NO_COLOR and TERM + w := newColourWriter(ctx.Stdout) + fmt.Fprint(w, sb.String()) + return nil } -// Argument represents a CLI argument +// argument represents a CLI argument type argument struct { name string help string } -// Flag represents a CLI flag +// flag represents a CLI flag type flag struct { flags string help string @@ -156,10 +158,15 @@ func getFlags(ctx *kong.Context) []flag { flagStr += "=" + strings.ToUpper(f.PlaceHolder) } + var defaultVal string + if f.HasDefault { + defaultVal = f.Default + } + flags = append(flags, flag{ flags: flagStr, help: f.Help, - defaultVal: f.FormatPlaceHolder(), + defaultVal: defaultVal, }) } diff --git a/internal/cli/help_test.go b/internal/cli/help_test.go new file mode 100644 index 0000000..cd2e836 --- /dev/null +++ b/internal/cli/help_test.go @@ -0,0 +1,95 @@ +package cli + +import ( + "bytes" + "testing" + + "github.com/alecthomas/kong" +) + +// helpFixture is a minimal CLI covering the flag shapes getFlags must handle. +type helpFixture struct { + Format string `help:"Output format." default:"mp3"` + Title string `help:"Episode title."` + Stereo bool `help:"Encode in stereo."` + Number int `short:"n" help:"Episode number."` +} + +// newFixtureContext builds a kong context that neither exits nor prints. +func newFixtureContext(t *testing.T) *kong.Context { + t.Helper() + + var fixture helpFixture + parser, err := kong.New(&fixture, + kong.Exit(func(int) {}), + kong.Writers(&bytes.Buffer{}, &bytes.Buffer{}), + ) + if err != nil { + t.Fatalf("kong.New() error = %v", err) + } + + ctx, err := parser.Parse([]string{}) + if err != nil { + t.Fatalf("parser.Parse() error = %v", err) + } + return ctx +} + +// findFlag returns the extracted flag whose rendered form matches exactly. +func findFlag(t *testing.T, flags []flag, rendered string) flag { + t.Helper() + for _, f := range flags { + if f.flags == rendered { + return f + } + } + t.Fatalf("flag %q not found in %+v", rendered, flags) + return flag{} +} + +func TestGetFlags(t *testing.T) { + flags := getFlags(newFixtureContext(t)) + + tests := []struct { + name string + rendered string + help string + defaultVal string + }{ + {"help flag always present", "-h, --help", "Show context-sensitive help.", ""}, + {"flag with default tag", "--format", "Output format.", "mp3"}, + // A defaultless flag must render no default clause. This guards + // against reintroducing the placeholder-formatting helper, which + // printed bogus values like "(default: STRING)". + {"flag without default", "--title", "Episode title.", ""}, + {"bool flag has no default", "--stereo", "Encode in stereo.", ""}, + {"short name captured", "-n, --number", "Episode number.", ""}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + f := findFlag(t, flags, tt.rendered) + if f.help != tt.help { + t.Errorf("help = %q; want %q", f.help, tt.help) + } + if f.defaultVal != tt.defaultVal { + t.Errorf("defaultVal = %q; want %q", f.defaultVal, tt.defaultVal) + } + }) + } + + // The help flag leads the list; the fixture contributes the other four. + if flags[0].flags != "-h, --help" { + t.Errorf("flags[0] = %q; want %q", flags[0].flags, "-h, --help") + } + if len(flags) != 5 { + t.Errorf("len(flags) = %d; want 5", len(flags)) + } +} + +func TestGetArguments(t *testing.T) { + // The fixture has no positional arguments, so none are extracted. + if args := getArguments(newFixtureContext(t)); len(args) != 0 { + t.Errorf("getArguments() = %+v; want none", args) + } +} diff --git a/internal/cli/styles.go b/internal/cli/styles.go index da6b54f..9ccdf8b 100644 --- a/internal/cli/styles.go +++ b/internal/cli/styles.go @@ -2,11 +2,27 @@ package cli import ( "fmt" + "io" "os" "charm.land/lipgloss/v2" + "github.com/charmbracelet/colorprofile" ) +// AppTitle is the styled application title shown in version and help output +const AppTitle = "Jive Encoder 🪩" + +var ( + // Colour-profile-aware writers: degrade colour for non-TTY output, + // honouring NO_COLOR and TERM + stdout = newColourWriter(os.Stdout) + stderr = newColourWriter(os.Stderr) +) + +func newColourWriter(w io.Writer) *colorprofile.Writer { + return colorprofile.NewWriter(w, os.Environ()) +} + var ( // Title style - bold blue with disco ball emoji TitleStyle = lipgloss.NewStyle(). @@ -53,38 +69,38 @@ var ( // PrintVersion prints version information func PrintVersion(version string) { - fmt.Println(TitleStyle.Render("Jive Encoder 🪩")) - fmt.Printf("%s %s\n", KeyStyle.Render("Version:"), ValueStyle.Render(version)) - fmt.Println() + fmt.Fprintln(stdout, TitleStyle.Render(AppTitle)) + fmt.Fprintf(stdout, "%s %s\n", KeyStyle.Render("Version:"), ValueStyle.Render(version)) + fmt.Fprintln(stdout) } // PrintError prints an error message func PrintError(message string) { - fmt.Fprintf(os.Stderr, "%s %s\n", ErrorStyle.Render("Error:"), message) + fmt.Fprintf(stderr, "%s %s\n", ErrorStyle.Render("Error:"), message) } // PrintWarning prints a warning message func PrintWarning(message string) { - fmt.Printf("%s %s\n", WarningStyle.Render("Warning:"), message) + fmt.Fprintf(stderr, "%s %s\n", WarningStyle.Render("Warning:"), message) } // PrintSuccess prints a success message func PrintSuccess(message string) { - fmt.Printf("%s %s\n", SuccessStyle.Render("✓"), message) + fmt.Fprintf(stdout, "%s %s\n", SuccessStyle.Render("✓"), message) } // PrintInfo prints an informational message func PrintInfo(message string) { - fmt.Printf("%s %s\n", KeyStyle.Render("•"), message) + fmt.Fprintf(stdout, "%s %s\n", KeyStyle.Render("•"), message) } // PrintLabelValue prints a label with muted style and a value // Used for summary output like "Episode: 67 - Title" func PrintLabelValue(label, value string) { - fmt.Printf("%s %s\n", KeyStyle.Render(label), value) + fmt.Fprintf(stdout, "%s %s\n", KeyStyle.Render(label), value) } // PrintSuccessLabel prints a success checkmark with a muted label and value func PrintSuccessLabel(label, value string) { - fmt.Printf("%s %s %s\n", SuccessStyle.Render("\u2713"), KeyStyle.Render(label), value) + fmt.Fprintf(stdout, "%s %s %s\n", SuccessStyle.Render("✓"), KeyStyle.Render(label), value) } diff --git a/internal/encoder/cover.go b/internal/encoder/cover.go new file mode 100644 index 0000000..7d21d42 --- /dev/null +++ b/internal/encoder/cover.go @@ -0,0 +1,67 @@ +package encoder + +import ( + "bytes" + "fmt" + "image/png" + "unsafe" + + "github.com/linuxmatters/ffmpeg-statigo" +) + +// addCoverStream creates the attached-picture stream that carries the scaled +// PNG cover. It is added after the audio stream, so the audio stream keeps +// index 0. The packet itself is written after AVFormatWriteHeader by +// writeCoverPacket. +func (e *Encoder) addCoverStream() error { + coverStream := ffmpeg.AVFormatNewStream(e.output.format, nil) + if coverStream == nil { + return fmt.Errorf("failed to create cover stream") + } + + // The mp3 and ipod muxers reject an attached-picture stream without + // dimensions, so read them from the PNG header. + cfg, err := png.DecodeConfig(bytes.NewReader(e.coverArt)) + if err != nil { + return fmt.Errorf("failed to read cover dimensions: %w", err) + } + + codecPar := coverStream.Codecpar() + codecPar.SetCodecType(ffmpeg.AVMediaTypeVideo) + // ScaleCoverArt always emits PNG, so the picture stream uses the PNG codec. + codecPar.SetCodecId(ffmpeg.AVCodecIdPng) + codecPar.SetWidth(cfg.Width) + codecPar.SetHeight(cfg.Height) + coverStream.SetDisposition(ffmpeg.AVDispositionAttachedPic) + + e.output.coverStreamIndex = coverStream.Index() + return nil +} + +// writeCoverPacket allocates a packet sized to the cover bytes, copies the PNG +// data into it, marks it a keyframe on the attached-picture stream, and writes +// it to the muxer. The packet is freed before returning, so Close never touches +// it. +func (e *Encoder) writeCoverPacket() error { + pkt := ffmpeg.AVPacketAlloc() + if pkt == nil { + return fmt.Errorf("failed to allocate cover packet") + } + defer ffmpeg.AVPacketFree(&pkt) + + if _, err := ffmpeg.AVNewPacket(pkt, len(e.coverArt)); err != nil { + return fmt.Errorf("failed to allocate cover packet data: %w", err) + } + + dst := unsafe.Slice((*byte)(pkt.Data()), len(e.coverArt)) + copy(dst, e.coverArt) + + pkt.SetStreamIndex(e.output.coverStreamIndex) + pkt.SetFlags(pkt.Flags() | ffmpeg.AVPktFlagKey) + + if _, err := ffmpeg.AVInterleavedWriteFrame(e.output.format, pkt); err != nil { + return fmt.Errorf("failed to write cover packet: %w", err) + } + + return nil +} diff --git a/internal/encoder/encoder.go b/internal/encoder/encoder.go index 1c950d9..f743b0d 100644 --- a/internal/encoder/encoder.go +++ b/internal/encoder/encoder.go @@ -1,13 +1,11 @@ package encoder import ( - "bytes" "errors" "fmt" - "image/png" + "path/filepath" "strings" "sync/atomic" - "unsafe" "github.com/linuxmatters/ffmpeg-statigo" ) @@ -22,47 +20,60 @@ const ( StereoBitrate = 192000 ) -// Encoder handles MP3 encoding from audio input files +// Encoder handles audio encoding (MP3, AAC, Opus) from audio input files type Encoder struct { inputPath string outputPath string stereo bool - ifmtCtx *ffmpeg.AVFormatContext - ofmtCtx *ffmpeg.AVFormatContext - - decCtx *ffmpeg.AVCodecContext - encCtx *ffmpeg.AVCodecContext - - decFrame *ffmpeg.AVFrame - encPkt *ffmpeg.AVPacket - - filterGraph *ffmpeg.AVFilterGraph - bufferSrcCtx *ffmpeg.AVFilterContext - bufferSinkCtx *ffmpeg.AVFilterContext - filteredFrame *ffmpeg.AVFrame + input inputState + output outputState + filter filterState + pipeline pipelineState preset formatPreset metadata Metadata coverArt []byte // scaled PNG cover bytes; empty disables the attached-picture stream - streamIndex int - outStreamIndex int // OUTPUT audio stream index, distinct from input streamIndex - coverStreamIndex int // attached-picture stream index, -1 when no cover stream - samplesRead int64 - totalSamples int64 - nextPts int64 // Track PTS for output frames - closed bool // Track if Close() has been called to prevent double-free + closed bool // Track if Close() has been called to prevent double-free // cancelled is set by Cancel and observed at the top of the decode loop so // Encode unwinds the cgo call chain before any Close frees the AV contexts. cancelled atomic.Bool } +type inputState struct { + format *ffmpeg.AVFormatContext + codec *ffmpeg.AVCodecContext + streamIndex int + totalSamples int64 +} + +type outputState struct { + format *ffmpeg.AVFormatContext + codec *ffmpeg.AVCodecContext + audioStreamIndex int + coverStreamIndex int +} + +type filterState struct { + graph *ffmpeg.AVFilterGraph + src *ffmpeg.AVFilterContext + sink *ffmpeg.AVFilterContext + frame *ffmpeg.AVFrame +} + +type pipelineState struct { + decoded *ffmpeg.AVFrame + packet *ffmpeg.AVPacket + samplesRead int64 + nextPts int64 +} + // Metadata carries episode tag fields into the encoder so it can write -// muxer-native metadata during Initialize/Encode. It mirrors the text fields of -// the id3 tag set but lives in the encoder package to avoid an encoder->id3 -// import cycle (id3 is being removed). +// muxer-native metadata during Initialize/Encode. It is the single tag-field +// carrier: the CLI workflows build it and the encoder maps its fields to +// standard muxer tag keys. type Metadata struct { EpisodeNumber string Title string @@ -99,17 +110,24 @@ func New(cfg Config) (*Encoder, error) { if !ok { return nil, fmt.Errorf("unknown output format: %q", format) } + if outputExt := filepath.Ext(cfg.OutputPath); !strings.EqualFold(outputExt, preset.extension) { + return nil, fmt.Errorf("output path extension %q does not match %s format extension %q", outputExt, preset.name, preset.extension) + } return &Encoder{ - inputPath: cfg.InputPath, - outputPath: cfg.OutputPath, - stereo: cfg.Stereo, - preset: preset, - metadata: cfg.Metadata, - coverArt: cfg.CoverArt, - streamIndex: -1, - outStreamIndex: -1, - coverStreamIndex: -1, + inputPath: cfg.InputPath, + outputPath: cfg.OutputPath, + stereo: cfg.Stereo, + preset: preset, + metadata: cfg.Metadata, + coverArt: cfg.CoverArt, + input: inputState{ + streamIndex: -1, + }, + output: outputState{ + audioStreamIndex: -1, + coverStreamIndex: -1, + }, }, nil } @@ -127,9 +145,9 @@ func (e *Encoder) Initialize() error { return fmt.Errorf("failed to open output: %w", err) } - e.decFrame = ffmpeg.AVFrameAlloc() - e.filteredFrame = ffmpeg.AVFrameAlloc() - e.encPkt = ffmpeg.AVPacketAlloc() + e.pipeline.decoded = ffmpeg.AVFrameAlloc() + e.filter.frame = ffmpeg.AVFrameAlloc() + e.pipeline.packet = ffmpeg.AVPacketAlloc() if err := e.initFilter(); err != nil { return fmt.Errorf("failed to initialize filter: %w", err) @@ -138,26 +156,26 @@ func (e *Encoder) Initialize() error { return nil } -// openInput opens and analyzes the input audio file +// openInput opens and analyses the input audio file func (e *Encoder) openInput() error { urlPtr := ffmpeg.ToCStr(e.inputPath) defer urlPtr.Free() - if _, err := ffmpeg.AVFormatOpenInput(&e.ifmtCtx, urlPtr, nil, nil); err != nil { + if _, err := ffmpeg.AVFormatOpenInput(&e.input.format, urlPtr, nil, nil); err != nil { return fmt.Errorf("cannot open input file: %w", err) } - if _, err := ffmpeg.AVFormatFindStreamInfo(e.ifmtCtx, nil); err != nil { + if _, err := ffmpeg.AVFormatFindStreamInfo(e.input.format, nil); err != nil { return fmt.Errorf("cannot find stream information: %w", err) } - streamIdx, err := ffmpeg.AVFindBestStream(e.ifmtCtx, ffmpeg.AVMediaTypeAudio, -1, -1, nil, 0) + streamIdx, err := ffmpeg.AVFindBestStream(e.input.format, ffmpeg.AVMediaTypeAudio, -1, -1, nil, 0) if err != nil { return fmt.Errorf("cannot find audio stream: %w", err) } - e.streamIndex = streamIdx + e.input.streamIndex = streamIdx - stream := e.ifmtCtx.Streams().Get(uintptr(e.streamIndex)) //nolint:gosec // streamIndex is validated by AVFindBestStream + stream := e.input.format.Streams().Get(uintptr(e.input.streamIndex)) //nolint:gosec // streamIndex is validated by AVFindBestStream codecPar := stream.Codecpar() decoder := ffmpeg.AVCodecFindDecoder(codecPar.CodecId()) @@ -165,16 +183,16 @@ func (e *Encoder) openInput() error { return fmt.Errorf("decoder not found for codec %d", codecPar.CodecId()) } - e.decCtx = ffmpeg.AVCodecAllocContext3(decoder) - if e.decCtx == nil { + e.input.codec = ffmpeg.AVCodecAllocContext3(decoder) + if e.input.codec == nil { return fmt.Errorf("failed to allocate decoder context") } - if _, err := ffmpeg.AVCodecParametersToContext(e.decCtx, codecPar); err != nil { + if _, err := ffmpeg.AVCodecParametersToContext(e.input.codec, codecPar); err != nil { return fmt.Errorf("failed to copy codec parameters: %w", err) } - if _, err := ffmpeg.AVCodecOpen2(e.decCtx, decoder, nil); err != nil { + if _, err := ffmpeg.AVCodecOpen2(e.input.codec, decoder, nil); err != nil { return fmt.Errorf("failed to open decoder: %w", err) } @@ -183,327 +201,7 @@ func (e *Encoder) openInput() error { timeBase := stream.TimeBase() if duration > 0 { durationSec := float64(duration) * float64(timeBase.Num()) / float64(timeBase.Den()) - e.totalSamples = int64(durationSec * float64(e.decCtx.SampleRate())) - } - - return nil -} - -// openOutput creates the output MP3 file and sets up the encoder -func (e *Encoder) openOutput() error { - namePtr := ffmpeg.ToCStr(e.outputPath) - defer namePtr.Free() - - if _, err := ffmpeg.AVFormatAllocOutputContext2(&e.ofmtCtx, nil, nil, namePtr); err != nil { - return fmt.Errorf("failed to create output context: %w", err) - } - - var encoder *ffmpeg.AVCodec - if e.preset.encoderName != "" { - namePtr := ffmpeg.ToCStr(e.preset.encoderName) - encoder = ffmpeg.AVCodecFindEncoderByName(namePtr) - namePtr.Free() - } - if encoder == nil { - encoder = ffmpeg.AVCodecFindEncoder(e.preset.codecID) - } - if encoder == nil { - return fmt.Errorf("%s encoder not found", e.preset.name) - } - - outStream := ffmpeg.AVFormatNewStream(e.ofmtCtx, encoder) - if outStream == nil { - return fmt.Errorf("failed to create output stream") - } - e.outStreamIndex = outStream.Index() - - e.encCtx = ffmpeg.AVCodecAllocContext3(encoder) - if e.encCtx == nil { - return fmt.Errorf("failed to allocate encoder context") - } - - if e.stereo { - e.encCtx.SetBitRate(int64(e.preset.stereoBitrate)) - ffmpeg.AVChannelLayoutDefault(e.encCtx.ChLayout(), 2) - } else { - e.encCtx.SetBitRate(int64(e.preset.monoBitrate)) - ffmpeg.AVChannelLayoutDefault(e.encCtx.ChLayout(), 1) - } - - e.encCtx.SetSampleRate(e.preset.sampleRate) - e.encCtx.SetSampleFmt(e.preset.sampleFmt) - - tb := &ffmpeg.AVRational{} - tb.SetNum(1) - tb.SetDen(e.encCtx.SampleRate()) - e.encCtx.SetTimeBase(tb) - - // Encoder tuning passed through AVDictionary, driven by the preset. - var opts *ffmpeg.AVDictionary - - for key, val := range e.preset.encoderOpts { - keyPtr := ffmpeg.ToCStr(key) - valPtr := ffmpeg.ToCStr(val) - _, err := ffmpeg.AVDictSet(&opts, keyPtr, valPtr, 0) - keyPtr.Free() - valPtr.Free() - if err != nil { - ffmpeg.AVDictFree(&opts) - return fmt.Errorf("failed to set encoder option %s: %w", key, err) - } - } - - if _, err := ffmpeg.AVCodecOpen2(e.encCtx, encoder, &opts); err != nil { - ffmpeg.AVDictFree(&opts) - return fmt.Errorf("failed to open encoder: %w", err) - } - ffmpeg.AVDictFree(&opts) - - if _, err := ffmpeg.AVCodecParametersFromContext(outStream.Codecpar(), e.encCtx); err != nil { - return fmt.Errorf("failed to copy encoder parameters: %w", err) - } - - outStream.SetTimeBase(e.encCtx.TimeBase()) - - // Formats without the NOFILE flag need an explicit AVIO output handle. - if e.ofmtCtx.Oformat().Flags()&ffmpeg.AVFmtNofile == 0 { - var pb *ffmpeg.AVIOContext - if _, err := ffmpeg.AVIOOpen(&pb, e.ofmtCtx.Url(), ffmpeg.AVIOFlagWrite); err != nil { - return fmt.Errorf("failed to open output file: %w", err) - } - e.ofmtCtx.SetPb(pb) - } - - if err := e.setMuxerMetadata(); err != nil { - return err - } - - // Add the attached-picture stream after the audio stream so audio keeps - // index 0 (outStreamIndex). Opus is not cover-capable and absent cover - // bytes mean no second stream, leaving the audio-only path unchanged. - if e.preset.coverCapable && len(e.coverArt) > 0 { - if err := e.addCoverStream(); err != nil { - return err - } - } - - // id3v2_version is an mp3-muxer-private option, so it goes through the - // WriteHeader options dict, not the format-context metadata. Other muxers - // ignore it. The dict is owned here and freed after WriteHeader. - var muxerOpts *ffmpeg.AVDictionary - if e.preset.name == "mp3" { - keyPtr := ffmpeg.ToCStr("id3v2_version") - valPtr := ffmpeg.ToCStr("4") - _, err := ffmpeg.AVDictSet(&muxerOpts, keyPtr, valPtr, 0) - keyPtr.Free() - valPtr.Free() - if err != nil { - ffmpeg.AVDictFree(&muxerOpts) - return fmt.Errorf("failed to set id3v2_version option: %w", err) - } - } - - if _, err := ffmpeg.AVFormatWriteHeader(e.ofmtCtx, &muxerOpts); err != nil { - ffmpeg.AVDictFree(&muxerOpts) - return fmt.Errorf("failed to write header: %w", err) - } - ffmpeg.AVDictFree(&muxerOpts) - - // Write the cover picture immediately after the header so the muxer carries - // it as the attached picture before any audio packet. - if e.coverStreamIndex >= 0 { - if err := e.writeCoverPacket(); err != nil { - return err - } - } - - return nil -} - -// addCoverStream creates the attached-picture stream that carries the scaled -// PNG cover. It is added after the audio stream, so the audio stream keeps -// index 0. The packet itself is written after AVFormatWriteHeader by -// writeCoverPacket. -func (e *Encoder) addCoverStream() error { - coverStream := ffmpeg.AVFormatNewStream(e.ofmtCtx, nil) - if coverStream == nil { - return fmt.Errorf("failed to create cover stream") - } - - // The mp3 and ipod muxers reject an attached-picture stream without - // dimensions, so read them from the PNG header. - cfg, err := png.DecodeConfig(bytes.NewReader(e.coverArt)) - if err != nil { - return fmt.Errorf("failed to read cover dimensions: %w", err) - } - - codecPar := coverStream.Codecpar() - codecPar.SetCodecType(ffmpeg.AVMediaTypeVideo) - // ScaleCoverArt always emits PNG, so the picture stream uses the PNG codec. - codecPar.SetCodecId(ffmpeg.AVCodecIdPng) - codecPar.SetWidth(cfg.Width) - codecPar.SetHeight(cfg.Height) - coverStream.SetDisposition(ffmpeg.AVDispositionAttachedPic) - - e.coverStreamIndex = coverStream.Index() - return nil -} - -// writeCoverPacket allocates a packet sized to the cover bytes, copies the PNG -// data into it, marks it a keyframe on the attached-picture stream, and writes -// it to the muxer. The packet is freed before returning, so Close never touches -// it. -func (e *Encoder) writeCoverPacket() error { - pkt := ffmpeg.AVPacketAlloc() - if pkt == nil { - return fmt.Errorf("failed to allocate cover packet") - } - defer ffmpeg.AVPacketFree(&pkt) - - if _, err := ffmpeg.AVNewPacket(pkt, len(e.coverArt)); err != nil { - return fmt.Errorf("failed to allocate cover packet data: %w", err) - } - - dst := unsafe.Slice((*byte)(pkt.Data()), len(e.coverArt)) - copy(dst, e.coverArt) - - pkt.SetStreamIndex(e.coverStreamIndex) - pkt.SetFlags(pkt.Flags() | ffmpeg.AVPktFlagKey) - - if _, err := ffmpeg.AVInterleavedWriteFrame(e.ofmtCtx, pkt); err != nil { - return fmt.Errorf("failed to write cover packet: %w", err) - } - - return nil -} - -// setMuxerMetadata builds the standard-key tag dictionary from the episode -// metadata and hands it to the output format context. SetMetadata transfers -// ownership to the context (freed by avformat_free_context), so this dict is -// never freed here. Preset-agnostic: every format gets the same standard keys. -func (e *Encoder) setMuxerMetadata() error { - tags := buildMuxerTags(e.metadata) - if len(tags) == 0 { - return nil - } - - var dict *ffmpeg.AVDictionary - for _, tag := range tags { - keyPtr := ffmpeg.ToCStr(tag.Key) - valPtr := ffmpeg.ToCStr(tag.Value) - _, err := ffmpeg.AVDictSet(&dict, keyPtr, valPtr, 0) - keyPtr.Free() - valPtr.Free() - if err != nil { - ffmpeg.AVDictFree(&dict) - return fmt.Errorf("failed to set metadata %s: %w", tag.Key, err) - } - } - - e.ofmtCtx.SetMetadata(dict) - return nil -} - -// initFilter sets up audio filter graph for resampling and frame buffering -func (e *Encoder) initFilter() error { - e.filterGraph = ffmpeg.AVFilterGraphAlloc() - if e.filterGraph == nil { - return fmt.Errorf("failed to allocate filter graph") - } - - bufferSrc := ffmpeg.AVFilterGetByName(ffmpeg.GlobalCStr("abuffer")) - bufferSink := ffmpeg.AVFilterGetByName(ffmpeg.GlobalCStr("abuffersink")) - if bufferSrc == nil || bufferSink == nil { - return fmt.Errorf("abuffer or abuffersink filter not found") - } - - layoutPtr := ffmpeg.AllocCStr(64) - defer layoutPtr.Free() - if _, err := ffmpeg.AVChannelLayoutDescribe(e.decCtx.ChLayout(), layoutPtr, 64); err != nil { - return fmt.Errorf("failed to describe channel layout: %w", err) - } - - pktTimebase := e.decCtx.PktTimebase() - args := fmt.Sprintf( - "time_base=%d/%d:sample_rate=%d:sample_fmt=%s:channel_layout=%s", - pktTimebase.Num(), pktTimebase.Den(), - e.decCtx.SampleRate(), - ffmpeg.AVGetSampleFmtName(e.decCtx.SampleFmt()).String(), - layoutPtr.String(), - ) - - argsC := ffmpeg.ToCStr(args) - defer argsC.Free() - - if _, err := ffmpeg.AVFilterGraphCreateFilter( - &e.bufferSrcCtx, - bufferSrc, - ffmpeg.GlobalCStr("in"), - argsC, - nil, - e.filterGraph, - ); err != nil { - return fmt.Errorf("failed to create buffer source: %w", err) - } - - if _, err := ffmpeg.AVFilterGraphCreateFilter( - &e.bufferSinkCtx, - bufferSink, - ffmpeg.GlobalCStr("out"), - nil, - nil, - e.filterGraph, - ); err != nil { - return fmt.Errorf("failed to create buffer sink: %w", err) - } - - // Parse filter graph - use aresample for format/rate/channel conversion - outputs := ffmpeg.AVFilterInoutAlloc() - inputs := ffmpeg.AVFilterInoutAlloc() - defer ffmpeg.AVFilterInoutFree(&outputs) - defer ffmpeg.AVFilterInoutFree(&inputs) - - outputs.SetName(ffmpeg.ToCStr("in")) - outputs.SetFilterCtx(e.bufferSrcCtx) - outputs.SetPadIdx(0) - outputs.SetNext(nil) - - inputs.SetName(ffmpeg.ToCStr("out")) - inputs.SetFilterCtx(e.bufferSinkCtx) - inputs.SetPadIdx(0) - inputs.SetNext(nil) - - // Build filter spec: resample to the preset's sample rate and sample format, - // set the target channel layout (stereo keeps channels, mono downmixes). - // Frame sizing is applied to the buffer sink after this graph is built - // (see below), not via asetnsamples, so each encoder gets its required size. - channelLayout := "mono" - if e.stereo { - channelLayout = "stereo" - } - sampleFmtName := ffmpeg.AVGetSampleFmtName(e.preset.sampleFmt).String() - filterSpec := fmt.Sprintf("aresample=%d:async=1,aformat=sample_fmts=%s:sample_rates=%d:channel_layouts=%s", - e.preset.sampleRate, sampleFmtName, e.preset.sampleRate, channelLayout) - - filterSpecC := ffmpeg.ToCStr(filterSpec) - defer filterSpecC.Free() - - if _, err := ffmpeg.AVFilterGraphParsePtr(e.filterGraph, filterSpecC, &inputs, &outputs, nil); err != nil { - return fmt.Errorf("failed to parse filter graph: %w", err) - } - - if _, err := ffmpeg.AVFilterGraphConfig(e.filterGraph, nil); err != nil { - return fmt.Errorf("failed to configure filter graph: %w", err) - } - - // Fix the buffer-sink frame size to the encoder's required frame size so the - // filter delivers exactly the frames the encoder expects (LAME 1152, native - // AAC 1024, libopus its own). Encoders that accept variable-size frames - // advertise AV_CODEC_CAP_VARIABLE_FRAME_SIZE and need no fixed size. - // openOutput runs before initFilter, so encCtx.FrameSize() is populated here. - if frameSize := e.encCtx.FrameSize(); frameSize > 0 && - e.encCtx.Codec().Capabilities()&ffmpeg.AVCodecCapVariableFrameSize == 0 { - ffmpeg.AVBuffersinkSetFrameSize(e.bufferSinkCtx, uint(frameSize)) + e.input.totalSamples = int64(durationSec * float64(e.input.codec.SampleRate())) } return nil @@ -517,7 +215,7 @@ func (e *Encoder) Encode(progressCb ProgressCallback) error { packet := ffmpeg.AVPacketAlloc() defer ffmpeg.AVPacketFree(&packet) - outStream := e.ofmtCtx.Streams().Get(uintptr(e.outStreamIndex)) //nolint:gosec // outStreamIndex is set from AVFormatNewStream in openOutput + outStream := e.output.format.Streams().Get(uintptr(e.output.audioStreamIndex)) //nolint:gosec // audioStreamIndex is set from AVFormatNewStream in openOutput for { // Observe cancellation before the next cgo call so Encode returns while @@ -526,19 +224,19 @@ func (e *Encoder) Encode(progressCb ProgressCallback) error { return ErrCancelled } - if _, err := ffmpeg.AVReadFrame(e.ifmtCtx, packet); err != nil { + if _, err := ffmpeg.AVReadFrame(e.input.format, packet); err != nil { if errors.Is(err, ffmpeg.AVErrorEOF) { break } return fmt.Errorf("read frame failed: %w", err) } - if packet.StreamIndex() != e.streamIndex { + if packet.StreamIndex() != e.input.streamIndex { ffmpeg.AVPacketUnref(packet) continue } - if _, err := ffmpeg.AVCodecSendPacket(e.decCtx, packet); err != nil { + if _, err := ffmpeg.AVCodecSendPacket(e.input.codec, packet); err != nil { ffmpeg.AVPacketUnref(packet) return fmt.Errorf("send packet to decoder failed: %w", err) } @@ -550,19 +248,19 @@ func (e *Encoder) Encode(progressCb ProgressCallback) error { return ErrCancelled } - if _, err := ffmpeg.AVCodecReceiveFrame(e.decCtx, e.decFrame); err != nil { + if _, err := ffmpeg.AVCodecReceiveFrame(e.input.codec, e.pipeline.decoded); err != nil { if errors.Is(err, ffmpeg.EAgain) || errors.Is(err, ffmpeg.AVErrorEOF) { break } return fmt.Errorf("receive frame from decoder failed: %w", err) } - e.samplesRead += int64(e.decFrame.NbSamples()) - if progressCb != nil && e.totalSamples > 0 { - progressCb(e.samplesRead, e.totalSamples) + e.pipeline.samplesRead += int64(e.pipeline.decoded.NbSamples()) + if progressCb != nil && e.input.totalSamples > 0 { + progressCb(e.pipeline.samplesRead, e.input.totalSamples) } - if _, err := ffmpeg.AVBuffersrcAddFrameFlags(e.bufferSrcCtx, e.decFrame, ffmpeg.AVBuffersrcFlagKeepRef); err != nil { + if _, err := ffmpeg.AVBuffersrcAddFrameFlags(e.filter.src, e.pipeline.decoded, ffmpeg.AVBuffersrcFlagKeepRef); err != nil { return fmt.Errorf("failed to feed filter graph: %w", err) } @@ -570,27 +268,27 @@ func (e *Encoder) Encode(progressCb ProgressCallback) error { return err } - ffmpeg.AVFrameUnref(e.decFrame) + ffmpeg.AVFrameUnref(e.pipeline.decoded) } } // Flush decoder - if _, err := ffmpeg.AVCodecSendPacket(e.decCtx, nil); err != nil { + if _, err := ffmpeg.AVCodecSendPacket(e.input.codec, nil); err != nil { return fmt.Errorf("flush decoder failed: %w", err) } for { - if _, err := ffmpeg.AVCodecReceiveFrame(e.decCtx, e.decFrame); err != nil { + if _, err := ffmpeg.AVCodecReceiveFrame(e.input.codec, e.pipeline.decoded); err != nil { if errors.Is(err, ffmpeg.EAgain) || errors.Is(err, ffmpeg.AVErrorEOF) { break } return fmt.Errorf("flush decoder receive failed: %w", err) } - // Keep a ref (AVBuffersrcFlagKeepRef) because we reuse e.decFrame each - // iteration and unref it ourselves below. The filter-graph flush feeds a + // Keep a ref (AVBuffersrcFlagKeepRef) because we reuse the decoded frame + // each iteration and unref it ourselves below. The filter-graph flush feeds a // nil frame, so KEEP_REF is inapplicable there and it passes 0. - if _, err := ffmpeg.AVBuffersrcAddFrameFlags(e.bufferSrcCtx, e.decFrame, ffmpeg.AVBuffersrcFlagKeepRef); err != nil { + if _, err := ffmpeg.AVBuffersrcAddFrameFlags(e.filter.src, e.pipeline.decoded, ffmpeg.AVBuffersrcFlagKeepRef); err != nil { return fmt.Errorf("failed to feed filter graph: %w", err) } @@ -598,11 +296,11 @@ func (e *Encoder) Encode(progressCb ProgressCallback) error { return err } - ffmpeg.AVFrameUnref(e.decFrame) + ffmpeg.AVFrameUnref(e.pipeline.decoded) } // Flush filter graph - if _, err := ffmpeg.AVBuffersrcAddFrameFlags(e.bufferSrcCtx, nil, 0); err != nil { + if _, err := ffmpeg.AVBuffersrcAddFrameFlags(e.filter.src, nil, 0); err != nil { return fmt.Errorf("failed to flush filter graph: %w", err) } @@ -616,7 +314,7 @@ func (e *Encoder) Encode(progressCb ProgressCallback) error { } // Write trailer - if _, err := ffmpeg.AVWriteTrailer(e.ofmtCtx); err != nil { + if _, err := ffmpeg.AVWriteTrailer(e.output.format); err != nil { return fmt.Errorf("write trailer failed: %w", err) } @@ -627,31 +325,31 @@ func (e *Encoder) Encode(progressCb ProgressCallback) error { // EOF, encoding each one. Callers feed the buffersrc before invoking this. func (e *Encoder) drainFilterGraph(outStream *ffmpeg.AVStream) error { for { - if _, err := ffmpeg.AVBuffersinkGetFrame(e.bufferSinkCtx, e.filteredFrame); err != nil { + if _, err := ffmpeg.AVBuffersinkGetFrame(e.filter.sink, e.filter.frame); err != nil { if errors.Is(err, ffmpeg.EAgain) || errors.Is(err, ffmpeg.AVErrorEOF) { break } return fmt.Errorf("failed to get filtered frame: %w", err) } - if err := e.encodeFrame(e.filteredFrame, outStream); err != nil { + if err := e.encodeFrame(e.filter.frame, outStream); err != nil { return err } - ffmpeg.AVFrameUnref(e.filteredFrame) + ffmpeg.AVFrameUnref(e.filter.frame) } return nil } -// encodeFrame encodes a single audio frame to MP3 +// encodeFrame encodes a single audio frame with the preset's codec func (e *Encoder) encodeFrame(frame *ffmpeg.AVFrame, outStream *ffmpeg.AVStream) error { // Stamp a monotonic PTS from the running sample counter so the filter's // reframing does not leave gaps the encoder would reject. - frame.SetPts(e.nextPts) - e.nextPts += int64(frame.NbSamples()) + frame.SetPts(e.pipeline.nextPts) + e.pipeline.nextPts += int64(frame.NbSamples()) - if _, err := ffmpeg.AVCodecSendFrame(e.encCtx, frame); err != nil { + if _, err := ffmpeg.AVCodecSendFrame(e.output.codec, frame); err != nil { return fmt.Errorf("send frame to encoder failed: %w", err) } @@ -663,9 +361,9 @@ func (e *Encoder) encodeFrame(frame *ffmpeg.AVFrame, outStream *ffmpeg.AVStream) // labels the receive-packet error so each caller keeps its existing wording. func (e *Encoder) drainEncoder(outStream *ffmpeg.AVStream, recvErrCtx string) error { for { - ffmpeg.AVPacketUnref(e.encPkt) + ffmpeg.AVPacketUnref(e.pipeline.packet) - if _, err := ffmpeg.AVCodecReceivePacket(e.encCtx, e.encPkt); err != nil { + if _, err := ffmpeg.AVCodecReceivePacket(e.output.codec, e.pipeline.packet); err != nil { if errors.Is(err, ffmpeg.EAgain) || errors.Is(err, ffmpeg.AVErrorEOF) { break } @@ -673,10 +371,10 @@ func (e *Encoder) drainEncoder(outStream *ffmpeg.AVStream, recvErrCtx string) er } // Rescale packet timestamps from encoder to output stream time base. - e.encPkt.SetStreamIndex(e.outStreamIndex) - ffmpeg.AVPacketRescaleTs(e.encPkt, e.encCtx.TimeBase(), outStream.TimeBase()) + e.pipeline.packet.SetStreamIndex(e.output.audioStreamIndex) + ffmpeg.AVPacketRescaleTs(e.pipeline.packet, e.output.codec.TimeBase(), outStream.TimeBase()) - if _, err := ffmpeg.AVInterleavedWriteFrame(e.ofmtCtx, e.encPkt); err != nil { + if _, err := ffmpeg.AVInterleavedWriteFrame(e.output.format, e.pipeline.packet); err != nil { return fmt.Errorf("write frame failed: %w", err) } } @@ -686,7 +384,7 @@ func (e *Encoder) drainEncoder(outStream *ffmpeg.AVStream, recvErrCtx string) er // flushEncoder flushes remaining packets from the encoder func (e *Encoder) flushEncoder(outStream *ffmpeg.AVStream) error { - if _, err := ffmpeg.AVCodecSendFrame(e.encCtx, nil); err != nil { + if _, err := ffmpeg.AVCodecSendFrame(e.output.codec, nil); err != nil { return fmt.Errorf("flush encoder failed: %w", err) } @@ -709,59 +407,59 @@ func (e *Encoder) Close() { } e.closed = true - if e.filteredFrame != nil { - ffmpeg.AVFrameFree(&e.filteredFrame) + if e.filter.frame != nil { + ffmpeg.AVFrameFree(&e.filter.frame) } - if e.encPkt != nil { - ffmpeg.AVPacketFree(&e.encPkt) + if e.pipeline.packet != nil { + ffmpeg.AVPacketFree(&e.pipeline.packet) } - if e.decFrame != nil { - ffmpeg.AVFrameFree(&e.decFrame) + if e.pipeline.decoded != nil { + ffmpeg.AVFrameFree(&e.pipeline.decoded) } - if e.filterGraph != nil { - ffmpeg.AVFilterGraphFree(&e.filterGraph) + if e.filter.graph != nil { + ffmpeg.AVFilterGraphFree(&e.filter.graph) } - if e.encCtx != nil { - ffmpeg.AVCodecFreeContext(&e.encCtx) + if e.output.codec != nil { + ffmpeg.AVCodecFreeContext(&e.output.codec) } - if e.decCtx != nil { - ffmpeg.AVCodecFreeContext(&e.decCtx) + if e.input.codec != nil { + ffmpeg.AVCodecFreeContext(&e.input.codec) } - if e.ofmtCtx != nil { - if e.ofmtCtx.Oformat().Flags()&ffmpeg.AVFmtNofile == 0 && e.ofmtCtx.Pb() != nil { - ffmpeg.AVIOClose(e.ofmtCtx.Pb()) - e.ofmtCtx.SetPb(nil) + if e.output.format != nil { + if e.output.format.Oformat().Flags()&ffmpeg.AVFmtNofile == 0 && e.output.format.Pb() != nil { + ffmpeg.AVIOClose(e.output.format.Pb()) + e.output.format.SetPb(nil) } - ffmpeg.AVFormatFreeContext(e.ofmtCtx) + ffmpeg.AVFormatFreeContext(e.output.format) } - if e.ifmtCtx != nil { - ffmpeg.AVFormatCloseInput(&e.ifmtCtx) + if e.input.format != nil { + ffmpeg.AVFormatCloseInput(&e.input.format) } } // GetInputInfo returns information about the input audio func (e *Encoder) GetInputInfo() (sampleRate, channels int, format string) { - if e.decCtx == nil { + if e.input.codec == nil { return 0, 0, "unknown" } - codecName := e.decCtx.Codec().Name() - return e.decCtx.SampleRate(), e.decCtx.ChLayout().NbChannels(), codecName.String() + codecName := e.input.codec.Codec().Name() + return e.input.codec.SampleRate(), e.input.codec.ChLayout().NbChannels(), codecName.String() } // GetDurationSecs returns the duration of the encoded audio in seconds. // This is calculated from the samples processed during encoding, avoiding // the need to re-open the output file. Should be called after Encode() completes. func (e *Encoder) GetDurationSecs() int64 { - if e.encCtx == nil { + if e.output.codec == nil { return 0 } - sampleRate := e.encCtx.SampleRate() + sampleRate := e.output.codec.SampleRate() if sampleRate <= 0 { return 0 } // nextPts tracks total samples written to the encoder; round to nearest second - return (e.nextPts + int64(sampleRate)/2) / int64(sampleRate) + return (e.pipeline.nextPts + int64(sampleRate)/2) / int64(sampleRate) } // Bitrate returns the output bitrate in kbps for the configured channel mode, diff --git a/internal/encoder/encoder_test.go b/internal/encoder/encoder_test.go index f80cde0..da400e0 100644 --- a/internal/encoder/encoder_test.go +++ b/internal/encoder/encoder_test.go @@ -9,7 +9,7 @@ import ( "sync" "testing" - "github.com/linuxmatters/jive-encoder/internal/id3" + "github.com/linuxmatters/jive-encoder/internal/artwork" ) // TestNewFormatResolution verifies that New defaults an empty Format to the @@ -36,8 +36,36 @@ func TestNewFormatResolution(t *testing.T) { if enc.preset.name != "mp3" { t.Fatalf("expected mp3 preset, got %q", enc.preset.name) } - if enc.outStreamIndex != -1 { - t.Fatalf("expected outStreamIndex -1, got %d", enc.outStreamIndex) + if enc.output.audioStreamIndex != -1 { + t.Fatalf("expected audioStreamIndex -1, got %d", enc.output.audioStreamIndex) + } + }) + + t.Run("mismatched explicit format extension errors", func(t *testing.T) { + if _, err := New(Config{ + InputPath: "in.flac", + OutputPath: "out.mp3", + Format: "aac", + }); err == nil { + t.Fatal("expected error for mismatched output extension, got nil") + } + }) + + t.Run("mismatched default format extension errors", func(t *testing.T) { + if _, err := New(Config{ + InputPath: "in.flac", + OutputPath: "out.m4a", + }); err == nil { + t.Fatal("expected error for mismatched default output extension, got nil") + } + }) + + t.Run("extension matching is case-insensitive", func(t *testing.T) { + if _, err := New(Config{ + InputPath: "in.flac", + OutputPath: "out.MP3", + }); err != nil { + t.Fatalf("New failed for case-insensitive extension match: %v", err) } }) } @@ -382,7 +410,7 @@ func probeFormatTags(t *testing.T, path string) map[string]string { // the attached-picture stream behaviour per format: MP3 and AAC are // cover-capable and must carry an attached-picture video stream, while Opus is // not cover-capable and must stay audio-only. The cover bytes come from -// id3.ScaleCoverArt on a real testdata PNG fixture. +// artwork.ScaleCoverArt on a real testdata PNG fixture. func TestEncodeCoverArt_Integration(t *testing.T) { inputPath := "../../testdata/LMP0.flac" if _, err := os.Stat(inputPath); os.IsNotExist(err) { @@ -396,7 +424,7 @@ func TestEncodeCoverArt_Integration(t *testing.T) { if _, err := os.Stat(coverPath); os.IsNotExist(err) { t.Skipf("Cover fixture not found: %s", coverPath) } - cover, err := id3.ScaleCoverArt(coverPath) + cover, err := artwork.ScaleCoverArt(coverPath) if err != nil { t.Fatalf("ScaleCoverArt failed: %v", err) } @@ -543,7 +571,7 @@ func TestEncoder_InvalidInput(t *testing.T) { } } -// TestEncoder_OutputExists tests behavior when output file already exists +// TestEncoder_OutputExists tests behaviour when output file already exists func TestEncoder_OutputExists(t *testing.T) { inputPath := "../../testdata/LMP0.flac" if _, err := os.Stat(inputPath); os.IsNotExist(err) { @@ -625,7 +653,7 @@ func TestEncoder_CloseSafety(t *testing.T) { if err != nil { t.Fatalf("Failed to create encoder: %v", err) } - // Don't call Initialize - Close should handle nil pointers gracefully + // Skip Initialize so Close must handle nil pointers gracefully. return enc }, }, @@ -715,7 +743,7 @@ func TestEncoder_ProgressCallback(t *testing.T) { t.Fatalf("Failed to initialize encoder: %v", err) } - totalSamples := enc.totalSamples + totalSamples := enc.input.totalSamples if totalSamples == 0 { t.Skip("Could not determine total samples from input file") } diff --git a/internal/encoder/filter.go b/internal/encoder/filter.go new file mode 100644 index 0000000..580b74d --- /dev/null +++ b/internal/encoder/filter.go @@ -0,0 +1,124 @@ +package encoder + +import ( + "fmt" + + "github.com/linuxmatters/ffmpeg-statigo" +) + +// initFilter sets up the audio filter graph for resampling and frame buffering. +func (e *Encoder) initFilter() error { + e.filter.graph = ffmpeg.AVFilterGraphAlloc() + if e.filter.graph == nil { + return fmt.Errorf("failed to allocate filter graph") + } + + bufferSrc := ffmpeg.AVFilterGetByName(ffmpeg.GlobalCStr("abuffer")) + bufferSink := ffmpeg.AVFilterGetByName(ffmpeg.GlobalCStr("abuffersink")) + if bufferSrc == nil || bufferSink == nil { + return fmt.Errorf("abuffer or abuffersink filter not found") + } + + layoutPtr := ffmpeg.AllocCStr(64) + defer layoutPtr.Free() + if _, err := ffmpeg.AVChannelLayoutDescribe(e.input.codec.ChLayout(), layoutPtr, 64); err != nil { + return fmt.Errorf("failed to describe channel layout: %w", err) + } + + pktTimebase := e.input.codec.PktTimebase() + args := fmt.Sprintf( + "time_base=%d/%d:sample_rate=%d:sample_fmt=%s:channel_layout=%s", + pktTimebase.Num(), pktTimebase.Den(), + e.input.codec.SampleRate(), + ffmpeg.AVGetSampleFmtName(e.input.codec.SampleFmt()).String(), + layoutPtr.String(), + ) + + argsC := ffmpeg.ToCStr(args) + defer argsC.Free() + + if _, err := ffmpeg.AVFilterGraphCreateFilter( + &e.filter.src, + bufferSrc, + ffmpeg.GlobalCStr("in"), + argsC, + nil, + e.filter.graph, + ); err != nil { + return fmt.Errorf("failed to create buffer source: %w", err) + } + + if _, err := ffmpeg.AVFilterGraphCreateFilter( + &e.filter.sink, + bufferSink, + ffmpeg.GlobalCStr("out"), + nil, + nil, + e.filter.graph, + ); err != nil { + return fmt.Errorf("failed to create buffer sink: %w", err) + } + + outputs := ffmpeg.AVFilterInoutAlloc() + inputs := ffmpeg.AVFilterInoutAlloc() + if outputs == nil || inputs == nil { + ffmpeg.AVFilterInoutFree(&outputs) + ffmpeg.AVFilterInoutFree(&inputs) + return fmt.Errorf("failed to allocate filter endpoints") + } + defer ffmpeg.AVFilterInoutFree(&outputs) + defer ffmpeg.AVFilterInoutFree(&inputs) + + outputs.SetName(ffmpeg.ToCStr("in")) + outputs.SetFilterCtx(e.filter.src) + outputs.SetPadIdx(0) + outputs.SetNext(nil) + + inputs.SetName(ffmpeg.ToCStr("out")) + inputs.SetFilterCtx(e.filter.sink) + inputs.SetPadIdx(0) + inputs.SetNext(nil) + + if err := e.parseFilterGraph(&inputs, &outputs); err != nil { + return err + } + + if _, err := ffmpeg.AVFilterGraphConfig(e.filter.graph, nil); err != nil { + return fmt.Errorf("failed to configure filter graph: %w", err) + } + + e.setFilterFrameSize() + return nil +} + +func (e *Encoder) parseFilterGraph(inputs, outputs **ffmpeg.AVFilterInOut) error { + channelLayout := "mono" + if e.stereo { + channelLayout = "stereo" + } + + sampleFmtName := ffmpeg.AVGetSampleFmtName(e.preset.sampleFmt).String() + filterSpec := fmt.Sprintf("aresample=%d:async=1,aformat=sample_fmts=%s:sample_rates=%d:channel_layouts=%s", + e.preset.sampleRate, sampleFmtName, e.preset.sampleRate, channelLayout) + + filterSpecC := ffmpeg.ToCStr(filterSpec) + defer filterSpecC.Free() + + if _, err := ffmpeg.AVFilterGraphParsePtr(e.filter.graph, filterSpecC, inputs, outputs, nil); err != nil { + return fmt.Errorf("failed to parse filter graph: %w", err) + } + + return nil +} + +func (e *Encoder) setFilterFrameSize() { + // Fix the buffer-sink frame size to the encoder's required frame size so the + // filter delivers exactly the frames the encoder expects (LAME 1152, native + // AAC 1024, libopus its own). Encoders that accept variable-size frames + // advertise AV_CODEC_CAP_VARIABLE_FRAME_SIZE and need no fixed size. + // openOutput runs before initFilter, so FrameSize is populated here. + if frameSize := e.output.codec.FrameSize(); frameSize > 0 && + e.output.codec.Codec().Capabilities()&ffmpeg.AVCodecCapVariableFrameSize == 0 { + ffmpeg.AVBuffersinkSetFrameSize(e.filter.sink, uint(frameSize)) + } +} diff --git a/internal/encoder/metadata.go b/internal/encoder/metadata.go index cbca606..b737600 100644 --- a/internal/encoder/metadata.go +++ b/internal/encoder/metadata.go @@ -14,7 +14,7 @@ import ( // ParseEpisodeNumber validates a raw episode number and returns it unchanged // when valid. An episode number must be non-empty and a non-negative integer, -// so it produces a well-formed ID3 TRCK frame and filename. Non-numeric input +// so it produces a well-formed muxer track tag and filename. Non-numeric input // such as "foo" or "67a" is rejected at the boundary. func ParseEpisodeNumber(s string) (string, error) { if s == "" { @@ -37,8 +37,8 @@ type muxerTag struct { } // buildMuxerTags renders the muxer metadata key/value set from the episode -// fields, skipping empty values. The title preserves the "{EpisodeNumber}: {Title}" -// format. The track key carries the episode number, matching the previous TRCK frame. +// fields, skipping empty values. The title uses the "{EpisodeNumber}: {Title}" +// format. The track key carries the episode number. func buildMuxerTags(m Metadata) []muxerTag { var tags []muxerTag @@ -73,8 +73,8 @@ type EpisodeMetadata struct { // UnmarshalYAML decodes EpisodeMetadata while accepting either the capitalised // "Date" key (used by all existing frontmatter) or a lowercase "date" key. // yaml.v3 matches struct tags case-sensitively, so without this a lowercase -// "date:" would silently parse to the zero time.Time and produce a wrong ID3 -// TDRC tag with no error surfaced. "Date" takes precedence when both appear. +// "date:" would silently parse to the zero time.Time and produce a wrong date +// tag with no error surfaced. "Date" takes precedence when both appear. func (m *EpisodeMetadata) UnmarshalYAML(value *yaml.Node) error { // Alias avoids infinite recursion into this method while reusing the tags. type rawMetadata EpisodeMetadata @@ -173,16 +173,7 @@ func ResolveCoverArtPath(markdownPath, episodeImage string) (string, error) { // A "./" prefix means the image sits beside the markdown file. if after, ok := strings.CutPrefix(episodeImage, "./"); ok { coverPath := filepath.Join(markdownDir, after) - coverPath, err := filepath.Abs(coverPath) - if err != nil { - return "", fmt.Errorf("failed to resolve cover art path: %w", err) - } - - if _, err := os.Stat(coverPath); err != nil { - return "", fmt.Errorf("cover art not found: %s", coverPath) - } - - return coverPath, nil + return verifyCoverArtPath(coverPath) } // Otherwise the path is rooted at the Hugo site, served from static/. @@ -192,7 +183,11 @@ func ResolveCoverArtPath(markdownPath, episodeImage string) (string, error) { } coverPath := filepath.Join(projectRoot, "static", strings.TrimPrefix(episodeImage, "/")) - coverPath, err = filepath.Abs(coverPath) + return verifyCoverArtPath(coverPath) +} + +func verifyCoverArtPath(coverPath string) (string, error) { + coverPath, err := filepath.Abs(coverPath) if err != nil { return "", fmt.Errorf("failed to resolve cover art path: %w", err) } @@ -226,7 +221,8 @@ func findProjectRoot(startPath string) (string, error) { } } -// FormatDateForID3 formats a time.Time to "YYYY-MM" format for ID3 TDRC tag +// FormatDateForID3 formats a time.Time to "YYYY-MM" for the muxer date tag +// (ID3 TDRC for MP3, and the equivalent date field in other muxers). func FormatDateForID3(t time.Time) string { return t.Format("2006-01") } @@ -253,7 +249,9 @@ func UpdateFrontmatter(markdownPath, duration string, bytes int64) error { line := lines[i] if strings.HasPrefix(strings.TrimSpace(line), "podcast_duration:") { - lines[i] = fmt.Sprintf("podcast_duration: %s", duration) + // Quote the duration: unquoted HH:MM:SS parses as a sexagesimal + // number under YAML 1.1. + lines[i] = fmt.Sprintf("podcast_duration: %q", duration) updated = true } @@ -267,7 +265,7 @@ func UpdateFrontmatter(markdownPath, duration string, bytes int64) error { if !updated || !bytesUpdated { var insertLines []string if !updated { - insertLines = append(insertLines, fmt.Sprintf("podcast_duration: %s", duration)) + insertLines = append(insertLines, fmt.Sprintf("podcast_duration: %q", duration)) } if !bytesUpdated { insertLines = append(insertLines, fmt.Sprintf("podcast_bytes: %d", bytes)) diff --git a/internal/encoder/metadata_test.go b/internal/encoder/metadata_test.go index 240087d..4e5d2d4 100644 --- a/internal/encoder/metadata_test.go +++ b/internal/encoder/metadata_test.go @@ -297,7 +297,7 @@ More content. updatedContent := string(updated) // Verify fields were added before closing delimiter - if !strings.Contains(updatedContent, "podcast_duration: 01:23:45") { + if !strings.Contains(updatedContent, `podcast_duration: "01:23:45"`) { t.Error("podcast_duration field not found in updated frontmatter") } if !strings.Contains(updatedContent, "podcast_bytes: 5555555") { @@ -375,7 +375,7 @@ Episode content. updatedContent := string(updated) // Verify fields were updated with new values - if !strings.Contains(updatedContent, "podcast_duration: 01:23:45") { + if !strings.Contains(updatedContent, `podcast_duration: "01:23:45"`) { t.Error("podcast_duration not updated correctly") } if !strings.Contains(updatedContent, "podcast_bytes: 5555555") { @@ -383,7 +383,7 @@ Episode content. } // Verify old values are gone - if strings.Contains(updatedContent, "podcast_duration: 00:10:00") { + if strings.Contains(updatedContent, `podcast_duration: "00:10:00"`) { t.Error("Old podcast_duration value still present") } if strings.Contains(updatedContent, "podcast_bytes: 1000000") { @@ -423,7 +423,7 @@ Episode content. updatedContent := string(updated) // Verify both fields are now correct - if !strings.Contains(updatedContent, "podcast_duration: 00:15:00") { + if !strings.Contains(updatedContent, `podcast_duration: "00:15:00"`) { t.Error("podcast_duration not updated") } if !strings.Contains(updatedContent, "podcast_bytes: 3333333") { @@ -572,7 +572,7 @@ Episode content. updatedContent := string(updated) - if !strings.Contains(updatedContent, "podcast_duration: 54:32:10") { + if !strings.Contains(updatedContent, `podcast_duration: "54:32:10"`) { t.Error("Duration not updated correctly") } if !strings.Contains(updatedContent, "podcast_bytes: 104857600") { @@ -611,7 +611,7 @@ Episode content. updatedContent := string(updated) - if !strings.Contains(updatedContent, "podcast_duration: 00:00:00") { + if !strings.Contains(updatedContent, `podcast_duration: "00:00:00"`) { t.Error("Zero duration not handled correctly") } if !strings.Contains(updatedContent, "podcast_bytes: 0") { @@ -647,7 +647,7 @@ episode_image: "/img/test.png" updatedContent := string(updated) // Verify fields were added - if !strings.Contains(updatedContent, "podcast_duration: 00:10:00") { + if !strings.Contains(updatedContent, `podcast_duration: "00:10:00"`) { t.Error("podcast_duration not added") } if !strings.Contains(updatedContent, "podcast_bytes: 1000000") { @@ -690,7 +690,7 @@ Episode content. updatedContent := string(updated) // Verify fields were added and multiline content preserved - if !strings.Contains(updatedContent, "podcast_duration: 00:25:00") { + if !strings.Contains(updatedContent, `podcast_duration: "00:25:00"`) { t.Error("podcast_duration not added") } if !strings.Contains(updatedContent, "This is a multiline") { diff --git a/internal/encoder/output.go b/internal/encoder/output.go new file mode 100644 index 0000000..f3d8da6 --- /dev/null +++ b/internal/encoder/output.go @@ -0,0 +1,204 @@ +package encoder + +import ( + "fmt" + "maps" + "strconv" + + "github.com/linuxmatters/ffmpeg-statigo" +) + +// openOutput creates the output file for the preset's format and sets up the +// output-side encoder, muxer, metadata, and optional attached-picture stream. +func (e *Encoder) openOutput() error { + namePtr := ffmpeg.ToCStr(e.outputPath) + defer namePtr.Free() + + if _, err := ffmpeg.AVFormatAllocOutputContext2(&e.output.format, nil, nil, namePtr); err != nil { + return fmt.Errorf("failed to create output context: %w", err) + } + + encoder, err := e.findOutputEncoder() + if err != nil { + return err + } + + outStream := ffmpeg.AVFormatNewStream(e.output.format, encoder) + if outStream == nil { + return fmt.Errorf("failed to create output stream") + } + e.output.audioStreamIndex = outStream.Index() + + if err := e.configureOutputCodec(encoder, outStream); err != nil { + return err + } + + // Formats without the NOFILE flag need an explicit AVIO output handle. + if e.output.format.Oformat().Flags()&ffmpeg.AVFmtNofile == 0 { + var pb *ffmpeg.AVIOContext + if _, err := ffmpeg.AVIOOpen(&pb, e.output.format.Url(), ffmpeg.AVIOFlagWrite); err != nil { + return fmt.Errorf("failed to open output file: %w", err) + } + e.output.format.SetPb(pb) + } + + if err := e.setMuxerMetadata(); err != nil { + return err + } + + // Add the attached-picture stream after the audio stream so audio keeps + // index 0 (audioStreamIndex). Opus is not cover-capable and absent cover + // bytes mean no second stream, leaving the audio-only path unchanged. + if e.preset.coverCapable && len(e.coverArt) > 0 { + if err := e.addCoverStream(); err != nil { + return err + } + } + + if err := e.writeHeader(); err != nil { + return err + } + + // Write the cover picture immediately after the header so the muxer carries + // it as the attached picture before any audio packet. + if e.output.coverStreamIndex >= 0 { + if err := e.writeCoverPacket(); err != nil { + return err + } + } + + return nil +} + +func (e *Encoder) findOutputEncoder() (*ffmpeg.AVCodec, error) { + var encoder *ffmpeg.AVCodec + if e.preset.encoderName != "" { + namePtr := ffmpeg.ToCStr(e.preset.encoderName) + encoder = ffmpeg.AVCodecFindEncoderByName(namePtr) + namePtr.Free() + } + if encoder == nil { + encoder = ffmpeg.AVCodecFindEncoder(e.preset.codecID) + } + if encoder == nil { + return nil, fmt.Errorf("%s encoder not found", e.preset.name) + } + + return encoder, nil +} + +func (e *Encoder) configureOutputCodec(encoder *ffmpeg.AVCodec, outStream *ffmpeg.AVStream) error { + e.output.codec = ffmpeg.AVCodecAllocContext3(encoder) + if e.output.codec == nil { + return fmt.Errorf("failed to allocate encoder context") + } + + if e.stereo { + e.output.codec.SetBitRate(int64(e.preset.stereoBitrate)) + ffmpeg.AVChannelLayoutDefault(e.output.codec.ChLayout(), 2) + } else { + e.output.codec.SetBitRate(int64(e.preset.monoBitrate)) + ffmpeg.AVChannelLayoutDefault(e.output.codec.ChLayout(), 1) + } + + e.output.codec.SetSampleRate(e.preset.sampleRate) + e.output.codec.SetSampleFmt(e.preset.sampleFmt) + + tb := &ffmpeg.AVRational{} + tb.SetNum(1) + tb.SetDen(e.output.codec.SampleRate()) + e.output.codec.SetTimeBase(tb) + + opts, err := e.encoderOptions() + if err != nil { + return err + } + defer ffmpeg.AVDictFree(&opts) + + if _, err := ffmpeg.AVCodecOpen2(e.output.codec, encoder, &opts); err != nil { + return fmt.Errorf("failed to open encoder: %w", err) + } + + if _, err := ffmpeg.AVCodecParametersFromContext(outStream.Codecpar(), e.output.codec); err != nil { + return fmt.Errorf("failed to copy encoder parameters: %w", err) + } + + outStream.SetTimeBase(e.output.codec.TimeBase()) + return nil +} + +func (e *Encoder) encoderOptions() (*ffmpeg.AVDictionary, error) { + encoderOpts := e.preset.encoderOpts + if e.preset.lowpassHz > 0 { + encoderOpts = make(map[string]string, len(e.preset.encoderOpts)+1) + maps.Copy(encoderOpts, e.preset.encoderOpts) + encoderOpts["cutoff"] = strconv.Itoa(e.preset.lowpassHz) + } + + var opts *ffmpeg.AVDictionary + for key, val := range encoderOpts { + keyPtr := ffmpeg.ToCStr(key) + valPtr := ffmpeg.ToCStr(val) + _, err := ffmpeg.AVDictSet(&opts, keyPtr, valPtr, 0) + keyPtr.Free() + valPtr.Free() + if err != nil { + ffmpeg.AVDictFree(&opts) + return nil, fmt.Errorf("failed to set encoder option %s: %w", key, err) + } + } + + return opts, nil +} + +func (e *Encoder) writeHeader() error { + var muxerOpts *ffmpeg.AVDictionary + defer ffmpeg.AVDictFree(&muxerOpts) + + // id3v2_version is an mp3-muxer-private option, so it goes through the + // WriteHeader options dict, not the format-context metadata. Other muxers + // ignore it. + if e.preset.name == "mp3" { + keyPtr := ffmpeg.ToCStr("id3v2_version") + valPtr := ffmpeg.ToCStr("4") + _, err := ffmpeg.AVDictSet(&muxerOpts, keyPtr, valPtr, 0) + keyPtr.Free() + valPtr.Free() + if err != nil { + return fmt.Errorf("failed to set id3v2_version option: %w", err) + } + } + + if _, err := ffmpeg.AVFormatWriteHeader(e.output.format, &muxerOpts); err != nil { + return fmt.Errorf("failed to write header: %w", err) + } + + return nil +} + +// setMuxerMetadata builds the standard-key tag dictionary from the episode +// metadata and hands it to the output format context. SetMetadata transfers +// ownership to the context (freed by avformat_free_context), so this dict is +// never freed here. Preset-agnostic: every format gets the same standard keys. +func (e *Encoder) setMuxerMetadata() error { + tags := buildMuxerTags(e.metadata) + if len(tags) == 0 { + return nil + } + + var dict *ffmpeg.AVDictionary + for _, tag := range tags { + keyPtr := ffmpeg.ToCStr(tag.Key) + valPtr := ffmpeg.ToCStr(tag.Value) + _, err := ffmpeg.AVDictSet(&dict, keyPtr, valPtr, 0) + keyPtr.Free() + valPtr.Free() + if err != nil { + ffmpeg.AVDictFree(&dict) + return fmt.Errorf("failed to set metadata %s: %w", tag.Key, err) + } + } + + e.output.format.SetMetadata(dict) + return nil +} diff --git a/internal/encoder/preset.go b/internal/encoder/preset.go index d37071e..96ac587 100644 --- a/internal/encoder/preset.go +++ b/internal/encoder/preset.go @@ -4,10 +4,10 @@ import ( "github.com/linuxmatters/ffmpeg-statigo" ) -// formatPreset describes how a single output format is encoded and muxed. The +// formatPreset describes how a single output format is encoded. The // table below is the single source of truth for codec, bitrate, sample format, -// muxer, extension, lowpass policy, and cover capability, so the encoder reads -// the preset rather than branching on the format name. +// extension, lowpass policy, and cover capability, so the encoder reads the +// preset rather than branching on the format name. type formatPreset struct { // name is the lowercase format identifier (mp3, aac, opus). name string @@ -16,8 +16,8 @@ type formatPreset struct { // encoderName, when set, names a specific encoder to try before falling // back to the codec ID (e.g. libopus). encoderName string - // monoBitrate and stereoBitrate are the constant bitrates in bits per - // second for each channel mode. + // monoBitrate and stereoBitrate are the target bitrates in bits per second + // for each channel mode (constant for MP3/AAC, the VBR target for Opus). monoBitrate int stereoBitrate int // vbr selects variable bitrate encoding when true. @@ -28,8 +28,6 @@ type formatPreset struct { // sampleRate is the output sample rate in Hz. MP3 and AAC use 44.1 kHz; // libopus rejects 44.1 kHz at open, so Opus uses 48 kHz. sampleRate int - // muxer is the output format name for AVFormatAllocOutputContext2. - muxer string // extension is the output file extension including the leading dot. extension string // lowpassHz is the lowpass cutoff in Hz, or 0 for no lowpass. @@ -53,13 +51,11 @@ var formatPresets = map[string]formatPreset{ vbr: false, sampleFmt: ffmpeg.AVSampleFmtS16P, sampleRate: 44100, - muxer: "mp3", extension: ".mp3", lowpassHz: 20500, coverCapable: true, encoderOpts: map[string]string{ "compression_level": "3", - "cutoff": "20500", }, }, "aac": { @@ -70,7 +66,6 @@ var formatPresets = map[string]formatPreset{ vbr: false, sampleFmt: ffmpeg.AVSampleFmtFltp, sampleRate: 44100, - muxer: "ipod", extension: ".m4a", lowpassHz: 0, coverCapable: true, @@ -85,7 +80,6 @@ var formatPresets = map[string]formatPreset{ vbr: true, sampleFmt: ffmpeg.AVSampleFmtFlt, sampleRate: 48000, - muxer: "opus", extension: ".opus", lowpassHz: 0, coverCapable: false, diff --git a/internal/encoder/stats_test.go b/internal/encoder/stats_test.go index 18a9a65..01574f1 100644 --- a/internal/encoder/stats_test.go +++ b/internal/encoder/stats_test.go @@ -2,6 +2,7 @@ package encoder import ( "os" + "path/filepath" "testing" ) @@ -28,31 +29,46 @@ func TestFormatDurationHMS(t *testing.T) { } } -// TestGetFileStats is an integration test that requires an actual MP3 file -// The MP3 is created by TestEncodeToMP3_Integration if it doesn't exist +// TestGetFileStats verifies size and duration reporting against temp files +// of known content, so it needs no encoded audio artefacts. func TestGetFileStats(t *testing.T) { - testFile := "../../testdata/LMP0.mp3" - - if _, err := os.Stat(testFile); os.IsNotExist(err) { - t.Skip("Test MP3 file not found - run tests to generate it via TestEncodeToMP3_Integration") + tests := []struct { + name string + sizeBytes int + durationSecs int64 + expected string + }{ + {"empty file, zero duration", 0, 0, "00:00:00"}, + {"small file, under a minute", 27, 27, "00:00:27"}, + {"typical episode length", 4096, 1695, "00:28:15"}, + {"over an hour", 1024, 3661, "01:01:01"}, } - // Use a known duration for testing (the test file is ~27 seconds) - testDurationSecs := int64(27) - stats, err := GetFileStats(testFile, testDurationSecs) - if err != nil { - t.Fatalf("GetFileStats() error = %v", err) - } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + path := filepath.Join(t.TempDir(), "episode.mp3") + if err := os.WriteFile(path, make([]byte, tt.sizeBytes), 0o644); err != nil { + t.Fatalf("failed to write temp file: %v", err) + } - // Verify duration format (should be HH:MM:SS) - if len(stats.DurationString) != 8 { - t.Errorf("Duration format incorrect: got %s, want HH:MM:SS format", stats.DurationString) - } + stats, err := GetFileStats(path, tt.durationSecs) + if err != nil { + t.Fatalf("GetFileStats() error = %v", err) + } - if stats.FileSizeBytes <= 0 { - t.Errorf("FileSizeBytes = %d; want > 0", stats.FileSizeBytes) + if stats.FileSizeBytes != int64(tt.sizeBytes) { + t.Errorf("FileSizeBytes = %d; want %d", stats.FileSizeBytes, tt.sizeBytes) + } + if stats.DurationString != tt.expected { + t.Errorf("DurationString = %s; want %s", stats.DurationString, tt.expected) + } + }) } - t.Logf("Stats for %s: duration=%s, size=%d bytes", - testFile, stats.DurationString, stats.FileSizeBytes) + t.Run("missing file returns error", func(t *testing.T) { + missing := filepath.Join(t.TempDir(), "missing.mp3") + if _, err := GetFileStats(missing, 27); err == nil { + t.Error("GetFileStats() on missing file: expected error, got nil") + } + }) } diff --git a/internal/id3/taginfo.go b/internal/id3/taginfo.go deleted file mode 100644 index 526f11b..0000000 --- a/internal/id3/taginfo.go +++ /dev/null @@ -1,12 +0,0 @@ -package id3 - -// TagInfo carries episode metadata from the CLI workflows to the encoder, which -// writes it as muxer-native tags during encoding. -type TagInfo struct { - EpisodeNumber string - Title string - Artist string // Optional: defaults to empty if not provided - Album string // Optional: defaults to empty if not provided - Date string // Optional: Format: "YYYY-MM" - Comment string // Optional: defaults to empty if not provided -} diff --git a/internal/ui/encode.go b/internal/ui/encode.go index 17a79b1..59d6794 100644 --- a/internal/ui/encode.go +++ b/internal/ui/encode.go @@ -124,7 +124,7 @@ func NewEncodeModel(enc *encoder.Encoder, outputMode string, outputBitrate int, } } -// Init initializes the model and starts encoding +// Init initialises the model and starts encoding func (m *EncodeModel) Init() tea.Cmd { return tea.Batch( m.startEncoding(), @@ -288,7 +288,7 @@ func (m *EncodeModel) calculateProgress() float64 { return float64(m.samplesProcessed) / float64(m.totalSamples) * 100 } -// calculateSpeed returns encoding speed (e.g., "101.2x realtime") +// calculateSpeed returns the encoding speed as a realtime multiple (e.g. 101.2). func (m *EncodeModel) calculateSpeed() float64 { if m.inputRate == 0 { return 0 @@ -299,10 +299,8 @@ func (m *EncodeModel) calculateSpeed() float64 { return 0 } - // Calculate audio duration processed (in seconds) + // Speed is audio seconds decoded divided by wall-clock seconds elapsed. audioProcessed := float64(m.samplesProcessed) / float64(m.inputRate) - - // Speed = audio duration / wall clock time return audioProcessed / elapsed } @@ -315,8 +313,8 @@ func (m *EncodeModel) calculateTimeRemaining() time.Duration { elapsed := time.Since(m.startTime) - // Use progress percentage for accurate estimation - // If we've completed X%, the remaining (100-X)% will take proportionally longer + // Extrapolate linearly from progress so far: elapsed time scaled to 100% + // gives the estimated total, minus what has already elapsed. totalEstimated := float64(elapsed) * 100.0 / progress remaining := time.Duration(totalEstimated) - elapsed diff --git a/internal/ui/styles.go b/internal/ui/styles.go index 5593cd6..b9bd3fd 100644 --- a/internal/ui/styles.go +++ b/internal/ui/styles.go @@ -5,13 +5,8 @@ import ( "github.com/linuxmatters/jive-encoder/internal/cli" ) -// Import shared colour palette from cli package +// Import shared gradient colours from cli package. var ( - primaryColor = cli.PrimaryColor - accentColor = cli.AccentColor - mutedColor = cli.MutedColor - - // Disco ball gradient colours gradientIndigo = cli.GradientIndigo gradientWhite = cli.GradientWhite ) @@ -19,12 +14,12 @@ var ( // Header style for section titles var headerStyle = lipgloss.NewStyle(). Bold(true). - Foreground(primaryColor). + Foreground(cli.PrimaryColor). MarginBottom(1) // Spinner style for the encoding progress indicator var spinnerStyle = lipgloss.NewStyle(). - Foreground(accentColor) + Foreground(cli.AccentColor) // Shared styles from cli package var ( @@ -38,7 +33,7 @@ var ( // Muted text style (no cli equivalent) var mutedStyle = lipgloss.NewStyle(). - Foreground(mutedColor). + Foreground(cli.MutedColor). Italic(true) // clockStyle renders the elapsed/remaining MM:SS values in white, matching the diff --git a/internal/ui/views.go b/internal/ui/views.go index 2c58f3c..0b45b70 100644 --- a/internal/ui/views.go +++ b/internal/ui/views.go @@ -13,7 +13,7 @@ func progressView(m *EncodeModel) string { var b strings.Builder // Persistent spinner accent rendered off the shared tick (see encode.go). - spinnerGlyph := spinnerStyle.Render(spinnerFrames[m.anim.spinnerFrame%len(spinnerFrames)]) + spinnerGlyph := spinnerStyle.Render(spinnerFrames[m.anim.spinnerFrame]) // Before the first non-zero progress, FFmpeg is still initialising, so the // spinner stands in as an indeterminate indicator with a "preparing" cue. @@ -56,7 +56,7 @@ func progressView(m *EncodeModel) string { " ", boltStyle.Render("⚡"), " ", - highlightStyle.Render(fmt.Sprintf("%.1f×", speed)), + speedLabel(speed), ) b.WriteString(stats) @@ -123,7 +123,7 @@ func completeView(m *EncodeModel) string { successStyle.Render("✓"), valueStyle.Render(elapsed), boltStyle.Render("⚡"), - highlightStyle.Render(fmt.Sprintf("%.1f×", speed)), + speedLabel(speed), ) return frameStyle.Render(lipgloss.JoinVertical(lipgloss.Left, msg)) @@ -138,3 +138,7 @@ func errorView(err error) string { return frameStyle.Render(lipgloss.JoinVertical(lipgloss.Left, msg)) } + +func speedLabel(speed float64) string { + return highlightStyle.Render(fmt.Sprintf("%.1f×", speed)) +} diff --git a/justfile b/justfile index f15a208..6e179af 100644 --- a/justfile +++ b/justfile @@ -199,10 +199,13 @@ test-encoder: build file="$out/LMP67.${ext[$fmt]}" rm -f "$file" - # Decline the frontmatter-update prompt. Gate on the encode exit status, - # not the SIGPIPE that "echo n" may receive once jive-encoder stops reading. + # Keep the encoder's output visible (the TUI renders when stdout is a + # terminal). "yes n" declines the frontmatter-update prompt: the TUI + # consumes stdin while it runs, so a single "echo n" would be eaten + # before the prompt reads it. Gate on the encode exit status, not the + # SIGPIPE that "yes" receives once jive-encoder exits. set +o pipefail - echo n | ./jive-encoder "$flac" "$meta" --format "$fmt" --output-path "$out/" >/dev/null + yes n | ./jive-encoder "$flac" "$meta" --format "$fmt" --output-path "$out/" rc=${PIPESTATUS[1]} set -o pipefail [ "$rc" -eq 0 ] || fail "$fmt: jive-encoder exited $rc"