Fix warnings, add additional comments

src/ImageSharp/Formats/WebP/ColorCache.cs

@@ -5,20 +5,23 @@ namespace SixLabors.ImageSharp.Formats.WebP
 {
     internal class ColorCache
     {
+        private const uint KHashMul = 0x1e35a7bdu;
+
         /// <summary>
-        /// Color entries.
+        /// Gets the color entries.
         /// </summary>
         public uint[] Colors { get; private set; }
 
         /// <summary>
-        /// Hash shift: 32 - hashBits.
+        /// Gets the hash shift: 32 - hashBits.
         /// </summary>
         public int HashShift { get; private set; }
 
+        /// <summary>
+        /// Gets the hash bits.
+        /// </summary>
         public int HashBits { get; private set; }
 
-        private const uint KHashMul = 0x1e35a7bdu;
-
         public void Init(int hashBits)
         {
             int hashSize = 1 << hashBits;

src/ImageSharp/Formats/WebP/HTreeGroup.cs

@@ -8,11 +8,10 @@ namespace SixLabors.ImageSharp.Formats.WebP
     /// <summary>
     /// 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'.
     /// </summary>
     internal class HTreeGroup
     {
@@ -27,32 +26,33 @@ namespace SixLabors.ImageSharp.Formats.WebP
         }
 
         /// <summary>
-        /// This has a maximum of HuffmanCodesPerMetaCode (5) entry's.
+        /// Gets the Huffman trees. This has a maximum of HuffmanCodesPerMetaCode (5) entry's.
         /// </summary>
-        public List<HuffmanCode[]> HTrees { get; private set; }
+        public List<HuffmanCode[]> HTrees { get; }
 
         /// <summary>
-        /// 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).
         /// </summary>
         public bool IsTrivialLiteral { get; set; }
 
         /// <summary>
-        /// 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.
         /// </summary>
         public uint LiteralArb { get; set; }
 
         /// <summary>
-        /// True if is_trivial_literal with only one code.
+        /// Gets or sets a value indicating whether there is only one code.
         /// </summary>
         public bool IsTrivialCode { get; set; }
 
         /// <summary>
-        /// use packed table below for short literal code
+        /// Gets or sets a value indicating whether to use packed table below for short literal code.
         /// </summary>
         public bool UsePackedTable { get; set; }
 
         /// <summary>
-        /// 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.
         /// </summary>
         public HuffmanCode[] PackedTable { get; set; }
     }

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));
         }
 
-
         /// <summary>
         /// Computes sampled size of 'size' when sampling using 'sampling bits'.
         /// </summary>

src/ImageSharp/Formats/WebP/Vp8LBitReader.cs

@@ -13,17 +13,17 @@ namespace SixLabors.ImageSharp.Formats.WebP
         /// <summary>
         /// Maximum number of bits (inclusive) the bit-reader can handle.
         /// </summary>
-        private const int VP8L_MAX_NUM_BIT_READ = 24;
+        private const int Vp8LMaxNumBitRead = 24;
 
         /// <summary>
         /// Number of bits prefetched (= bit-size of vp8l_val_t).
         /// </summary>
-        private const int VP8L_LBITS = 64;
+        private const int Vp8LLbits = 64;
 
         /// <summary>
         /// Minimum number of bytes ready after VP8LFillBitWindow.
         /// </summary>
-        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;
 
+        /// <summary>
+        /// Pre-fetched bits.
+        /// </summary>
+        private ulong value;
+
+        /// <summary>
+        /// Buffer length.
+        /// </summary>
+        private readonly long len;
+
+        /// <summary>
+        /// Byte position in buffer.
+        /// </summary>
+        private long pos;
+
+        /// <summary>
+        /// Current bit-reading position in value.
+        /// </summary>
+        private int bitPos;
+
+        /// <summary>
+        /// True if a bit was read past the end of buffer.
+        /// </summary>
+        private bool eos;
+
         /// <summary>
         /// Initializes a new instance of the <see cref="Vp8LBitReader"/> class.
         /// </summary>
@@ -72,31 +97,6 @@ namespace SixLabors.ImageSharp.Formats.WebP
             this.pos = length;
         }
 
-        /// <summary>
-        /// Pre-fetched bits.
-        /// </summary>
-        private ulong value;
-
-        /// <summary>
-        /// Buffer length.
-        /// </summary>
-        private readonly long len;
-
-        /// <summary>
-        /// Byte position in buffer.
-        /// </summary>
-        private long pos;
-
-        /// <summary>
-        /// Current bit-reading position in value.
-        /// </summary>
-        private int bitPos;
-
-        /// <summary>
-        /// True if a bit was read past the end of buffer.
-        /// </summary>
-        private bool eos;
-
         /// <summary>
         /// Reads a unsigned short value from the inputStream. The bits of each byte are read in least-significant-bit-first order.
         /// </summary>
@@ -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;
             }

src/ImageSharp/Formats/WebP/Vp8LTransform.cs

@@ -24,7 +24,7 @@ namespace SixLabors.ImageSharp.Formats.WebP
         public Vp8LTransformType TransformType { get; }
 
         /// <summary>
-        /// Subsampling bits defining transform window.
+        /// Gets or sets the subsampling bits defining the transform window.
         /// </summary>
         public int Bits { get; set; }
 

src/ImageSharp/Formats/WebP/WebPDecoderCore.cs

@@ -64,6 +64,12 @@ namespace SixLabors.ImageSharp.Formats.WebP
             this.options = options;
         }
 
+        /// <summary>
+        /// Decodes the image from the specified <see cref="Stream"/> and sets the data to the image.
+        /// </summary>
+        /// <typeparam name="TPixel">The pixel format.</typeparam>
+        /// <param name="stream">The stream, where the image should be.</param>
+        /// <returns>The decoded image.</returns>
         public Image<TPixel> Decode<TPixel>(Stream stream)
             where TPixel : struct, IPixel<TPixel>
         {
@@ -80,7 +86,7 @@ namespace SixLabors.ImageSharp.Formats.WebP
             Buffer2D<TPixel> 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);
         }
 
+        /// <summary>
+        /// Reads and skips over the image header.
+        /// </summary>
+        /// <returns>The chunk size in bytes.</returns>
         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
                    };
         }
 
+        /// <summary>
+        /// Parses optional metadata chunks.
+        /// </summary>
+        /// <param name="features">The webp features.</param>
         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
         /// </exception>
         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
         /// <returns>The chunk size in bytes.</returns>
         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.");

src/ImageSharp/Formats/WebP/WebPFeatures.cs

@@ -9,27 +9,27 @@ namespace SixLabors.ImageSharp.Formats.WebP
     public class WebPFeatures
     {
         /// <summary>
-        /// Gets or sets whether this image has a ICC Profile.
+        /// Gets or sets a value indicating whether this image has a ICC Profile.
         /// </summary>
         public bool IccProfile { get; set; }
 
         /// <summary>
-        /// Gets or sets whether this image has a alpha channel.
+        /// Gets or sets a value indicating whether this image has a alpha channel.
         /// </summary>
         public bool Alpha { get; set; }
 
         /// <summary>
-        /// Gets or sets whether this image has a EXIF Profile.
+        /// Gets or sets a value indicating whether this image has a EXIF Profile.
         /// </summary>
         public bool ExifProfile { get; set; }
 
         /// <summary>
-        /// Gets or sets whether this image has XMP Metadata.
+        /// Gets or sets a value indicating whether this image has XMP Metadata.
         /// </summary>
         public bool XmpMetaData { get; set; }
 
         /// <summary>
-        /// Gets or sets whether this image is a animation.
+        /// Gets or sets a value indicating whether this image is a animation.
         /// </summary>
         public bool Animation { get; set; }
     }

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)
+        /// <summary>
+        /// Initializes a new instance of the <see cref="WebPLosslessDecoder"/> class.
+        /// </summary>
+        /// <param name="bitReader">Bitreader to read from the stream.</param>
+        public WebPLosslessDecoder(Vp8LBitReader bitReader)
         {
             this.bitReader = bitReader;
-            this.imageDataSize = imageDataSize;
         }
 
+        /// <summary>
+        /// Decodes the image from the stream using the bitreader.
+        /// </summary>
+        /// <typeparam name="TPixel">The pixel format.</typeparam>
+        /// <param name="pixels">The pixel buffer to store the decoded data.</param>
+        /// <param name="width">The width of the image.</param>
+        /// <param name="height">The height of the image.</param>
         public void Decode<TPixel>(Buffer2D<TPixel> pixels, int width, int height)
             where TPixel : struct, IPixel<TPixel>
         {
@@ -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
         /// <summary>
         /// Reads the transformations, if any are present.
         /// </summary>
+        /// <param name="xSize">The width of the image.</param>
+        /// <param name="ySize">The height of the image.</param>
         /// <param name="decoder">Vp8LDecoder where the transformations will be stored.</param>
         private void ReadTransformation(int xSize, int ySize, Vp8LDecoder decoder)
         {

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;
 

src/ImageSharp/Formats/WebP/WebPMetadata.cs

@@ -28,12 +28,12 @@ namespace SixLabors.ImageSharp.Formats.WebP
         }
 
         /// <summary>
-        /// The webp format used. Either lossless or lossy.
+        /// Gets or sets the webp format used. Either lossless or lossy.
         /// </summary>
         public WebPFormatType Format { get; set; }
 
         /// <summary>
-        /// All found chunk types ordered by appearance.
+        /// Gets or sets all found chunk types ordered by appearance.
         /// </summary>
         public Queue<WebPChunkType> ChunkTypes { get; set; } = new Queue<WebPChunkType>();