Fix recorder chapter anchor edge cases

Author: cursoragent

server/lib/recorder/ffmpeg.go

@@ -71,12 +71,12 @@ type FFmpegRecorder struct {
 	// chapter offsets to the media timeline; zero means never detected and
 	// callers fall back to startTime (stamped before the process spawned).
 	captureAnchor time.Time
-	ffmpegErr  error
-	exitCode   int
-	exited     chan struct{}
-	deleted    bool
-	markers    []Marker
-	stz        *scaletozero.Oncer
+	ffmpegErr     error
+	exitCode      int
+	exited        chan struct{}
+	deleted       bool
+	markers       []Marker
+	stz           *scaletozero.Oncer
 
 	// flight coordinates concurrent operations using different keys:
 	// - "stop": prevents multiple SIGINTs from being sent to ffmpeg
@@ -520,12 +520,16 @@ func (fr *FFmpegRecorder) Mark(name string) (string, int64, error) {
 }
 
 // watchCaptureStart stamps captureAnchor with the moment ffmpeg writes its
-// first output bytes. The muxer only writes them once the capture pipeline is
-// fully initialized (input opened, encoder ready, first frame in flight), so
-// this tracks media t=0 far more closely than startTime, which is stamped
-// before the process is even spawned. If no bytes ever appear the anchor
-// stays zero and callers fall back to startTime.
+// first output bytes for the current Start() session. The muxer only writes
+// them once the capture pipeline is fully initialized (input opened, encoder
+// ready, first frame in flight), so this tracks media t=0 far more closely
+// than startTime, which is stamped before the process is even spawned. If no
+// bytes ever appear the anchor stays zero and callers fall back to startTime.
 func (fr *FFmpegRecorder) watchCaptureStart() {
+	fr.mu.Lock()
+	sessionStart := fr.startTime
+	fr.mu.Unlock()
+
 	ticker := time.NewTicker(10 * time.Millisecond)
 	defer ticker.Stop()
 	for {
@@ -534,6 +538,12 @@ func (fr *FFmpegRecorder) watchCaptureStart() {
 			return
 		case <-ticker.C:
 			if info, err := os.Stat(fr.outputPath); err == nil && info.Size() > 0 {
+				// Ignore stale bytes from a previous recording session that used the
+				// same output path. We only anchor once this file has been updated
+				// after the current Start() timestamp.
+				if !sessionStart.IsZero() && info.ModTime().Before(sessionStart) {
+					continue
+				}
 				now := time.Now()
 				fr.mu.Lock()
 				fr.captureAnchor = now

server/lib/recorder/markers.go

@@ -23,8 +23,9 @@ const sentinelChapterName = "_recording_start"
 
 // buildChapterMetadata writes an ffmetadata file describing one MP4 chapter per
 // marker and returns its path. Each marker's chapter starts at its offset from
-// startTime; markers before startTime or at/after durationMs are dropped.
-// durationMs is the recording length and becomes the END of the final chapter.
+// startTime; markers before startTime are clamped to 0 and markers at/after
+// durationMs are dropped. durationMs is the recording length and becomes the
+// END of the final chapter.
 // Chapter ENDs are clamped to never precede their START, since ffmpeg rejects
 // an END-before-START chapter outright and would fail the whole remux.
 //
@@ -40,9 +41,12 @@ func buildChapterMetadata(outputPath string, markers []Marker, startTime time.Ti
 	chapters := make([]chapter, 0, len(markers))
 	for _, m := range markers {
 		startMs := m.At.Sub(startTime).Milliseconds()
-		if startMs < 0 || startMs >= durationMs {
+		if startMs >= durationMs {
 			continue
 		}
+		if startMs < 0 {
+			startMs = 0
+		}
 		chapters = append(chapters, chapter{startMs: startMs, title: m.Name})
 	}
 	if len(chapters) == 0 {

server/lib/recorder/markers_test.go

@@ -47,7 +47,7 @@ func TestBuildChapterMetadata_SentinelAndOrdering(t *testing.T) {
 	assert.Equal(t, expected, meta)
 }
 
-func TestBuildChapterMetadata_DropsNegativeOffsets(t *testing.T) {
+func TestBuildChapterMetadata_ClampsNegativeOffsetsToZero(t *testing.T) {
 	start := time.Unix(1000, 0)
 	out := filepath.Join(t.TempDir(), "rec.mp4")
 	markers := []Marker{
@@ -60,9 +60,9 @@ func TestBuildChapterMetadata_DropsNegativeOffsets(t *testing.T) {
 	require.True(t, ok)
 
 	meta := readMeta(t, path)
-	assert.NotContains(t, meta, "before-start")
+	assert.Contains(t, meta, "START=0\nEND=4000\ntitle=before-start")
 	assert.Contains(t, meta, "title=kept")
-	// Sentinel + one kept marker = two chapters.
+	// Two usable chapters, no sentinel needed because the first starts at 0.
 	assert.Equal(t, 2, strings.Count(meta, "[CHAPTER]"))
 }
 
@@ -329,6 +329,38 @@ func TestWatchCaptureStart_AnchorsToFirstOutputBytes(t *testing.T) {
 	rec.mu.Unlock()
 }
 
+func TestWatchCaptureStart_IgnoresPreexistingOutputUntilCurrentSessionWrite(t *testing.T) {
+	outputPath := filepath.Join(t.TempDir(), "anchor-stale.mp4")
+	require.NoError(t, os.WriteFile(outputPath, []byte("stale"), 0o644))
+	time.Sleep(20 * time.Millisecond) // ensure stale file mtime predates startTime
+
+	rec := &FFmpegRecorder{
+		id:         "anchor-stale",
+		outputPath: outputPath,
+		startTime:  time.Now(),
+		exited:     make(chan struct{}),
+	}
+	defer close(rec.exited)
+	go rec.watchCaptureStart()
+
+	time.Sleep(50 * time.Millisecond)
+	rec.mu.Lock()
+	assert.True(t, rec.captureAnchor.IsZero(), "stale output must not anchor this session")
+	rec.mu.Unlock()
+
+	beforeRewrite := time.Now()
+	require.NoError(t, os.WriteFile(outputPath, []byte("fresh"), 0o644))
+	require.Eventually(t, func() bool {
+		rec.mu.Lock()
+		defer rec.mu.Unlock()
+		return !rec.captureAnchor.IsZero()
+	}, 2*time.Second, 5*time.Millisecond)
+
+	rec.mu.Lock()
+	assert.False(t, rec.captureAnchor.Before(beforeRewrite), "anchor stamped at/after fresh session write")
+	rec.mu.Unlock()
+}
+
 func TestWatchCaptureStart_StopsOnExitWithoutOutput(t *testing.T) {
 	rec := &FFmpegRecorder{
 		id:         "anchor-exit",