Browse Source

Release AV1 frame state when neighbor context construction fails

pull/2633/head
James Jackson-South 4 weeks ago
parent
commit
b305e6e896
  1. 23
      HEIF_IMPLEMENTATION_PLAN.md
  2. 23
      src/ImageSharp/Formats/Heif/Av1/Tiling/Av1TileReader.cs
  3. 89
      tests/ImageSharp.Tests/Formats/Heif/Av1/Av1TilingTests.cs

23
HEIF_IMPLEMENTATION_PLAN.md

@ -96,6 +96,29 @@ Reference-edge correction checkpoint `578ec34d9`, verified on 2026-09-05:
output parity, complete encoder control flow, or decoder-wide conformance. No benchmark was run. output parity, complete encoder control flow, or decoder-wide conformance. No benchmark was run.
Temporary launch scripts, native comparison output, logs, and reports remain local and are excluded from commits. Temporary launch scripts, native comparison output, logs, and reports remain local and are excluded from commits.
Tile-reader construction ownership correction, verified on 2026-09-05:
- At checkpoint `330d4c4ea`, `Av1TileReader.cs:279-316` acquired frame syntax storage before constructing
above/left neighbor contexts. Failure in the above constructor bypassed cleanup; failure in the left
constructor returned only the above context. Both leaked the completed `FrameInfo` owner. This is a
demonstrated lifetime defect, independent of reconstruction quality and encoder search completeness.
- Reference `av1/common/alloccommon.c:370-408,411-452` keeps partial above-context allocations reachable
from common state; decoder destruction calls `av1_remove_common` (`av1/decoder/decoder.c:239`), which
frees those contexts (`alloccommon.c:501-506`). Native contexts are reused until dimensions outgrow them
(`av1/decoder/decodeframe.c:5125-5135`); the managed per-frame allocation lifetime remains a separate
architectural deviation. This correction only restores cleanup at the existing managed owning boundary.
- The new regression failed when the ninth allocator request was rejected: all eight successful frame-state
allocations had no matching return (`tile-ownership-red.trx`). Serialized VSTest stopped on that failure.
The existing first catch now also covers above-context construction, and the left-context catch returns
both preceding owners. No new buffer, owner, guard, or native dependency was introduced.
- Four standalone-reader cases cover monochrome/color and 64/128 superblocks, reject every allocator request
in turn, and require exactly one return per successful allocation. After the final edit, the focused tiling,
reference-store, reference-motion, and decoder conformance set passed **137/137** through serialized
Release .NET 11 Visual Studio VSTest in 1.8500 minutes (`tile-ownership-final.trx`). The final build had
zero warnings/errors; Roslynk reported zero compiler errors. Reports remain in the temporary directory
`D:\GitHub\ynse01\av1-takeover-20260905`, outside the repository. No benchmark was run; this does not
establish separate-encoder sample parity or complete decoder ownership/reference-lifetime equivalence.
Sequence-construction ownership correction, verified subsequently on 2026-09-05: Sequence-construction ownership correction, verified subsequently on 2026-09-05:
- At checkpoint `578ec34d9`, `Av1FrameEncoder.cs:1492-1560,1668-1713,1778-1824` allocated common state and - At checkpoint `578ec34d9`, `Av1FrameEncoder.cs:1492-1560,1668-1713,1778-1824` allocated common state and

23
src/ImageSharp/Formats/Heif/Av1/Tiling/Av1TileReader.cs

@ -272,35 +272,36 @@ internal sealed class Av1TileReader : IAv1TileReader, IDisposable
} }
} }
// Above contexts span the aligned frame width, while left contexts are reused for each superblock row.
int planesCount = sequenceHeader.ColorConfig.PlaneCount;
int modeInfoWideColumnCount = Av1Math.AlignPowerOf2(
frameHeader.ModeInfoColumnCount,
sequenceHeader.SuperblockSizeLog2 - Av1Constants.ModeInfoSizeLog2);
try try
{ {
this.FrameInfo.InitializeSegmentIds(this.FrameHeader, this.primaryReferenceState); this.FrameInfo.InitializeSegmentIds(this.FrameHeader, this.primaryReferenceState);
this.FrameInfo.InitializeLoopRestoration(this.SequenceHeader, this.FrameHeader); this.FrameInfo.InitializeLoopRestoration(this.SequenceHeader, this.FrameHeader);
this.aboveNeighborContext = new Av1ParseAboveNeighbor4x4Context(configuration, planesCount, modeInfoWideColumnCount);
} }
catch catch
{ {
// FrameInfo has already rented the active frame's syntax storage. Return every successful rent if // FrameInfo owns the active frame's syntax storage. Return it if segmentation, restoration, or the
// a later segmentation or restoration allocation prevents this reader from being constructed. // first neighbor context fails, because no constructed reader reaches the caller's using statement.
this.FrameInfo.Dispose(); this.FrameInfo.Dispose();
throw; throw;
} }
// Above contexts span the aligned frame width, while left contexts are reused for each superblock row.
int planesCount = sequenceHeader.ColorConfig.PlaneCount;
int modeInfoWideColumnCount = Av1Math.AlignPowerOf2(
frameHeader.ModeInfoColumnCount,
sequenceHeader.SuperblockSizeLog2 - Av1Constants.ModeInfoSizeLog2);
this.aboveNeighborContext = new Av1ParseAboveNeighbor4x4Context(configuration, planesCount, modeInfoWideColumnCount);
try try
{ {
this.leftNeighborContext = new Av1ParseLeftNeighbor4x4Context(configuration, planesCount, sequenceHeader.SuperblockModeInfoSize); this.leftNeighborContext = new Av1ParseLeftNeighbor4x4Context(configuration, planesCount, sequenceHeader.SuperblockModeInfoSize);
} }
catch catch
{ {
// The reader is not returned when its second context allocation fails, so release the first rent here // The second context can fail after both frame state and the above context have acquired owners.
// rather than relying on an owner that the caller cannot reach. // Neither survives failed construction, so unwind both completed owners here.
this.aboveNeighborContext.Dispose(); this.aboveNeighborContext.Dispose();
this.FrameInfo.Dispose();
throw; throw;
} }

89
tests/ImageSharp.Tests/Formats/Heif/Av1/Av1TilingTests.cs

@ -12,12 +12,80 @@ using SixLabors.ImageSharp.Formats.Heif.Av1.Tiling;
using SixLabors.ImageSharp.Formats.Heif.Av1.Transform; using SixLabors.ImageSharp.Formats.Heif.Av1.Transform;
using SixLabors.ImageSharp.Memory; using SixLabors.ImageSharp.Memory;
using SixLabors.ImageSharp.PixelFormats; using SixLabors.ImageSharp.PixelFormats;
using SixLabors.ImageSharp.Tests.Memory;
namespace SixLabors.ImageSharp.Tests.Formats.Heif.Av1; namespace SixLabors.ImageSharp.Tests.Formats.Heif.Av1;
[Trait("Format", "Avif")] [Trait("Format", "Avif")]
public class Av1TilingTests public class Av1TilingTests
{ {
[Theory]
[InlineData(false, false)]
[InlineData(false, true)]
[InlineData(true, false)]
[InlineData(true, true)]
public void ConstructionFailureReturnsEveryAllocation(bool use128x128Superblock, bool monochrome)
{
ObuSequenceHeader sequenceHeader = new()
{
MaxFrameWidth = 128,
MaxFrameHeight = 128,
Use128x128Superblock = use128x128Superblock,
ColorConfig = new ObuColorConfig
{
BitDepth = Av1BitDepth.EightBit,
IsMonochrome = monochrome,
SubSamplingX = true,
SubSamplingY = true
}
};
ObuFrameHeader frameHeader = new()
{
FrameSize = new ObuFrameSize
{
FrameWidth = 128,
FrameHeight = 128,
SuperResolutionUpscaledWidth = 128,
RenderWidth = 128,
RenderHeight = 128
},
ModeInfoColumnCount = 32,
ModeInfoRowCount = 32,
ModeInfoStride = 32
};
Configuration configuration = Configuration.Default.Clone();
TestMemoryAllocator successfulAllocator = new();
successfulAllocator.EnableNonThreadSafeLogging();
configuration.MemoryAllocator = successfulAllocator;
using (Av1TileReader reader = new(configuration, sequenceHeader, frameHeader))
{
Assert.NotEmpty(successfulAllocator.AllocationLog);
}
Assert.Equal(successfulAllocator.AllocationLog.Count, successfulAllocator.ReturnLog.Count);
for (int failureIndex = 0; failureIndex < successfulAllocator.AllocationLog.Count; failureIndex++)
{
FailingTileAllocator allocator = new(failureIndex);
configuration.MemoryAllocator = allocator;
// Fail each actual rent, including nested frame-state and neighbor-context constructors. No reader
// reaches the caller's using statement on failure, so construction must return every completed owner.
InvalidMemoryOperationException exception = Assert.Throws<InvalidMemoryOperationException>(() =>
{
using Av1TileReader reader = new(configuration, sequenceHeader, frameHeader);
});
Assert.Equal("Tile allocation failure.", exception.Message);
Assert.Equal(failureIndex, allocator.AllocationLog.Count);
Assert.All(
allocator.AllocationLog,
allocation => Assert.Single(allocator.ReturnLog, returned => returned.AllocationId == allocation.AllocationId));
Assert.Equal(allocator.AllocationLog.Count, allocator.ReturnLog.Count);
}
}
/// <summary> /// <summary>
/// Verifies that frame mode-information indices do not wrap at the unsigned 16-bit boundary. /// Verifies that frame mode-information indices do not wrap at the unsigned 16-bit boundary.
/// </summary> /// </summary>
@ -347,4 +415,25 @@ public class Av1TilingTests
Assert.Equal(dataSize * 8, bitStreamReader.BitPosition); Assert.Equal(dataSize * 8, bitStreamReader.BitPosition);
Assert.Equal(superblockCount, frameDecoder.SuperblockCount); Assert.Equal(superblockCount, frameDecoder.SuperblockCount);
} }
private sealed class FailingTileAllocator : TestMemoryAllocator
{
private readonly int failureIndex;
public FailingTileAllocator(int failureIndex)
{
this.failureIndex = failureIndex;
this.EnableNonThreadSafeLogging();
}
protected override AllocationTrackedMemoryManager<T> AllocateCore<T>(int length, AllocationOptions options)
{
if (this.AllocationLog.Count == this.failureIndex)
{
throw new InvalidMemoryOperationException("Tile allocation failure.");
}
return base.AllocateCore<T>(length, options);
}
}
} }

Loading…
Cancel
Save