From b8cb750fb4b2c4832f1047ceb20a8b1c916a6f07 Mon Sep 17 00:00:00 2001 From: Dmitry Sergeev Date: Wed, 2 Sep 2026 00:39:30 +0300 Subject: [PATCH] frame pacing fix --- cmd/mxl-player/frame_pacing.go | 24 +++++++++++++ cmd/mxl-player/frame_pacing_test.go | 29 +++++++++++++++ cmd/mxl-player/main.go | 4 ++- internal/playback/video_bridge.go | 13 +++++++ internal/playback/video_bridge_test.go | 49 ++++++++++++++++++++++++++ 5 files changed, 118 insertions(+), 1 deletion(-) create mode 100644 cmd/mxl-player/frame_pacing.go create mode 100644 cmd/mxl-player/frame_pacing_test.go diff --git a/cmd/mxl-player/frame_pacing.go b/cmd/mxl-player/frame_pacing.go new file mode 100644 index 0000000..e5ae1c2 --- /dev/null +++ b/cmd/mxl-player/frame_pacing.go @@ -0,0 +1,24 @@ +package main + +import "time" + +const guiFrameInterval = time.Second / 60 + +// videoPollInterval bounds GUI latency when no video producer is waiting. +// It is not a video-rate cap: Next returns immediately whenever a frame +// arrives, including for sources faster than this interval. +const videoPollInterval = 8 * time.Millisecond + +func remainingFrameTime(start, now time.Time, interval time.Duration) time.Duration { + remaining := interval - now.Sub(start) + if remaining < 0 { + return 0 + } + return remaining +} + +func paceFrame(start time.Time) { + if remaining := remainingFrameTime(start, time.Now(), guiFrameInterval); remaining > 0 { + time.Sleep(remaining) + } +} diff --git a/cmd/mxl-player/frame_pacing_test.go b/cmd/mxl-player/frame_pacing_test.go new file mode 100644 index 0000000..7550c20 --- /dev/null +++ b/cmd/mxl-player/frame_pacing_test.go @@ -0,0 +1,29 @@ +package main + +import ( + "testing" + "time" +) + +func TestRemainingFrameTime(t *testing.T) { + start := time.Date(2026, time.September, 2, 0, 0, 0, 0, time.UTC) + interval := 16 * time.Millisecond + tests := []struct { + name string + elapsed time.Duration + want time.Duration + }{ + {name: "no work", want: interval}, + {name: "partial budget", elapsed: 5 * time.Millisecond, want: 11 * time.Millisecond}, + {name: "exact budget", elapsed: interval}, + {name: "over budget", elapsed: 20 * time.Millisecond}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + got := remainingFrameTime(start, start.Add(test.elapsed), interval) + if got != test.want { + t.Fatalf("remainingFrameTime() = %v, want %v", got, test.want) + } + }) + } +} diff --git a/cmd/mxl-player/main.go b/cmd/mxl-player/main.go index 7dd41ef..13952ad 100644 --- a/cmd/mxl-player/main.go +++ b/cmd/mxl-player/main.go @@ -532,6 +532,7 @@ func main() { if err := r.RecreateSwapchain(); err != nil { if errors.Is(err, renderer.ErrMinimized) { resized = true + paceFrame(frameStart) continue } panic(err) @@ -543,7 +544,7 @@ func main() { var shownSource playback.FeedConfig hasFrame := false - frameCtx, frameCancel := context.WithTimeout(ctx, 100*time.Millisecond) + frameCtx, frameCancel := context.WithTimeout(ctx, videoPollInterval) pendingFrame, frameErr := videoBridge.Next(frameCtx) frameCancel() @@ -1094,6 +1095,7 @@ func main() { if rerr := r.RecreateSwapchain(); rerr != nil { if errors.Is(rerr, renderer.ErrMinimized) { resized = true + paceFrame(frameStart) continue } panic(rerr) diff --git a/internal/playback/video_bridge.go b/internal/playback/video_bridge.go index aa971e6..1dfefc4 100644 --- a/internal/playback/video_bridge.go +++ b/internal/playback/video_bridge.go @@ -57,6 +57,19 @@ func (b *VideoBridge) Next( } } +// TryNext returns a frame only when a producer is already waiting. It never +// blocks the caller, allowing UI/render loops to run independently of video +// frame cadence. A returned frame has the same completion requirements as +// one returned by Next. +func (b *VideoBridge) TryNext() (*PendingVideoFrame, bool) { + select { + case pending := <-b.requests: + return pending, true + default: + return nil, false + } +} + func (f *PendingVideoFrame) Complete(err error) { f.completeOnce.Do(func() { f.result <- err diff --git a/internal/playback/video_bridge_test.go b/internal/playback/video_bridge_test.go index d66a424..0fbe697 100644 --- a/internal/playback/video_bridge_test.go +++ b/internal/playback/video_bridge_test.go @@ -144,3 +144,52 @@ func TestVideoBridgeNextHonorsCancellation(t *testing.T) { t.Fatalf("Next() error = %v, want %v", err, context.Canceled) } } + +func TestVideoBridgeTryNextReturnsImmediatelyWhenEmpty(t *testing.T) { + bridge := NewVideoBridge() + if pending, ok := bridge.TryNext(); ok || pending != nil { + t.Fatalf("TryNext() = %#v, %t; want nil, false", pending, ok) + } +} + +func TestVideoBridgeTryNextDeliversWithoutCopyAndRequiresCompletion(t *testing.T) { + bridge := NewVideoBridge() + frame := VideoFrame{Index: 9, Payload: []byte{1, 2, 3}} + consumeResult := make(chan error, 1) + started := make(chan struct{}) + go func() { + close(started) + consumeResult <- bridge.ConsumeVideo(context.Background(), frame) + }() + <-started + + deadline := time.Now().Add(videoBridgeTestTimeout) + var pending *PendingVideoFrame + for pending == nil && time.Now().Before(deadline) { + pending, _ = bridge.TryNext() + if pending == nil { + time.Sleep(time.Millisecond) + } + } + if pending == nil { + t.Fatal("TryNext() did not receive waiting producer") + } + if &pending.Frame.Payload[0] != &frame.Payload[0] { + t.Fatal("TryNext() copied borrowed payload") + } + select { + case err := <-consumeResult: + t.Fatalf("ConsumeVideo() returned before completion: %v", err) + default: + } + + pending.Complete(nil) + select { + case err := <-consumeResult: + if err != nil { + t.Fatalf("ConsumeVideo() error = %v", err) + } + case <-time.After(videoBridgeTestTimeout): + t.Fatal("ConsumeVideo() did not return after completion") + } +}