Merge pull request #2703 from SixLabors/v4/fix-webp-alpha-flags

V4 Ensure VP8X alpha flag is updated correctly.

src/ImageSharp/Formats/Webp/BitWriter/BitWriterBase.cs

@@ -88,7 +88,8 @@ internal abstract class BitWriterBase
     /// <param name="iccProfile">The color profile.</param>
     /// <param name="hasAlpha">Flag indicating, if a alpha channel is present.</param>
     /// <param name="hasAnimation">Flag indicating, if an animation parameter is present.</param>
-    public static void WriteTrunksBeforeData(
+    /// <returns>A <see cref="WebpVp8X"/> or a default instance.</returns>
+    public static WebpVp8X WriteTrunksBeforeData(
         Stream stream,
         uint width,
         uint height,
@@ -102,16 +103,19 @@ internal abstract class BitWriterBase
         RiffHelper.BeginWriteRiffFile(stream, WebpConstants.WebpFourCc);
 
         // Write VP8X, header if necessary.
+        WebpVp8X vp8x = default;
         bool isVp8X = exifProfile != null || xmpProfile != null || iccProfile != null || hasAlpha || hasAnimation;
         if (isVp8X)
         {
-            WriteVp8XHeader(stream, exifProfile, xmpProfile, iccProfile, width, height, hasAlpha, hasAnimation);
+            vp8x = WriteVp8XHeader(stream, exifProfile, xmpProfile, iccProfile, width, height, hasAlpha, hasAnimation);
 
             if (iccProfile != null)
             {
                 RiffHelper.WriteChunk(stream, (uint)WebpChunkType.Iccp, iccProfile.ToByteArray());
             }
         }
+
+        return vp8x;
     }
 
     /// <summary>
@@ -124,10 +128,16 @@ internal abstract class BitWriterBase
     /// Write the trunks after data trunk.
     /// </summary>
     /// <param name="stream">The stream to write to.</param>
-    /// <param name="exifProfile">The exif profile.</param>
+    /// <param name="vp8x">The VP8X chunk.</param>
+    /// <param name="updateVp8x">Whether to update the chunk.</param>
+    /// <param name="initialPosition">The initial position of the stream before encoding.</param>
+    /// <param name="exifProfile">The EXIF profile.</param>
     /// <param name="xmpProfile">The XMP profile.</param>
     public static void WriteTrunksAfterData(
         Stream stream,
+        in WebpVp8X vp8x,
+        bool updateVp8x,
+        long initialPosition,
         ExifProfile? exifProfile,
         XmpProfile? xmpProfile)
     {
@@ -141,7 +151,7 @@ internal abstract class BitWriterBase
             RiffHelper.WriteChunk(stream, (uint)WebpChunkType.Xmp, xmpProfile.Data);
         }
 
-        RiffHelper.EndWriteRiffFile(stream, 4);
+        RiffHelper.EndWriteRiffFile(stream, in vp8x, updateVp8x, initialPosition);
     }
 
     /// <summary>
@@ -186,19 +196,21 @@ internal abstract class BitWriterBase
     /// Writes a VP8X header to the stream.
     /// </summary>
     /// <param name="stream">The stream to write to.</param>
-    /// <param name="exifProfile">A exif profile or null, if it does not exist.</param>
-    /// <param name="xmpProfile">A XMP profile or null, if it does not exist.</param>
+    /// <param name="exifProfile">An EXIF profile or null, if it does not exist.</param>
+    /// <param name="xmpProfile">An XMP profile or null, if it does not exist.</param>
     /// <param name="iccProfile">The color profile.</param>
     /// <param name="width">The width of the image.</param>
     /// <param name="height">The height of the image.</param>
     /// <param name="hasAlpha">Flag indicating, if a alpha channel is present.</param>
     /// <param name="hasAnimation">Flag indicating, if an animation parameter is present.</param>
-    protected static void WriteVp8XHeader(Stream stream, ExifProfile? exifProfile, XmpProfile? xmpProfile, IccProfile? iccProfile, uint width, uint height, bool hasAlpha, bool hasAnimation)
+    protected static WebpVp8X WriteVp8XHeader(Stream stream, ExifProfile? exifProfile, XmpProfile? xmpProfile, IccProfile? iccProfile, uint width, uint height, bool hasAlpha, bool hasAnimation)
     {
         WebpVp8X chunk = new(hasAnimation, xmpProfile != null, exifProfile != null, hasAlpha, iccProfile != null, width, height);
 
         chunk.Validate(MaxDimension, MaxCanvasPixels);
 
         chunk.WriteTo(stream);
+
+        return chunk;
     }
 }

src/ImageSharp/Formats/Webp/Chunks/WebpVp8X.cs

@@ -3,7 +3,7 @@
 
 namespace SixLabors.ImageSharp.Formats.Webp.Chunks;
 
-internal readonly struct WebpVp8X
+internal readonly struct WebpVp8X : IEquatable<WebpVp8X>
 {
     public WebpVp8X(bool hasAnimation, bool hasXmp, bool hasExif, bool hasAlpha, bool hasIcc, uint width, uint height)
     {
@@ -51,6 +51,24 @@ internal readonly struct WebpVp8X
     /// </summary>
     public uint Height { get; }
 
+    public static bool operator ==(WebpVp8X left, WebpVp8X right) => left.Equals(right);
+
+    public static bool operator !=(WebpVp8X left, WebpVp8X right) => !(left == right);
+
+    public override bool Equals(object? obj) => obj is WebpVp8X x && this.Equals(x);
+
+    public bool Equals(WebpVp8X other)
+        => this.HasAnimation == other.HasAnimation
+        && this.HasXmp == other.HasXmp
+        && this.HasExif == other.HasExif
+        && this.HasAlpha == other.HasAlpha
+        && this.HasIcc == other.HasIcc
+        && this.Width == other.Width
+        && this.Height == other.Height;
+
+    public override int GetHashCode()
+        => HashCode.Combine(this.HasAnimation, this.HasXmp, this.HasExif, this.HasAlpha, this.HasIcc, this.Width, this.Height);
+
     public void Validate(uint maxDimension, ulong maxCanvasPixels)
     {
         if (this.Width > maxDimension || this.Height > maxDimension)
@@ -65,6 +83,9 @@ internal readonly struct WebpVp8X
         }
     }
 
+    public WebpVp8X WithAlpha(bool hasAlpha)
+        => new(this.HasAnimation, this.HasXmp, this.HasExif, hasAlpha, this.HasIcc, this.Width, this.Height);
+
     public void WriteTo(Stream stream)
     {
         byte flags = 0;

src/ImageSharp/Formats/Webp/Lossless/Vp8LEncoder.cs

@@ -236,7 +236,7 @@ internal class Vp8LEncoder : IDisposable
     /// </summary>
     public Vp8LHashChain HashChain { get; }
 
-    public void EncodeHeader<TPixel>(Image<TPixel> image, Stream stream, bool hasAnimation)
+    public WebpVp8X EncodeHeader<TPixel>(Image<TPixel> image, Stream stream, bool hasAnimation)
         where TPixel : unmanaged, IPixel<TPixel>
     {
         // Write bytes from the bit-writer buffer to the stream.
@@ -246,7 +246,8 @@ internal class Vp8LEncoder : IDisposable
         ExifProfile exifProfile = this.skipMetadata ? null : metadata.ExifProfile;
         XmpProfile xmpProfile = this.skipMetadata ? null : metadata.XmpProfile;
 
-        BitWriterBase.WriteTrunksBeforeData(
+        // The alpha flag is updated following encoding.
+        WebpVp8X vp8x = BitWriterBase.WriteTrunksBeforeData(
             stream,
             (uint)image.Width,
             (uint)image.Height,
@@ -261,9 +262,11 @@ internal class Vp8LEncoder : IDisposable
             WebpMetadata webpMetadata = WebpCommonUtils.GetWebpMetadata(image);
             BitWriterBase.WriteAnimationParameter(stream, webpMetadata.BackgroundColor, webpMetadata.RepeatCount);
         }
+
+        return vp8x;
     }
 
-    public void EncodeFooter<TPixel>(Image<TPixel> image, Stream stream)
+    public void EncodeFooter<TPixel>(Image<TPixel> image, in WebpVp8X vp8x, bool hasAlpha, Stream stream, long initialPosition)
         where TPixel : unmanaged, IPixel<TPixel>
     {
         // Write bytes from the bit-writer buffer to the stream.
@@ -272,7 +275,9 @@ internal class Vp8LEncoder : IDisposable
         ExifProfile exifProfile = this.skipMetadata ? null : metadata.ExifProfile;
         XmpProfile xmpProfile = this.skipMetadata ? null : metadata.XmpProfile;
 
-        BitWriterBase.WriteTrunksAfterData(stream, exifProfile, xmpProfile);
+        bool updateVp8x = hasAlpha && vp8x != default;
+        WebpVp8X updated = updateVp8x ? vp8x.WithAlpha(true) : vp8x;
+        BitWriterBase.WriteTrunksAfterData(stream, in updated, updateVp8x, initialPosition, exifProfile, xmpProfile);
     }
 
     /// <summary>
@@ -284,7 +289,8 @@ internal class Vp8LEncoder : IDisposable
     /// <param name="frameMetadata">The frame metadata.</param>
     /// <param name="stream">The <see cref="Stream"/> to encode the image data to.</param>
     /// <param name="hasAnimation">Flag indicating, if an animation parameter is present.</param>
-    public void Encode<TPixel>(ImageFrame<TPixel> frame, Rectangle bounds, WebpFrameMetadata frameMetadata, Stream stream, bool hasAnimation)
+    /// <returns>A <see cref="bool"/> indicating whether the frame contains an alpha channel.</returns>
+    public bool Encode<TPixel>(ImageFrame<TPixel> frame, Rectangle bounds, WebpFrameMetadata frameMetadata, Stream stream, bool hasAnimation)
         where TPixel : unmanaged, IPixel<TPixel>
     {
         // Convert image pixels to bgra array.
@@ -323,6 +329,8 @@ internal class Vp8LEncoder : IDisposable
         {
             RiffHelper.EndWriteChunk(stream, prevPosition);
         }
+
+        return hasAlpha;
     }
 
     /// <summary>
@@ -501,7 +509,7 @@ internal class Vp8LEncoder : IDisposable
     /// <typeparam name="TPixel">The type of the pixels.</typeparam>
     /// <param name="pixels">The frame pixel buffer to convert.</param>
     /// <returns>true, if the image is non opaque.</returns>
-    private bool ConvertPixelsToBgra<TPixel>(Buffer2DRegion<TPixel> pixels)
+    public bool ConvertPixelsToBgra<TPixel>(Buffer2DRegion<TPixel> pixels)
         where TPixel : unmanaged, IPixel<TPixel>
     {
         bool nonOpaque = false;

src/ImageSharp/Formats/Webp/Lossy/Vp8Encoder.cs

@@ -310,7 +310,7 @@ internal class Vp8Encoder : IDisposable
     /// </summary>
     private int MbHeaderLimit { get; }
 
-    public void EncodeHeader<TPixel>(Image<TPixel> image, Stream stream, bool hasAlpha, bool hasAnimation)
+    public WebpVp8X EncodeHeader<TPixel>(Image<TPixel> image, Stream stream, bool hasAlpha, bool hasAnimation)
         where TPixel : unmanaged, IPixel<TPixel>
     {
         // Write bytes from the bitwriter buffer to the stream.
@@ -320,7 +320,7 @@ internal class Vp8Encoder : IDisposable
         ExifProfile exifProfile = this.skipMetadata ? null : metadata.ExifProfile;
         XmpProfile xmpProfile = this.skipMetadata ? null : metadata.XmpProfile;
 
-        BitWriterBase.WriteTrunksBeforeData(
+        WebpVp8X vp8x = BitWriterBase.WriteTrunksBeforeData(
             stream,
             (uint)image.Width,
             (uint)image.Height,
@@ -335,9 +335,11 @@ internal class Vp8Encoder : IDisposable
             WebpMetadata webpMetadata = WebpCommonUtils.GetWebpMetadata(image);
             BitWriterBase.WriteAnimationParameter(stream, webpMetadata.BackgroundColor, webpMetadata.RepeatCount);
         }
+
+        return vp8x;
     }
 
-    public void EncodeFooter<TPixel>(Image<TPixel> image, Stream stream)
+    public void EncodeFooter<TPixel>(Image<TPixel> image, in WebpVp8X vp8x, bool hasAlpha, Stream stream, long initialPosition)
         where TPixel : unmanaged, IPixel<TPixel>
     {
         // Write bytes from the bitwriter buffer to the stream.
@@ -346,7 +348,9 @@ internal class Vp8Encoder : IDisposable
         ExifProfile exifProfile = this.skipMetadata ? null : metadata.ExifProfile;
         XmpProfile xmpProfile = this.skipMetadata ? null : metadata.XmpProfile;
 
-        BitWriterBase.WriteTrunksAfterData(stream, exifProfile, xmpProfile);
+        bool updateVp8x = hasAlpha && vp8x != default;
+        WebpVp8X updated = updateVp8x ? vp8x.WithAlpha(true) : vp8x;
+        BitWriterBase.WriteTrunksAfterData(stream, in updated, updateVp8x, initialPosition, exifProfile, xmpProfile);
     }
 
     /// <summary>
@@ -357,9 +361,10 @@ internal class Vp8Encoder : IDisposable
     /// <param name="stream">The stream to encode the image data to.</param>
     /// <param name="bounds">The region of interest within the frame to encode.</param>
     /// <param name="frameMetadata">The frame metadata.</param>
-    public void EncodeAnimation<TPixel>(ImageFrame<TPixel> frame, Stream stream, Rectangle bounds, WebpFrameMetadata frameMetadata)
-        where TPixel : unmanaged, IPixel<TPixel> =>
-        this.Encode(stream, frame, bounds, frameMetadata, true, null);
+    /// <returns>A <see cref="bool"/> indicating whether the frame contains an alpha channel.</returns>
+    public bool EncodeAnimation<TPixel>(ImageFrame<TPixel> frame, Stream stream, Rectangle bounds, WebpFrameMetadata frameMetadata)
+        where TPixel : unmanaged, IPixel<TPixel>
+        => this.Encode(stream, frame, bounds, frameMetadata, true, null);
 
     /// <summary>
     /// Encodes the static image frame to the specified stream.
@@ -384,7 +389,8 @@ internal class Vp8Encoder : IDisposable
     /// <param name="frameMetadata">The frame metadata.</param>
     /// <param name="hasAnimation">Flag indicating, if an animation parameter is present.</param>
     /// <param name="image">The image to encode from.</param>
-    private void Encode<TPixel>(Stream stream, ImageFrame<TPixel> frame, Rectangle bounds, WebpFrameMetadata frameMetadata, bool hasAnimation, Image<TPixel> image)
+    /// <returns>A <see cref="bool"/> indicating whether the frame contains an alpha channel.</returns>
+    private bool Encode<TPixel>(Stream stream, ImageFrame<TPixel> frame, Rectangle bounds, WebpFrameMetadata frameMetadata, bool hasAnimation, Image<TPixel> image)
         where TPixel : unmanaged, IPixel<TPixel>
     {
         int width = bounds.Width;
@@ -514,6 +520,8 @@ internal class Vp8Encoder : IDisposable
         {
             encodedAlphaData?.Dispose();
         }
+
+        return hasAlpha;
     }
 
     /// <inheritdoc/>

src/ImageSharp/Formats/Webp/RiffHelper.cs

@@ -3,6 +3,7 @@
 
 using System.Buffers.Binary;
 using System.Text;
+using SixLabors.ImageSharp.Formats.Webp.Chunks;
 
 namespace SixLabors.ImageSharp.Formats.Webp;
 
@@ -107,6 +108,7 @@ internal static class RiffHelper
             position++;
         }
 
+        // Add the size of the encoded file to the Riff header.
         BinaryPrimitives.WriteUInt32LittleEndian(buffer, dataSize);
         stream.Position = sizePosition;
         stream.Write(buffer);
@@ -120,5 +122,18 @@ internal static class RiffHelper
         return sizePosition;
     }
 
-    public static void EndWriteRiffFile(Stream stream, long sizePosition) => EndWriteChunk(stream, sizePosition);
+    public static void EndWriteRiffFile(Stream stream, in WebpVp8X vp8x, bool updateVp8x, long sizePosition)
+    {
+        EndWriteChunk(stream, sizePosition + 4);
+
+        // Write the VP8X chunk if necessary.
+        if (updateVp8x)
+        {
+            long position = stream.Position;
+
+            stream.Position = sizePosition + 12;
+            vp8x.WriteTo(stream);
+            stream.Position = position;
+        }
+    }
 }

src/ImageSharp/Formats/Webp/WebpChunkParsingUtils.cs

@@ -2,7 +2,6 @@
 // Licensed under the Six Labors Split License.
 
 using System.Buffers.Binary;
-using System.Drawing;
 using SixLabors.ImageSharp.Formats.Webp.BitReader;
 using SixLabors.ImageSharp.Formats.Webp.Lossy;
 using SixLabors.ImageSharp.IO;

src/ImageSharp/Formats/Webp/WebpDecoderCore.cs

@@ -54,7 +54,7 @@ internal sealed class WebpDecoderCore : IImageDecoderInternals, IDisposable
     /// <summary>
     /// The flag to decide how to handle the background color in the Animation Chunk.
     /// </summary>
-    private BackgroundColorHandling backgroundColorHandling;
+    private readonly BackgroundColorHandling backgroundColorHandling;
 
     /// <summary>
     /// Initializes a new instance of the <see cref="WebpDecoderCore"/> class.

src/ImageSharp/Formats/Webp/WebpEncoderCore.cs

@@ -1,6 +1,7 @@
 // Copyright (c) Six Labors.
 // Licensed under the Six Labors Split License.
 
+using SixLabors.ImageSharp.Formats.Webp.Chunks;
 using SixLabors.ImageSharp.Formats.Webp.Lossless;
 using SixLabors.ImageSharp.Formats.Webp.Lossy;
 using SixLabors.ImageSharp.Memory;
@@ -143,12 +144,14 @@ internal sealed class WebpEncoderCore : IImageEncoderInternals
                 this.nearLossless,
                 this.nearLosslessQuality);
 
-            encoder.EncodeHeader(image, stream, hasAnimation);
+            long initialPosition = stream.Position;
+            bool hasAlpha = false;
+            WebpVp8X vp8x = encoder.EncodeHeader(image, stream, hasAnimation);
 
             // Encode the first frame.
             ImageFrame<TPixel> previousFrame = image.Frames.RootFrame;
             WebpFrameMetadata frameMetadata = WebpCommonUtils.GetWebpFrameMetadata(previousFrame);
-            encoder.Encode(previousFrame, previousFrame.Bounds(), frameMetadata, stream, hasAnimation);
+            hasAlpha |= encoder.Encode(previousFrame, previousFrame.Bounds(), frameMetadata, stream, hasAnimation);
 
             if (hasAnimation)
             {
@@ -190,14 +193,14 @@ internal sealed class WebpEncoderCore : IImageEncoderInternals
                         this.nearLossless,
                         this.nearLosslessQuality);
 
-                    animatedEncoder.Encode(encodingFrame, bounds, frameMetadata, stream, hasAnimation);
+                    hasAlpha |= animatedEncoder.Encode(encodingFrame, bounds, frameMetadata, stream, hasAnimation);
 
                     previousFrame = currentFrame;
                     previousDisposal = frameMetadata.DisposalMethod;
                 }
             }
 
-            encoder.EncodeFooter(image, stream);
+            encoder.EncodeFooter(image, in vp8x, hasAlpha, stream, initialPosition);
         }
         else
         {
@@ -214,17 +217,20 @@ internal sealed class WebpEncoderCore : IImageEncoderInternals
                 this.spatialNoiseShaping,
                 this.alphaCompression);
 
+            long initialPosition = stream.Position;
+            bool hasAlpha = false;
+            WebpVp8X vp8x = default;
             if (image.Frames.Count > 1)
             {
-                // TODO: What about alpha here?
-                encoder.EncodeHeader(image, stream, false, true);
+                // The alpha flag is updated following encoding.
+                vp8x = encoder.EncodeHeader(image, stream, false, true);
 
                 // Encode the first frame.
                 ImageFrame<TPixel> previousFrame = image.Frames.RootFrame;
                 WebpFrameMetadata frameMetadata = WebpCommonUtils.GetWebpFrameMetadata(previousFrame);
                 WebpDisposalMethod previousDisposal = frameMetadata.DisposalMethod;
 
-                encoder.EncodeAnimation(previousFrame, stream, previousFrame.Bounds(), frameMetadata);
+                hasAlpha |= encoder.EncodeAnimation(previousFrame, stream, previousFrame.Bounds(), frameMetadata);
 
                 // Encode additional frames
                 // This frame is reused to store de-duplicated pixel buffers.
@@ -263,18 +269,19 @@ internal sealed class WebpEncoderCore : IImageEncoderInternals
                         this.spatialNoiseShaping,
                         this.alphaCompression);
 
-                    animatedEncoder.EncodeAnimation(encodingFrame, stream, bounds, frameMetadata);
+                    hasAlpha |= animatedEncoder.EncodeAnimation(encodingFrame, stream, bounds, frameMetadata);
 
                     previousFrame = currentFrame;
                     previousDisposal = frameMetadata.DisposalMethod;
                 }
+
+                encoder.EncodeFooter(image, in vp8x, hasAlpha, stream, initialPosition);
             }
             else
             {
                 encoder.EncodeStatic(stream, image);
+                encoder.EncodeFooter(image, in vp8x, hasAlpha, stream, initialPosition);
             }
-
-            encoder.EncodeFooter(image, stream);
         }
     }
 }