diff --git a/README.md b/README.md index a09fdca..c104e12 100644 --- a/README.md +++ b/README.md @@ -54,8 +54,10 @@ directory.PumpEvents(TimeSpan.FromMilliseconds(200)); using var client = directory.CreateClient(index: 0); -// Frames are Microsoft's IOSurface binding directly - dispose when done (it releases the retain). -using IOSurface.IOSurface? frame = client.TryGetFrame(); +// Frames are Microsoft's IOSurface binding directly. The surface belongs to the client - read it and +// let it go, never dispose it: a server recycles one surface, and the bindings keep a single managed +// peer per native object, so disposing would zero the handle every later frame comes back through. +IOSurface.IOSurface? frame = client.TryGetFrame(); if (frame is not null) { (int w, int h) = frame.PixelSize(); @@ -78,6 +80,7 @@ using Syphon.NET; (int w, int h) = surface.PixelSize(); // int-typed dimensions bool bgra = surface.IsBgra(); // format predicates (IsBgra / IsNv12) +int planes = surface.PlaneCount(); // 1 for packed BGRA, 2 for NV12 (int cw, int ch, int stride) = surface.PlaneInfo(1); // per-plane dims + row stride // Scoped CPU access yielding a Span; unlocks on dispose: diff --git a/samples/Syphon.NET.Peer/Program.cs b/samples/Syphon.NET.Peer/Program.cs index 3477609..ee68c02 100644 --- a/samples/Syphon.NET.Peer/Program.cs +++ b/samples/Syphon.NET.Peer/Program.cs @@ -74,7 +74,7 @@ static bool Loopback(int w, int h, out string detail) using SyphonClient client = server.CreateLoopbackClient(); // Publish repeatedly and keep the most recent delivered frame; discard an initial stale one - // by requiring a few publishes before accepting. + // by requiring a few publishes before accepting. Frames belong to the client - never dispose one. IOSurface.IOSurface? frame = null; var sw = Stopwatch.StartNew(); int published = 0; @@ -85,7 +85,6 @@ static bool Loopback(int w, int h, out string detail) IOSurface.IOSurface? f = client.TryGetFrame(); if (f is not null) { - frame?.Dispose(); frame = f; if (published >= 4) break; } @@ -94,7 +93,6 @@ static bool Loopback(int w, int h, out string detail) if (frame is null) { detail = "no frame delivered"; return false; } - using (frame) { (int gotW, int gotH) = frame.PixelSize(); if (gotW != w || gotH != h) @@ -242,12 +240,11 @@ static int CrossTest() while (psw.Elapsed < TimeSpan.FromSeconds(8)) { IOSurface.IOSurface? f = client.TryGetFrame(); - if (f is not null) { frame?.Dispose(); frame = f; if (++got >= 3) break; } + if (f is not null) { frame = f; if (++got >= 3) break; } Thread.Sleep(16); } if (frame is null) { Log("CROSS: no frame delivered across processes"); return 1; } - using (frame) { (int gotW, int gotH) = frame.PixelSize(); if (gotW != w || gotH != h) @@ -308,7 +305,6 @@ static int ForeignClient(string[] cmdArgs) } if (frame is null) { Log("[cs-client] no frame received from foreign server"); return 4; } - using (frame) { (int w, int h) = frame.PixelSize(); byte[] got = new byte[w * h * 4]; diff --git a/src/Syphon.NET/IOSurfaceExtensions.cs b/src/Syphon.NET/IOSurfaceExtensions.cs index bb58b3d..3828bd3 100644 --- a/src/Syphon.NET/IOSurfaceExtensions.cs +++ b/src/Syphon.NET/IOSurfaceExtensions.cs @@ -29,11 +29,29 @@ public static bool IsNv12(this IOSurface.IOSurface surface) => surface.PixelFormat == FourCcNv12VideoRange || surface.PixelFormat == FourCcNv12FullRange; /// Number of planes (1 for packed BGRA, 2 for NV12). - public static int PlaneCount(this IOSurface.IOSurface surface) => (int)surface.PlaneCount; + /// + /// IOSurface itself reports 0 for a non-planar surface and rejects its per-plane accessors + /// outright. A packed surface is one plane everywhere it matters (it is what + /// describes at index 0), so it is reported as such here - matching CoreVideo's plane model. + /// + public static int PlaneCount(this IOSurface.IOSurface surface) => Math.Max(1, (int)surface.PlaneCount); /// Dimensions and row stride of a plane (plane 0 = luma/BGRA, plane 1 = NV12 CbCr), as ints. - public static (int Width, int Height, int BytesPerRow) PlaneInfo(this IOSurface.IOSurface surface, int plane) => - ((int)surface.GetWidth((nuint)plane), (int)surface.GetHeight((nuint)plane), (int)surface.GetBytesPerRow((nuint)plane)); + /// + /// On a non-planar surface the per-plane accessors raise NSGenericException ("surface is not + /// planar"), so plane 0 is answered from the surface itself. + /// + /// The surface has no such plane. + public static (int Width, int Height, int BytesPerRow) PlaneInfo(this IOSurface.IOSurface surface, int plane) + { + ArgumentOutOfRangeException.ThrowIfNegative(plane); + ArgumentOutOfRangeException.ThrowIfGreaterThanOrEqual(plane, surface.PlaneCount()); + + if (surface.PlaneCount == 0) + return ((int)surface.Width, (int)surface.Height, (int)surface.BytesPerRow); + + return ((int)surface.GetWidth((nuint)plane), (int)surface.GetHeight((nuint)plane), (int)surface.GetBytesPerRow((nuint)plane)); + } // ---- Scoped CPU access ------------------------------------------------------------------------------- diff --git a/src/Syphon.NET/SyphonClient.cs b/src/Syphon.NET/SyphonClient.cs index b6907a2..3b83971 100644 --- a/src/Syphon.NET/SyphonClient.cs +++ b/src/Syphon.NET/SyphonClient.cs @@ -18,6 +18,9 @@ public sealed partial class SyphonClient : IDisposable private nint _handle; private GCHandle _self; private bool _firstFrameLogged; + // Strong reference to the most recent frame so the peer that callers hold is not finalized (and the + // surface released) between calls. + private IOSurface.IOSurface? _lastFrame; /// Raised on an arbitrary thread when a new frame becomes available. public event Action? FrameReady; @@ -70,16 +73,27 @@ public static SyphonClient Connect(ReadOnlySpan description, Action? onFra /// /// Return the latest frame's backing , or null if no new frame is available - /// since the last call. The surface is returned retained; dispose it once consumed (it releases the - /// retain). Zero-copy - the bytes live in shared GPU memory. + /// since the last call. Zero-copy - the bytes live in shared GPU memory. /// + /// + /// The surface belongs to the client: read it (for example with ) + /// and let it go - do not dispose it. A server publishes frames into one recycled surface, so + /// successive calls hand back the very same managed instance, and macOS bindings keep exactly one + /// managed peer per native object: disposing it would zero the handle of an instance the caller, the + /// publishing server and every later call still share, which then reports a surface with no size, no + /// planes and no pixels. The client holds the frame retained until the next call or until it is + /// disposed; keep the pixels, not the surface, if you need them longer. + /// public IOSurface.IOSurface? TryGetFrame() { ObjectDisposedException.ThrowIf(_handle == 0, this); nint surface = SyphonNative.sy_client_copy_new_frame(_handle); if (surface == 0) return null; if (!_firstFrameLogged) { _firstFrameLogged = true; LogFirstFrame(); } - return Runtime.GetINativeObject(surface, owns: true); + // owns: true consumes the shim's retain. When a peer for this surface already exists the runtime + // returns that instance and drops the extra retain, so the count stays flat across a frame loop. + _lastFrame = Runtime.GetINativeObject(surface, owns: true); + return _lastFrame; } [UnmanagedCallersOnly] @@ -102,6 +116,9 @@ public void Dispose() nint h = Interlocked.Exchange(ref _handle, 0); if (h != 0) SyphonNative.sy_client_destroy(h); if (_self.IsAllocated) _self.Free(); + // Drop the frame reference rather than disposing it: the peer may be shared with the publishing + // server (loopback) and with callers still holding it. + _lastFrame = null; } [LoggerMessage(Level = LogLevel.Debug, Message = "client created")] diff --git a/src/Syphon.NET/SyphonServer.cs b/src/Syphon.NET/SyphonServer.cs index 0bde053..4148016 100644 --- a/src/Syphon.NET/SyphonServer.cs +++ b/src/Syphon.NET/SyphonServer.cs @@ -26,6 +26,8 @@ public sealed partial class SyphonServer : IDisposable private readonly ILogger _logger; private nint _handle; private bool _firstPublishLogged; + // Strong reference to the surface most recently handed out by AcquireSurface; see the note there. + private IOSurface.IOSurface? _currentSurface; /// Create a server advertised to other applications under . /// Server name advertised to clients. @@ -68,6 +70,12 @@ public void Publish(IOSurface.IOSurface surface, bool flipped = false) /// Get a server-owned writable surface of the given size and format, recreated when the /// dimensions or format change. Write pixels into it, then call . /// + /// + /// The surface belongs to the server - do not dispose it. It is recycled while the size and + /// format hold, so successive calls hand back the very same managed instance (macOS bindings keep one + /// managed peer per native object); disposing it would zero the handle of an instance the server and + /// every later call still share, which then reports a surface with no size, no planes and no pixels. + /// public IOSurface.IOSurface AcquireSurface(int width, int height, CVPixelFormatType format = CVPixelFormatType.CV32BGRA) { ObjectDisposedException.ThrowIf(_handle == 0, this); @@ -75,9 +83,11 @@ public IOSurface.IOSurface AcquireSurface(int width, int height, CVPixelFormatTy ArgumentOutOfRangeException.ThrowIfNegativeOrZero(height); nint surface = SyphonNative.sy_server_acquire_surface(_handle, (uint)width, (uint)height, (uint)format); if (surface == 0) throw new InvalidOperationException("Failed to acquire a surface."); - // The server owns the surface (it recreates/releases it), so wrap it non-owning. - return Runtime.GetINativeObject(surface, owns: false) + // The server owns the surface (it recreates/releases it), so wrap it non-owning. Held onto so the + // peer callers write through is not finalized between acquire and publish. + _currentSurface = Runtime.GetINativeObject(surface, owns: false) ?? throw new InvalidOperationException("Failed to wrap the acquired surface."); + return _currentSurface; } /// Publish the surface most recently returned by . @@ -159,6 +169,9 @@ public void Dispose() { nint h = Interlocked.Exchange(ref _handle, 0); if (h != 0) SyphonNative.sy_server_destroy(h); + // Drop the reference rather than disposing: the peer is shared with any loopback client that + // received this surface, and with callers still holding it. + _currentSurface = null; } [LoggerMessage(Level = LogLevel.Debug, Message = "server created")] diff --git a/tests/Syphon.NET.Tests/SyphonTests.cs b/tests/Syphon.NET.Tests/SyphonTests.cs index 97d6c0c..9898420 100644 --- a/tests/Syphon.NET.Tests/SyphonTests.cs +++ b/tests/Syphon.NET.Tests/SyphonTests.cs @@ -58,7 +58,8 @@ private static void AssertLoopback(int w, int h) byte[] expected = Pattern(w, h); using SyphonClient client = server.CreateLoopbackClient(); - using IOSurface.IOSurface? surface = PollLatest(server, client, expected, w, h); + // Not disposed: delivered frames belong to the client (see SyphonClient.TryGetFrame). + IOSurface.IOSurface? surface = PollLatest(server, client, expected, w, h); Assert.IsNotNull(surface, "a published frame should be delivered to the loopback client"); (int gotW, int gotH) = surface.PixelSize(); @@ -76,7 +77,9 @@ private static void AssertLoopback(int w, int h) SyphonServer server, SyphonClient client, byte[] src, int w, int h) { // Publish repeatedly, keeping the most recent delivered frame and discarding an initial - // stale one by requiring a few publishes before accepting. + // stale one by requiring a few publishes before accepting. The frames are the client's to + // own - never dispose one here, or every later frame (the same managed peer) comes back with + // a zeroed handle. IOSurface.IOSurface? frame = null; var sw = Stopwatch.StartNew(); int published = 0; @@ -87,7 +90,6 @@ private static void AssertLoopback(int w, int h) IOSurface.IOSurface? f = client.TryGetFrame(); if (f is not null) { - frame?.Dispose(); frame = f; if (published >= 4) break; } @@ -96,6 +98,57 @@ private static void AssertLoopback(int w, int h) return frame; } + /// + /// A server recycles one surface, so every delivered frame is the same native object and the + /// bindings hand back the same managed peer. Polling a long run of frames has to keep yielding a + /// readable surface - it did not while the loop disposed each frame, which zeroed that shared peer. + /// + [TestMethod] + [TestCategory("Transport")] + public void RepeatedFrames_StayReadable() + { + const int w = 32, h = 16; + SyphonServer server = null!; + try + { + server = new SyphonServer("Syphon.NET Repeat Test"); + } + catch (DllNotFoundException) + { + Assert.Inconclusive("Native Syphon shim not present on this host."); + } + catch (PlatformNotSupportedException) + { + Assert.Inconclusive("No Metal device available on this host."); + } + + using (server) + { + byte[] expected = Pattern(w, h); + using SyphonClient client = server.CreateLoopbackClient(); + + int received = 0; + var sw = Stopwatch.StartNew(); + while (received < 10 && sw.Elapsed < TimeSpan.FromSeconds(10)) + { + server.PublishPixels(expected, w, h, CVPixelFormatType.CV32BGRA); + IOSurface.IOSurface? frame = client.TryGetFrame(); + if (frame is null) { Thread.Sleep(16); continue; } + + received++; + (int gotW, int gotH) = frame.PixelSize(); + Assert.AreEqual(w, gotW, $"frame {received} should still report its width"); + Assert.AreEqual(h, gotH, $"frame {received} should still report its height"); + + byte[] got = new byte[w * h * 4]; + frame.CopyTightlyPacked(got); + CollectionAssert.AreEqual(expected, got, $"frame {received} must round-trip"); + } + + Assert.AreEqual(10, received, "ten published frames should have been delivered"); + } + } + private static byte[] Pattern(int w, int h) { byte[] p = new byte[w * h * 4]; @@ -160,6 +213,74 @@ public void WritePixels_ThenCopyTightlyPacked_RoundTripsByteExact() } } + /// + /// IOSurface reports no planes for a packed surface and raises an Objective-C exception from its + /// per-plane accessors; the helpers present that surface as the single plane it is. + /// + [TestMethod] + [TestCategory("Transport")] + public void PlaneInfo_OnPackedBgra_DescribesTheWholeSurface() + { + const int w = 48, h = 32; + SyphonServer server = null!; + try + { + server = new SyphonServer("Syphon.NET Plane Test"); + } + catch (DllNotFoundException) + { + Assert.Inconclusive("Native Syphon shim not present on this host."); + } + catch (PlatformNotSupportedException) + { + Assert.Inconclusive("No Metal device available on this host."); + } + + using (server) + { + IOSurface.IOSurface surface = server.AcquireSurface(w, h, CVPixelFormatType.CV32BGRA); + Assert.AreEqual(1, surface.PlaneCount()); + + (int pw, int ph, int stride) = surface.PlaneInfo(0); + Assert.AreEqual(w, pw); + Assert.AreEqual(h, ph); + Assert.IsTrue(stride >= w * 4, "the stride must cover a row of pixels"); + + Assert.ThrowsExactly(() => surface.PlaneInfo(1)); + } + } + + /// + /// The server recycles its surface, and the bindings keep one managed peer per native object, so + /// repeated acquires are the same instance - which is why callers must not dispose it. + /// + [TestMethod] + [TestCategory("Transport")] + public void AcquireSurface_ReturnsTheRecycledInstance() + { + SyphonServer server = null!; + try + { + server = new SyphonServer("Syphon.NET Recycle Test"); + } + catch (DllNotFoundException) + { + Assert.Inconclusive("Native Syphon shim not present on this host."); + } + catch (PlatformNotSupportedException) + { + Assert.Inconclusive("No Metal device available on this host."); + } + + using (server) + { + IOSurface.IOSurface first = server.AcquireSurface(64, 64); + IOSurface.IOSurface second = server.AcquireSurface(64, 64); + Assert.AreSame(first, second); + Assert.AreEqual((64, 64), second.PixelSize()); + } + } + private static byte[] Pattern(int w, int h) { byte[] p = new byte[w * h * 4];