harden ffmpeg pipeline failure handling and stream mapping

internal/app/app.go

@@ -2,7 +2,9 @@ package app
 
 import (
 	"context"
+	"errors"
 	"fmt"
+	"os/exec"
 	"path/filepath"
 	"regexp"
 	"sort"
@@ -903,6 +905,10 @@ func (m *Main) ripTrack(ctx context.Context, p provider.Client, source, id, fall
 		}
 	}
 	if err = m.Tagger.TagFLAC(outPath, tagMeta, coverPath); err != nil {
+		if isFFmpegMissingError(err) {
+			_ = m.Store.MarkFailed(ctx, source, "track", id)
+			return fmt.Errorf("id=%s title=%q tag: %w", id, title, err)
+		}
 		m.logf("warning: tag failed for %s: %v\n", filepath.Base(outPath), err)
 	}
 
@@ -1342,3 +1348,13 @@ func applyPlaylistMetadataOverrides(meta map[string]any, cfg config.MetadataConf
 	}
 	artist["name"] = "Various Artists"
 }
+
+func isFFmpegMissingError(err error) bool {
+	if err == nil {
+		return false
+	}
+	if errors.Is(err, exec.ErrNotFound) {
+		return true
+	}
+	return strings.Contains(strings.ToLower(err.Error()), "ffmpeg not found")
+}

internal/app/app_test.go

@@ -2,9 +2,11 @@ package app
 
 import (
 	"context"
+	"fmt"
 	"net/http"
 	"net/http/httptest"
 	"os"
+	"os/exec"
 	"path/filepath"
 	"strings"
 	"testing"
@@ -20,6 +22,12 @@ type noopTagger struct{}
 
 func (n noopTagger) TagFLAC(string, tag.Metadata, string) error { return nil }
 
+type failingTagger struct {
+	err error
+}
+
+func (f failingTagger) TagFLAC(string, tag.Metadata, string) error { return f.err }
+
 type fakeProvider struct {
 	url string
 }
@@ -237,6 +245,56 @@ func TestTrackRipPipeline(t *testing.T) {
 	}
 }
 
+func TestTrackRipFailsWhenTaggerReportsMissingFFmpeg(t *testing.T) {
+	tmp := t.TempDir()
+
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		_, _ = w.Write([]byte("audio-bytes"))
+	}))
+	defer ts.Close()
+
+	d := config.DefaultConfigData()
+	d.Downloads.Folder = tmp
+	d.Downloads.SourceSubdirectories = false
+	cfg := &config.Config{File: d, Session: d}
+
+	sqlite, err := store.NewSQLite(filepath.Join(tmp, "db.sqlite"))
+	if err != nil {
+		t.Fatalf("NewSQLite() error = %v", err)
+	}
+	defer func() { _ = sqlite.Close() }()
+
+	m := &Main{
+		Config: cfg,
+		Providers: map[string]provider.Client{
+			"qobuz": &fakeProvider{url: ts.URL},
+		},
+		Store:  sqlite,
+		DL:     download.NewWithOptions(true, false),
+		Tagger: failingTagger{err: fmt.Errorf("ffmpeg not found: %w", exec.ErrNotFound)},
+	}
+
+	ctx := context.Background()
+	if err = m.AddByID(ctx, "qobuz", "track", "19512574"); err != nil {
+		t.Fatalf("AddByID() error = %v", err)
+	}
+	if err = m.Resolve(ctx); err != nil {
+		t.Fatalf("Resolve() error = %v", err)
+	}
+	err = m.Rip(ctx)
+	if err == nil {
+		t.Fatalf("expected rip failure")
+	}
+
+	ok, err := sqlite.IsDownloaded(ctx, "qobuz", "19512574")
+	if err != nil {
+		t.Fatalf("IsDownloaded() error = %v", err)
+	}
+	if ok {
+		t.Fatalf("expected track not marked downloaded")
+	}
+}
+
 func TestAlbumRipPipeline(t *testing.T) {
 	tmp := t.TempDir()
 

internal/audio/convert/convert.go

@@ -52,13 +52,13 @@ func Convert(path string, cfg config.ConversionConfig) (string, error) {
 		return path, fmt.Errorf("conversion failed: %w: %s", err, string(output))
 	}
 
-	if path != finalPath {
-		_ = os.Remove(path)
-	}
 	if err = os.Rename(tmpPath, finalPath); err != nil {
 		_ = os.Remove(tmpPath)
 		return path, err
 	}
+	if path != finalPath {
+		_ = os.Remove(path)
+	}
 
 	return finalPath, nil
 }

internal/audio/convert/convert_test.go

@@ -1,6 +1,8 @@
 package convert
 
 import (
+	"os"
+	"path/filepath"
 	"strings"
 	"testing"
 
@@ -59,3 +61,43 @@ func TestBuildFFmpegArgsNoCoverForOpus(t *testing.T) {
 		t.Fatalf("unexpected cover map args=%s", joined)
 	}
 }
+
+func TestConvertKeepsSourceWhenRenameFails(t *testing.T) {
+	tmp := t.TempDir()
+	in := filepath.Join(tmp, "song.flac")
+	if err := os.WriteFile(in, []byte("src"), 0o644); err != nil {
+		t.Fatalf("write input: %v", err)
+	}
+
+	fakeBin := filepath.Join(tmp, "bin")
+	if err := os.MkdirAll(fakeBin, 0o755); err != nil {
+		t.Fatalf("mkdir bin: %v", err)
+	}
+	fakeFFmpeg := filepath.Join(fakeBin, "ffmpeg")
+	script := "#!/bin/sh\nout=\"\"\nfor arg in \"$@\"; do out=\"$arg\"; done\n: > \"$out\"\n"
+	if err := os.WriteFile(fakeFFmpeg, []byte(script), 0o755); err != nil {
+		t.Fatalf("write fake ffmpeg: %v", err)
+	}
+	t.Setenv("PATH", fakeBin)
+
+	finalPath := strings.TrimSuffix(in, filepath.Ext(in)) + ".m4a"
+	if err := os.Mkdir(finalPath, 0o755); err != nil {
+		t.Fatalf("mkdir final path: %v", err)
+	}
+
+	cfg := config.ConversionConfig{Enabled: true, Codec: "ALAC"}
+	out, err := Convert(in, cfg)
+	if err == nil {
+		t.Fatalf("expected rename failure")
+	}
+	if out != in {
+		t.Fatalf("returned path = %q, want %q", out, in)
+	}
+	if _, statErr := os.Stat(in); statErr != nil {
+		t.Fatalf("expected source to remain, stat err=%v", statErr)
+	}
+	tmpPath := finalPath + ".tmp.m4a"
+	if _, statErr := os.Stat(tmpPath); !os.IsNotExist(statErr) {
+		t.Fatalf("expected temp output cleanup, stat err=%v", statErr)
+	}
+}

internal/download/downloader.go

@@ -323,17 +323,7 @@ func (d *Downloader) streamManifestWithFFmpeg(ctx context.Context, sourceURL, ou
 		return fmt.Errorf("ffmpeg not found for manifest stream: %w", err)
 	}
 
-	args := []string{
+	args := buildFFmpegStreamArgs(sourceURL, outputPath, includeVideo)
-		"-y",
-		"-protocol_whitelist", "file,http,https,tcp,tls,crypto,data",
-		"-i", sourceURL,
-	}
-	if includeVideo {
-		args = append(args, "-map", "0")
-	} else {
-		args = append(args, "-map", "0:a:0")
-	}
-	args = append(args, "-c", "copy", "-hide_banner", "-nostats", "-progress", "pipe:2", outputPath)
 
 	if !d.ProgressEnabled() {
 		cmd := exec.CommandContext(ctx, "ffmpeg", args...)
@@ -458,6 +448,24 @@ func (d *Downloader) streamManifestWithFFmpeg(ctx context.Context, sourceURL, ou
 	return nil
 }
 
+func buildFFmpegStreamArgs(sourceURL, outputPath string, includeVideo bool) []string {
+	args := []string{
+		"-y",
+		"-protocol_whitelist", "file,http,https,tcp,tls,crypto,data",
+		"-i", sourceURL,
+	}
+	if includeVideo {
+		args = append(args,
+			"-map", "0:v:0?",
+			"-map", "0:a:0?",
+		)
+	} else {
+		args = append(args, "-map", "0:a:0")
+	}
+	args = append(args, "-c", "copy", "-hide_banner", "-nostats", "-progress", "pipe:2", outputPath)
+	return args
+}
+
 type scanState struct {
 	totalMS   int64
 	currentMS int64

internal/download/downloader_test.go

@@ -260,3 +260,38 @@ func TestParseClockDurationMSInvalid(t *testing.T) {
 		t.Fatalf("expected short duration to fail")
 	}
 }
+
+func TestBuildFFmpegStreamArgsAudioOnly(t *testing.T) {
+	args := buildFFmpegStreamArgs("https://example.com/master.m3u8", "/tmp/out.m4a", false)
+	if !containsArgPair(args, "-map", "0:a:0") {
+		t.Fatalf("expected audio map in args: %v", args)
+	}
+	if containsArgPair(args, "-map", "0:v:0?") {
+		t.Fatalf("did not expect video map in audio-only args: %v", args)
+	}
+	if containsArgPair(args, "-map", "0") {
+		t.Fatalf("did not expect broad map=0 in args: %v", args)
+	}
+}
+
+func TestBuildFFmpegStreamArgsIncludeVideo(t *testing.T) {
+	args := buildFFmpegStreamArgs("https://example.com/master.m3u8", "/tmp/out.mp4", true)
+	if !containsArgPair(args, "-map", "0:v:0?") {
+		t.Fatalf("expected video map in args: %v", args)
+	}
+	if !containsArgPair(args, "-map", "0:a:0?") {
+		t.Fatalf("expected audio map in args: %v", args)
+	}
+	if containsArgPair(args, "-map", "0") {
+		t.Fatalf("did not expect broad map=0 in args: %v", args)
+	}
+}
+
+func containsArgPair(args []string, key, value string) bool {
+	for i := 0; i+1 < len(args); i++ {
+		if args[i] == key && args[i+1] == value {
+			return true
+		}
+	}
+	return false
+}