Require a real first DXGI frame and read the FreeBSD framebuffer at its true pitch

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) {

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 }
 

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")
+		})
+	}
+}