From 6e23ec3455d83aec3359001fc494f4d1bdb5f08d Mon Sep 17 00:00:00 2001 From: quepasaquepasa <293237286+quepasaquepasa@users.noreply.github.com> Date: Fri, 31 Jul 2026 00:06:44 -0400 Subject: [PATCH 1/2] Keep embedded cover art when transcoding to mp3 and flac --- consts/consts.go | 7 ++- core/ffmpeg/ffmpeg_test.go | 4 +- ..._keep_cover_art_in_default_transcodings.go | 56 +++++++++++++++++++ 3 files changed, 63 insertions(+), 4 deletions(-) create mode 100644 db/migrations/20260731004500_keep_cover_art_in_default_transcodings.go diff --git a/consts/consts.go b/consts/consts.go index f453ac125..7eba445bd 100644 --- a/consts/consts.go +++ b/consts/consts.go @@ -166,7 +166,10 @@ var ( Name: "mp3 audio", TargetFormat: "mp3", DefaultBitRate: 192, - Command: "ffmpeg -ss %t -i %s -map 0:a:0 -map_metadata 0 -map_metadata 0:s:a:0 -b:a %bk -v 0 -f mp3 -", + // 0:v:0? also carries over embedded cover art, when the source has any. Only + // mp3 and flac are given this treatment: the opus and adts muxers reject a + // video stream outright. + Command: "ffmpeg -ss %t -i %s -map 0:a:0 -map 0:v:0? -map_metadata 0 -map_metadata 0:s:a:0 -b:a %bk -v 0 -c:v copy -disposition:v attached_pic -f mp3 -", }, { Name: "opus audio", @@ -184,7 +187,7 @@ var ( Name: "flac audio", TargetFormat: "flac", DefaultBitRate: 0, - Command: "ffmpeg -ss %t -i %s -map 0:a:0 -map_metadata 0 -map_metadata 0:s:a:0 -v 0 -c:a flac -f flac -", + Command: "ffmpeg -ss %t -i %s -map 0:a:0 -map 0:v:0? -map_metadata 0 -map_metadata 0:s:a:0 -v 0 -c:a flac -c:v copy -disposition:v attached_pic -f flac -", }, } ) diff --git a/core/ffmpeg/ffmpeg_test.go b/core/ffmpeg/ffmpeg_test.go index 0fa3de111..3912bc2e6 100644 --- a/core/ffmpeg/ffmpeg_test.go +++ b/core/ffmpeg/ffmpeg_test.go @@ -83,7 +83,7 @@ var _ = Describe("ffmpeg", func() { Describe("isDefaultCommand", func() { It("returns true for known default mp3 command", func() { - Expect(isDefaultCommand("mp3", "ffmpeg -ss %t -i %s -map 0:a:0 -map_metadata 0 -map_metadata 0:s:a:0 -b:a %bk -v 0 -f mp3 -")).To(BeTrue()) + Expect(isDefaultCommand("mp3", "ffmpeg -ss %t -i %s -map 0:a:0 -map 0:v:0? -map_metadata 0 -map_metadata 0:s:a:0 -b:a %bk -v 0 -c:v copy -disposition:v attached_pic -f mp3 -")).To(BeTrue()) }) It("returns true for known default opus command", func() { Expect(isDefaultCommand("opus", "ffmpeg -ss %t -i %s -map 0:a:0 -map_metadata 0 -map_metadata 0:s:a:0 -b:a %bk -v 0 -c:a libopus -f opus -")).To(BeTrue()) @@ -92,7 +92,7 @@ var _ = Describe("ffmpeg", func() { Expect(isDefaultCommand("aac", "ffmpeg -ss %t -i %s -map 0:a:0 -map_metadata 0 -map_metadata 0:s:a:0 -b:a %bk -v 0 -c:a aac -f adts -")).To(BeTrue()) }) It("returns true for known default flac command", func() { - Expect(isDefaultCommand("flac", "ffmpeg -ss %t -i %s -map 0:a:0 -map_metadata 0 -map_metadata 0:s:a:0 -v 0 -c:a flac -f flac -")).To(BeTrue()) + Expect(isDefaultCommand("flac", "ffmpeg -ss %t -i %s -map 0:a:0 -map 0:v:0? -map_metadata 0 -map_metadata 0:s:a:0 -v 0 -c:a flac -c:v copy -disposition:v attached_pic -f flac -")).To(BeTrue()) }) It("returns false for a custom command", func() { Expect(isDefaultCommand("mp3", "ffmpeg -i %s -b:a %bk -custom-flag -f mp3 -")).To(BeFalse()) diff --git a/db/migrations/20260731004500_keep_cover_art_in_default_transcodings.go b/db/migrations/20260731004500_keep_cover_art_in_default_transcodings.go new file mode 100644 index 000000000..25e2f8fd2 --- /dev/null +++ b/db/migrations/20260731004500_keep_cover_art_in_default_transcodings.go @@ -0,0 +1,56 @@ +package migrations + +import ( + "context" + "database/sql" + + "github.com/pressly/goose/v3" +) + +func init() { + goose.AddMigrationContext(upKeepCoverArtInDefaultTranscodings, downKeepCoverArtInDefaultTranscodings) +} + +// coverArtPairs maps the current default commands (audio stream only) to the new +// defaults that also carry over embedded cover art. Index 0 = old, index 1 = new. +// +// `-map 0:v:0?` picks up the attached picture when the source has one, and the +// trailing `?` keeps the command working for sources without artwork. `-c:v copy` +// avoids re-encoding it and `-disposition:v attached_pic` marks it as cover art +// rather than a video track. +// +// Only mp3 and flac are updated: the opus muxer rejects the mjpeg stream +// ("Unsupported codec id in stream 1") and adts refuses any video stream +// ("adts muxer does not support any stream of type video"), so adding the +// mapping there would break transcoding to those formats outright. +// +// Only rows still holding the exact unmodified default are updated, so any +// user-customized command is left untouched. +var coverArtPairs = [][2]string{ + { + "ffmpeg -ss %t -i %s -map 0:a:0 -map_metadata 0 -map_metadata 0:s:a:0 -b:a %bk -v 0 -f mp3 -", + "ffmpeg -ss %t -i %s -map 0:a:0 -map 0:v:0? -map_metadata 0 -map_metadata 0:s:a:0 -b:a %bk -v 0 -c:v copy -disposition:v attached_pic -f mp3 -", + }, + { + "ffmpeg -ss %t -i %s -map 0:a:0 -map_metadata 0 -map_metadata 0:s:a:0 -v 0 -c:a flac -f flac -", + "ffmpeg -ss %t -i %s -map 0:a:0 -map 0:v:0? -map_metadata 0 -map_metadata 0:s:a:0 -v 0 -c:a flac -c:v copy -disposition:v attached_pic -f flac -", + }, +} + +func upKeepCoverArtInDefaultTranscodings(ctx context.Context, tx *sql.Tx) error { + for _, p := range coverArtPairs { + if _, err := tx.ExecContext(ctx, `UPDATE transcoding SET command = ? WHERE command = ?`, p[1], p[0]); err != nil { + return err + } + } + return nil +} + +func downKeepCoverArtInDefaultTranscodings(ctx context.Context, tx *sql.Tx) error { + for _, p := range coverArtPairs { + if _, err := tx.ExecContext(ctx, `UPDATE transcoding SET command = ? WHERE command = ?`, p[0], p[1]); err != nil { + return err + } + } + return nil +} From 68cde4d35b1588f1a8e1d64d050bd210c550cadc Mon Sep 17 00:00:00 2001 From: quepasaquepasa <293237286+quepasaquepasa@users.noreply.github.com> Date: Fri, 31 Jul 2026 23:27:19 -0400 Subject: [PATCH 2/2] Map cover art in the dynamic command builder too isDefaultCommand routes unmodified defaults to buildDynamicArgs, which builds the argument list programmatically and never reads the stored command, so changing the default template alone left artwork stripped for exactly the users the change was aimed at. --- core/ffmpeg/ffmpeg.go | 16 ++++++++++++++++ core/ffmpeg/ffmpeg_test.go | 18 ++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/core/ffmpeg/ffmpeg.go b/core/ffmpeg/ffmpeg.go index af2dab647..0ff77a77a 100644 --- a/core/ffmpeg/ffmpeg.go +++ b/core/ffmpeg/ffmpeg.go @@ -441,6 +441,14 @@ var formatOutputMap = map[string]string{ "flac": "flac", } +// formatKeepsCoverArt lists the target formats whose muxer accepts an attached +// picture. opus ("Unsupported codec id in stream 1") and adts ("adts muxer does +// not support any stream of type video") reject one, so artwork is dropped there. +var formatKeepsCoverArt = map[string]bool{ + "mp3": true, + "flac": true, +} + // defaultCommands is used to detect whether a user has customized their transcoding command. var defaultCommands = func() map[string]string { m := make(map[string]string, len(consts.DefaultTranscodings)) @@ -468,6 +476,14 @@ func buildDynamicArgs(opts TranscodeOptions) []string { args = append(args, "-i", opts.FilePath) args = append(args, "-map", "0:a:0") + // Carry over embedded cover art, when the source has any. The trailing "?" + // keeps the mapping optional so sources without artwork still transcode. + // Only mp3 and flac: the opus muxer rejects the mjpeg stream and adts + // refuses any video stream, so mapping it there breaks transcoding outright. + if formatKeepsCoverArt[opts.Format] { + args = append(args, "-map", "0:v:0?", "-c:v", "copy", "-disposition:v", "attached_pic") + } + // Preserve source tags. -map_metadata 0 copies format-level tags (MP3/FLAC); // -map_metadata 0:s:a:0 copies tags from the first audio stream (OPUS/OGG). // Both are needed because the two source families store tags at different diff --git a/core/ffmpeg/ffmpeg_test.go b/core/ffmpeg/ffmpeg_test.go index 3912bc2e6..b8dc653e0 100644 --- a/core/ffmpeg/ffmpeg_test.go +++ b/core/ffmpeg/ffmpeg_test.go @@ -114,6 +114,7 @@ var _ = Describe("ffmpeg", func() { Expect(args).To(Equal([]string{ "ffmpeg", "-i", "/music/file.flac", "-map", "0:a:0", + "-map", "0:v:0?", "-c:v", "copy", "-disposition:v", "attached_pic", "-map_metadata", "0", "-map_metadata", "0:s:a:0", "-c:a", "libmp3lame", "-b:a", "256k", @@ -134,6 +135,7 @@ var _ = Describe("ffmpeg", func() { Expect(args).To(Equal([]string{ "ffmpeg", "-i", "/music/file.dsf", "-map", "0:a:0", + "-map", "0:v:0?", "-c:v", "copy", "-disposition:v", "attached_pic", "-map_metadata", "0", "-map_metadata", "0:s:a:0", "-c:a", "flac", "-ar", "48000", @@ -161,6 +163,20 @@ var _ = Describe("ffmpeg", func() { })) }) + It("does not map cover art for muxers that reject it", func() { + // The opus and adts muxers refuse a video stream outright, so mapping + // the attached picture there would break transcoding rather than + // preserve artwork. + for _, format := range []string{"opus", "aac"} { + args := buildDynamicArgs(TranscodeOptions{ + Format: format, + FilePath: "/music/file.flac", + BitRate: 128, + }) + Expect(strings.Join(args, " ")).ToNot(ContainSubstring("0:v:0?"), format) + } + }) + It("includes offset when specified", func() { args := buildDynamicArgs(TranscodeOptions{ Format: "mp3", @@ -173,6 +189,7 @@ var _ = Describe("ffmpeg", func() { "-ss", "30", "-i", "/music/file.mp3", "-map", "0:a:0", + "-map", "0:v:0?", "-c:v", "copy", "-disposition:v", "attached_pic", "-map_metadata", "0", "-map_metadata", "0:s:a:0", "-c:a", "libmp3lame", "-b:a", "192k", @@ -209,6 +226,7 @@ var _ = Describe("ffmpeg", func() { Expect(args).To(Equal([]string{ "ffmpeg", "-i", "/music/file.dsf", "-map", "0:a:0", + "-map", "0:v:0?", "-c:v", "copy", "-disposition:v", "attached_pic", "-map_metadata", "0", "-map_metadata", "0:s:a:0", "-c:a", "flac", "-sample_fmt", "s32",