diff --git a/cmd/root.go b/cmd/root.go index 9618b16e6..6ae24beb5 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 7292c7dfe..c8b0251a0 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 @@ -489,6 +490,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..4f089ab83 100644 --- a/core/ffmpeg/ffmpeg_test.go +++ b/core/ffmpeg/ffmpeg_test.go @@ -1,7 +1,11 @@ package ffmpeg import ( + "context" + "io" + "os/exec" "testing" + "time" "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/tests" @@ -65,4 +69,114 @@ var _ = Describe("ffmpeg", func() { Expect(args).To(Equal([]string{"/usr/bin/with spaces/ffmpeg.exe", "-i", "one.mp3", "-f", "ffmetadata"})) }) }) + + Describe("Context Cancellation", func() { + Context("when FFmpeg is available", func() { + var ff FFmpeg + + BeforeEach(func() { + 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.WithCancel(context.Background()) + defer cancel() + + // Start a long-running FFmpeg process (slow transcode to give time for cancellation) + // Use a command that will take some time to complete + command := "ffmpeg -f lavfi -i sine=frequency=1000:duration=30 -acodec pcm_s16le -" + + errChan := make(chan error, 1) + var stream io.ReadCloser + + go func() { + var err error + // 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) + errChan <- err + }() + + // Give FFmpeg a moment to start + time.Sleep(100 * time.Millisecond) + + // Cancel the context + cancel() + + // The operation should fail due to cancellation + select { + case err := <-errChan: + if err == nil { + // If no error during start, the stream should be cancelled when we try to read + if stream != nil { + defer stream.Close() + buf := make([]byte, 1024) + _, readErr := stream.Read(buf) + Expect(readErr).To(HaveOccurred(), "Expected read to fail due to cancelled context") + } + } else { + // Starting should fail due to cancelled context + Expect(err).To(HaveOccurred()) + } + case <-time.After(5 * time.Second): + Fail("Expected FFmpeg to be cancelled within 5 seconds") + } + }) + + It("should handle immediate context cancellation", func() { + ctx, cancel := context.WithCancel(context.Background()) + 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 originalFfmpegPath string + + BeforeEach(func() { + originalFfmpegPath = ffmpegPath + // Use a long-running command for testing cancellation + ffmpegPath = "sleep" + }) + + AfterEach(func() { + ffmpegPath = originalFfmpegPath + }) + + It("should terminate the underlying process when context is cancelled", func() { + ff := New() + ctx, cancel := context.WithCancel(context.Background()) + + // Start a process that will run for a while + stream, err := ff.Transcode(ctx, "sleep 10", "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") + + // Give some time for cleanup + time.Sleep(100 * time.Millisecond) + + // Verify no sleep processes are left running + checkCmd := exec.Command("pgrep", "-f", "sleep 10") + err = checkCmd.Run() + Expect(err).To(HaveOccurred(), "Expected no 'sleep 10' processes to be running") + }) + }) + }) }) 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