From 0d1d657bf832197dc5868e0aa914aa5bc2a25943 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Fri, 10 Jan 2020 16:46:09 +0100 Subject: [PATCH] Fix warnings, add additional comments --- src/ImageSharp/Formats/WebP/ColorCache.cs | 11 ++-- src/ImageSharp/Formats/WebP/HTreeGroup.cs | 24 +++---- src/ImageSharp/Formats/WebP/LosslessUtils.cs | 7 +- src/ImageSharp/Formats/WebP/Vp8LBitReader.cs | 66 +++++++++---------- src/ImageSharp/Formats/WebP/Vp8LTransform.cs | 2 +- .../Formats/WebP/WebPDecoderCore.cs | 26 ++++++-- src/ImageSharp/Formats/WebP/WebPFeatures.cs | 10 +-- .../Formats/WebP/WebPLosslessDecoder.cs | 32 ++++++--- .../Formats/WebP/WebPLossyDecoder.cs | 3 + src/ImageSharp/Formats/WebP/WebPMetadata.cs | 4 +- 10 files changed, 109 insertions(+), 76 deletions(-) diff --git a/src/ImageSharp/Formats/WebP/ColorCache.cs b/src/ImageSharp/Formats/WebP/ColorCache.cs index e9b4bc7482..1fc47180f7 100644 --- a/src/ImageSharp/Formats/WebP/ColorCache.cs +++ b/src/ImageSharp/Formats/WebP/ColorCache.cs @@ -5,20 +5,23 @@ namespace SixLabors.ImageSharp.Formats.WebP { internal class ColorCache { + private const uint KHashMul = 0x1e35a7bdu; + /// - /// Color entries. + /// Gets the color entries. /// public uint[] Colors { get; private set; } /// - /// Hash shift: 32 - hashBits. + /// Gets the hash shift: 32 - hashBits. /// public int HashShift { get; private set; } + /// + /// Gets the hash bits. + /// public int HashBits { get; private set; } - private const uint KHashMul = 0x1e35a7bdu; - public void Init(int hashBits) { int hashSize = 1 << hashBits; diff --git a/src/ImageSharp/Formats/WebP/HTreeGroup.cs b/src/ImageSharp/Formats/WebP/HTreeGroup.cs index 99d26844c6..c311601bb5 100644 --- a/src/ImageSharp/Formats/WebP/HTreeGroup.cs +++ b/src/ImageSharp/Formats/WebP/HTreeGroup.cs @@ -8,11 +8,10 @@ namespace SixLabors.ImageSharp.Formats.WebP /// /// Huffman table group. /// Includes special handling for the following cases: - /// - is_trivial_literal: one common literal base for RED/BLUE/ALPHA (not GREEN) - /// - is_trivial_code: only 1 code (no bit is read from bitstream) - /// - use_packed_table: few enough literal symbols, so all the bit codes - /// can fit into a small look-up table packed_table[] - /// The common literal base, if applicable, is stored in 'literal_arb'. + /// - IsTrivialLiteral: one common literal base for RED/BLUE/ALPHA (not GREEN) + /// - IsTrivialCode: only 1 code (no bit is read from the bitstream) + /// - UsePackedTable: few enough literal symbols, so all the bit codes can fit into a small look-up table PackedTable[] + /// The common literal base, if applicable, is stored in 'LiteralArb'. /// internal class HTreeGroup { @@ -27,32 +26,33 @@ namespace SixLabors.ImageSharp.Formats.WebP } /// - /// This has a maximum of HuffmanCodesPerMetaCode (5) entry's. + /// Gets the Huffman trees. This has a maximum of HuffmanCodesPerMetaCode (5) entry's. /// - public List HTrees { get; private set; } + public List HTrees { get; } /// - /// True, if huffman trees for Red, Blue & Alpha Symbols are trivial (have a single code). + /// Gets or sets a value indicating whether huffman trees for Red, Blue and Alpha Symbols are trivial (have a single code). /// public bool IsTrivialLiteral { get; set; } /// - /// If is_trivial_literal is true, this is the ARGB value of the pixel, with Green channel being set to zero. + /// Gets or sets a the literal argb value of the pixel. + /// If IsTrivialLiteral is true, this is the ARGB value of the pixel, with Green channel being set to zero. /// public uint LiteralArb { get; set; } /// - /// True if is_trivial_literal with only one code. + /// Gets or sets a value indicating whether there is only one code. /// public bool IsTrivialCode { get; set; } /// - /// use packed table below for short literal code + /// Gets or sets a value indicating whether to use packed table below for short literal code. /// public bool UsePackedTable { get; set; } /// - /// Table mapping input bits to a packed values, or escape case to literal code. + /// Gets or sets table mapping input bits to packed values, or escape case to literal code. /// public HuffmanCode[] PackedTable { get; set; } } diff --git a/src/ImageSharp/Formats/WebP/LosslessUtils.cs b/src/ImageSharp/Formats/WebP/LosslessUtils.cs index e047f18d39..f2a754a92d 100644 --- a/src/ImageSharp/Formats/WebP/LosslessUtils.cs +++ b/src/ImageSharp/Formats/WebP/LosslessUtils.cs @@ -138,7 +138,7 @@ namespace SixLabors.ImageSharp.Formats.WebP newBlue += ColorTransformDelta((sbyte)m.RedToBlue, (sbyte)newRed); newBlue &= 0xff; - pixelData[i] = (uint)((argb & 0xff00ff00u) | (newRed << 16) | newBlue); + pixelData[i] = (argb & 0xff00ff00u) | ((uint)newRed << 16) | (uint)newBlue; } } @@ -511,7 +511,7 @@ namespace SixLabors.ImageSharp.Formats.WebP (int)((c1 >> 8) & 0xff), (int)((c2 >> 8) & 0xff)); int b = AddSubtractComponentFull((int)(c0 & 0xff), (int)(c1 & 0xff), (int)(c2 & 0xff)); - return (uint)(((uint)a << 24) | (r << 16) | (g << 8) | b); + return ((uint)a << 24) | ((uint)r << 16) | ((uint)g << 8) | (uint)b; } private static uint ClampedAddSubtractHalf(uint c0, uint c1, uint c2) @@ -521,7 +521,7 @@ namespace SixLabors.ImageSharp.Formats.WebP int r = AddSubtractComponentHalf((int)((ave >> 16) & 0xff), (int)((c2 >> 16) & 0xff)); int g = AddSubtractComponentHalf((int)((ave >> 8) & 0xff), (int)((c2 >> 8) & 0xff)); int b = AddSubtractComponentHalf((int)(ave & 0xff), (int)(c2 & 0xff)); - return (uint)(((uint)a << 24) | (r << 16) | (g << 8) | b); + return ((uint)a << 24) | ((uint)r << 16) | ((uint)g << 8) | (uint)b; } private static int AddSubtractComponentHalf(int a, int b) @@ -576,7 +576,6 @@ namespace SixLabors.ImageSharp.Formats.WebP return Average2(Average2(a0, a1), Average2(a2, a3)); } - /// /// Computes sampled size of 'size' when sampling using 'sampling bits'. /// diff --git a/src/ImageSharp/Formats/WebP/Vp8LBitReader.cs b/src/ImageSharp/Formats/WebP/Vp8LBitReader.cs index 0bb69311f4..180f3cb5b2 100644 --- a/src/ImageSharp/Formats/WebP/Vp8LBitReader.cs +++ b/src/ImageSharp/Formats/WebP/Vp8LBitReader.cs @@ -13,17 +13,17 @@ namespace SixLabors.ImageSharp.Formats.WebP /// /// Maximum number of bits (inclusive) the bit-reader can handle. /// - private const int VP8L_MAX_NUM_BIT_READ = 24; + private const int Vp8LMaxNumBitRead = 24; /// /// Number of bits prefetched (= bit-size of vp8l_val_t). /// - private const int VP8L_LBITS = 64; + private const int Vp8LLbits = 64; /// /// Minimum number of bytes ready after VP8LFillBitWindow. /// - private const int VP8L_WBITS = 32; + private const int Vp8LWbits = 32; private readonly uint[] kBitMask = { @@ -38,6 +38,31 @@ namespace SixLabors.ImageSharp.Formats.WebP private readonly byte[] data; + /// + /// Pre-fetched bits. + /// + private ulong value; + + /// + /// Buffer length. + /// + private readonly long len; + + /// + /// Byte position in buffer. + /// + private long pos; + + /// + /// Current bit-reading position in value. + /// + private int bitPos; + + /// + /// True if a bit was read past the end of buffer. + /// + private bool eos; + /// /// Initializes a new instance of the class. /// @@ -72,31 +97,6 @@ namespace SixLabors.ImageSharp.Formats.WebP this.pos = length; } - /// - /// Pre-fetched bits. - /// - private ulong value; - - /// - /// Buffer length. - /// - private readonly long len; - - /// - /// Byte position in buffer. - /// - private long pos; - - /// - /// Current bit-reading position in value. - /// - private int bitPos; - - /// - /// True if a bit was read past the end of buffer. - /// - private bool eos; - /// /// Reads a unsigned short value from the inputStream. The bits of each byte are read in least-significant-bit-first order. /// @@ -106,7 +106,7 @@ namespace SixLabors.ImageSharp.Formats.WebP { Guard.MustBeGreaterThan(nBits, 0, nameof(nBits)); - if (!this.eos && nBits <= VP8L_MAX_NUM_BIT_READ) + if (!this.eos && nBits <= Vp8LMaxNumBitRead) { ulong val = this.PrefetchBits() & this.kBitMask[nBits]; int newBits = this.bitPos + nBits; @@ -134,12 +134,12 @@ namespace SixLabors.ImageSharp.Formats.WebP public ulong PrefetchBits() { - return this.value >> (this.bitPos & (VP8L_LBITS - 1)); + return this.value >> (this.bitPos & (Vp8LLbits - 1)); } public void FillBitWindow() { - if (this.bitPos >= VP8L_WBITS) + if (this.bitPos >= Vp8LWbits) { this.DoFillBitWindow(); } @@ -152,7 +152,7 @@ namespace SixLabors.ImageSharp.Formats.WebP public bool IsEndOfStream() { - return this.eos || ((this.pos == this.len) && (this.bitPos > VP8L_LBITS)); + return this.eos || ((this.pos == this.len) && (this.bitPos > Vp8LLbits)); } private void ShiftBytes() @@ -160,7 +160,7 @@ namespace SixLabors.ImageSharp.Formats.WebP while (this.bitPos >= 8 && this.pos < this.len) { this.value >>= 8; - this.value |= (ulong)this.data[this.pos] << (VP8L_LBITS - 8); + this.value |= (ulong)this.data[this.pos] << (Vp8LLbits - 8); ++this.pos; this.bitPos -= 8; } diff --git a/src/ImageSharp/Formats/WebP/Vp8LTransform.cs b/src/ImageSharp/Formats/WebP/Vp8LTransform.cs index 580c03dc77..78874554a1 100644 --- a/src/ImageSharp/Formats/WebP/Vp8LTransform.cs +++ b/src/ImageSharp/Formats/WebP/Vp8LTransform.cs @@ -24,7 +24,7 @@ namespace SixLabors.ImageSharp.Formats.WebP public Vp8LTransformType TransformType { get; } /// - /// Subsampling bits defining transform window. + /// Gets or sets the subsampling bits defining the transform window. /// public int Bits { get; set; } diff --git a/src/ImageSharp/Formats/WebP/WebPDecoderCore.cs b/src/ImageSharp/Formats/WebP/WebPDecoderCore.cs index 5d61035437..0e5e79994f 100644 --- a/src/ImageSharp/Formats/WebP/WebPDecoderCore.cs +++ b/src/ImageSharp/Formats/WebP/WebPDecoderCore.cs @@ -64,6 +64,12 @@ namespace SixLabors.ImageSharp.Formats.WebP this.options = options; } + /// + /// Decodes the image from the specified and sets the data to the image. + /// + /// The pixel format. + /// The stream, where the image should be. + /// The decoded image. public Image Decode(Stream stream) where TPixel : struct, IPixel { @@ -80,7 +86,7 @@ namespace SixLabors.ImageSharp.Formats.WebP Buffer2D pixels = image.GetRootFramePixelBuffer(); if (imageInfo.IsLossLess) { - var losslessDecoder = new WebPLosslessDecoder(imageInfo.Vp9LBitReader, (int)imageInfo.ImageDataSize); + var losslessDecoder = new WebPLosslessDecoder(imageInfo.Vp9LBitReader); losslessDecoder.Decode(pixels, image.Width, image.Height); } else @@ -114,6 +120,10 @@ namespace SixLabors.ImageSharp.Formats.WebP return new ImageInfo(new PixelTypeInfo(bitsPerPixel), imageInfo.Width, imageInfo.Height, this.metadata); } + /// + /// Reads and skips over the image header. + /// + /// The chunk size in bytes. private uint ReadImageHeader() { // Skip FourCC header, we already know its a RIFF file at this point. @@ -133,7 +143,7 @@ namespace SixLabors.ImageSharp.Formats.WebP private WebPImageInfo ReadVp8Info() { this.metadata = new ImageMetadata(); - this.webpMetadata = metadata.GetFormatMetadata(WebPFormat.Instance); + this.webpMetadata = this.metadata.GetFormatMetadata(WebPFormat.Instance); WebPChunkType chunkType = this.ReadChunkType(); @@ -328,9 +338,13 @@ namespace SixLabors.ImageSharp.Formats.WebP }; } + /// + /// Parses optional metadata chunks. + /// + /// The webp features. private void ParseOptionalChunks(WebPFeatures features) { - if (features.ExifProfile == false && features.XmpMetaData == false) + if (features.ExifProfile is false && features.XmpMetaData is false) { return; } @@ -354,7 +368,7 @@ namespace SixLabors.ImageSharp.Formats.WebP /// private WebPChunkType ReadChunkType() { - if (this.currentStream.Read(this.buffer, 0, 4) == 4) + if (this.currentStream.Read(this.buffer, 0, 4) is 4) { var chunkType = (WebPChunkType)BinaryPrimitives.ReadUInt32BigEndian(this.buffer); this.webpMetadata.ChunkTypes.Enqueue(chunkType); @@ -371,10 +385,10 @@ namespace SixLabors.ImageSharp.Formats.WebP /// The chunk size in bytes. private uint ReadChunkSize() { - if (this.currentStream.Read(this.buffer, 0, 4) == 4) + if (this.currentStream.Read(this.buffer, 0, 4) is 4) { uint chunkSize = BinaryPrimitives.ReadUInt32LittleEndian(this.buffer); - return (chunkSize % 2 == 0) ? chunkSize : chunkSize + 1; + return (chunkSize % 2 is 0) ? chunkSize : chunkSize + 1; } throw new ImageFormatException("Invalid WebP data."); diff --git a/src/ImageSharp/Formats/WebP/WebPFeatures.cs b/src/ImageSharp/Formats/WebP/WebPFeatures.cs index 653c8fa672..6ae4f1e9d4 100644 --- a/src/ImageSharp/Formats/WebP/WebPFeatures.cs +++ b/src/ImageSharp/Formats/WebP/WebPFeatures.cs @@ -9,27 +9,27 @@ namespace SixLabors.ImageSharp.Formats.WebP public class WebPFeatures { /// - /// Gets or sets whether this image has a ICC Profile. + /// Gets or sets a value indicating whether this image has a ICC Profile. /// public bool IccProfile { get; set; } /// - /// Gets or sets whether this image has a alpha channel. + /// Gets or sets a value indicating whether this image has a alpha channel. /// public bool Alpha { get; set; } /// - /// Gets or sets whether this image has a EXIF Profile. + /// Gets or sets a value indicating whether this image has a EXIF Profile. /// public bool ExifProfile { get; set; } /// - /// Gets or sets whether this image has XMP Metadata. + /// Gets or sets a value indicating whether this image has XMP Metadata. /// public bool XmpMetaData { get; set; } /// - /// Gets or sets whether this image is a animation. + /// Gets or sets a value indicating whether this image is a animation. /// public bool Animation { get; set; } } diff --git a/src/ImageSharp/Formats/WebP/WebPLosslessDecoder.cs b/src/ImageSharp/Formats/WebP/WebPLosslessDecoder.cs index 8d7de75132..887ed8691d 100644 --- a/src/ImageSharp/Formats/WebP/WebPLosslessDecoder.cs +++ b/src/ImageSharp/Formats/WebP/WebPLosslessDecoder.cs @@ -21,8 +21,6 @@ namespace SixLabors.ImageSharp.Formats.WebP { private readonly Vp8LBitReader bitReader; - private readonly int imageDataSize; - private static readonly int BitsSpecialMarker = 0x100; private static readonly uint PackedNonLiteralCode = 0; @@ -72,12 +70,22 @@ namespace SixLabors.ImageSharp.Formats.WebP 0, 1, 1, 1, 0 }; - public WebPLosslessDecoder(Vp8LBitReader bitReader, int imageDataSize) + /// + /// Initializes a new instance of the class. + /// + /// Bitreader to read from the stream. + public WebPLosslessDecoder(Vp8LBitReader bitReader) { this.bitReader = bitReader; - this.imageDataSize = imageDataSize; } + /// + /// Decodes the image from the stream using the bitreader. + /// + /// The pixel format. + /// The pixel buffer to store the decoded data. + /// The width of the image. + /// The height of the image. public void Decode(Buffer2D pixels, int width, int height) where TPixel : struct, IPixel { @@ -113,7 +121,11 @@ namespace SixLabors.ImageSharp.Formats.WebP if (colorCachePresent) { colorCacheBits = (int)this.bitReader.ReadBits(4); - // TODO: error check color cache bits + bool coloCacheBitsIsValid = colorCacheBits >= 1 && colorCacheBits <= WebPConstants.MaxColorCacheBits; + if (!coloCacheBitsIsValid) + { + WebPThrowHelper.ThrowImageFormatException("Invalid color cache bits found"); + } } // Read the Huffman codes (may recurse). @@ -192,7 +204,7 @@ namespace SixLabors.ImageSharp.Formats.WebP int lastCached = decodedPixels; while (decodedPixels < totalPixels) { - int code = 0; + int code; if ((col & mask) == 0) { hTreeGroup = this.GetHTreeGroupForPos(decoder.Metadata, col, row); @@ -235,7 +247,7 @@ namespace SixLabors.ImageSharp.Formats.WebP { if (hTreeGroup[0].IsTrivialLiteral) { - pixelData[decodedPixels] = (uint)(hTreeGroup[0].LiteralArb | (code << 8)); + pixelData[decodedPixels] = hTreeGroup[0].LiteralArb | ((uint)code << 8); } else { @@ -306,7 +318,7 @@ namespace SixLabors.ImageSharp.Formats.WebP } else { - // TODO: throw appropriate error msg + WebPThrowHelper.ThrowImageFormatException("Webp parsing error"); } } @@ -483,7 +495,7 @@ namespace SixLabors.ImageSharp.Formats.WebP codeLengths[symbol] = 1; // The second code (if present), is always 8 bit long. - if (numSymbols == 2) + if (numSymbols is 2) { symbol = this.bitReader.ReadBits(8); codeLengths[symbol] = 1; @@ -583,6 +595,8 @@ namespace SixLabors.ImageSharp.Formats.WebP /// /// Reads the transformations, if any are present. /// + /// The width of the image. + /// The height of the image. /// Vp8LDecoder where the transformations will be stored. private void ReadTransformation(int xSize, int ySize, Vp8LDecoder decoder) { diff --git a/src/ImageSharp/Formats/WebP/WebPLossyDecoder.cs b/src/ImageSharp/Formats/WebP/WebPLossyDecoder.cs index cd8f986789..906f11efda 100644 --- a/src/ImageSharp/Formats/WebP/WebPLossyDecoder.cs +++ b/src/ImageSharp/Formats/WebP/WebPLossyDecoder.cs @@ -1,3 +1,6 @@ +// Copyright (c) Six Labors and contributors. +// Licensed under the Apache License, Version 2.0. + using System; using System.IO; diff --git a/src/ImageSharp/Formats/WebP/WebPMetadata.cs b/src/ImageSharp/Formats/WebP/WebPMetadata.cs index 65b09d5ec6..4788d2b0bd 100644 --- a/src/ImageSharp/Formats/WebP/WebPMetadata.cs +++ b/src/ImageSharp/Formats/WebP/WebPMetadata.cs @@ -28,12 +28,12 @@ namespace SixLabors.ImageSharp.Formats.WebP } /// - /// The webp format used. Either lossless or lossy. + /// Gets or sets the webp format used. Either lossless or lossy. /// public WebPFormatType Format { get; set; } /// - /// All found chunk types ordered by appearance. + /// Gets or sets all found chunk types ordered by appearance. /// public Queue ChunkTypes { get; set; } = new Queue();