Browse Source

Resolve incompatible AV1 color settings before allocating conversion storage

pull/2633/head
James Jackson-South 4 weeks ago
parent
commit
182f39ae5e
  1. 36
      HEIF_IMPLEMENTATION_PLAN.md
  2. 6
      src/ImageSharp/Formats/Heif/Av1/Pipeline/Av1FrameEncoder.cs
  3. 7
      src/ImageSharp/Formats/Heif/HeifEncoderCore.Sequence.cs
  4. 22
      tests/ImageSharp.Tests/Formats/Heif/Av1/Av1EncoderFrameTests.cs
  5. 29
      tests/ImageSharp.Tests/Formats/Heif/HeifEncoderTests.cs

36
HEIF_IMPLEMENTATION_PLAN.md

@ -191,6 +191,42 @@ Coefficient optimization and evaluation-stage investigation:
`av1/encoder/speed_features.c:2709-2776` show usage-specific CPU settings and feature initialization.
ImageSharp's 0-10 effort scale has not yet been reconciled with these policies. No new effort mapping is assumed.
Color-conversion boundary correction after checkpoint `f7bd907d6`, verified on 2026-09-05:
- `HeifEncoderCore.Sequence.cs:74-194` resolved output sampling and preserved reversible YCgCo matrix metadata,
including when default 4:2:0 or requested 4:2:2 was incompatible. The shared converter's established contract
rejects that combination at `HeifColorConversionParameters.cs:265-282`. The public save regression failed
on default sampling with that exact exception (`matrix-fallback-before.trx`).
- PNG and TIFF resolve incompatible options through conversion at `PngEncoderCore.cs:1640-1654` and
`TiffEncoderCore.cs:378-444`. HEIF now extends its existing identity-matrix fallback to incompatible
YCgCo-Re/Ro sampling, converts with BT.601, and writes the matching matrix metadata. Explicit 4:4:4 remains
eligible for the existing reversible operator. This changes encoder option resolution, not decoder acceptance.
- Seven public cases retain the original profile object and values, inspect decoded matrix/range metadata,
and require byte-identical output to the same packed pixels explicitly encoded with the fallback matrix.
The cases include the original identity fallback and YCgCo-Re/Ro with default, 4:2:0, and 4:2:2 sampling.
- A separate lifetime defect existed at `Av1FrameEncoder.cs:1404-1434`: conversion parameters were resolved
after renting row storage. The internal-factory regression confirmed that rejected conversion had already
made one allocator request (`conversion-allocation-before.trx`). Resolution now precedes storage allocation;
the regression requires both allocation and return logs to remain empty. No new guard or owner was added.
- Final Release .NET 11 build: zero reported warnings and errors on the incremental build. Roslynk reports
zero compiler errors. Serialized Visual Studio VSTest with stop-on-failure passes 265/265 affected cases
(`conversion-final.trx` in the local takeover report directory).
The regenerated color and partition streams again match optimized native decoding on all 23,396 samples,
maximum error 0 and zero exceeding one. No benchmark or separate-encoder parity comparison was run.
Color/output source coverage and remaining limits:
- `Av1YuvConverter.cs:24-135,175-274` dispatches byte/high-bit-depth, complete/cropped/scaled output, and alpha
through shared HEIF adapters. `HeifPlanarColorConverter.cs:39-320,329-870` was read through both traversals:
conversion owns reusable row scratch, interpolates chroma before matrix conversion, and uses existing
`PixelOperations<TPixel>` packing/unpacking or Rgb48/Rgba64 conversion.
- `HeifColorConverter.Operator.cs:164-431` uses closed semantic operators and descending SIMD widths.
These architecture observations are not proof that every H.273 operator or pixel format is numerically correct.
Independent color-conversion and complete SIMD coverage remain open.
- The reference codec interface exposes native planes and strides (`aom/aom_image.h:284-292`).
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.
### Required completion gates
Motion-controller investigation continued after correction checkpoint `578ec34d9`:

6
src/ImageSharp/Formats/Heif/Av1/Pipeline/Av1FrameEncoder.cs

@ -1408,6 +1408,10 @@ internal static class Av1FrameEncoder
bool encodeAlpha,
bool usesHighBitDepth)
{
// Resolve conversion before renting storage: a rejected color description must not strand an owner
// in a constructor that never returns to the sequence encoder's disposal boundary.
this.parameters = Av1YuvConverter.GetConversionParameters(colorConfig, out HeifColorConversionMode mode);
this.colorConverter = HeifColorConverterBase.Create(mode, in this.parameters, colorConfig.IsMonochrome);
int subsamplingY = colorConfig.SubSamplingY ? 1 : 0;
int componentLength = encodeAlpha
? HeifPlanarAlphaEncoder.GetRowStorageLength(width)
@ -1427,8 +1431,6 @@ internal static class Av1FrameEncoder
Memory<float> storage = this.storageOwner.Memory;
this.componentMemory = storage[..componentLength];
this.packedMemory = storage.Slice(componentLength, packedFloatLength);
this.parameters = Av1YuvConverter.GetConversionParameters(colorConfig, out HeifColorConversionMode mode);
this.colorConverter = HeifColorConverterBase.Create(mode, in this.parameters, colorConfig.IsMonochrome);
this.encodeAlpha = encodeAlpha;
this.packedPixelCount = usesHighBitDepth && !encodeAlpha ? width : 0;
}

7
src/ImageSharp/Formats/Heif/HeifEncoderCore.Sequence.cs

@ -130,10 +130,13 @@ internal sealed partial class HeifEncoderCore
&& sourceColorProfile.ColorPrimaries == CicpColorPrimaries.ItuRBt709_6
&& sourceColorProfile.TransferCharacteristics == CicpTransferCharacteristics.Iec61966_2_1;
bool reversibleMatrix = sourceColorProfile.MatrixCoefficients is CicpMatrixCoefficients.YCgCoRe or CicpMatrixCoefficients.YCgCoRo;
if (sourceColorProfile.MatrixCoefficients == CicpMatrixCoefficients.Unspecified
|| (identityMatrix && !legalIdentityMatrix))
|| (identityMatrix && !legalIdentityMatrix)
|| (reversibleMatrix && !isMonochrome && chromaSubsampling != HeifChromaSubsampling.Yuv444))
{
// The converter uses BT.601 for unspecified or incompatible identity signaling, so record that actual matrix.
// Packed source pixels can be converted to the requested sampling even when their metadata
// describes a matrix that requires 4:4:4. Use and signal BT.601 without changing source metadata.
colorProfile = new CicpProfile(
(byte)sourceColorProfile.ColorPrimaries,
(byte)sourceColorProfile.TransferCharacteristics,

22
tests/ImageSharp.Tests/Formats/Heif/Av1/Av1EncoderFrameTests.cs

@ -621,6 +621,28 @@ public class Av1EncoderFrameTests
Assert.Equal(second.Size, decodedSecond.Size);
}
[Fact]
public void SequenceEncoderRejectsInvalidConversionBeforeAllocatingStorage()
{
ObuColorConfig colorConfig = CreateColorConfig(Av1BitDepth.TenBit, Av1ColorFormat.Yuv420);
colorConfig.MatrixCoefficients = ObuMatrixCoefficients.YCgCoRe;
Configuration configuration = Configuration.Default.Clone();
TestMemoryAllocator allocator = new();
allocator.EnableNonThreadSafeLogging();
configuration.MemoryAllocator = allocator;
// This internal factory receives resolved AV1 settings. The shared converter already rejects a
// reversible matrix with subsampling; that rejection must occur before any owner can be stranded.
Assert.Throws<InvalidImageContentException>(() =>
{
using Av1FrameEncoder.SequenceEncoder encoder = Av1FrameEncoder.CreateColorSequenceEncoder(
configuration, 32, 32, colorConfig, 17, 9);
});
Assert.Empty(allocator.AllocationLog);
Assert.Empty(allocator.ReturnLog);
}
[Theory]
[InlineData(false, EightBit)]
[InlineData(false, TenBit)]

29
tests/ImageSharp.Tests/Formats/Heif/HeifEncoderTests.cs

@ -990,23 +990,32 @@ public class HeifEncoderTests
Assert.Equal(Av1BitDepth.TenBit, sequenceHeader.ColorConfig.BitDepth);
}
[Fact]
public void Av1SanitizesIncompatibleIdentityMatrixWithoutMutatingSourceMetadata()
[Theory]
[InlineData(CicpMatrixCoefficients.Identity, HeifChromaSubsampling.Yuv420)]
[InlineData(CicpMatrixCoefficients.YCgCoRe, null)]
[InlineData(CicpMatrixCoefficients.YCgCoRe, HeifChromaSubsampling.Yuv420)]
[InlineData(CicpMatrixCoefficients.YCgCoRe, HeifChromaSubsampling.Yuv422)]
[InlineData(CicpMatrixCoefficients.YCgCoRo, null)]
[InlineData(CicpMatrixCoefficients.YCgCoRo, HeifChromaSubsampling.Yuv420)]
[InlineData(CicpMatrixCoefficients.YCgCoRo, HeifChromaSubsampling.Yuv422)]
public void Av1SanitizesIncompatibleMatrixWithoutMutatingSourceMetadata(
CicpMatrixCoefficients matrix,
HeifChromaSubsampling? subsampling)
{
using Image<Rgb24> image = new(8, 8);
CicpProfile sourceProfile = new(1, 13, 0, false);
using Image<Rgb24> image = new(8, 8, new Rgb24(32, 96, 192));
CicpProfile sourceProfile = new(1, 13, (byte)matrix, false);
image.Metadata.CicpProfile = sourceProfile;
using MemoryStream stream = new();
HeifEncoder encoder = new()
{
CompressionMethod = HeifCompressionMethod.Av1,
ChromaSubsampling = HeifChromaSubsampling.Yuv420,
ChromaSubsampling = subsampling,
Effort = 0
};
image.Save(stream, encoder);
Assert.Same(sourceProfile, image.Metadata.CicpProfile);
Assert.Equal(CicpMatrixCoefficients.Identity, sourceProfile.MatrixCoefficients);
Assert.Equal(matrix, sourceProfile.MatrixCoefficients);
Assert.False(sourceProfile.FullRange);
stream.Position = 0;
@ -1014,6 +1023,14 @@ public class HeifEncoderTests
CicpProfile decodedProfile = Assert.IsType<CicpProfile>(decoded.Metadata.CicpProfile);
Assert.Equal(CicpMatrixCoefficients.ItuRBt601_7_525, decodedProfile.MatrixCoefficients);
Assert.False(decodedProfile.FullRange);
// A fallback must change the actual encoded conversion as well as its metadata. Compare with the
// same packed pixels explicitly encoded using that fallback matrix and the requested sampling.
using Image<Rgb24> explicitConversion = image.Clone();
explicitConversion.Metadata.CicpProfile = new CicpProfile(1, 13, (byte)CicpMatrixCoefficients.ItuRBt601_7_525, false);
using MemoryStream expected = new();
explicitConversion.Save(expected, encoder);
Assert.Equal(expected.ToArray(), stream.ToArray());
}
[Fact]

Loading…
Cancel
Save