fix cleaning of recordings in case of multiple recordDeleteAfter values (#3557)

internal/api/api.go

@@ -22,7 +22,7 @@ import (
 	"github.com/bluenviron/mediamtx/internal/defs"
 	"github.com/bluenviron/mediamtx/internal/logger"
 	"github.com/bluenviron/mediamtx/internal/protocols/httpp"
-	"github.com/bluenviron/mediamtx/internal/record"
+	"github.com/bluenviron/mediamtx/internal/recordstore"
 	"github.com/bluenviron/mediamtx/internal/restrictnetwork"
 	"github.com/bluenviron/mediamtx/internal/servers/hls"
 	"github.com/bluenviron/mediamtx/internal/servers/rtmp"
@@ -56,6 +56,27 @@ func paramName(ctx *gin.Context) (string, bool) {
 	return name[1:], true
 }
 
+func recordingsOfPath(
+	pathConf *conf.Path,
+	pathName string,
+) *defs.APIRecording {
+	ret := &defs.APIRecording{
+		Name: pathName,
+	}
+
+	segments, _ := recordstore.FindSegments(pathConf, pathName)
+
+	ret.Segments = make([]*defs.APIRecordingSegment, len(segments))
+
+	for i, seg := range segments {
+		ret.Segments[i] = &defs.APIRecordingSegment{
+			Start: seg.Start,
+		}
+	}
+
+	return ret
+}
+
 // PathManager contains methods used by the API and Metrics server.
 type PathManager interface {
 	APIPathsList() (*defs.APIPathList, error)
@@ -1062,7 +1083,7 @@ func (a *API) onRecordingsList(ctx *gin.Context) {
 	c := a.Conf
 	a.mutex.RUnlock()
 
-	pathNames := getAllPathsWithRecordings(c.Paths)
+	pathNames := recordstore.FindAllPathsWithSegments(c.Paths)
 
 	data := defs.APIRecordingList{}
 
@@ -1077,8 +1098,8 @@ func (a *API) onRecordingsList(ctx *gin.Context) {
 	data.Items = make([]*defs.APIRecording, len(pathNames))
 
 	for i, pathName := range pathNames {
-		_, pathConf, _, _ := conf.FindPathConf(c.Paths, pathName)
-		data.Items[i] = recordingEntry(pathConf, pathName)
+		pathConf, _, _ := conf.FindPathConf(c.Paths, pathName)
+		data.Items[i] = recordingsOfPath(pathConf, pathName)
 	}
 
 	ctx.JSON(http.StatusOK, data)
@@ -1095,13 +1116,13 @@ func (a *API) onRecordingsGet(ctx *gin.Context) {
 	c := a.Conf
 	a.mutex.RUnlock()
 
-	_, pathConf, _, err := conf.FindPathConf(c.Paths, pathName)
+	pathConf, _, err := conf.FindPathConf(c.Paths, pathName)
 	if err != nil {
 		a.writeError(ctx, http.StatusBadRequest, err)
 		return
 	}
 
-	ctx.JSON(http.StatusOK, recordingEntry(pathConf, pathName))
+	ctx.JSON(http.StatusOK, recordingsOfPath(pathConf, pathName))
 }
 
 func (a *API) onRecordingDeleteSegment(ctx *gin.Context) {
@@ -1117,18 +1138,18 @@ func (a *API) onRecordingDeleteSegment(ctx *gin.Context) {
 	c := a.Conf
 	a.mutex.RUnlock()
 
-	_, pathConf, _, err := conf.FindPathConf(c.Paths, pathName)
+	pathConf, _, err := conf.FindPathConf(c.Paths, pathName)
 	if err != nil {
 		a.writeError(ctx, http.StatusBadRequest, err)
 		return
 	}
 
-	pathFormat := record.PathAddExtension(
+	pathFormat := recordstore.PathAddExtension(
 		strings.ReplaceAll(pathConf.RecordPath, "%path", pathName),
 		pathConf.RecordFormat,
 	)
 
-	segmentPath := record.Path{
+	segmentPath := recordstore.Path{
 		Start: start,
 	}.Encode(pathFormat)
 

internal/api/api_test.go

@@ -595,6 +595,7 @@ func TestRecordingsList(t *testing.T) {
 	cnf := tempConf(t, "pathDefaults:\n"+
 		"  recordPath: "+filepath.Join(dir, "%path/%Y-%m-%d_%H-%M-%S-%f")+"\n"+
 		"paths:\n"+
+		"  mypath1:\n"+
 		"  all_others:\n")
 
 	api := API{

internal/conf/path.go

@@ -46,38 +46,38 @@
 }
 
 // FindPathConf returns the configuration corresponding to the given path name.
-func FindPathConf(pathConfs map[string]*Path, name string) (string, *Path, []string, error) {
+func FindPathConf(pathConfs map[string]*Path, name string) (*Path, []string, error) {
 	err := isValidPathName(name)
 	if err != nil {
-		return "", nil, nil, fmt.Errorf("invalid path name: %w (%s)", err, name)
+		return nil, nil, fmt.Errorf("invalid path name: %w (%s)", err, name)
 	}
 
 	// normal path
 	if pathConf, ok := pathConfs[name]; ok {
-		return name, pathConf, nil, nil
+		return pathConf, nil, nil
 	}
 
 	// regular expression-based path
 	for pathConfName, pathConf := range pathConfs {
 		if pathConf.Regexp != nil && pathConfName != "all" && pathConfName != "all_others" {
 			m := pathConf.Regexp.FindStringSubmatch(name)
 			if m != nil {
-				return pathConfName, pathConf, m, nil
+				return pathConf, m, nil
 			}
 		}
 	}
 
-	// all_others
+	// process all_others after every other entry
 	for pathConfName, pathConf := range pathConfs {
 		if pathConfName == "all" || pathConfName == "all_others" {
 			m := pathConf.Regexp.FindStringSubmatch(name)
 			if m != nil {
-				return pathConfName, pathConf, m, nil
+				return pathConf, m, nil
 			}
 		}
 	}
 
-	return "", nil, nil, fmt.Errorf("path '%s' is not configured", name)
+	return nil, nil, fmt.Errorf("path '%s' is not configured", name)
 }
 
 // Path is a path configuration.

internal/core/core.go

@@ -8,7 +8,6 @@ import (
 	"os/signal"
 	"path/filepath"
 	"reflect"
-	"sort"
 	"strings"
 	"time"
 
@@ -25,7 +24,7 @@ import (
 	"github.com/bluenviron/mediamtx/internal/metrics"
 	"github.com/bluenviron/mediamtx/internal/playback"
 	"github.com/bluenviron/mediamtx/internal/pprof"
-	"github.com/bluenviron/mediamtx/internal/record"
+	"github.com/bluenviron/mediamtx/internal/recordcleaner"
 	"github.com/bluenviron/mediamtx/internal/rlimit"
 	"github.com/bluenviron/mediamtx/internal/servers/hls"
 	"github.com/bluenviron/mediamtx/internal/servers/rtmp"
@@ -44,38 +43,6 @@ var defaultConfPaths = []string{
 	"/etc/mediamtx/mediamtx.yml",
 }
 
-func gatherCleanerEntries(paths map[string]*conf.Path) []record.CleanerEntry {
-	out := make(map[record.CleanerEntry]struct{})
-
-	for _, pa := range paths {
-		if pa.Record && pa.RecordDeleteAfter != 0 {
-			entry := record.CleanerEntry{
-				Path:        pa.RecordPath,
-				Format:      pa.RecordFormat,
-				DeleteAfter: time.Duration(pa.RecordDeleteAfter),
-			}
-			out[entry] = struct{}{}
-		}
-	}
-
-	out2 := make([]record.CleanerEntry, len(out))
-	i := 0
-
-	for v := range out {
-		out2[i] = v
-		i++
-	}
-
-	sort.Slice(out2, func(i, j int) bool {
-		if out2[i].Path != out2[j].Path {
-			return out2[i].Path < out2[j].Path
-		}
-		return out2[i].DeleteAfter < out2[j].DeleteAfter
-	})
-
-	return out2
-}
-
 var cli struct {
 	Version  bool   `help:"print version"`
 	Confpath string `arg:"" default:""`
@@ -92,7 +59,7 @@ type Core struct {
 	authManager     *auth.Manager
 	metrics         *metrics.Metrics
 	pprof           *pprof.PPROF
-	recordCleaner   *record.Cleaner
+	recordCleaner   *recordcleaner.Cleaner
 	playbackServer  *playback.Server
 	pathManager     *pathManager
 	rtspServer      *rtsp.Server
@@ -333,12 +300,10 @@ func (p *Core) createResources(initial bool) error {
 		p.pprof = i
 	}
 
-	cleanerEntries := gatherCleanerEntries(p.conf.Paths)
-	if len(cleanerEntries) != 0 &&
-		p.recordCleaner == nil {
-		p.recordCleaner = &record.Cleaner{
-			Entries: cleanerEntries,
-			Parent:  p,
+	if p.recordCleaner == nil {
+		p.recordCleaner = &recordcleaner.Cleaner{
+			PathConfs: p.conf.Paths,
+			Parent:    p,
 		}
 		p.recordCleaner.Initialize()
 	}
@@ -707,8 +672,10 @@ func (p *Core) closeResources(newConf *conf.Conf, calledByAPI bool) {
 		closeLogger
 
 	closeRecorderCleaner := newConf == nil ||
-		!reflect.DeepEqual(gatherCleanerEntries(newConf.Paths), gatherCleanerEntries(p.conf.Paths)) ||
 		closeLogger
+	if !closeRecorderCleaner && !reflect.DeepEqual(newConf.Paths, p.conf.Paths) {
+		p.recordCleaner.ReloadPathConfs(newConf.Paths)
+	}
 
 	closePlaybackServer := newConf == nil ||
 		newConf.Playback != p.conf.Playback ||