From e104cef490d1a75ce5144c4f847be4bacd6a52f2 Mon Sep 17 00:00:00 2001 From: Viktor Liu Date: Sat, 29 Aug 2026 15:51:19 +0200 Subject: [PATCH] Require a real first DXGI frame and read the FreeBSD framebuffer at its true pitch --- client/vnc/server/capture_dxgi_windows.go | 40 ++++++++++++- client/vnc/server/capture_fb_freebsd.go | 28 +++++++++- client/vnc/server/capture_fb_freebsd_test.go | 59 ++++++++++++++++++++ 3 files changed, 123 insertions(+), 4 deletions(-) create mode 100644 client/vnc/server/capture_fb_freebsd_test.go diff --git a/client/vnc/server/capture_dxgi_windows.go b/client/vnc/server/capture_dxgi_windows.go index f64ec7158..e4a050e63 100644 --- a/client/vnc/server/capture_dxgi_windows.go +++ b/client/vnc/server/capture_dxgi_windows.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "image" + "time" "github.com/kirides/go-d3d/d3d11" "github.com/kirides/go-d3d/outputduplication" @@ -61,13 +62,46 @@ func newDXGICapturer() (*dxgiCapturer, error) { height: h, } - // Grab the initial frame with a longer timeout to ensure we have - // a valid image before returning. - _ = dup.GetImage(c.img, 2000) + if err := c.grabFirstFrame(); err != nil { + c.close() + return nil, err + } return c, nil } +// Bounds the wait for the very first frame. Each attempt blocks for +// firstFrameAttempt, so the deadline allows a few of them. +const ( + firstFrameAttemptMS = 2000 + firstFrameDeadline = 6 * time.Second +) + +// grabFirstFrame fills c.img with a real desktop frame before the capturer is +// handed out. +// +// capture() deliberately tolerates ErrNoImageYet, because on an idle desktop +// DXGI reports "nothing new" and the right answer is the frame already in hand. +// At construction there is no such frame: accepting the timeout there would +// publish the all-zero buffer, and every session attaching in that window would +// be served a black screen that looks like a successful capture. Failing +// instead lets createCapturer fall back to GDI. +func (c *dxgiCapturer) grabFirstFrame() error { + deadline := time.Now().Add(firstFrameDeadline) + for { + err := c.dup.GetImage(c.img, firstFrameAttemptMS) + if err == nil { + return nil + } + if !errors.Is(err, outputduplication.ErrNoImageYet) { + return fmt.Errorf("acquire first desktop frame: %w", err) + } + if time.Now().After(deadline) { + return fmt.Errorf("no desktop frame within %s", firstFrameDeadline) + } + } +} + func (c *dxgiCapturer) capture() (*image.RGBA, error) { err := c.dup.GetImage(c.img, 100) if err != nil && !errors.Is(err, outputduplication.ErrNoImageYet) { diff --git a/client/vnc/server/capture_fb_freebsd.go b/client/vnc/server/capture_fb_freebsd.go index 5e5578556..697857e91 100644 --- a/client/vnc/server/capture_fb_freebsd.go +++ b/client/vnc/server/capture_fb_freebsd.go @@ -79,7 +79,12 @@ func NewFBCapturer(path string) (*FBCapturer, error) { } bpp := int(fbt.FbDepth) - stride := int(fbt.FbWidth) * (bpp / 8) + stride, err := freebsdFBStride(fbt) + if err != nil { + _ = unix.Munmap(mm) + unix.Close(fd) + return nil, err + } c := &FBCapturer{ path: path, fd: fd, // valid fd >= 0; we use -1 as the closed sentinel @@ -93,6 +98,27 @@ func NewFBCapturer(path string) (*FBCapturer, error) { return c, nil } +// freebsdFBStride returns the framebuffer's row pitch in bytes. +// +// Not width*bpp/8: a KMS-backed framebuffer commonly pads each row up to an +// alignment, and reading at the unpadded width starts every row a little +// further off than the last, which shears the whole image. fbtype exposes no +// stride field, so the pitch is derived from the mapping, which covers FbHeight +// rows. +// +// A size that cannot even hold the unpadded geometry means the two disagree; +// reading rows at the width the device reports would then run off the end of +// the mapping, so that is refused rather than guessed at. +func freebsdFBStride(fbt fbType) (int, error) { + unpadded := int(fbt.FbWidth) * (int(fbt.FbDepth) / 8) + stride := int(fbt.FbSize) / int(fbt.FbHeight) + if stride < unpadded { + return 0, fmt.Errorf("framebuffer size %d covers %d bytes per row, less than %dx%d at %d bpp needs", + fbt.FbSize, stride, fbt.FbWidth, fbt.FbHeight, fbt.FbDepth) + } + return stride, nil +} + // Width returns the framebuffer width. func (c *FBCapturer) Width() int { return c.w } diff --git a/client/vnc/server/capture_fb_freebsd_test.go b/client/vnc/server/capture_fb_freebsd_test.go new file mode 100644 index 000000000..6b1d6cbd0 --- /dev/null +++ b/client/vnc/server/capture_fb_freebsd_test.go @@ -0,0 +1,59 @@ +//go:build freebsd + +package server + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// The row pitch decides where every row after the first begins, so getting it +// from the reported width instead of the mapping shears the whole image on any +// device that pads its rows. +func TestFreebsdFBStride(t *testing.T) { + tests := []struct { + name string + fbt fbType + want int + wantErr bool + }{ + { + name: "unpadded 32bpp", + fbt: fbType{FbWidth: 1920, FbHeight: 1080, FbDepth: 32, FbSize: 1920 * 4 * 1080}, + want: 1920 * 4, + }, + { + name: "row padded up to an alignment", + fbt: fbType{FbWidth: 1366, FbHeight: 768, FbDepth: 32, FbSize: 5504 * 768}, + want: 5504, // 1366*4 = 5464, padded to 5504 + }, + { + name: "unpadded 16bpp", + fbt: fbType{FbWidth: 800, FbHeight: 600, FbDepth: 16, FbSize: 800 * 2 * 600}, + want: 800 * 2, + }, + { + // A mapping too small for the geometry it reports: reading rows at + // the reported width would run off the end of it. + name: "size cannot hold the geometry", + fbt: fbType{FbWidth: 1920, FbHeight: 1080, FbDepth: 32, FbSize: 1920 * 4 * 500}, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := freebsdFBStride(tt.fbt) + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + assert.Equal(t, tt.want, got) + assert.GreaterOrEqual(t, got, int(tt.fbt.FbWidth)*(int(tt.fbt.FbDepth)/8), + "the pitch can pad a row but never truncate it") + }) + } +}