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.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 0fa3de111..b8dc653e0 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()) @@ -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", 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 +}