From 2424ff9f91923f34e6bdb8d18c6320a298665f8e Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Sat, 5 Sep 2026 16:21:10 +1000 Subject: [PATCH] Bound AV1 intra reference extensions by coded frame extent --- HEIF_IMPLEMENTATION_PLAN.md | 44 +++++++++++++ ...traSuperblockEncoder.ChromaModeDecision.cs | 12 +++- .../Av1IntraSuperblockEncoder.ModeDecision.cs | 16 ++++- .../Formats/Heif/Av1/Av1EncoderFrameTests.cs | 62 +++++++++++-------- .../Av1/Av1IntraSuperblockEncoderTests.cs | 35 ++++++----- 5 files changed, 123 insertions(+), 46 deletions(-) diff --git a/HEIF_IMPLEMENTATION_PLAN.md b/HEIF_IMPLEMENTATION_PLAN.md index 50b6152433..35c18d22fe 100644 --- a/HEIF_IMPLEMENTATION_PLAN.md +++ b/HEIF_IMPLEMENTATION_PLAN.md @@ -227,6 +227,50 @@ Color/output source coverage and remaining limits: The temporary comparison adapter supplies I420 using the managed RGB conversion. Its output cannot independently validate that RGB conversion, even when both codec decoders agree on native planes. +Intra-reference frame-extent correction after checkpoint `182f39ae5`, verified on 2026-09-05: + +- Source comparison found that extension availability and extension length had been conflated. + `Av1IntraSuperblockEncoder.ChromaModeDecision.cs:1211-1240` and + `Av1IntraSuperblockEncoder.ModeDecision.cs:2393-2449` previously copied a complete adjacent extent whenever + its coding-order availability flag was true. Reference `av1/common/reconintra.c:1737-1742,1817-1820` + also clips the available count to the remaining coded frame extent, then repeats the final available + sample in the edge preparation at `reconintra.c:1149-1184`. +- The new 56x56 production partition case failed before the correction with `ArgumentOutOfRangeException` + at the top-right copy (`edge-extent-before.trx`: two existing 32x32 cases passed, then execution stopped + on the new failure). This is an implementation defect, not a search-performance hypothesis. + Bottom-edge reads also require the bound: `Buffer2DRegion{T}.cs:89-97` limits row width but resolves the + row index against the backing buffer, whose encoder border can contain samples outside the coded region. +- Both shared luma/chroma references and tiled candidate references now bound adjacent samples by the + plane's coded extent before endpoint repetition. The existing availability rules and allocation ownership + remain the governing contracts; no new guard, rejection policy, owner, or scratch buffer was introduced. +- The rectangular reference cases retain all twelve earlier checks and add six explicit clipped-extent + cases across 8, 10, and 12 bits and both orientations. They use a larger backing buffer with distinct + values beyond the coded region, fixed expected edge sequences, and destination sentinels. + The production mixed-partition test retains both 32x32 orientations and adds both 56x56 orientations. +- Final Release net11.0 build: zero errors and 1,009 existing warnings. Serialized Visual Studio VSTest: + **273/273 passed** in `edge-extent-final.trx` (20.3789 seconds), covering encoder frames, intra-superblocks, + HEIF encoder contracts, and the retained empty-transform cost-helper test. Roslynk reports zero compiler errors. +- Fresh optimized-reference decoding of four regenerated partition streams matches all 8,320 retained luma samples. + Twelve regenerated moving color streams match all 21,348 Y/U/V samples. Combined maximum error is **0** across + **29,668** samples, with **0** samples exceeding one. These are same-bitstream decoder/reconstruction comparisons; + they do not establish separate-encoder parity or performance. No benchmark was run. + +The intra-edge investigation also confirmed these unresolved integration requirements: + +- `Av1PredictionDecoder.cs:989-1837` owns separate directional preparation, edge smoothing, upsampling, + strength selection, and neighboring-mode selection. Native `reconintra.c:989-1082,1349-1381` uses endpoint + extension, rounded nonnegative smoothing kernels, and clipped signed four-tap half-sample interpolation. + No new numerical discrepancy in those arithmetic kernels has been established by this comparison. +- Encoder `Av1EncoderModeDecisionWorkspace.cs:50-53,133-135` retains four raw edge spans with one prefix sample. + The decoder needs writable prefix positions -1 and -2 and candidate-specific filtering; mutating those raw + encoder spans across mode trials would contaminate later candidates. `Av1EncoderBlockWorkspace.cs:143-144` + exposes transform scratch whose lifetime must be reconciled with directional prediction before sharing it. +- `Av1IntraSuperblockEncoder.ModeDecision.cs:2272-2466` draws tiled edges from both committed reconstruction + and the current candidate mosaic. Enabling filtering must preserve that distinction, coded extents, + chroma neighbor ownership, corner preparation, and smooth-neighbor-dependent thresholds across all callers. + The sequence flag remains disabled pending that complete integration. The decoder's private kernels are + not a substitute for the required shared closed-generic traversal and semantic-operator architecture. + ### Required completion gates Motion-controller investigation continued after correction checkpoint `578ec34d9`: diff --git a/src/ImageSharp/Formats/Heif/Av1/Pipeline/Av1IntraSuperblockEncoder.ChromaModeDecision.cs b/src/ImageSharp/Formats/Heif/Av1/Pipeline/Av1IntraSuperblockEncoder.ChromaModeDecision.cs index 27db92d751..61ffe4e8a9 100644 --- a/src/ImageSharp/Formats/Heif/Av1/Pipeline/Av1IntraSuperblockEncoder.ChromaModeDecision.cs +++ b/src/ImageSharp/Formats/Heif/Av1/Pipeline/Av1IntraSuperblockEncoder.ChromaModeDecision.cs @@ -1208,7 +1208,12 @@ internal static partial class Av1IntraSuperblockEncoder left[..height].Fill(hasAbove ? above[0] : TOperator.CreateSample(midpoint + 1)); } - int topRightCount = hasTopRight ? Math.Min(width, height) : 0; + // Availability describes coding order, not the number of samples left at the frame boundary. + // A partially present adjacent block contributes only its coded samples before endpoint repetition. + int topRightCount = hasTopRight + ? Math.Min(Math.Min(width, height), reconstructionPlane.Width - blockOrigin.X - width) + : 0; + if (hasTopRight) { reconstructionPlane.DangerousGetRowSpan(blockOrigin.Y - 1) @@ -1219,7 +1224,10 @@ internal static partial class Av1IntraSuperblockEncoder int topCount = width + topRightCount; above[topCount..].Fill(above[topCount - 1]); - int bottomLeftCount = hasBottomLeft ? Math.Min(height, width) : 0; + int bottomLeftCount = hasBottomLeft + ? Math.Min(Math.Min(height, width), reconstructionPlane.Height - blockOrigin.Y - height) + : 0; + for (int row = height; row < height + bottomLeftCount; row++) { left[row] = reconstructionPlane.DangerousGetRowSpan(blockOrigin.Y + row)[blockOrigin.X - 1]; diff --git a/src/ImageSharp/Formats/Heif/Av1/Pipeline/Av1IntraSuperblockEncoder.ModeDecision.cs b/src/ImageSharp/Formats/Heif/Av1/Pipeline/Av1IntraSuperblockEncoder.ModeDecision.cs index 7ba63c6d02..f24353c8d6 100644 --- a/src/ImageSharp/Formats/Heif/Av1/Pipeline/Av1IntraSuperblockEncoder.ModeDecision.cs +++ b/src/ImageSharp/Formats/Heif/Av1/Pipeline/Av1IntraSuperblockEncoder.ModeDecision.cs @@ -2390,7 +2390,14 @@ internal static partial class Av1IntraSuperblockEncoder left[..transformHeight].Fill(hasAbove ? above[0] : TOperator.CreateSample(midpoint + 1)); } - int topRightCount = hasTopRight ? Math.Min(transformWidth, transformHeight) : 0; + // Candidate mosaics share the committed frame's coded extent. Padding beyond that extent is + // never a reference sample, even when coding order makes the adjacent block available. + int topRightCount = hasTopRight + ? Math.Min( + Math.Min(transformWidth, transformHeight), + reconstructionPlane.Width - planeBlockOrigin.X - columnOffset - transformWidth) + : 0; + if (hasTopRight) { if (transformRow > 0) @@ -2412,7 +2419,12 @@ internal static partial class Av1IntraSuperblockEncoder int topCount = transformWidth + topRightCount; above[topCount..].Fill(above[topCount - 1]); - int bottomLeftCount = hasBottomLeft ? Math.Min(transformHeight, transformWidth) : 0; + int bottomLeftCount = hasBottomLeft + ? Math.Min( + Math.Min(transformHeight, transformWidth), + reconstructionPlane.Height - planeBlockOrigin.Y - rowOffset - transformHeight) + : 0; + if (hasBottomLeft) { if (transformColumn > 0) diff --git a/tests/ImageSharp.Tests/Formats/Heif/Av1/Av1EncoderFrameTests.cs b/tests/ImageSharp.Tests/Formats/Heif/Av1/Av1EncoderFrameTests.cs index 092922eef3..1716de243e 100644 --- a/tests/ImageSharp.Tests/Formats/Heif/Av1/Av1EncoderFrameTests.cs +++ b/tests/ImageSharp.Tests/Formats/Heif/Av1/Av1EncoderFrameTests.cs @@ -30,35 +30,42 @@ public class Av1EncoderFrameTests private const int Yuv444 = (int)Av1ColorFormat.Yuv444; [Theory] - [InlineData(EightBit, false, false)] - [InlineData(EightBit, false, true)] - [InlineData(EightBit, true, false)] - [InlineData(EightBit, true, true)] - [InlineData(TenBit, false, false)] - [InlineData(TenBit, false, true)] - [InlineData(TenBit, true, false)] - [InlineData(TenBit, true, true)] - [InlineData(TwelveBit, false, false)] - [InlineData(TwelveBit, false, true)] - [InlineData(TwelveBit, true, false)] - [InlineData(TwelveBit, true, true)] - public void RectangularIntraReferencesExtendTheLastAvailableSample(int bitDepthValue, bool transpose, bool extensionAvailable) + [InlineData(EightBit, false, false, false)] + [InlineData(EightBit, false, true, false)] + [InlineData(EightBit, true, false, false)] + [InlineData(EightBit, true, true, false)] + [InlineData(TenBit, false, false, false)] + [InlineData(TenBit, false, true, false)] + [InlineData(TenBit, true, false, false)] + [InlineData(TenBit, true, true, false)] + [InlineData(TwelveBit, false, false, false)] + [InlineData(TwelveBit, false, true, false)] + [InlineData(TwelveBit, true, false, false)] + [InlineData(TwelveBit, true, true, false)] + [InlineData(EightBit, false, true, true)] + [InlineData(EightBit, true, true, true)] + [InlineData(TenBit, false, true, true)] + [InlineData(TenBit, true, true, true)] + [InlineData(TwelveBit, false, true, true)] + [InlineData(TwelveBit, true, true, true)] + public void RectangularIntraReferencesExtendTheLastAvailableSample(int bitDepthValue, bool transpose, bool extensionAvailable, bool limitedExtent) { Av1BitDepth bitDepth = (Av1BitDepth)bitDepthValue; if (bitDepth == Av1BitDepth.EightBit) { - AssertRectangularIntraReferences(bitDepth, transpose, extensionAvailable); + AssertRectangularIntraReferences(bitDepth, transpose, extensionAvailable, limitedExtent); } else { - AssertRectangularIntraReferences(bitDepth, transpose, extensionAvailable); + AssertRectangularIntraReferences(bitDepth, transpose, extensionAvailable, limitedExtent); } } private static void AssertRectangularIntraReferences( Av1BitDepth bitDepth, bool transpose, - bool extensionAvailable) + bool extensionAvailable, + bool limitedExtent) where TSample : unmanaged where TOperator : struct, Av1IntraSuperblockEncoder.IBlockEncodingOperator { @@ -76,14 +83,19 @@ public class Av1EncoderFrameTests // Native reconintra.c extends a four-sample edge through its four-sample neighbor, then // repeats sample seven to cover the twenty samples required by a 4x16 directional ray. - // These explicit offsets also distinguish unavailable neighbors from available extension. - int[] shortEdge = extensionAvailable - ? [0, 1, 2, 3, 4, 5, 6, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7] - : [0, 1, 2, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3]; - - int[] longEdge = extensionAvailable - ? [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19] - : [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 15, 15, 15, 15]; + // A clipped frame leaves only two adjacent samples; backing-buffer values beyond the region + // must not contribute. These explicit offsets distinguish all three extension cases. + int[] shortEdge = limitedExtent + ? [0, 1, 2, 3, 4, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5] + : extensionAvailable + ? [0, 1, 2, 3, 4, 5, 6, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7] + : [0, 1, 2, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3]; + + int[] longEdge = limitedExtent + ? [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 17, 17] + : extensionAvailable + ? [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19] + : [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 15, 15, 15, 15]; int[] expectedAbove = transpose ? longEdge : shortEdge; int[] expectedLeft = transpose ? shortEdge : longEdge; @@ -96,7 +108,7 @@ public class Av1EncoderFrameTests // The exact-sized interior includes the corner and twenty projected samples. Sentinel samples // on either side detect writes outside the reference view, including the former 2*long-edge span. Av1IntraSuperblockEncoder.ModeDecision.PrepareReferenceSamples( - plane.GetRegion(), + plane.GetRegion(0, 0, limitedExtent ? width + 3 : 33, limitedExtent ? height + 3 : 33), new Point(1, 1), width, height, diff --git a/tests/ImageSharp.Tests/Formats/Heif/Av1/Av1IntraSuperblockEncoderTests.cs b/tests/ImageSharp.Tests/Formats/Heif/Av1/Av1IntraSuperblockEncoderTests.cs index ff05a26f51..6dda775e2b 100644 --- a/tests/ImageSharp.Tests/Formats/Heif/Av1/Av1IntraSuperblockEncoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Heif/Av1/Av1IntraSuperblockEncoderTests.cs @@ -2918,11 +2918,12 @@ public class Av1IntraSuperblockEncoderTests /// Verifies that mixed partition trials and final writing retain the decoder's reconstruction order. /// [Theory] - [InlineData(false)] - [InlineData(true)] - public void ProductionMixedPartitionsPreserveReconstructionOrder(bool transpose) + [InlineData(false, 32)] + [InlineData(true, 32)] + [InlineData(false, 56)] + [InlineData(true, 56)] + public void ProductionMixedPartitionsPreserveReconstructionOrder(bool transpose, int size) { - const int Size = 32; const int QIndex = 4; ObuColorConfig colorConfig = new() { @@ -2932,12 +2933,12 @@ public class Av1IntraSuperblockEncoderTests BitDepth = Av1BitDepth.EightBit }; - using Av1EncoderFrameBuffer source = new(Configuration.Default, Size, Size, 8, Av1ColorFormat.Yuv400, 0, 0); - using Av1EncoderFrameBuffer reconstruction = new(Configuration.Default, Size, Size, 8, Av1ColorFormat.Yuv400, 0, 0); + using Av1EncoderFrameBuffer source = new(Configuration.Default, size, size, 8, Av1ColorFormat.Yuv400, 0, 0); + using Av1EncoderFrameBuffer reconstruction = new(Configuration.Default, size, size, 8, Av1ColorFormat.Yuv400, 0, 0); Buffer2DRegion sourcePlane = source.Frame.CodedView.GetPlane(Av1Plane.Y); - for (int y = 0; y < Size; y++) + for (int y = 0; y < size; y++) { - for (int x = 0; x < Size; x++) + for (int x = 0; x < size; x++) { // The lower-right quadrant contains two different square surfaces beside one vertical // surface. Transposition exercises the corresponding horizontal reconstruction order. @@ -2953,19 +2954,19 @@ public class Av1IntraSuperblockEncoderTests } ClearPlane(reconstruction.Luma); - using Av1EncoderModeInfoBuffer modeInfo = new(Configuration.Default, Size, Size, disallow4x4AllFrames: false); + using Av1EncoderModeInfoBuffer modeInfo = new(Configuration.Default, size, size, disallow4x4AllFrames: false); Av1PictureControlSet template = CreatePicture(modeInfo, colorConfig, use128x128Superblock: false, QIndex); using Av1EncoderPictureBuffer picture = new( - Configuration.Default, template.Sequence.SequenceHeader, template.Parent.FrameHeader, Size, Size, disallow4x4AllFrames: false); + Configuration.Default, template.Sequence.SequenceHeader, template.Parent.FrameHeader, size, size, disallow4x4AllFrames: false); - using Av1EncoderCoefficientBuffer coefficients = new(Configuration.Default, template.Sequence.SequenceHeader, Size, Size); + using Av1EncoderCoefficientBuffer coefficients = new(Configuration.Default, template.Sequence.SequenceHeader, size, size); using Av1EncoderSuperblockWorkspace superblockWorkspace = new(Configuration.Default); using Av1EncoderBlockWorkspace blockWorkspace = new(Configuration.Default); using Av1SymbolEncoder symbolEncoder = CreateTileSymbolEncoder(picture.Picture, 8192); Av1TileEncoder tileWriter = new( symbolEncoder, source.Frame, reconstruction.Frame, picture.Picture, coefficients, superblockWorkspace, blockWorkspace, effort: 9); - byte[] payload = WriteCompleteTileObu(picture.Picture, tileWriter, Size, Size); + byte[] payload = WriteCompleteTileObu(picture.Picture, tileWriter, size, size); using Av1Decoder decoder = new(Configuration.Default); decoder.DecodeSequenceReference(payload, null, null); Av1FrameInfo decodedInfo = Assert.IsType(decoder.FrameInfo); @@ -2973,10 +2974,10 @@ public class Av1IntraSuperblockEncoderTests Buffer2DRegion decodedPlane = decodedFrame.DeriveBlockPointer(Av1Plane.Y, 0, 0); Buffer2DRegion retainedPlane = reconstruction.Frame.CodedView.GetPlane(Av1Plane.Y); bool hasMixedPartition = false; - for (int y = 0; y < Size; y++) + for (int y = 0; y < size; y++) { Assert.Equal(retainedPlane.DangerousGetRowSpan(y).ToArray(), decodedPlane.DangerousGetRowSpan(y).ToArray()); - for (int x = 0; x < Size; x += 4) + for (int x = 0; x < size; x += 4) { Point position = new(x >> 2, y >> 2); Av1PartitionType partition = decodedInfo.GetModeInfoAt(position).PartitionType; @@ -2995,9 +2996,9 @@ public class Av1IntraSuperblockEncoderTests TestEnvironment.ActualOutputDirectoryFullPath, "Heif", "Av1", nameof(this.ProductionMixedPartitionsPreserveReconstructionOrder)); Directory.CreateDirectory(directory); - File.WriteAllBytes(Path.Combine(directory, $"{transpose}.obu"), payload); - using FileStream raw = File.Create(Path.Combine(directory, $"{transpose}.retained.yuv")); - for (int y = 0; y < Size; y++) + File.WriteAllBytes(Path.Combine(directory, $"{size}-{transpose}.obu"), payload); + using FileStream raw = File.Create(Path.Combine(directory, $"{size}-{transpose}.retained.yuv")); + for (int y = 0; y < size; y++) { raw.Write(retainedPlane.DangerousGetRowSpan(y)); }