diff --git a/HEIF_IMPLEMENTATION_PLAN.md b/HEIF_IMPLEMENTATION_PLAN.md index a4365c25e..9cad6257e 100644 --- a/HEIF_IMPLEMENTATION_PLAN.md +++ b/HEIF_IMPLEMENTATION_PLAN.md @@ -496,6 +496,7 @@ Exit gate: Implement and verify in dependency order: - [ ] OBU framing, sequence headers, frame headers, tile groups, byte alignment, and trailing bits. + - [x] Isolate every declared OBU payload with a zero-copy bounded span reader, advance ignored metadata and reserved units without parsing their bytes as headers, validate padding and trailing bytes, enforce primary/redundant/combined frame-header order, and reject lengths that cross the containing image-item boundary. Focused malformed-input tests and independent 8/10/12-bit AVIF reconstruction pass in Release. - [ ] One coherent decoder lifecycle that retains parsed frame and tile state and disposes all buffers deterministically. - [ ] Tile partitioning, mode information, segmentation, delta quantization, transform-size selection, coefficient token decode, inverse quantization, and inverse transforms. - [x] Match libaom's depth-first traversal and frame-edge behavior for all ten AV1 partition types. Independent 8/10/12-bit streams collectively select every terminal partition shape and contain nested block geometry that requires recursive `Split` traversal; their complete native planes remain byte-exact under normal hardware dispatch and the scalar fallback. diff --git a/src/ImageSharp/Formats/Heif/Av1/Av1BitStreamReader.cs b/src/ImageSharp/Formats/Heif/Av1/Av1BitStreamReader.cs index 35945069b..e8cc52496 100644 --- a/src/ImageSharp/Formats/Heif/Av1/Av1BitStreamReader.cs +++ b/src/ImageSharp/Formats/Heif/Av1/Av1BitStreamReader.cs @@ -87,18 +87,20 @@ internal ref struct Av1BitStreamReader ulong value = 0; length = 0; - for (int i = 0; i < 56; i += 7) + for (int shift = 0; shift < 56; shift += 7) { uint leb128Byte = this.ReadLiteral(8); - value |= (leb128Byte & 0x7FUL) << i; + value |= (leb128Byte & 0x7FUL) << shift; length++; if ((leb128Byte & 0x80U) == 0) { - break; + return value; } } - return value; + // AV1 limits unsigned LEB128 fields to eight bytes. A continuation bit in the eighth byte does not describe + // another value byte; accepting it would move the following OBU header into the declared size field. + throw new InvalidImageContentException("The AV1 LEB128 value is not terminated within eight bytes."); } /// @@ -204,11 +206,24 @@ internal ref struct Av1BitStreamReader /// The tile payload length in bytes. /// The tile payload span. public Span GetSymbolReader(int tileDataSize) + => this.ReadBytes(tileDataSize); + + /// + /// Gets the next byte-aligned portion of the encoded data and advances past it. + /// + /// The number of bytes to read. + /// The requested bytes. + public Span ReadBytes(int byteCount) { - DebugGuard.IsTrue(Av1Math.Modulus8(this.BitPosition) == 0, "Symbol reading needs to start on byte boundary."); - int bytesRead = Av1Math.DivideBy8Floor(this.BitPosition); - Span span = this.data.Slice(bytesRead, tileDataSize); - this.Skip(tileDataSize << 3); - return span; + DebugGuard.IsTrue(Av1Math.Modulus8(this.BitPosition) == 0, "Byte spans must start on a byte boundary."); + int byteOffset = Av1Math.DivideBy8Floor(this.BitPosition); + if ((uint)byteOffset > (uint)this.data.Length || (uint)byteCount > (uint)(this.data.Length - byteOffset)) + { + throw new InvalidImageContentException("The AV1 payload exceeds its declared data boundary."); + } + + Span payload = this.data.Slice(byteOffset, byteCount); + this.Skip(byteCount << 3); + return payload; } } diff --git a/src/ImageSharp/Formats/Heif/Av1/OpenBitstreamUnit/ObuHeader.cs b/src/ImageSharp/Formats/Heif/Av1/OpenBitstreamUnit/ObuHeader.cs index 35277bbe7..ed8deedcb 100644 --- a/src/ImageSharp/Formats/Heif/Av1/OpenBitstreamUnit/ObuHeader.cs +++ b/src/ImageSharp/Formats/Heif/Av1/OpenBitstreamUnit/ObuHeader.cs @@ -9,7 +9,7 @@ namespace SixLabors.ImageSharp.Formats.Heif.Av1.OpenBitstreamUnit; internal class ObuHeader { /// - /// Gets or sets the number of bytes occupied by the OBU header and its size field. + /// Gets or sets the number of bytes occupied by the fixed OBU header and optional extension. /// public int Size { get; set; } diff --git a/src/ImageSharp/Formats/Heif/Av1/OpenBitstreamUnit/ObuReader.cs b/src/ImageSharp/Formats/Heif/Av1/OpenBitstreamUnit/ObuReader.cs index 56411f41e..3b28ca151 100644 --- a/src/ImageSharp/Formats/Heif/Av1/OpenBitstreamUnit/ObuReader.cs +++ b/src/ImageSharp/Formats/Heif/Av1/OpenBitstreamUnit/ObuReader.cs @@ -35,101 +35,219 @@ internal class ObuReader /// A value indicating whether each OBU is prefixed by an Annex B length field. public void ReadAll(ref Av1BitStreamReader reader, int dataSize, Func creator, bool isAnnexB = false) { - bool seenFrameHeader = false; - bool frameDecodingFinished = false; - while (!frameDecodingFinished) + try { - int lengthSize = 0; - int payloadSize = 0; - if (isAnnexB) + int availableByteCount = reader.Length - Av1Math.DivideBy8Floor(reader.BitPosition); + if ((reader.BitPosition & 0x7) != 0 || (uint)dataSize > (uint)availableByteCount) { - ReadObuSize(ref reader, out payloadSize, out lengthSize); + throw new InvalidImageContentException("The AV1 OBU data boundary is invalid."); } - ObuHeader header = ReadObuHeaderSize(ref reader, out lengthSize); - if (isAnnexB) + bool seenFrameHeader = false; + bool frameDecodingFinished = false; + Span primaryFrameHeaderPayload = default; + while (dataSize > 0 && !frameDecodingFinished) { - header.PayloadSize -= header.Size; - dataSize -= lengthSize; - lengthSize = 0; - } - - payloadSize = header.PayloadSize; - dataSize -= header.Size + lengthSize; - if (isAnnexB && dataSize < payloadSize) - { - throw new InvalidImageContentException("Corrupt frame"); - } - - switch (header.Type) - { - case ObuType.SequenceHeader: - this.SequenceHeader = new(); - ReadSequenceHeader(ref reader, this.SequenceHeader); - break; - case ObuType.FrameHeader: - case ObuType.RedundantFrameHeader: - case ObuType.Frame: - if (header.Type != ObuType.Frame) - { - // Nothing to do here. - } - else if (header.Type != ObuType.FrameHeader) + int annexObuSize = 0; + if (isAnnexB) + { + ReadObuSize(ref reader, out annexObuSize, out int annexLengthSize); + if (annexLengthSize > dataSize || annexObuSize < 1) { - Guard.IsFalse(seenFrameHeader, nameof(seenFrameHeader), "Frame header expected"); + throw new InvalidImageContentException("The Annex B AV1 OBU length is invalid."); } - else + + dataSize -= annexLengthSize; + if (annexObuSize > dataSize) { - Guard.IsTrue(seenFrameHeader, nameof(seenFrameHeader), "Already decoded a frame header"); + throw new InvalidImageContentException("The Annex B AV1 OBU exceeds its temporal-unit boundary."); } + } + else if (dataSize < 1) + { + throw new InvalidImageContentException("The AV1 OBU header is truncated."); + } + + int obuStartBitPosition = reader.BitPosition; + ObuHeader header = ReadObuHeaderSize(ref reader, out _); + if (!isAnnexB && !header.HasSize) + { + // AV1 section 5 requires every low-overhead OBU to carry its own payload size. Only Annex B may + // derive the payload length from the outer obu_length field. + throw new InvalidImageContentException("A low-overhead AV1 OBU is missing its payload-size field."); + } + + int headerAndLengthSize = (reader.BitPosition - obuStartBitPosition) >> 3; + int boundedObuSize = isAnnexB ? annexObuSize : dataSize; + if (headerAndLengthSize > boundedObuSize) + { + throw new InvalidImageContentException("The AV1 OBU header exceeds its declared boundary."); + } + + int payloadSize = header.HasSize ? header.PayloadSize : boundedObuSize - headerAndLengthSize; + if ((uint)payloadSize > (uint)(boundedObuSize - headerAndLengthSize)) + { + throw new InvalidImageContentException("The AV1 OBU payload exceeds its declared boundary."); + } + + int completeObuSize = headerAndLengthSize + payloadSize; + if (isAnnexB && completeObuSize != annexObuSize) + { + throw new InvalidImageContentException("The nested and Annex B AV1 OBU lengths do not match."); + } + + dataSize -= isAnnexB ? annexObuSize : completeObuSize; + header.PayloadSize = payloadSize; + + // A dedicated payload reader prevents malformed syntax from consuming the following OBU. The parent + // advances once here, so ignored metadata, padding, and reserved OBUs are skipped without copying. + Span obuPayload = reader.ReadBytes(payloadSize); + Av1BitStreamReader payloadReader = new(obuPayload); + int decodedPayloadSize; + + switch (header.Type) + { + case ObuType.SequenceHeader: + this.SequenceHeader = new(); + ReadSequenceHeader(ref payloadReader, this.SequenceHeader); + decodedPayloadSize = Av1Math.DivideBy8Floor(payloadReader.BitPosition); + break; + case ObuType.FrameHeader: + if (this.SequenceHeader is null) + { + throw new InvalidImageContentException("An AV1 frame header appears before its sequence header."); + } + + if (seenFrameHeader) + { + throw new InvalidImageContentException("An AV1 frame contains more than one primary frame header."); + } - if (!seenFrameHeader) - { seenFrameHeader = true; this.FrameHeader = new(); - this.ReadFrameHeader(ref reader, header, header.Type != ObuType.Frame); - } + this.ReadFrameHeader(ref payloadReader, header, trailingBit: true); + decodedPayloadSize = Av1Math.DivideBy8Floor(payloadReader.BitPosition); + primaryFrameHeaderPayload = obuPayload[..decodedPayloadSize]; + break; + case ObuType.RedundantFrameHeader: + if (!seenFrameHeader) + { + throw new InvalidImageContentException("A redundant AV1 frame header appears before its primary frame header."); + } - if (header.Type != ObuType.Frame) - { - break; // For OBU_TILE_GROUP comes under OBU_FRAME - } + if (primaryFrameHeaderPayload.Length > obuPayload.Length + || !obuPayload[..primaryFrameHeaderPayload.Length].SequenceEqual(primaryFrameHeaderPayload)) + { + throw new InvalidImageContentException("The redundant AV1 frame header does not match its primary header."); + } - goto TILE_GROUP; - case ObuType.TileGroup: - TILE_GROUP: - if (!seenFrameHeader) - { - throw new InvalidImageContentException("Corrupt frame"); - } + // The primary header already owns the decoded frame state. Matching its encoded bytes avoids + // parsing the same adaptive frame-header syntax twice, as in libaom's decoder. + decodedPayloadSize = primaryFrameHeaderPayload.Length; + break; + case ObuType.Frame: + if (this.SequenceHeader is null) + { + throw new InvalidImageContentException("An AV1 frame appears before its sequence header."); + } - this.decoder ??= creator(); + if (seenFrameHeader) + { + throw new InvalidImageContentException("A combined AV1 frame OBU follows a separate frame header."); + } - // A combined frame OBU reaches this label after its frame-header portion has - // been consumed, leaving the same tile-group syntax as a standalone tile OBU. - this.ReadTileGroup(ref reader, this.decoder, header, out frameDecodingFinished); - if (frameDecodingFinished) - { + seenFrameHeader = true; + this.FrameHeader = new(); + this.ReadFrameHeader(ref payloadReader, header, trailingBit: false); + primaryFrameHeaderPayload = obuPayload[..Av1Math.DivideBy8Floor(payloadReader.BitPosition)]; + goto TILE_GROUP; + case ObuType.TileGroup: + TILE_GROUP: + if (!seenFrameHeader) + { + throw new InvalidImageContentException("An AV1 tile group appears before its frame header."); + } + + this.decoder ??= creator(); + + // A combined frame OBU reaches this label after its frame-header portion has + // been consumed, leaving the same tile-group syntax as a standalone tile OBU. + this.ReadTileGroup(ref payloadReader, this.decoder, header, out frameDecodingFinished); + decodedPayloadSize = Av1Math.DivideBy8Floor(payloadReader.BitPosition); + if (frameDecodingFinished) + { + seenFrameHeader = false; + } + + break; + case ObuType.TemporalDelimiter: + // AV1 section 5.6 defines no delimiter syntax. The common post-switch validation still permits + // zero bytes between the empty syntax and the declared payload boundary, matching libaom. seenFrameHeader = false; - } + decodedPayloadSize = 0; + break; + case ObuType.Padding: + int lastNonzeroIndex = obuPayload.Length - 1; + while (lastNonzeroIndex >= 0 && obuPayload[lastNonzeroIndex] == 0) + { + lastNonzeroIndex--; + } - break; - case ObuType.TemporalDelimiter: - // 5.6. Temporal delimiter obu syntax. - seenFrameHeader = false; - break; - default: - // Ignore unknown OBU types. - // throw new InvalidImageContentException($"Unknown OBU header found: {header.Type.ToString()}"); - break; - } + // AV1 padding contains only its trailing one bit and optional zero bytes. A header-only + // padding OBU is also valid, so the empty payload bypasses this final-byte check. + if (lastNonzeroIndex >= 0 && obuPayload[lastNonzeroIndex] != 0x80) + { + throw new InvalidImageContentException("The AV1 padding OBU has invalid trailing bits."); + } - dataSize -= payloadSize; - if (dataSize <= 0) - { - frameDecodingFinished = true; + if (obuPayload.Length > 0 && lastNonzeroIndex < 0) + { + throw new InvalidImageContentException("The AV1 padding OBU is missing its trailing one bit."); + } + + decodedPayloadSize = payloadSize; + break; + default: + // Metadata, tile-list, and reserved OBUs do not contribute to this still-image reconstruction + // pass. Their declared payload has already been skipped by the parent reader. libaom rejects a + // nonempty unrecognized payload that contains only zeros because it has no trailing one bit. + if (payloadSize > 0) + { + int ignoredLastNonzeroIndex = payloadSize - 1; + while (ignoredLastNonzeroIndex >= 0 && obuPayload[ignoredLastNonzeroIndex] == 0) + { + ignoredLastNonzeroIndex--; + } + + if (ignoredLastNonzeroIndex < 0) + { + throw new InvalidImageContentException("The ignored AV1 OBU is missing its trailing one bit."); + } + } + + decodedPayloadSize = payloadSize; + break; + } + + // Parsed syntax may be followed only by zero bytes within its declared OBU payload. Ignored metadata + // and reserved OBUs set decodedPayloadSize to the full payload because their syntax is not consumed here. + for (int i = decodedPayloadSize; i < obuPayload.Length; i++) + { + if (obuPayload[i] != 0) + { + throw new InvalidImageContentException("The AV1 OBU contains nonzero data after its decoded syntax."); + } + } } } + catch (IndexOutOfRangeException exception) + { + throw new InvalidImageContentException("The AV1 OBU syntax exceeds its payload boundary.", exception); + } + catch (ArgumentOutOfRangeException exception) + { + throw new InvalidImageContentException("The AV1 OBU syntax exceeds its payload boundary.", exception); + } } /// @@ -182,9 +300,9 @@ internal class ObuReader private static void ReadObuSize(ref Av1BitStreamReader reader, out int obuSize, out int lengthSize) { ulong rawSize = reader.ReadLittleEndianBytes128(out lengthSize); - if (rawSize > uint.MaxValue) + if (rawSize > int.MaxValue) { - throw new ImageFormatException("OBU block too large."); + throw new InvalidImageContentException("The AV1 OBU size exceeds the supported image payload limit."); } obuSize = (int)rawSize; diff --git a/tests/ImageSharp.Tests/Formats/Heif/Av1/ObuFrameHeaderTests.cs b/tests/ImageSharp.Tests/Formats/Heif/Av1/ObuFrameHeaderTests.cs index 5ef28f780..963e3c63d 100644 --- a/tests/ImageSharp.Tests/Formats/Heif/Av1/ObuFrameHeaderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Heif/Av1/ObuFrameHeaderTests.cs @@ -12,8 +12,6 @@ public class ObuFrameHeaderTests private static readonly byte[] DefaultSequenceHeaderBitStream = [0x0a, 0x06, 0b001_1_1_000, 0b00_1000_01, 0b11_110101, 0b001_11101, 0b111_1_1_1_0_1, 0b1_0_0_1_1_1_10]; - private static readonly byte[] KeyFrameHeaderBitStream = [0x32, 0x06, 0x10, 0x00]; - // Bits Syntax element Value // 1 obu_forbidden_bit 0 // 4 obu_type 2 (OBU_TEMPORAL_DELIMITER) @@ -133,16 +131,17 @@ public class ObuFrameHeaderTests } [Fact] - public void ReadHeaderWithoutSizeField() + public void ReadAnnexBHeaderWithoutSizeField() { // Arrange - byte[] bitStream = [0x10]; + // Annex B's outer obu_length is one byte and covers the size-less temporal-delimiter header. + byte[] bitStream = [0x01, 0x10]; Av1BitStreamReader reader = new(bitStream); ObuReader obuReader = new(); IAv1TileReader tileDecoder = new Av1TileDecoderStub(); // Act - obuReader.ReadAll(ref reader, bitStream.Length, () => tileDecoder); + obuReader.ReadAll(ref reader, bitStream.Length, () => tileDecoder, isAnnexB: true); // Assert Assert.Null(obuReader.SequenceHeader); @@ -168,6 +167,54 @@ public class ObuFrameHeaderTests Assert.Equal(ObuPrettyPrint.PrettyPrintProperties(expected), ObuPrettyPrint.PrettyPrintProperties(obuReader.SequenceHeader)); } + /// + /// Verifies that ignored OBU payloads are bounded and skipped before parsing the following sequence header. + /// + [Fact] + public void ReadIgnoredObusSkipsEachDeclaredPayload() + { + // 0x7A identifies padding and 0x4A identifies reserved OBU type 9. Both carry explicit sizes so their payload + // bytes must never be interpreted as another OBU header. + byte[] bitStream = + [ + 0x7A, 0x02, 0x80, 0x00, + 0x4A, 0x01, 0x80, + .. DefaultSequenceHeaderBitStream + ]; + + Av1BitStreamReader reader = new(bitStream); + ObuReader obuReader = new(); + IAv1TileReader tileDecoder = new Av1TileDecoderStub(); + + obuReader.ReadAll(ref reader, bitStream.Length, () => tileDecoder); + + Assert.NotNull(obuReader.SequenceHeader); + Assert.Equal(bitStream.Length * 8, reader.BitPosition); + } + + /// + /// Verifies that invalid OBU boundaries, size fields, and trailing bytes are rejected. + /// + /// The malformed OBU stream. + [Theory] + [InlineData(new byte[] { 0x7A, 0x02, 0x11 })] + [InlineData(new byte[] { 0x7A, 0x01, 0x00 })] + [InlineData(new byte[] { 0x12, 0x01, 0x01 })] + [InlineData(new byte[] { 0x10 })] + [InlineData(new byte[] { 0x7A, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80, 0x80 })] + public void ReadInvalidObuBoundaryThrows(byte[] bitStream) + => Assert.Throws(() => ReadObuStream(bitStream)); + + /// + /// Verifies that an empty temporal delimiter may occupy a payload containing only zero padding bytes. + /// + [Fact] + public void ReadTemporalDelimiterAllowsZeroPayloadPadding() + { + byte[] bitStream = [0x12, 0x02, 0x00, 0x00]; + ReadObuStream(bitStream); + } + [Fact] public void WriteTemporalDelimiter() { @@ -201,6 +248,9 @@ public class ObuFrameHeaderTests Assert.Equal(DefaultSequenceHeaderBitStream, actual); } + /// + /// Verifies that the combined frame OBU declares exactly the payload bytes emitted by the writer. + /// [Fact] public void WriteFrameHeader() { @@ -215,12 +265,25 @@ public class ObuFrameHeaderTests // Act obuWriter.WriteAll(Configuration.Default, stream, sequenceInput, frameInput, tileStub); - byte[] buffer = stream.GetBuffer(); + byte[] bitStream = stream.ToArray(); // Assert - // Skip over Temporal Delimiter and Sequence header. - byte[] actual = buffer.AsSpan().Slice(DefaultTemporalDelimiterBitStream.Length + DefaultSequenceHeaderBitStream.Length, KeyFrameHeaderBitStream.Length).ToArray(); - Assert.Equal(KeyFrameHeaderBitStream, actual); + int frameOffset = DefaultTemporalDelimiterBitStream.Length + DefaultSequenceHeaderBitStream.Length; + Span frameObu = bitStream.AsSpan(frameOffset); + byte expectedHeader = (byte)(((byte)ObuType.Frame << 3) | 0x02); + Assert.Equal(expectedHeader, frameObu[0]); + + Av1BitStreamReader sizeReader = new(frameObu[1..]); + ulong declaredPayloadSize = sizeReader.ReadLittleEndianBytes128(out int encodedSizeLength); + Assert.Equal(frameObu.Length - 1 - encodedSizeLength, (int)declaredPayloadSize); + + Av1BitStreamReader reader = new(bitStream); + ObuReader obuReader = new(); + obuReader.ReadAll(ref reader, bitStream.Length, () => new Av1TileDecoderStub()); + + Assert.NotNull(obuReader.SequenceHeader); + Assert.NotNull(obuReader.FrameHeader); + Assert.Equal(bitStream.Length * 8, reader.BitPosition); } private static ObuSequenceHeader GetDefaultSequenceHeader() @@ -290,6 +353,19 @@ public class ObuFrameHeaderTests AreFilmGrainingParametersPresent = true, }; + /// + /// Reads one complete OBU stream for malformed-input assertions that cannot capture a ref-struct reader. + /// + /// The complete encoded OBU stream. + private static void ReadObuStream(byte[] bitStream) + { + Av1BitStreamReader reader = new(bitStream); + ObuReader obuReader = new(); + IAv1TileReader tileDecoder = new Av1TileDecoderStub(); + + obuReader.ReadAll(ref reader, bitStream.Length, () => tileDecoder); + } + private static ObuFrameHeader GetKeyFrameHeader() => new() {