From c256c18965408720aceeff8e6a3ee6c9d029852a Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Wed, 2 Sep 2026 16:18:45 +1000 Subject: [PATCH] Align and consolidate AV1 frame storage --- HEIF_IMPLEMENTATION_PLAN.md | 8 +- .../Formats/Heif/Av1/Av1FrameBuffer.cs | 261 +++++++++--------- .../Formats/Heif/Av1/Av1FrameBufferTests.cs | 42 +-- .../Formats/Heif/Av1/Av1YuvConverterTests.cs | 2 +- 4 files changed, 157 insertions(+), 156 deletions(-) diff --git a/HEIF_IMPLEMENTATION_PLAN.md b/HEIF_IMPLEMENTATION_PLAN.md index 7f03b54e33..70d372c0e5 100644 --- a/HEIF_IMPLEMENTATION_PLAN.md +++ b/HEIF_IMPLEMENTATION_PLAN.md @@ -93,10 +93,10 @@ The checkpoint is complete through `c4b4e4e0386328dea574a884b6fa36c360ad5a9b`. I - Current `Av1ReferenceMotionVectors` uses the same two-entry stop condition and retains an eight-entry stack for earlier candidates and DRL selection. - No production change is required. This remains spatial single-reference extension, not temporal extension. - [x] Establish and enforce the contiguous frame-plane invariant used by `Av1FrameBuffer` and inter reconstruction. - - Every frame plane is allocated with `preferContiguosImageBuffers: true`, so a constrained allocator cannot split a representable padded plane into normal memory groups. - - `Av1FrameBuffer` now rejects an external frame geometry whose padded plane reaches the `int.MaxValue` fallback boundary before any allocation. This makes every direct `DangerousGetSingleSpan` call an enforced owner invariant rather than a memory-group accident. - - `ConstructorRequestsContiguousPaddedPlanes` proves that a plane larger than the allocator's group capacity is one group. `ConstructorRejectsPaddedPlaneThatCannotBeContiguous` proves that an unrepresentable plane is rejected before allocation. - - The production path performs no plane copy and no per-block, per-row, or per-scanline allocation. + - One ImageSharp allocator owner now contains the aligned Y, U, and V storage, matching libaom's frame-buffer ownership while non-owning `Buffer2D` views preserve ImageSharp's row API. Coded dimensions are aligned to eight samples, the luma stride is aligned to 32 samples, and chroma strides and heights are derived from that luma layout exactly once. A 4K eight-bit 4:2:0 frame owner occupies about 17.3 MiB. + - The single owner removes the previous three-rent constructor and its allocation-cleanup `try/catch`. `Av1FrameBuffer` rejects external geometry whose complete aligned frame reaches the contiguous `int.MaxValue` boundary before allocation, making every direct `DangerousGetSingleSpan` call an enforced owner invariant. + - `ConstructorRequestsContiguousPaddedPlanes` proves that a frame larger than the allocator's group capacity remains one group. `ConstructorUsesOneFrameOwnerForAllPaddedPlanes` proves exact one-rent Y/U/V ownership and exactly-once return. `ConstructorRejectsPaddedPlaneThatCannotBeContiguous` proves that an unrepresentable frame is rejected before allocation, and the high-bit-depth stride regression proves the 608-sample libaom layout for a three-pixel coded row. + - The complete HEIF/AV1 namespace passes 8,808 of 8,808 direct net11 VSTest cases in Release after the physical layout change. The production path performs no plane copy and no per-block, per-row, or per-scanline allocation. - [x] Prove the real `Av1BlockDecoder.DecodeBlock` inter-reconstruction branch. - Decode the progressive dependent-frame fixture through the complete public production path. - Compare the final frame's native Y, Cb, and Cr planes exactly with current-main libaom output. diff --git a/src/ImageSharp/Formats/Heif/Av1/Av1FrameBuffer.cs b/src/ImageSharp/Formats/Heif/Av1/Av1FrameBuffer.cs index 98ff939cca..1a8c494db7 100644 --- a/src/ImageSharp/Formats/Heif/Av1/Av1FrameBuffer.cs +++ b/src/ImageSharp/Formats/Heif/Av1/Av1FrameBuffer.cs @@ -1,6 +1,7 @@ // Copyright (c) Six Labors. // Licensed under the Six Labors Split License. +using System.Buffers; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using SixLabors.ImageSharp.Formats.Heif.Av1.OpenBitstreamUnit; @@ -22,31 +23,6 @@ internal sealed class Av1FrameBuffer : IDisposable // taps. The normative 288-sample border keeps that entire source window directly addressable without block copies. public const int DecoderPaddingValue = 288; - /// - /// The allocation-mask bit for the luma plane. - /// - private const int PictureBufferYFlag = 1 << 0; - - /// - /// The allocation-mask bit for the first chroma plane. - /// - private const int PictureBufferCbFlag = 1 << 1; - - /// - /// The allocation-mask bit for the second chroma plane. - /// - private const int PictureBufferCrFlag = 1 << 2; - - /// - /// The allocation mask for a monochrome frame. - /// - private const int PictureBufferLumaMask = PictureBufferYFlag; - - /// - /// The allocation mask for a frame containing all three planes. - /// - private const int PictureBufferFullMask = PictureBufferYFlag | PictureBufferCbFlag | PictureBufferCrFlag; - /// /// The number of elements occupied by one logical sample. /// @@ -109,90 +85,45 @@ internal sealed class Av1FrameBuffer : IDisposable this.ColorFormat = colorFormat; this.Is16BitPipeline = is16BitPipeline; - int bufferEnableMask = sequenceHeader.ColorConfig.IsMonochrome ? PictureBufferLumaMask : PictureBufferFullMask; - - int leftPadding = DecoderPaddingValue; - int rightPadding = DecoderPaddingValue; - int topPadding = DecoderPaddingValue; - int bottomPadding = DecoderPaddingValue; - - this.StartPosition = new Point(leftPadding, topPadding); + this.StartPosition = new Point(DecoderPaddingValue, DecoderPaddingValue); this.Width = this.MaxWidth; this.Height = this.MaxHeight; - int strideY = this.MaxWidth + leftPadding + rightPadding; - int heightY = this.MaxHeight + topPadding + bottomPadding; - this.OriginX = leftPadding; - this.OriginY = topPadding; - int strideChroma = 0; - int heightChroma = 0; - switch (this.ColorFormat) + this.OriginX = DecoderPaddingValue; + this.OriginY = DecoderPaddingValue; + + FrameBufferLayout layout = CreateFrameBufferLayout( + allocationWidth, + allocationHeight, + colorFormat, + this.storageElementsPerSample); + + // Libaom stores Y, U, and V in one aligned frame allocation. Non-owning Buffer2D views retain ImageSharp's + // row API without introducing separate plane rents or constructor rollback paths. + IMemoryOwner owner = configuration.MemoryAllocator.Allocate(layout.StorageLength); + Memory storage = owner.Memory; + Buffer2D luma = Buffer2D.WrapMemory( + storage.Slice(0, layout.LumaElementCount), + layout.LumaStorageWidth, + layout.LumaHeight); + + ChromaPlanes? chroma = null; + if (!sequenceHeader.ColorConfig.IsMonochrome) { - case Av1ColorFormat.Yuv420: - strideChroma = (strideY + 1) >> 1; - heightChroma = (heightY + 1) >> 1; - break; - case Av1ColorFormat.Yuv422: - strideChroma = (strideY + 1) >> 1; - heightChroma = heightY; - break; - case Av1ColorFormat.Yuv444: - strideChroma = strideY; - heightChroma = heightY; - break; - } + Buffer2D chromaBlue = Buffer2D.WrapMemory( + storage.Slice(layout.ChromaBlueOffset, layout.ChromaElementCount), + layout.ChromaStorageWidth, + layout.ChromaHeight); - long lumaElementCount = (long)strideY * this.storageElementsPerSample * heightY; - long chromaElementCount = (long)strideChroma * this.storageElementsPerSample * heightChroma; - bool planesExceedContiguousLimit = - lumaElementCount >= int.MaxValue || - (bufferEnableMask == PictureBufferFullMask && chromaElementCount >= int.MaxValue); + Buffer2D chromaRed = Buffer2D.WrapMemory( + storage.Slice(layout.ChromaRedOffset, layout.ChromaElementCount), + layout.ChromaStorageWidth, + layout.ChromaHeight); - if (planesExceedContiguousLimit) - { - // The reconstruction operators use one span plus a constant stride to address padded neighbors. Reject an - // external geometry that cannot satisfy that ownership contract before Allocate2D falls back to groups. - throw new InvalidImageContentException("The AV1 frame dimensions exceed the contiguous decoder plane limit."); + chroma = new ChromaPlanes(chromaBlue, chromaRed); } - // Block reconstruction and the SIMD predictors address decoder padding through one span plus a constant row - // stride. Establish that invariant at the plane owner instead of copying fragmented groups in every hot path. - Buffer2D luma = configuration.MemoryAllocator.Allocate2D( - strideY * this.storageElementsPerSample, - heightY, - preferContiguosImageBuffers: true); - - Buffer2D? chromaBlue = null; - Buffer2D? chromaRed = null; - try - { - ChromaPlanes? chroma = null; - if (bufferEnableMask == PictureBufferFullMask) - { - chromaBlue = configuration.MemoryAllocator.Allocate2D( - strideChroma * this.storageElementsPerSample, - heightChroma, - preferContiguosImageBuffers: true); - - chromaRed = configuration.MemoryAllocator.Allocate2D( - strideChroma * this.storageElementsPerSample, - heightChroma, - preferContiguosImageBuffers: true); - - chroma = new ChromaPlanes(chromaBlue, chromaRed); - } - - this.planes = new(luma, chroma); - } - catch - { - // Construction publishes the owner only after every required plane has been rented. Release earlier planes - // here because a later allocation failure leaves no constructed frame buffer for the caller to dispose. - chromaRed?.Dispose(); - chromaBlue?.Dispose(); - luma.Dispose(); - throw; - } + this.planes = new(owner, luma, chroma); } /// @@ -291,37 +222,12 @@ internal sealed class Av1FrameBuffer : IDisposable (bytesPerSample + Unsafe.SizeOf() - 1) / Unsafe.SizeOf(), 1); - long strideY = (long)sequenceHeader.MaxFrameWidth + (DecoderPaddingValue * 2L); - long heightY = (long)sequenceHeader.MaxFrameHeight + (DecoderPaddingValue * 2L); Av1ColorFormat colorFormat = sequenceHeader.ColorConfig.IsMonochrome ? Av1ColorFormat.Yuv400 : maxColorFormat; - long strideChroma = 0; - long heightChroma = 0; - - switch (colorFormat) - { - case Av1ColorFormat.Yuv420: - strideChroma = (strideY + 1) >> 1; - heightChroma = (heightY + 1) >> 1; - break; - case Av1ColorFormat.Yuv422: - strideChroma = (strideY + 1) >> 1; - heightChroma = heightY; - break; - case Av1ColorFormat.Yuv444: - strideChroma = strideY; - heightChroma = heightY; - break; - } - - long lumaElementCount = strideY * storageElementsPerSample * heightY; - long chromaElementCount = strideChroma * storageElementsPerSample * heightChroma; - if (lumaElementCount >= int.MaxValue || - (!sequenceHeader.ColorConfig.IsMonochrome && chromaElementCount >= int.MaxValue)) - { - // Every decoder operator addresses padding through one contiguous span. Reject external sequence geometry - // before frame-wide syntax state is allocated so hostile dimensions cannot bypass allocator limits. - throw new InvalidImageContentException("The AV1 frame dimensions exceed the contiguous decoder plane limit."); - } + _ = CreateFrameBufferLayout( + sequenceHeader.MaxFrameWidth, + sequenceHeader.MaxFrameHeight, + colorFormat, + storageElementsPerSample); } /// @@ -413,6 +319,8 @@ internal sealed class Av1FrameBuffer : IDisposable chroma.Value.Blue.Dispose(); chroma.Value.Red.Dispose(); } + + activePlanes.Owner.Dispose(); } /// @@ -624,10 +532,64 @@ internal sealed class Av1FrameBuffer : IDisposable } /// - /// Carries the mandatory luma owner and the optional complete chroma pair as one state. + /// Calculates the aligned physical plane layout retained by one frame owner. + /// + private static FrameBufferLayout CreateFrameBufferLayout( + int width, + int height, + Av1ColorFormat colorFormat, + int storageElementsPerSample) + { + long alignedWidth = (width + 7L) & ~7L; + long alignedHeight = (height + 7L) & ~7L; + long lumaStride = (alignedWidth + (2L * DecoderPaddingValue) + 31L) & ~31L; + long lumaHeight = alignedHeight + (2L * DecoderPaddingValue); + int subsamplingX = colorFormat is Av1ColorFormat.Yuv420 or Av1ColorFormat.Yuv422 ? 1 : 0; + int subsamplingY = colorFormat == Av1ColorFormat.Yuv420 ? 1 : 0; + long chromaStride = colorFormat == Av1ColorFormat.Yuv400 ? 0 : lumaStride >> subsamplingX; + long chromaHeight = colorFormat == Av1ColorFormat.Yuv400 + ? 0 + : (alignedHeight >> subsamplingY) + (2L * (DecoderPaddingValue >> subsamplingY)); + + long lumaStorageWidth = lumaStride * storageElementsPerSample; + long chromaStorageWidth = chromaStride * storageElementsPerSample; + long lumaElementCount = lumaStorageWidth * lumaHeight; + long chromaElementCount = chromaStorageWidth * chromaHeight; + long planeAlignment = Math.Max(32 / Unsafe.SizeOf(), 1); + long chromaBlueOffset = ((lumaElementCount + planeAlignment - 1) / planeAlignment) * planeAlignment; + long chromaRedOffset = ((chromaBlueOffset + chromaElementCount + planeAlignment - 1) / planeAlignment) * planeAlignment; + long storageLength = colorFormat == Av1ColorFormat.Yuv400 + ? lumaElementCount + : chromaRedOffset + chromaElementCount; + + if (storageLength >= int.MaxValue) + { + // Reconstruction operators require one contiguous owner so every padded row remains directly addressable. + throw new InvalidImageContentException("The AV1 frame dimensions exceed the contiguous decoder frame limit."); + } + + return new FrameBufferLayout( + (int)lumaStorageWidth, + (int)lumaHeight, + (int)lumaElementCount, + (int)chromaStorageWidth, + (int)chromaHeight, + (int)chromaElementCount, + (int)chromaBlueOffset, + (int)chromaRedOffset, + (int)storageLength); + } + + /// + /// Carries the one frame owner, mandatory luma view, and optional complete chroma pair as one state. /// - private readonly struct FramePlanes(Buffer2D luma, ChromaPlanes? chroma) + private readonly struct FramePlanes(IMemoryOwner owner, Buffer2D luma, ChromaPlanes? chroma) { + /// + /// Gets the complete frame allocation. + /// + public IMemoryOwner Owner { get; } = owner; + /// /// Gets the padded luma plane. /// @@ -639,6 +601,39 @@ internal sealed class Av1FrameBuffer : IDisposable public ChromaPlanes? Chroma { get; } = chroma; } + /// + /// Describes the physical storage slices used by the component-plane views. + /// + private readonly struct FrameBufferLayout( + int lumaStorageWidth, + int lumaHeight, + int lumaElementCount, + int chromaStorageWidth, + int chromaHeight, + int chromaElementCount, + int chromaBlueOffset, + int chromaRedOffset, + int storageLength) + { + public int LumaStorageWidth { get; } = lumaStorageWidth; + + public int LumaHeight { get; } = lumaHeight; + + public int LumaElementCount { get; } = lumaElementCount; + + public int ChromaStorageWidth { get; } = chromaStorageWidth; + + public int ChromaHeight { get; } = chromaHeight; + + public int ChromaElementCount { get; } = chromaElementCount; + + public int ChromaBlueOffset { get; } = chromaBlueOffset; + + public int ChromaRedOffset { get; } = chromaRedOffset; + + public int StorageLength { get; } = storageLength; + } + private readonly struct ChromaPlanes(Buffer2D blue, Buffer2D red) { /// diff --git a/tests/ImageSharp.Tests/Formats/Heif/Av1/Av1FrameBufferTests.cs b/tests/ImageSharp.Tests/Formats/Heif/Av1/Av1FrameBufferTests.cs index f895399719..443e2f8688 100644 --- a/tests/ImageSharp.Tests/Formats/Heif/Av1/Av1FrameBufferTests.cs +++ b/tests/ImageSharp.Tests/Formats/Heif/Av1/Av1FrameBufferTests.cs @@ -17,7 +17,7 @@ using SixLabors.ImageSharp.Tests.Memory; namespace SixLabors.ImageSharp.Tests.Formats.Heif.Av1; /// -/// Verifies AV1 frame-plane allocation contracts and constructor rollback ownership. +/// Verifies AV1 frame-plane allocation contracts. /// [Trait("Format", "Avif")] public class Av1FrameBufferTests @@ -81,18 +81,19 @@ public class Av1FrameBufferTests } /// - /// Verifies that a failure while renting the final chroma plane releases every previously rented plane. + /// Verifies that all padded component planes share one frame owner. /// [Fact] - public void ConstructorFailureOnThirdPlaneReleasesEarlierPlanes() + public void ConstructorUsesOneFrameOwnerForAllPaddedPlanes() { - FailingTestMemoryAllocator allocator = new(failureAllocationNumber: 3); + TestMemoryAllocator allocator = new(); + allocator.EnableNonThreadSafeLogging(); Configuration configuration = Configuration.Default.Clone(); configuration.MemoryAllocator = allocator; ObuSequenceHeader sequenceHeader = new() { - MaxFrameWidth = 1, - MaxFrameHeight = 1, + MaxFrameWidth = 64, + MaxFrameHeight = 64, ColorConfig = new ObuColorConfig { IsMonochrome = false, @@ -102,19 +103,24 @@ public class Av1FrameBufferTests } }; - // All three padded planes fit in one backing owner each, making attempt three the Cr plane rent after Y and - // Cb have succeeded. The allocator log therefore contains exactly the two owners requiring rollback. - Assert.Throws( - () => new Av1FrameBuffer(configuration, sequenceHeader, Av1ColorFormat.Yuv420, false)); + TestMemoryAllocator.AllocationRequest allocation; + using (Av1FrameBuffer frameBuffer = new( + configuration, + sequenceHeader, + Av1ColorFormat.Yuv420, + false)) + { + allocation = Assert.Single(allocator.AllocationLog); + Assert.Empty(allocator.ReturnLog); + Assert.Equal(typeof(byte), allocation.ElementType); + Assert.Equal(614_400, allocation.Length); + Assert.Single(frameBuffer.GetPlaneBuffer(Av1Plane.Y).MemoryGroup); + Assert.Single(frameBuffer.GetPlaneBuffer(Av1Plane.U).MemoryGroup); + Assert.Single(frameBuffer.GetPlaneBuffer(Av1Plane.V).MemoryGroup); + } - Assert.Equal(3, allocator.AllocationAttemptCount); - Assert.Equal(2, allocator.AllocationLog.Count); - Assert.Equal(2, allocator.ReturnLog.Count); - Assert.All( - allocator.AllocationLog, - allocation => Assert.Single( - allocator.ReturnLog, - returned => returned.AllocationId == allocation.AllocationId)); + TestMemoryAllocator.ReturnRequest returned = Assert.Single(allocator.ReturnLog); + Assert.Equal(allocation.AllocationId, returned.AllocationId); } /// diff --git a/tests/ImageSharp.Tests/Formats/Heif/Av1/Av1YuvConverterTests.cs b/tests/ImageSharp.Tests/Formats/Heif/Av1/Av1YuvConverterTests.cs index 159b0fef01..0f0be7062e 100644 --- a/tests/ImageSharp.Tests/Formats/Heif/Av1/Av1YuvConverterTests.cs +++ b/tests/ImageSharp.Tests/Formats/Heif/Av1/Av1YuvConverterTests.cs @@ -253,7 +253,7 @@ public class Av1YuvConverterTests // Assert Assert.Equal(2, frameBuffer.BytesPerSample); - Assert.Equal(3 + (frameBuffer.OriginX * 2), stride); + Assert.Equal(608, stride); Assert.Equal(stride * 2, frameBuffer.GetPlaneBuffer(Av1Plane.Y).Width); Assert.Equal(321, frameBuffer.GetHighBitDepthRowSpan(Av1Plane.Y, 0, 0, 0)[0]); Assert.Equal(2, chromaRow.Length);