diff --git a/cmd/root.go b/cmd/root.go index 4a1305cad..5e91ecd5f 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -374,6 +374,7 @@ func init() { rootCmd.Flags().Duration("scaninterval", viper.GetDuration("scaninterval"), "how frequently to scan for changes in your music library") rootCmd.Flags().String("uiloginbackgroundurl", viper.GetString("uiloginbackgroundurl"), "URL to a backaground image used in the Login page") rootCmd.Flags().Bool("enabletranscodingconfig", viper.GetBool("enabletranscodingconfig"), "enables transcoding configuration in the UI") + rootCmd.Flags().Bool("enabletranscodingcancellation", viper.GetBool("enabletranscodingcancellation"), "enables transcoding context cancellation") rootCmd.Flags().String("transcodingcachesize", viper.GetString("transcodingcachesize"), "size of transcoding cache") rootCmd.Flags().String("imagecachesize", viper.GetString("imagecachesize"), "size of image (art work) cache. set to 0 to disable cache") rootCmd.Flags().String("albumplaycountmode", viper.GetString("albumplaycountmode"), "how to compute playcount for albums. absolute (default) or normalized") @@ -397,6 +398,7 @@ func init() { _ = viper.BindPFlag("prometheus.metricspath", rootCmd.Flags().Lookup("prometheus.metricspath")) _ = viper.BindPFlag("enabletranscodingconfig", rootCmd.Flags().Lookup("enabletranscodingconfig")) + _ = viper.BindPFlag("enabletranscodingcancellation", rootCmd.Flags().Lookup("enabletranscodingcancellation")) _ = viper.BindPFlag("transcodingcachesize", rootCmd.Flags().Lookup("transcodingcachesize")) _ = viper.BindPFlag("imagecachesize", rootCmd.Flags().Lookup("imagecachesize")) } diff --git a/conf/configuration.go b/conf/configuration.go index f6b1c4cb7..f8e4a8084 100644 --- a/conf/configuration.go +++ b/conf/configuration.go @@ -41,6 +41,7 @@ type configOptions struct { UIWelcomeMessage string MaxSidebarPlaylists int EnableTranscodingConfig bool + EnableTranscodingCancellation bool EnableDownloads bool EnableExternalServices bool EnableInsightsCollector bool @@ -492,6 +493,7 @@ func setViperDefaults() { viper.SetDefault("uiwelcomemessage", "") viper.SetDefault("maxsidebarplaylists", consts.DefaultMaxSidebarPlaylists) viper.SetDefault("enabletranscodingconfig", false) + viper.SetDefault("enabletranscodingcancellation", false) viper.SetDefault("transcodingcachesize", "100MB") viper.SetDefault("imagecachesize", "100MB") viper.SetDefault("albumplaycountmode", consts.AlbumPlayCountModeAbsolute) diff --git a/core/ffmpeg/ffmpeg.go b/core/ffmpeg/ffmpeg.go index 2e0d5a4b7..d134077ce 100644 --- a/core/ffmpeg/ffmpeg.go +++ b/core/ffmpeg/ffmpeg.go @@ -112,7 +112,7 @@ func (e *ffmpeg) start(ctx context.Context, args []string) (io.ReadCloser, error log.Trace(ctx, "Executing ffmpeg command", "cmd", args) j := &ffCmd{args: args} j.PipeReader, j.out = io.Pipe() - err := j.start() + err := j.start(ctx) if err != nil { return nil, err } @@ -127,8 +127,8 @@ type ffCmd struct { cmd *exec.Cmd } -func (j *ffCmd) start() error { - cmd := exec.Command(j.args[0], j.args[1:]...) // #nosec +func (j *ffCmd) start(ctx context.Context) error { + cmd := exec.CommandContext(ctx, j.args[0], j.args[1:]...) // #nosec cmd.Stdout = j.out if log.IsGreaterOrEqualTo(log.LevelTrace) { cmd.Stderr = os.Stderr diff --git a/core/ffmpeg/ffmpeg_test.go b/core/ffmpeg/ffmpeg_test.go index 7e67a2a6a..debe0b51e 100644 --- a/core/ffmpeg/ffmpeg_test.go +++ b/core/ffmpeg/ffmpeg_test.go @@ -1,7 +1,11 @@ package ffmpeg import ( + "context" + "runtime" + sync "sync" "testing" + "time" "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/tests" @@ -65,4 +69,98 @@ var _ = Describe("ffmpeg", func() { Expect(args).To(Equal([]string{"/usr/bin/with spaces/ffmpeg.exe", "-i", "one.mp3", "-f", "ffmetadata"})) }) }) + + Describe("FFmpeg", func() { + Context("when FFmpeg is available", func() { + var ff FFmpeg + + BeforeEach(func() { + ffOnce = sync.Once{} + ff = New() + // Skip if FFmpeg is not available + if !ff.IsAvailable() { + Skip("FFmpeg not available on this system") + } + }) + + It("should interrupt transcoding when context is cancelled", func() { + ctx, cancel := context.WithTimeout(GinkgoT().Context(), 5*time.Second) + defer cancel() + + // Use a command that generates audio indefinitely + // -f lavfi uses FFmpeg's built-in audio source + // -t 0 means no time limit (runs forever) + command := "ffmpeg -f lavfi -i sine=frequency=1000:duration=0 -f mp3 -" + + // The input file is not used here, but we need to provide a valid path to the Transcode function + stream, err := ff.Transcode(ctx, command, "tests/fixtures/test.mp3", 128, 0) + Expect(err).ToNot(HaveOccurred()) + defer stream.Close() + + // Read some data first to ensure FFmpeg is running + buf := make([]byte, 1024) + _, err = stream.Read(buf) + Expect(err).ToNot(HaveOccurred()) + + // Cancel the context + cancel() + + // Next read should fail due to cancelled context + _, err = stream.Read(buf) + Expect(err).To(HaveOccurred()) + }) + + It("should handle immediate context cancellation", func() { + ctx, cancel := context.WithCancel(GinkgoT().Context()) + cancel() // Cancel immediately + + // This should fail immediately + _, err := ff.Transcode(ctx, "ffmpeg -i %s -f mp3 -", "tests/fixtures/test.mp3", 128, 0) + Expect(err).To(MatchError(context.Canceled)) + }) + }) + + Context("with mock process behavior", func() { + var longRunningCmd string + BeforeEach(func() { + // Use a long-running command for testing cancellation + switch runtime.GOOS { + case "windows": + // Use PowerShell's Start-Sleep + ffmpegPath = "powershell" + longRunningCmd = "powershell -Command Start-Sleep -Seconds 10" + default: + // Use sleep on Unix-like systems + ffmpegPath = "sleep" + longRunningCmd = "sleep 10" + } + }) + + It("should terminate the underlying process when context is cancelled", func() { + ff := New() + ctx, cancel := context.WithTimeout(GinkgoT().Context(), 5*time.Second) + defer cancel() + + // Start a process that will run for a while + stream, err := ff.Transcode(ctx, longRunningCmd, "tests/fixtures/test.mp3", 0, 0) + Expect(err).ToNot(HaveOccurred()) + defer stream.Close() + + // Give the process time to start + time.Sleep(50 * time.Millisecond) + + // Cancel the context + cancel() + + // Try to read from the stream, which should fail + buf := make([]byte, 100) + _, err = stream.Read(buf) + Expect(err).To(HaveOccurred(), "Expected stream to be closed due to process termination") + + // Verify the stream is closed by attempting another read + _, err = stream.Read(buf) + Expect(err).To(HaveOccurred()) + }) + }) + }) }) diff --git a/core/media_streamer.go b/core/media_streamer.go index b3593c4eb..c741ed476 100644 --- a/core/media_streamer.go +++ b/core/media_streamer.go @@ -204,7 +204,20 @@ func NewTranscodingCache() TranscodingCache { log.Error(ctx, "Error loading transcoding command", "format", job.format, err) return nil, os.ErrInvalid } - out, err := job.ms.transcoder.Transcode(ctx, t.Command, job.filePath, job.bitRate, job.offset) + + // Choose the appropriate context based on EnableTranscodingCancellation configuration. + // This is where we decide whether transcoding processes should be cancellable or not. + var transcodingCtx context.Context + if conf.Server.EnableTranscodingCancellation { + // Use the request context directly, allowing cancellation when client disconnects + transcodingCtx = ctx + } else { + // Use background context with request values preserved. + // This prevents cancellation but maintains request metadata (user, client, etc.) + transcodingCtx = request.AddValues(context.Background(), ctx) + } + + out, err := job.ms.transcoder.Transcode(transcodingCtx, t.Command, job.filePath, job.bitRate, job.offset) if err != nil { log.Error(ctx, "Error starting transcoder", "id", job.mf.ID, err) return nil, os.ErrInvalid