Fallback extracting tags using ffmpeg

cmd/wire_gen.go

@@ -12,8 +12,8 @@ import (
 	"github.com/navidrome/navidrome/core/agents"
 	"github.com/navidrome/navidrome/core/agents/lastfm"
 	"github.com/navidrome/navidrome/core/agents/listenbrainz"
+	"github.com/navidrome/navidrome/core/ffmpeg"
 	"github.com/navidrome/navidrome/core/scrobbler"
-	"github.com/navidrome/navidrome/core/transcoder"
 	"github.com/navidrome/navidrome/db"
 	"github.com/navidrome/navidrome/persistence"
 	"github.com/navidrome/navidrome/scanner"
@@ -46,8 +46,8 @@ func CreateSubsonicAPIRouter() *subsonic.Router {
 	sqlDB := db.Db()
 	dataStore := persistence.New(sqlDB)
 	fileCache := core.GetImageCache()
-	artwork := core.NewArtwork(dataStore, fileCache)
+	transcoderTranscoder := ffmpeg.New()
-	transcoderTranscoder := transcoder.New()
+	artwork := core.NewArtwork(dataStore, fileCache, transcoderTranscoder)
 	transcodingCache := core.GetTranscodingCache()
 	mediaStreamer := core.NewMediaStreamer(dataStore, transcoderTranscoder, transcodingCache)
 	archiver := core.NewArchiver(mediaStreamer, dataStore)

core/artwork.go

@@ -12,6 +12,8 @@ import (
 	"io"
 	"os"
 	"path/filepath"
+	"reflect"
+	"runtime"
 	"strings"
 	"time"
 
@@ -19,6 +21,7 @@ import (
 	"github.com/disintegration/imaging"
 	"github.com/navidrome/navidrome/conf"
 	"github.com/navidrome/navidrome/consts"
+	"github.com/navidrome/navidrome/core/ffmpeg"
 	"github.com/navidrome/navidrome/log"
 	"github.com/navidrome/navidrome/model"
 	"github.com/navidrome/navidrome/resources"
@@ -31,13 +34,14 @@ type Artwork interface {
 	Get(ctx context.Context, id string, size int) (io.ReadCloser, error)
 }
 
-func NewArtwork(ds model.DataStore, cache cache.FileCache) Artwork {
+func NewArtwork(ds model.DataStore, cache cache.FileCache, ffmpeg ffmpeg.FFmpeg) Artwork {
-	return &artwork{ds: ds, cache: cache}
+	return &artwork{ds: ds, cache: cache, ffmpeg: ffmpeg}
 }
 
 type artwork struct {
-	ds    model.DataStore
+	ds     model.DataStore
-	cache cache.FileCache
+	cache  cache.FileCache
+	ffmpeg ffmpeg.FFmpeg
 }
 
 func (a *artwork) Get(ctx context.Context, id string, size int) (io.ReadCloser, error) {
@@ -95,6 +99,7 @@ func (a *artwork) extractAlbumImage(ctx context.Context, artID model.ArtworkID)
 		fromExternalFile(al.ImageFiles, "albumart.png", "albumart.jpg", "albumart.jpeg", "albumart.webp"),
 		fromExternalFile(al.ImageFiles, "front.png", "front.jpg", "front.jpeg", "front.webp"),
 		fromTag(al.EmbedArtPath),
+		fromFFmpegTag(ctx, a.ffmpeg, al.EmbedArtPath),
 		fromPlaceholder(),
 	)
 }
@@ -112,6 +117,7 @@ func (a *artwork) extractMediaFileImage(ctx context.Context, artID model.Artwork
 
 	return extractImage(ctx, artID,
 		fromTag(mf.Path),
+		fromFFmpegTag(ctx, a.ffmpeg, mf.Path),
 		a.fromAlbum(ctx, mf.AlbumCoverArtID()),
 	)
 }
@@ -135,8 +141,9 @@ func (a *artwork) resizedFromOriginal(ctx context.Context, artID model.ArtworkID
 	usePng := strings.ToLower(filepath.Ext(path)) == ".png"
 	r, err = resizeImage(r, size, usePng)
 	if err != nil {
+		log.Warn("Could not resize image", "artID", artID, "size", size, err)
 		r, path := fromPlaceholder()()
-		return r, path, err
+		return r, path, nil
 	}
 	return r, fmt.Sprintf("%s@%d", path, size), nil
 }
@@ -145,7 +152,7 @@ func extractImage(ctx context.Context, artID model.ArtworkID, extractFuncs ...fu
 	for _, f := range extractFuncs {
 		r, path := f()
 		if r != nil {
-			log.Trace(ctx, "Found artwork", "artID", artID, "path", path)
+			log.Trace(ctx, "Found artwork", "artID", artID, "path", path, "from", getFunctionName(f))
 			return r, path
 		}
 	}
@@ -153,6 +160,13 @@ func extractImage(ctx context.Context, artID model.ArtworkID, extractFuncs ...fu
 	return nil, ""
 }
 
+func getFunctionName(i interface{}) string {
+	name := runtime.FuncForPC(reflect.ValueOf(i).Pointer()).Name()
+	name = strings.TrimPrefix(name, "github.com/navidrome/navidrome/core.")
+	name = strings.TrimSuffix(name, ".func1")
+	return name
+}
+
 // This is a bit unoptimized, but we need to make sure the priority order of validNames
 // is preserved (i.e. png is better than jpg)
 func fromExternalFile(files string, validNames ...string) func() (io.ReadCloser, string) {
@@ -199,6 +213,19 @@ func fromTag(path string) func() (io.ReadCloser, string) {
 	}
 }
 
+func fromFFmpegTag(ctx context.Context, ffmpeg ffmpeg.FFmpeg, path string) func() (io.ReadCloser, string) {
+	return func() (io.ReadCloser, string) {
+		if path == "" {
+			return nil, ""
+		}
+		r, err := ffmpeg.ExtractImage(ctx, path)
+		if err != nil {
+			return nil, ""
+		}
+		return r, path
+	}
+}
+
 func fromPlaceholder() func() (io.ReadCloser, string) {
 	return func() (io.ReadCloser, string) {
 		r, _ := resources.FS().Open(consts.PlaceholderAlbumArt)

core/artwork_internal_test.go

@@ -2,6 +2,7 @@ package core
 
 import (
 	"context"
+	"errors"
 	"image"
 
 	"github.com/navidrome/navidrome/conf"
@@ -17,6 +18,7 @@ import (
 var _ = Describe("Artwork", func() {
 	var aw *artwork
 	var ds model.DataStore
+	var ffmpeg *tests.MockFFmpeg
 	ctx := log.NewContext(context.TODO())
 	var alOnlyEmbed, alEmbedNotFound, alOnlyExternal, alExternalNotFound, alAllOptions model.Album
 	var mfWithEmbed, mfWithoutEmbed, mfCorruptedCover model.MediaFile
@@ -38,7 +40,8 @@ var _ = Describe("Artwork", func() {
 		conf.Server.ImageCacheSize = "0" // Disable cache
 
 		cache := GetImageCache()
-		aw = NewArtwork(ds, cache).(*artwork)
+		ffmpeg = tests.NewMockFFmpeg("")
+		aw = NewArtwork(ds, cache, ffmpeg).(*artwork)
 	})
 
 	Context("Empty ID", func() {
@@ -70,6 +73,7 @@ var _ = Describe("Artwork", func() {
 				Expect(path).To(Equal("tests/fixtures/test.mp3"))
 			})
 			It("returns placeholder if embed path is not available", func() {
+				ffmpeg.Error = errors.New("not available")
 				_, path, err := aw.get(context.Background(), alEmbedNotFound.CoverArtID(), 0)
 				Expect(err).ToNot(HaveOccurred())
 				Expect(path).To(Equal(consts.PlaceholderAlbumArt))
@@ -124,13 +128,19 @@ var _ = Describe("Artwork", func() {
 				Expect(err).ToNot(HaveOccurred())
 				Expect(path).To(Equal("tests/fixtures/test.mp3"))
 			})
-			It("returns album cover if media file has no cover art", func() {
+			It("returns embed cover if successfully extracted by ffmpeg", func() {
-				_, path, err := aw.get(context.Background(), mfWithoutEmbed.CoverArtID(), 0)
+				_, path, err := aw.get(context.Background(), mfCorruptedCover.CoverArtID(), 0)
+				Expect(err).ToNot(HaveOccurred())
+				Expect(path).To(Equal("tests/fixtures/test.ogg"))
+			})
+			It("returns album cover if cannot read embed artwork", func() {
+				ffmpeg.Error = errors.New("not available")
+				_, path, err := aw.get(context.Background(), mfCorruptedCover.CoverArtID(), 0)
 				Expect(err).ToNot(HaveOccurred())
 				Expect(path).To(Equal("tests/fixtures/front.png"))
 			})
-			It("returns album cover if cannot read embed artwork", func() {
+			It("returns album cover if media file has no cover art", func() {
-				_, path, err := aw.get(context.Background(), mfCorruptedCover.CoverArtID(), 0)
+				_, path, err := aw.get(context.Background(), mfWithoutEmbed.CoverArtID(), 0)
 				Expect(err).ToNot(HaveOccurred())
 				Expect(path).To(Equal("tests/fixtures/front.png"))
 			})

core/ffmpeg/ffmpeg.go

@@ -1,4 +1,4 @@
-package transcoder
+package ffmpeg
 
 import (
 	"context"
@@ -13,19 +13,32 @@ import (
 	"github.com/navidrome/navidrome/log"
 )
 
-type Transcoder interface {
+type FFmpeg interface {
-	Start(ctx context.Context, command, path string, maxBitRate int) (io.ReadCloser, error)
+	Transcode(ctx context.Context, command, path string, maxBitRate int) (io.ReadCloser, error)
+	ExtractImage(ctx context.Context, path string) (io.ReadCloser, error)
+	// TODO Move scanner ffmpeg probe to here
 }
 
-func New() Transcoder {
+func New() FFmpeg {
-	return &externalTranscoder{}
+	return &ffmpeg{}
 }
 
-type externalTranscoder struct{}
+const extractImageCmd = "ffmpeg -i %s -an -vcodec copy -f image2pipe -"
 
-func (e *externalTranscoder) Start(ctx context.Context, command, path string, maxBitRate int) (io.ReadCloser, error) {
+type ffmpeg struct{}
-	args := createTranscodeCommand(command, path, maxBitRate)
+
-	log.Trace(ctx, "Executing transcoding command", "cmd", args)
+func (e *ffmpeg) Transcode(ctx context.Context, command, path string, maxBitRate int) (io.ReadCloser, error) {
+	args := createFFmpegCommand(command, path, maxBitRate)
+	return e.start(ctx, args)
+}
+
+func (e *ffmpeg) ExtractImage(ctx context.Context, path string) (io.ReadCloser, error) {
+	args := createFFmpegCommand(extractImageCmd, path, 0)
+	return e.start(ctx, args)
+}
+
+func (e *ffmpeg) start(ctx context.Context, args []string) (io.ReadCloser, error) {
+	log.Trace(ctx, "Executing ffmpeg command", "cmd", args)
 	j := &Cmd{ctx: ctx, args: args}
 	j.PipeReader, j.out = io.Pipe()
 	err := j.start()
@@ -47,7 +60,11 @@ type Cmd struct {
 func (j *Cmd) start() error {
 	cmd := exec.CommandContext(j.ctx, j.args[0], j.args[1:]...) // #nosec
 	cmd.Stdout = j.out
-	cmd.Stderr = os.Stderr
+	if log.CurrentLevel() >= log.LevelTrace {
+		cmd.Stderr = os.Stderr
+	} else {
+		cmd.Stderr = io.Discard
+	}
 	j.cmd = cmd
 
 	if err := cmd.Start(); err != nil {
@@ -74,7 +91,7 @@ func (j *Cmd) wait() {
 }
 
 // Path will always be an absolute path
-func createTranscodeCommand(cmd, path string, maxBitRate int) []string {
+func createFFmpegCommand(cmd, path string, maxBitRate int) []string {
 	split := strings.Split(cmd, " ")
 	for i, s := range split {
 		s = strings.ReplaceAll(s, "%s", path)

core/ffmpeg/ffmpeg_test.go

@@ -1,4 +1,4 @@
-package transcoder
+package ffmpeg
 
 import (
 	"testing"
@@ -9,16 +9,16 @@ import (
 	. "github.com/onsi/gomega"
 )
 
-func TestTranscoder(t *testing.T) {
+func TestFFmpeg(t *testing.T) {
 	tests.Init(t, false)
 	log.SetLevel(log.LevelFatal)
 	RegisterFailHandler(Fail)
-	RunSpecs(t, "Transcoder Suite")
+	RunSpecs(t, "FFmpeg Suite")
 }
 
-var _ = Describe("createTranscodeCommand", func() {
+var _ = Describe("createFFmpegCommand", func() {
 	It("creates a valid command line", func() {
-		args := createTranscodeCommand("ffmpeg -i %s -b:a %bk mp3 -", "/music library/file.mp3", 123)
+		args := createFFmpegCommand("ffmpeg -i %s -b:a %bk mp3 -", "/music library/file.mp3", 123)
 		Expect(args).To(Equal([]string{"ffmpeg", "-i", "/music library/file.mp3", "-b:a", "123k", "mp3", "-"}))
 	})
 })

core/media_streamer.go

@@ -11,7 +11,7 @@ import (
 
 	"github.com/navidrome/navidrome/conf"
 	"github.com/navidrome/navidrome/consts"
-	"github.com/navidrome/navidrome/core/transcoder"
+	"github.com/navidrome/navidrome/core/ffmpeg"
 	"github.com/navidrome/navidrome/log"
 	"github.com/navidrome/navidrome/model"
 	"github.com/navidrome/navidrome/model/request"
@@ -25,13 +25,13 @@ type MediaStreamer interface {
 
 type TranscodingCache cache.FileCache
 
-func NewMediaStreamer(ds model.DataStore, t transcoder.Transcoder, cache TranscodingCache) MediaStreamer {
+func NewMediaStreamer(ds model.DataStore, t ffmpeg.FFmpeg, cache TranscodingCache) MediaStreamer {
 	return &mediaStreamer{ds: ds, transcoder: t, cache: cache}
 }
 
 type mediaStreamer struct {
 	ds         model.DataStore
-	transcoder transcoder.Transcoder
+	transcoder ffmpeg.FFmpeg
 	cache      cache.FileCache
 }
 
@@ -191,7 +191,7 @@ func GetTranscodingCache() TranscodingCache {
 					log.Error(ctx, "Error loading transcoding command", "format", job.format, err)
 					return nil, os.ErrInvalid
 				}
-				out, err := job.ms.transcoder.Start(ctx, t.Command, job.mf.Path, job.bitRate)
+				out, err := job.ms.transcoder.Transcode(ctx, t.Command, job.mf.Path, job.bitRate)
 				if err != nil {
 					log.Error(ctx, "Error starting transcoder", "id", job.mf.ID, err)
 					return nil, os.ErrInvalid

core/media_streamer_test.go

@@ -4,24 +4,21 @@ import (
 	"context"
 	"io"
 	"os"
-	"strings"
-	"sync"
 
 	"github.com/navidrome/navidrome/conf"
 	"github.com/navidrome/navidrome/conf/configtest"
-	. "github.com/navidrome/navidrome/core"
+	"github.com/navidrome/navidrome/core"
 	"github.com/navidrome/navidrome/log"
 	"github.com/navidrome/navidrome/model"
 	"github.com/navidrome/navidrome/tests"
-	"github.com/navidrome/navidrome/utils"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 )
 
 var _ = Describe("MediaStreamer", func() {
-	var streamer MediaStreamer
+	var streamer core.MediaStreamer
 	var ds model.DataStore
-	ffmpeg := newFakeFFmpeg("fake data")
+	ffmpeg := tests.NewMockFFmpeg("fake data")
 	ctx := log.NewContext(context.TODO())
 
 	BeforeEach(func() {
@@ -32,9 +29,9 @@ var _ = Describe("MediaStreamer", func() {
 		ds.MediaFile(ctx).(*tests.MockMediaFileRepo).SetData(model.MediaFiles{
 			{ID: "123", Path: "tests/fixtures/test.mp3", Suffix: "mp3", BitRate: 128, Duration: 257.0},
 		})
-		testCache := GetTranscodingCache()
+		testCache := core.GetTranscodingCache()
 		Eventually(func() bool { return testCache.Ready(context.TODO()) }).Should(BeTrue())
-		streamer = NewMediaStreamer(ds, ffmpeg, testCache)
+		streamer = core.NewMediaStreamer(ds, ffmpeg, testCache)
 	})
 	AfterEach(func() {
 		_ = os.RemoveAll(conf.Server.DataFolder)
@@ -75,32 +72,3 @@ var _ = Describe("MediaStreamer", func() {
 		})
 	})
 })
-
-func newFakeFFmpeg(data string) *fakeFFmpeg {
-	return &fakeFFmpeg{Reader: strings.NewReader(data)}
-}
-
-type fakeFFmpeg struct {
-	io.Reader
-	lock   sync.Mutex
-	closed utils.AtomicBool
-}
-
-func (ff *fakeFFmpeg) Start(ctx context.Context, cmd, path string, maxBitRate int) (f io.ReadCloser, err error) {
-	return ff, nil
-}
-
-func (ff *fakeFFmpeg) Read(p []byte) (n int, err error) {
-	ff.lock.Lock()
-	defer ff.lock.Unlock()
-	return ff.Reader.Read(p)
-}
-
-func (ff *fakeFFmpeg) Close() error {
-	ff.closed.Set(true)
-	return nil
-}
-
-func (ff *fakeFFmpeg) IsClosed() bool {
-	return ff.closed.Get()
-}

core/wire_providers.go

@@ -3,8 +3,8 @@ package core
 import (
 	"github.com/google/wire"
 	"github.com/navidrome/navidrome/core/agents"
+	"github.com/navidrome/navidrome/core/ffmpeg"
 	"github.com/navidrome/navidrome/core/scrobbler"
-	"github.com/navidrome/navidrome/core/transcoder"
 )
 
 var Set = wire.NewSet(
@@ -16,7 +16,7 @@ var Set = wire.NewSet(
 	NewExternalMetadata,
 	NewPlayers,
 	agents.New,
-	transcoder.New,
+	ffmpeg.New,
 	scrobbler.GetPlayTracker,
 	NewShare,
 	NewPlaylists,

server/subsonic/api.go

@@ -185,7 +185,7 @@ func hr(r chi.Router, path string, f handlerRaw) {
 				if errors.Is(err, model.ErrNotFound) {
 					err = newError(responses.ErrorDataNotFound, "data not found")
 				} else {
-					err = newError(responses.ErrorGeneric, "Internal Error")
+					err = newError(responses.ErrorGeneric, fmt.Sprintf("Internal Server Error: %s", err))
 				}
 			}
 			sendError(w, r, err)

tests/mock_ffmpeg.go

@@ -0,0 +1,52 @@
+package tests
+
+import (
+	"context"
+	"io"
+	"strings"
+	"sync"
+
+	"github.com/navidrome/navidrome/consts"
+	"github.com/navidrome/navidrome/resources"
+	"github.com/navidrome/navidrome/utils"
+)
+
+func NewMockFFmpeg(data string) *MockFFmpeg {
+	return &MockFFmpeg{Reader: strings.NewReader(data)}
+}
+
+type MockFFmpeg struct {
+	io.Reader
+	lock   sync.Mutex
+	closed utils.AtomicBool
+	Error  error
+}
+
+func (ff *MockFFmpeg) Transcode(ctx context.Context, cmd, path string, maxBitRate int) (f io.ReadCloser, err error) {
+	if ff.Error != nil {
+		return nil, ff.Error
+	}
+	return ff, nil
+}
+
+func (ff *MockFFmpeg) ExtractImage(ctx context.Context, path string) (io.ReadCloser, error) {
+	if ff.Error != nil {
+		return nil, ff.Error
+	}
+	return resources.FS().Open(consts.PlaceholderAlbumArt)
+}
+
+func (ff *MockFFmpeg) Read(p []byte) (n int, err error) {
+	ff.lock.Lock()
+	defer ff.lock.Unlock()
+	return ff.Reader.Read(p)
+}
+
+func (ff *MockFFmpeg) Close() error {
+	ff.closed.Set(true)
+	return nil
+}
+
+func (ff *MockFFmpeg) IsClosed() bool {
+	return ff.closed.Get()
+}