From 759ff844e2d0b89661b6c5c83c5609d3f859bbc5 Mon Sep 17 00:00:00 2001 From: Deluan Date: Tue, 7 Feb 2023 13:08:25 -0500 Subject: [PATCH] Make ffmpeg path configurable, also finds it automatically in current folder. Fixes #1932 --- conf/configuration.go | 4 +- core/ffmpeg/ffmpeg.go | 89 +++++++++++++++++++++++++- core/ffmpeg/ffmpeg_test.go | 22 +++++-- persistence/mediafile_repository.go | 2 +- scanner/metadata/ffmpeg/ffmpeg.go | 38 ++++------- scanner/metadata/ffmpeg/ffmpeg_test.go | 5 -- server/initial_setup.go | 6 +- tests/mock_ffmpeg.go | 17 ++++- 8 files changed, 137 insertions(+), 46 deletions(-) diff --git a/conf/configuration.go b/conf/configuration.go index 24d33cd4..74c3cfc4 100644 --- a/conf/configuration.go +++ b/conf/configuration.go @@ -47,7 +47,7 @@ type configOptions struct { IgnoredArticles string IndexGroups string SubsonicArtistParticipations bool - ProbeCommand string + FFmpegPath string CoverArtPriority string CoverJpegQuality int EnableGravatar bool @@ -246,7 +246,7 @@ func init() { viper.SetDefault("ignoredarticles", "The El La Los Las Le Les Os As O A") viper.SetDefault("indexgroups", "A B C D E F G H I J K L M N O P Q R S T U V W X-Z(XYZ) [Unknown]([)") viper.SetDefault("subsonicartistparticipations", false) - viper.SetDefault("probecommand", "ffmpeg %s -f ffmetadata") + viper.SetDefault("ffmpegpath", "") viper.SetDefault("coverartpriority", "cover.*, folder.*, front.*, embedded, external") viper.SetDefault("coverjpegquality", 75) viper.SetDefault("enablegravatar", false) diff --git a/core/ffmpeg/ffmpeg.go b/core/ffmpeg/ffmpeg.go index bf6dafac..4085d56c 100644 --- a/core/ffmpeg/ffmpeg.go +++ b/core/ffmpeg/ffmpeg.go @@ -9,34 +9,61 @@ import ( "os/exec" "strconv" "strings" + "sync" + "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/log" ) type FFmpeg interface { 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 + Probe(ctx context.Context, files []string) (string, error) + CmdPath() (string, error) } func New() FFmpeg { return &ffmpeg{} } -const extractImageCmd = "ffmpeg -i %s -an -vcodec copy -f image2pipe -" +const ( + extractImageCmd = "ffmpeg -i %s -an -vcodec copy -f image2pipe -" + probeCmd = "ffmpeg %s -f ffmetadata" +) type ffmpeg struct{} func (e *ffmpeg) Transcode(ctx context.Context, command, path string, maxBitRate int) (io.ReadCloser, error) { + if _, err := ffmpegCmd(); err != nil { + return nil, err + } args := createFFmpegCommand(command, path, maxBitRate) return e.start(ctx, args) } func (e *ffmpeg) ExtractImage(ctx context.Context, path string) (io.ReadCloser, error) { + if _, err := ffmpegCmd(); err != nil { + return nil, err + } args := createFFmpegCommand(extractImageCmd, path, 0) return e.start(ctx, args) } +func (e *ffmpeg) Probe(ctx context.Context, files []string) (string, error) { + if _, err := ffmpegCmd(); err != nil { + return "", err + } + args := createProbeCommand(probeCmd, files) + log.Trace(ctx, "Executing ffmpeg command", "args", args) + cmd := exec.CommandContext(ctx, args[0], args[1:]...) // #nosec + output, _ := cmd.CombinedOutput() + return string(output), nil +} + +func (e *ffmpeg) CmdPath() (string, error) { + return ffmpegCmd() +} + func (e *ffmpeg) start(ctx context.Context, args []string) (io.ReadCloser, error) { log.Trace(ctx, "Executing ffmpeg command", "cmd", args) j := &ffCmd{args: args} @@ -87,7 +114,7 @@ func (j *ffCmd) wait() { // Path will always be an absolute path func createFFmpegCommand(cmd, path string, maxBitRate int) []string { - split := strings.Split(cmd, " ") + split := strings.Split(fixCmd(cmd), " ") for i, s := range split { s = strings.ReplaceAll(s, "%s", path) s = strings.ReplaceAll(s, "%b", strconv.Itoa(maxBitRate)) @@ -96,3 +123,59 @@ func createFFmpegCommand(cmd, path string, maxBitRate int) []string { return split } + +func createProbeCommand(cmd string, inputs []string) []string { + split := strings.Split(fixCmd(cmd), " ") + var args []string + + for _, s := range split { + if s == "%s" { + for _, inp := range inputs { + args = append(args, "-i", inp) + } + } else { + args = append(args, s) + } + } + return args +} + +func fixCmd(cmd string) string { + split := strings.Split(cmd, " ") + var result []string + cmdPath, _ := ffmpegCmd() + for _, s := range split { + if s == "ffmpeg" || s == "ffmpeg.exe" { + result = append(result, cmdPath) + } else { + result = append(result, s) + } + } + return strings.Join(result, " ") +} + +func ffmpegCmd() (string, error) { + ffOnce.Do(func() { + if conf.Server.FFmpegPath != "" { + ffmpegPath = conf.Server.FFmpegPath + ffmpegPath, ffmpegErr = exec.LookPath(ffmpegPath) + } else { + ffmpegPath, ffmpegErr = exec.LookPath("ffmpeg") + if errors.Is(ffmpegErr, exec.ErrDot) { + log.Trace("ffmpeg found in current folder '.'") + ffmpegPath, ffmpegErr = exec.LookPath("./ffmpeg") + } + } + if ffmpegErr == nil { + log.Info("Found ffmpeg", "path", ffmpegPath) + return + } + }) + return ffmpegPath, ffmpegErr +} + +var ( + ffOnce sync.Once + ffmpegPath string + ffmpegErr error +) diff --git a/core/ffmpeg/ffmpeg_test.go b/core/ffmpeg/ffmpeg_test.go index be8dad3b..ea8ea6a7 100644 --- a/core/ffmpeg/ffmpeg_test.go +++ b/core/ffmpeg/ffmpeg_test.go @@ -16,9 +16,23 @@ func TestFFmpeg(t *testing.T) { RunSpecs(t, "FFmpeg Suite") } -var _ = Describe("createFFmpegCommand", func() { - It("creates a valid command line", func() { - 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", "-"})) +var _ = Describe("ffmpeg", func() { + BeforeEach(func() { + _, _ = ffmpegCmd() + ffmpegPath = "ffmpeg" + ffmpegErr = nil + }) + Describe("createFFmpegCommand", func() { + It("creates a valid command line", func() { + 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", "-"})) + }) + }) + + Describe("createProbeCommand", func() { + It("creates a valid command line", func() { + args := createProbeCommand(probeCmd, []string{"/music library/one.mp3", "/music library/two.mp3"}) + Expect(args).To(Equal([]string{"ffmpeg", "-i", "/music library/one.mp3", "-i", "/music library/two.mp3", "-f", "ffmetadata"})) + }) }) }) diff --git a/persistence/mediafile_repository.go b/persistence/mediafile_repository.go index 9c93068a..55295888 100644 --- a/persistence/mediafile_repository.go +++ b/persistence/mediafile_repository.go @@ -179,7 +179,7 @@ func (r *mediaFileRepository) DeleteByPath(basePath string) (int64, error) { func (r *mediaFileRepository) removeNonAlbumArtistIds() error { upd := Update(r.tableName).Set("artist_id", "").Where(notExists("artist", ConcatExpr("id = artist_id"))) - log.Debug(r.ctx, "Removing non-album artist_id") + log.Debug(r.ctx, "Removing non-album artist_ids") _, err := r.executeSQL(upd) return err } diff --git a/scanner/metadata/ffmpeg/ffmpeg.go b/scanner/metadata/ffmpeg/ffmpeg.go index bbccf164..b23ab8a0 100644 --- a/scanner/metadata/ffmpeg/ffmpeg.go +++ b/scanner/metadata/ffmpeg/ffmpeg.go @@ -2,33 +2,35 @@ package ffmpeg import ( "bufio" + "context" "errors" - "os/exec" "regexp" "strconv" "strings" "time" - "github.com/navidrome/navidrome/conf" + "github.com/navidrome/navidrome/core/ffmpeg" "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/scanner/metadata" ) const ExtractorID = "ffmpeg" -type Extractor struct{} +type Extractor struct { + ffmpeg ffmpeg.FFmpeg +} func (e *Extractor) Parse(files ...string) (map[string]metadata.ParsedTags, error) { - args := e.createProbeCommand(files) - - log.Trace("Executing command", "args", args) - cmd := exec.Command(args[0], args[1:]...) // #nosec - output, _ := cmd.CombinedOutput() + output, err := e.ffmpeg.Probe(context.TODO(), files) + if err != nil { + log.Error("Cannot use ffmpeg to extract tags. Aborting", err) + return nil, err + } fileTags := map[string]metadata.ParsedTags{} if len(output) == 0 { return fileTags, errors.New("error extracting metadata files") } - infos := e.parseOutput(string(output)) + infos := e.parseOutput(output) for file, info := range infos { tags, err := e.extractMetadata(file, info) // Skip files with errors @@ -197,22 +199,6 @@ func (e *Extractor) parseChannels(tag string) string { } // Inputs will always be absolute paths -func (e *Extractor) createProbeCommand(inputs []string) []string { - split := strings.Split(conf.Server.ProbeCommand, " ") - args := make([]string, 0) - - for _, s := range split { - if s == "%s" { - for _, inp := range inputs { - args = append(args, "-i", inp) - } - } else { - args = append(args, s) - } - } - return args -} - func init() { - metadata.RegisterExtractor(ExtractorID, &Extractor{}) + metadata.RegisterExtractor(ExtractorID, &Extractor{ffmpeg: ffmpeg.New()}) } diff --git a/scanner/metadata/ffmpeg/ffmpeg_test.go b/scanner/metadata/ffmpeg/ffmpeg_test.go index 67a3b2b4..7f1c63bf 100644 --- a/scanner/metadata/ffmpeg/ffmpeg_test.go +++ b/scanner/metadata/ffmpeg/ffmpeg_test.go @@ -280,11 +280,6 @@ Input #0, mp3, from '/Users/deluan/Music/Music/Media/_/Wyclef Jean - From the Hu }) }) - It("creates a valid command line", func() { - args := e.createProbeCommand([]string{"/music library/one.mp3", "/music library/two.mp3"}) - Expect(args).To(Equal([]string{"ffmpeg", "-i", "/music library/one.mp3", "-i", "/music library/two.mp3", "-f", "ffmetadata"})) - }) - It("parses an integer TBPM tag", func() { const output = ` Input #0, mp3, from 'tests/fixtures/test.mp3': diff --git a/server/initial_setup.go b/server/initial_setup.go index 53aeca2b..39f6f388 100644 --- a/server/initial_setup.go +++ b/server/initial_setup.go @@ -3,12 +3,12 @@ package server import ( "context" "fmt" - "os/exec" "time" "github.com/google/uuid" "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" ) @@ -77,9 +77,9 @@ func createJWTSecret(ds model.DataStore) error { } func checkFfmpegInstallation() { - path, err := exec.LookPath("ffmpeg") + f := ffmpeg.New() + _, err := f.CmdPath() if err == nil { - log.Info("Found ffmpeg", "path", path) return } log.Warn("Unable to find ffmpeg. Transcoding will fail if used", err) diff --git a/tests/mock_ffmpeg.go b/tests/mock_ffmpeg.go index aa21acae..9431c157 100644 --- a/tests/mock_ffmpeg.go +++ b/tests/mock_ffmpeg.go @@ -20,20 +20,33 @@ type MockFFmpeg struct { Error error } -func (ff *MockFFmpeg) Transcode(ctx context.Context, cmd, path string, maxBitRate int) (f io.ReadCloser, err error) { +func (ff *MockFFmpeg) Transcode(_ context.Context, _, _ string, _ 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) { +func (ff *MockFFmpeg) ExtractImage(context.Context, string) (io.ReadCloser, error) { if ff.Error != nil { return nil, ff.Error } return ff, nil } +func (ff *MockFFmpeg) Probe(context.Context, []string) (string, error) { + if ff.Error != nil { + return "", ff.Error + } + return "", nil +} +func (ff *MockFFmpeg) CmdPath() (string, error) { + if ff.Error != nil { + return "", ff.Error + } + return "ffmpeg", nil +} + func (ff *MockFFmpeg) Read(p []byte) (n int, err error) { ff.lock.Lock() defer ff.lock.Unlock()