Compare commits

...

5 Commits

Author SHA1 Message Date
Deluan Quintão
0cdfff43c2
Merge 578375ca7132e1b3b7b4fd96fa5887cfed652bd0 into e86dc03619ffb8477083de23bb4daed567ef0a2c 2025-11-02 12:05:17 -05:00
Deluan
578375ca71 test: improve FFmpeg context cancellation tests for cross-platform compatibility
Signed-off-by: Deluan <deluan@navidrome.org>
2025-07-31 13:23:13 -04:00
Deluan
187eac268f test: reset FFmpeg initialization
Signed-off-by: Deluan <deluan@navidrome.org>
2025-07-31 12:10:27 -04:00
Deluan
f085446cc6 test: refactor FFmpeg context cancellation tests for improved clarity and reliability
Signed-off-by: Deluan <deluan@navidrome.org>
2025-07-31 11:55:10 -04:00
Deluan
b12e3ff367 feat: add configurable transcoding cancellation
Implemented EnableTranscodingCancellation configuration option to control whether
FFmpeg transcoding processes can be interrupted when client requests are cancelled.
This addresses resource management issues on low-power hardware where transcoding
processes would accumulate and cause CPU spikes.

Key changes:
- Added EnableTranscodingCancellation bool to configuration (default: false)
- Added CLI flag --enabletranscodingcancellation and TOML/env support
- Modified FFmpeg package to always use exec.CommandContext for consistency
- Implemented conditional context handling in NewTranscodingCache function
- When enabled: uses request context directly (allows cancellation)
- When disabled: uses background context with request metadata preserved
- Added comprehensive tests for both FFmpeg and transcoding layers
- Maintained backward compatibility with existing behavior as default

The implementation follows proper layered architecture with policy decisions
at the media streaming layer and execution utilities remaining focused on
their core responsibilities.

Signed-off-by: Deluan <deluan@navidrome.org>
2025-07-31 11:11:04 -04:00
5 changed files with 119 additions and 4 deletions

View File

@ -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"))
}

View File

@ -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)

View File

@ -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

View File

@ -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())
})
})
})
})

View File

@ -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