feat: gracefully stop and restart recordings during display resize by hiroTamada · Pull Request #158 · kernel/kernel-images

hiroTamada · 2026-02-23T20:28:16Z

Summary

When a display resize is requested with require_idle=false, active FFmpeg recordings are gracefully stopped and new recording segments are started with unique suffixed IDs (e.g. my-rec -> my-rec-1709312345678).
Old (stopped) recorders remain registered in the manager, so their finalized files stay discoverable via GET /recording/list and downloadable via GET /recording/download.
When require_idle=true, the original strict behavior is preserved: resize is refused with 409 Conflict if any live view or recording is active.
Adds Params() accessor with deep-copy clone() on FFmpegRecordingParams to safely capture recording parameters before restart.

Test plan

Unit tests for stopActiveRecordings (verifies old recorder stays registered but stopped)
Unit tests for startNewRecordingSegments (verifies new segment created with suffixed ID)
Round-trip test (stop + start, both old and new coexist in manager)
Params() deep-copy test
End-to-end Docker test: start recording -> resize (require_idle=false) -> verify 2 segments in list -> resize again -> verify 3 segments -> stop -> require_idle=true succeeds -> all segments downloadable as valid MP4

Note

Medium Risk
Touches recording lifecycle during display resize (stop/finalize/restart) and introduces async segment creation, which could affect recording continuity and resource usage if edge cases aren’t handled.

Overview
Display resize (PATCH /display) now coordinates with active recordings. The handler stops any active FFmpeg recordings before attempting the resolution change, and if the resize succeeds it starts new recording segments with unique suffixed IDs so old segments remain discoverable/downloadable.

Recording restart preserves original limits. New segments reuse captured recorder params via a new FFmpegRecorder.Params() deep-copy accessor, and adjust remaining MaxDurationInSeconds/MaxSizeInMB based on prior segment metadata and file size.

Adds focused unit tests covering stop-and-preserve behavior, segment restart/ID suffixing, round-trip stop+restart, budget adjustment logic, and the new params deep-copy behavior.

^{Written by Cursor Bugbot for commit f4d3313. This will update automatically on new commits. Configure here.}

Instead of refusing display resize requests (409) when recordings are active, PatchDisplay now stops active recordings, performs the resize, then restarts them with the same ID and params. The pre-resize segment is preserved by renaming it (e.g. `<id>-before-resize-<ts>.mp4`). Live view sessions still block the resize as before. Co-authored-by: Cursor <cursoragent@cursor.com>

server/cmd/api/api/display.go

server/lib/recorder/ffmpeg.go

- Don't block recording restart when rename of old file fails (rename is best-effort to preserve the pre-resize segment) - Deep copy pointer fields in FFmpegRecordingParams.clone() so Params() callers cannot mutate recorder internals Co-authored-by: Cursor <cursoragent@cursor.com>

server/cmd/api/api/display.go

If DeregisterRecorder fails, the old recorder remains registered. Appending to the stopped list would cause RegisterRecorder to fail with a duplicate ID during restart. Co-authored-by: Cursor <cursoragent@cursor.com>

server/cmd/api/api/display.go

Instead of deregistering old recorders and renaming files, stopped recorders now remain registered (keeping their finalized files discoverable via the API) and new segments are started with unique suffixed IDs. This removes file-rename complexity and keeps all recording segments accessible through existing list/download endpoints. Also moves the graceful stop/restart logic outside the requireIdle block so it applies unconditionally when recordings are active. Made-with: Cursor

cursor · 2026-03-03T17:01:39Z

server/cmd/api/api/display.go

 		}
 	}

+	// Gracefully stop active recordings so the resize can proceed.


Recording check blocks graceful stop/restart under defaults

High Severity

The requireIdle block at line 68–82 still includes isRecording in the resizableNow condition ((live == 0) && !isRecording), so any active recording causes a 409 return before the new stopActiveRecordings code at line 86 is ever reached. Since requireIdle defaults to true, the graceful stop-and-restart feature introduced by this PR is effectively unreachable under normal usage. Per the PR description, the live-view check and recording check were supposed to be separated, with only live sessions triggering a 409.

Additional Locations (1)

server/cmd/api/api/display.go#L83-L97

This is intentional. The two modes are by design:

require_idle=true (default): strict mode — returns 409 if anything is active (live views or recordings). This preserves the original safety behavior.

require_idle=false: graceful mode — stops active recordings, resizes, then starts new recording segments with suffixed IDs.

The graceful stop/restart path is reachable when the caller explicitly opts in via require_idle=false. A follow-up change in the kernel API will thread a user-facing flag down to this parameter.

Sayan-

I'm not opposed to this direction but I think there's material trade offs here:

Users now get segments (e.g., rec-123 then rec-123-) instead of a single uninterrupted file.
The original recording ID no longer represents the full post-resize timeline by itself.
There’s no stitching in this PR, so consumers need to handle multiple files if they want a continuous playback experience.

So the UX benefit is reliability during resize. The UX cost is fragmentation unless product/UI layers present these segments as one logical recording, which is confusing in a different way

Sayan- · 2026-03-03T22:39:25Z

server/cmd/api/api/display.go

+	// New recording segments (with unique IDs) will be started after the resize.
+	stopped, stopErr := s.stopActiveRecordings(ctx)
+	if len(stopped) > 0 {
+		defer s.startNewRecordingSegments(context.WithoutCancel(ctx), stopped)


This leaves me a bit split brained on the design here. We don't block on all recordings being started so we're exposed to data loss but also increases the latency for this endpoint further

Sayan- · 2026-03-03T22:48:57Z

server/cmd/api/api/display.go

+// stopActiveRecordings gracefully stops every recording that is currently in
+// progress. The old recorders remain registered in the manager so their
+// finalized files stay discoverable and downloadable. It returns info needed
+// to start a new recording segment for each stopped recorder.
+func (s *ApiService) stopActiveRecordings(ctx context.Context) ([]stoppedRecordingInfo, error) {
+	log := logger.FromContext(ctx)
+	var stopped []stoppedRecordingInfo


I think this is another place where we're breaking the record manager interface. I wonder if it's simpler in thinking of these operations as stop and clone but nbd

also segments is a new concept. you could encode it into the record manager interface and treat the replays as disjoint and perhaps join them back together at the end. unclear

Sayan- · 2026-03-03T22:51:12Z

server/lib/recorder/ffmpeg.go

+	if p.MaxSizeInMB != nil {
+		v := *p.MaxSizeInMB
+		c.MaxSizeInMB = &v
+	}
+	if p.MaxDurationInSeconds != nil {
+		v := *p.MaxDurationInSeconds
+		c.MaxDurationInSeconds = &v
+	}


users don't really use these params but these break entirely over the boundary (e.g. initial replay was expected to be bound to size X. due to resize we'll exceed)

hiroTamada · 2026-03-04T00:23:26Z

yeah maybe I will do stitching @Sayan-

When a display resize splits a recording into segments, the new segment's MaxDurationInSeconds and MaxSizeInMB are now reduced by what the prior segment already consumed, keeping cumulative usage within the originally requested limits. Made-with: Cursor

server/cmd/api/api/display.go

Move segment restart from a defer (which fires on all return paths) to an explicit call after the resize succeeds. This avoids restarting recordings at the old resolution when the stop partially fails or the resize itself errors out. Also round up fractional MB when computing consumed file size budget to prevent cumulative overestimation across multiple resize cycles. Made-with: Cursor

cursor

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

server/cmd/api/api/display.go

Move segment restart back to a defer so recordings are restarted regardless of whether the resize succeeds. If the resize fails the display is still at the old resolution, so restarting there is correct. Losing recording data is worse than a brief gap. The defer is placed after the stopErr check so it only fires when all stops succeeded cleanly. Made-with: Cursor

cursor bot reviewed Feb 23, 2026

View reviewed changes

server/cmd/api/api/display.go Show resolved Hide resolved

server/lib/recorder/ffmpeg.go Show resolved Hide resolved

cursor bot reviewed Feb 23, 2026

View reviewed changes

server/cmd/api/api/display.go Show resolved Hide resolved

server/cmd/api/api/display.go Show resolved Hide resolved

fix: skip restart when deregister fails to avoid ID conflict

0820c4a

If DeregisterRecorder fails, the old recorder remains registered. Appending to the stopped list would cause RegisterRecorder to fail with a duplicate ID during restart. Co-authored-by: Cursor <cursoragent@cursor.com>

cursor bot reviewed Feb 23, 2026

View reviewed changes

server/cmd/api/api/display.go Show resolved Hide resolved

hiroTamada requested review from Sayan- and rgarcia March 3, 2026 16:55

cursor bot reviewed Mar 3, 2026

View reviewed changes

Sayan- reviewed Mar 3, 2026

View reviewed changes

cursor bot reviewed Mar 4, 2026

View reviewed changes

server/cmd/api/api/display.go Show resolved Hide resolved

server/cmd/api/api/display.go Outdated Show resolved Hide resolved

cursor bot reviewed Mar 4, 2026

View reviewed changes

server/cmd/api/api/display.go Outdated Show resolved Hide resolved

hiroTamada requested a review from Sayan- March 4, 2026 18:17

rgarcia approved these changes Mar 4, 2026

View reviewed changes

hiroTamada merged commit 689e284 into main Mar 4, 2026
7 of 8 checks passed

hiroTamada deleted the htamada/graceful-recording-resize branch March 4, 2026 18:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: gracefully stop and restart recordings during display resize#158

feat: gracefully stop and restart recordings during display resize#158
hiroTamada merged 7 commits intomainfrom
htamada/graceful-recording-resize

hiroTamada commented Feb 23, 2026 •

edited by cursor bot

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cursor bot Mar 3, 2026

Uh oh!

hiroTamada Mar 3, 2026

Uh oh!

Sayan- left a comment

Uh oh!

Sayan- Mar 3, 2026

Uh oh!

Sayan- Mar 3, 2026

Uh oh!

Sayan- Mar 3, 2026

Uh oh!

hiroTamada commented Mar 4, 2026

Uh oh!

Uh oh!

Uh oh!

cursor bot left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

hiroTamada commented Feb 23, 2026 • edited by cursor bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cursor bot Mar 3, 2026

Choose a reason for hiding this comment

Recording check blocks graceful stop/restart under defaults

Uh oh!

hiroTamada Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

Sayan- left a comment

Choose a reason for hiding this comment

Uh oh!

Sayan- Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

Sayan- Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

Sayan- Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

hiroTamada commented Mar 4, 2026

Uh oh!

Uh oh!

Uh oh!

cursor bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

hiroTamada commented Feb 23, 2026 •

edited by cursor bot

Loading