fix(core): exclude encoder time from repeat scheduling
- commit
- dbee877aa2d82781760f82050359e9090a161819
- author
- brice · · 2026-09-30 08:29 UTC
- committer
- brice · · 2026-09-30 14:48 UTC
- parent
- 8a7c43d
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +64 -9
M internal/core/service.go +25 -9
| @@ -34,7 +34,7 @@ func New(source ports.Source, png ports.ScreenshotWriter, video ports.VideoWrite | |||
| 34 | 34 | // A timed recording (Duration > 0) encodes exactly ceil(Duration*FPS) frames, | |
| 35 | 35 | // so its encoded length is Duration rounded up to a frame. Wall time may run | |
| 36 | 36 | // longer when Capture or Write is slow: missed slots repeat the current frame | |
| 37 | - | // (at most two per fresh Capture) and the schedule then moves | |
| 37 | + | // (at most two repeats per fresh Capture) and the schedule then moves | |
| 38 | 38 | // forward, so a slow encoder lengthens the run instead of starving Capture. | |
| 39 | 39 | // The clock starts after Start and the first Write succeed. | |
| 40 | 40 | // | |
| @@ -104,8 +104,9 @@ func (s *Service) abort(err error) error { | |||
| 104 | 104 | } | |
| 105 | 105 | ||
| 106 | 106 | // maxRepeatSlots bounds how many missed slots repeat the current frame before | |
| 107 | - | // the next fresh Capture. A larger lag moves the schedule forward instead, so | |
| 108 | - | // encoder backpressure becomes wall-time overrun rather than a stale-frame spiral. | |
| 107 | + | // the next fresh Capture (so at most three identical consecutive writes). A | |
| 108 | + | // larger lag moves the schedule forward instead, so encoder backpressure | |
| 109 | + | // becomes wall-time overrun rather than a stale-frame spiral. | |
| 109 | 110 | const maxRepeatSlots = 2 | |
| 110 | 111 | ||
| 111 | 112 | // schedule maps elapsed time since the first frame to fixed-rate frame slots. | |
| @@ -152,11 +153,21 @@ func (sc *schedule) catchUp(next int64) int64 { | |||
| 152 | 153 | return cur | |
| 153 | 154 | } | |
| 154 | 155 | ||
| 156 | + | // writeTimed writes frame and returns how long the encoder took. | |
| 157 | + | func (s *Service) writeTimed(ctx context.Context, frame ports.Frame) (time.Duration, error) { | |
| 158 | + | began := time.Now() | |
| 159 | + | err := s.video.Write(ctx, frame) | |
| 160 | + | return time.Since(began), err | |
| 161 | + | } | |
| 162 | + | ||
| 155 | 163 | // recordLoop fills slots 1.. after the already written first frame. A slot | |
| 156 | - | // missed because Capture or Write ran long reuses the current frame, which is | |
| 157 | - | // still valid until the next Capture, so the encoded duration stays exact | |
| 158 | - | // even when wall time overruns. At most maxRepeatSlots slots repeat per fresh | |
| 159 | - | // Capture; see catchUp. A timed recording ends after ceil(duration*fps) | |
| 164 | + | // missed because Capture ran long reuses the current frame, which is still | |
| 165 | + | // valid until the next Capture, so the encoded duration stays exact even when | |
| 166 | + | // wall time overruns. Encoder time does not cause repeats: the full duration | |
| 167 | + | // of a repeated Write, and the part of a fresh Write beyond one frame | |
| 168 | + | // interval, is added to the schedule start, so only Capture delay is | |
| 169 | + | // duplicated. catchUp is a backstop that keeps at most maxRepeatSlots repeats | |
| 170 | + | // per fresh Capture. A timed recording ends after ceil(duration*fps) | |
| 160 | 171 | // frames, so a duration shorter than one frame interval encodes one frame and | |
| 161 | 172 | // returns without waiting. Cancellation ends the recording at the last frame | |
| 162 | 173 | // already written: nothing is written afterwards, because the writer is | |
| @@ -197,9 +208,11 @@ func (s *Service) recordLoop(ctx context.Context, frame ports.Frame, sel ports.S | |||
| 197 | 208 | if ctx.Err() != nil { | |
| 198 | 209 | return nil | |
| 199 | 210 | } | |
| 200 | - | if err := s.video.Write(ctx, frame); err != nil { | |
| 211 | + | took, err := s.writeTimed(ctx, frame) | |
| 212 | + | if err != nil { | |
| 201 | 213 | return writeResult(ctx, err) | |
| 202 | 214 | } | |
| 215 | + | sc.start = sc.start.Add(took) // repeated encoder time is not lag | |
| 203 | 216 | } | |
| 204 | 217 | if past { | |
| 205 | 218 | return nil | |
| @@ -218,9 +231,12 @@ func (s *Service) recordLoop(ctx context.Context, frame ports.Frame, sel ports.S | |||
| 218 | 231 | if ctx.Err() != nil { | |
| 219 | 232 | return nil | |
| 220 | 233 | } | |
| 221 | - | if err := s.video.Write(ctx, frame); err != nil { | |
| 234 | + | took, err := s.writeTimed(ctx, frame) | |
| 235 | + | if err != nil { | |
| 222 | 236 | return writeResult(ctx, err) | |
| 223 | 237 | } | |
| 238 | + | // Only the part of a fresh Write beyond its own slot is delay. | |
| 239 | + | sc.start = sc.start.Add(max(0, took-sc.due(1))) | |
| 224 | 240 | next++ | |
| 225 | 241 | } | |
| 226 | 242 | return nil | |
| @@ -34,7 +34,7 @@ func New(source ports.Source, png ports.ScreenshotWriter, video ports.VideoWrite | |||||
| 34 | // A timed recording (Duration > 0) encodes exactly ceil(Duration*FPS) frames, | 34 | // A timed recording (Duration > 0) encodes exactly ceil(Duration*FPS) frames, | ||
| 35 | // so its encoded length is Duration rounded up to a frame. Wall time may run | 35 | // so its encoded length is Duration rounded up to a frame. Wall time may run | ||
| 36 | // longer when Capture or Write is slow: missed slots repeat the current frame | 36 | // longer when Capture or Write is slow: missed slots repeat the current frame | ||
| 37 | - | // (at most two per fresh Capture) and the schedule then moves | 37 | + | // (at most two repeats per fresh Capture) and the schedule then moves |
| 38 | // forward, so a slow encoder lengthens the run instead of starving Capture. | 38 | // forward, so a slow encoder lengthens the run instead of starving Capture. | ||
| 39 | // The clock starts after Start and the first Write succeed. | 39 | // The clock starts after Start and the first Write succeed. | ||
| 40 | // | 40 | // | ||
| @@ -104,8 +104,9 @@ func (s *Service) abort(err error) error { | |||||
| 104 | } | 104 | } | ||
| 105 | 105 | ||||
| 106 | // maxRepeatSlots bounds how many missed slots repeat the current frame before | 106 | // maxRepeatSlots bounds how many missed slots repeat the current frame before | ||
| 107 | - | // the next fresh Capture. A larger lag moves the schedule forward instead, so | 107 | + | // the next fresh Capture (so at most three identical consecutive writes). A |
| 108 | - | // encoder backpressure becomes wall-time overrun rather than a stale-frame spiral. | 108 | + | // larger lag moves the schedule forward instead, so encoder backpressure |
| 109 | + | // becomes wall-time overrun rather than a stale-frame spiral. | |||
| 109 | const maxRepeatSlots = 2 | 110 | const maxRepeatSlots = 2 | ||
| 110 | 111 | ||||
| 111 | // schedule maps elapsed time since the first frame to fixed-rate frame slots. | 112 | // schedule maps elapsed time since the first frame to fixed-rate frame slots. | ||
| @@ -152,11 +153,21 @@ func (sc *schedule) catchUp(next int64) int64 { | |||||
| 152 | return cur | 153 | return cur | ||
| 153 | } | 154 | } | ||
| 154 | 155 | ||||
| 156 | + | // writeTimed writes frame and returns how long the encoder took. | |||
| 157 | + | func (s *Service) writeTimed(ctx context.Context, frame ports.Frame) (time.Duration, error) { | |||
| 158 | + | began := time.Now() | |||
| 159 | + | err := s.video.Write(ctx, frame) | |||
| 160 | + | return time.Since(began), err | |||
| 161 | + | } | |||
| 162 | + | ||||
| 155 | // recordLoop fills slots 1.. after the already written first frame. A slot | 163 | // recordLoop fills slots 1.. after the already written first frame. A slot | ||
| 156 | - | // missed because Capture or Write ran long reuses the current frame, which is | 164 | + | // missed because Capture ran long reuses the current frame, which is still |
| 157 | - | // still valid until the next Capture, so the encoded duration stays exact | 165 | + | // valid until the next Capture, so the encoded duration stays exact even when |
| 158 | - | // even when wall time overruns. At most maxRepeatSlots slots repeat per fresh | 166 | + | // wall time overruns. Encoder time does not cause repeats: the full duration |
| 159 | - | // Capture; see catchUp. A timed recording ends after ceil(duration*fps) | 167 | + | // of a repeated Write, and the part of a fresh Write beyond one frame |
| 168 | + | // interval, is added to the schedule start, so only Capture delay is | |||
| 169 | + | // duplicated. catchUp is a backstop that keeps at most maxRepeatSlots repeats | |||
| 170 | + | // per fresh Capture. A timed recording ends after ceil(duration*fps) | |||
| 160 | // frames, so a duration shorter than one frame interval encodes one frame and | 171 | // frames, so a duration shorter than one frame interval encodes one frame and | ||
| 161 | // returns without waiting. Cancellation ends the recording at the last frame | 172 | // returns without waiting. Cancellation ends the recording at the last frame | ||
| 162 | // already written: nothing is written afterwards, because the writer is | 173 | // already written: nothing is written afterwards, because the writer is | ||
| @@ -197,9 +208,11 @@ func (s *Service) recordLoop(ctx context.Context, frame ports.Frame, sel ports.S | |||||
| 197 | if ctx.Err() != nil { | 208 | if ctx.Err() != nil { | ||
| 198 | return nil | 209 | return nil | ||
| 199 | } | 210 | } | ||
| 200 | - | if err := s.video.Write(ctx, frame); err != nil { | 211 | + | took, err := s.writeTimed(ctx, frame) |
| 212 | + | if err != nil { | |||
| 201 | return writeResult(ctx, err) | 213 | return writeResult(ctx, err) | ||
| 202 | } | 214 | } | ||
| 215 | + | sc.start = sc.start.Add(took) // repeated encoder time is not lag | |||
| 203 | } | 216 | } | ||
| 204 | if past { | 217 | if past { | ||
| 205 | return nil | 218 | return nil | ||
| @@ -218,9 +231,12 @@ func (s *Service) recordLoop(ctx context.Context, frame ports.Frame, sel ports.S | |||||
| 218 | if ctx.Err() != nil { | 231 | if ctx.Err() != nil { | ||
| 219 | return nil | 232 | return nil | ||
| 220 | } | 233 | } | ||
| 221 | - | if err := s.video.Write(ctx, frame); err != nil { | 234 | + | took, err := s.writeTimed(ctx, frame) |
| 235 | + | if err != nil { | |||
| 222 | return writeResult(ctx, err) | 236 | return writeResult(ctx, err) | ||
| 223 | } | 237 | } | ||
| 238 | + | // Only the part of a fresh Write beyond its own slot is delay. | |||
| 239 | + | sc.start = sc.start.Add(max(0, took-sc.due(1))) | |||
| 224 | next++ | 240 | next++ | ||
| 225 | } | 241 | } | ||
| 226 | return nil | 242 | return nil | ||
M internal/core/service_test.go +39 -0
| @@ -448,6 +448,45 @@ func TestRecordEncoderBackpressureKeepsCapturesFresh(t *testing.T) { | |||
| 448 | 448 | assert.LessOrEqual(t, bp.maxRun, 2, "repeat backlog is bounded") | |
| 449 | 449 | } | |
| 450 | 450 | ||
| 451 | + | func TestRecordSlightWriteOverrunStaysFresh(t *testing.T) { | |
| 452 | + | h := newHarness(t) | |
| 453 | + | sel := rec(time.Second) | |
| 454 | + | sel.Video.FPS = 60 | |
| 455 | + | start := recordOrder(h, sel) | |
| 456 | + | var bp backpressure | |
| 457 | + | bp.wire(h, start, 17*time.Millisecond) // just over the 16.7ms slot | |
| 458 | + | h.vid.EXPECT().Close().Return(nil).Once() | |
| 459 | + | ||
| 460 | + | require.NoError(t, h.svc.Run(context.Background(), sel)) | |
| 461 | + | assert.Equal(t, int32(60), bp.writes.Load(), "exact encoded length") | |
| 462 | + | assert.GreaterOrEqual(t, bp.captures.Load(), int32(54), "over 90 percent of frames are fresh") | |
| 463 | + | } | |
| 464 | + | ||
| 465 | + | func TestRecordSlowCaptureStillRepeats(t *testing.T) { | |
| 466 | + | h := newHarness(t) | |
| 467 | + | sel := rec(time.Second) | |
| 468 | + | sel.Video.FPS = 60 | |
| 469 | + | start := recordOrder(h, sel) | |
| 470 | + | var captures atomic.Int32 | |
| 471 | + | h.src.EXPECT().Capture(mock.Anything, target).RunAndReturn(func(context.Context, ports.Target) (ports.Frame, error) { | |
| 472 | + | captures.Add(1) | |
| 473 | + | time.Sleep(25 * time.Millisecond) | |
| 474 | + | return newFrame(), nil | |
| 475 | + | }).Maybe() | |
| 476 | + | var writes atomic.Int32 | |
| 477 | + | h.vid.EXPECT().Write(mock.Anything, mock.Anything).RunAndReturn(func(_ context.Context, _ ports.Frame) error { | |
| 478 | + | time.Sleep(time.Millisecond) | |
| 479 | + | writes.Add(1) | |
| 480 | + | return nil | |
| 481 | + | }).NotBefore(start).Maybe() | |
| 482 | + | h.vid.EXPECT().Close().Return(nil).Once() | |
| 483 | + | ||
| 484 | + | require.NoError(t, h.svc.Run(context.Background(), sel)) | |
| 485 | + | assert.Equal(t, int32(60), writes.Load(), "exact encoded length") | |
| 486 | + | assert.Less(t, captures.Load(), int32(45), "slow captures are covered by repeated frames") | |
| 487 | + | assert.Greater(t, captures.Load(), int32(10)) | |
| 488 | + | } | |
| 489 | + | ||
| 451 | 490 | func TestRecordUntimedBackpressureBoundsRepeats(t *testing.T) { | |
| 452 | 491 | h := newHarness(t) | |
| 453 | 492 | sel := rec(0) | |
| @@ -448,6 +448,45 @@ func TestRecordEncoderBackpressureKeepsCapturesFresh(t *testing.T) { | |||||
| 448 | assert.LessOrEqual(t, bp.maxRun, 2, "repeat backlog is bounded") | 448 | assert.LessOrEqual(t, bp.maxRun, 2, "repeat backlog is bounded") | ||
| 449 | } | 449 | } | ||
| 450 | 450 | ||||
| 451 | + | func TestRecordSlightWriteOverrunStaysFresh(t *testing.T) { | |||
| 452 | + | h := newHarness(t) | |||
| 453 | + | sel := rec(time.Second) | |||
| 454 | + | sel.Video.FPS = 60 | |||
| 455 | + | start := recordOrder(h, sel) | |||
| 456 | + | var bp backpressure | |||
| 457 | + | bp.wire(h, start, 17*time.Millisecond) // just over the 16.7ms slot | |||
| 458 | + | h.vid.EXPECT().Close().Return(nil).Once() | |||
| 459 | + | ||||
| 460 | + | require.NoError(t, h.svc.Run(context.Background(), sel)) | |||
| 461 | + | assert.Equal(t, int32(60), bp.writes.Load(), "exact encoded length") | |||
| 462 | + | assert.GreaterOrEqual(t, bp.captures.Load(), int32(54), "over 90 percent of frames are fresh") | |||
| 463 | + | } | |||
| 464 | + | ||||
| 465 | + | func TestRecordSlowCaptureStillRepeats(t *testing.T) { | |||
| 466 | + | h := newHarness(t) | |||
| 467 | + | sel := rec(time.Second) | |||
| 468 | + | sel.Video.FPS = 60 | |||
| 469 | + | start := recordOrder(h, sel) | |||
| 470 | + | var captures atomic.Int32 | |||
| 471 | + | h.src.EXPECT().Capture(mock.Anything, target).RunAndReturn(func(context.Context, ports.Target) (ports.Frame, error) { | |||
| 472 | + | captures.Add(1) | |||
| 473 | + | time.Sleep(25 * time.Millisecond) | |||
| 474 | + | return newFrame(), nil | |||
| 475 | + | }).Maybe() | |||
| 476 | + | var writes atomic.Int32 | |||
| 477 | + | h.vid.EXPECT().Write(mock.Anything, mock.Anything).RunAndReturn(func(_ context.Context, _ ports.Frame) error { | |||
| 478 | + | time.Sleep(time.Millisecond) | |||
| 479 | + | writes.Add(1) | |||
| 480 | + | return nil | |||
| 481 | + | }).NotBefore(start).Maybe() | |||
| 482 | + | h.vid.EXPECT().Close().Return(nil).Once() | |||
| 483 | + | ||||
| 484 | + | require.NoError(t, h.svc.Run(context.Background(), sel)) | |||
| 485 | + | assert.Equal(t, int32(60), writes.Load(), "exact encoded length") | |||
| 486 | + | assert.Less(t, captures.Load(), int32(45), "slow captures are covered by repeated frames") | |||
| 487 | + | assert.Greater(t, captures.Load(), int32(10)) | |||
| 488 | + | } | |||
| 489 | + | ||||
| 451 | func TestRecordUntimedBackpressureBoundsRepeats(t *testing.T) { | 490 | func TestRecordUntimedBackpressureBoundsRepeats(t *testing.T) { | ||
| 452 | h := newHarness(t) | 491 | h := newHarness(t) | ||
| 453 | sel := rec(0) | 492 | sel := rec(0) | ||