Merge pull request #1798 from SixLabors/bp/webpexifwithpadding

Write exif profile with padding if needed

Author: JimBobSquarePants

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

@@ -10,11 +10,22 @@ namespace SixLabors.ImageSharp.Formats.Webp.BitWriter
 {
     internal abstract class BitWriterBase
     {
+        private const uint MaxDimension = 16777215;
+
+        private const ulong MaxCanvasPixels = 4294967295ul;
+
+        protected const uint ExtendedFileChunkSize = WebpConstants.ChunkHeaderSize + WebpConstants.Vp8XChunkSize;
+
         /// <summary>
         /// Buffer to write to.
         /// </summary>
         private byte[] buffer;
 
+        /// <summary>
+        /// A scratch buffer to reduce allocations.
+        /// </summary>
+        private readonly byte[] scratchBuffer = new byte[4];
+
         /// <summary>
         /// Initializes a new instance of the <see cref="BitWriterBase"/> class.
         /// </summary>
@@ -52,15 +63,6 @@ internal abstract class BitWriterBase
         /// </summary>
         public abstract void Finish();
 
-        /// <summary>
-        /// Writes the encoded image to the stream.
-        /// </summary>
-        /// <param name="stream">The stream to write to.</param>
-        /// <param name="exifProfile">The exif profile.</param>
-        /// <param name="width">The width of the image.</param>
-        /// <param name="height">The height of the image.</param>
-        public abstract void WriteEncodedImageToStream(Stream stream, ExifProfile exifProfile, uint width, uint height);
-
         protected void ResizeBuffer(int maxBytes, int sizeRequired)
         {
             int newSize = (3 * maxBytes) >> 1;
@@ -81,13 +83,25 @@ protected void ResizeBuffer(int maxBytes, int sizeRequired)
         /// <param name="riffSize">The block length.</param>
         protected void WriteRiffHeader(Stream stream, uint riffSize)
         {
-            Span<byte> buf = stackalloc byte[4];
             stream.Write(WebpConstants.RiffFourCc);
-            BinaryPrimitives.WriteUInt32LittleEndian(buf, riffSize);
-            stream.Write(buf);
+            BinaryPrimitives.WriteUInt32LittleEndian(this.scratchBuffer, riffSize);
+            stream.Write(this.scratchBuffer.AsSpan(0, 4));
             stream.Write(WebpConstants.WebpHeader);
         }
 
+        /// <summary>
+        /// Calculates the exif chunk size.
+        /// </summary>
+        /// <param name="exifBytes">The exif profile bytes.</param>
+        /// <returns>The exif chunk size in bytes.</returns>
+        protected uint ExifChunkSize(byte[] exifBytes)
+        {
+            uint exifSize = (uint)exifBytes.Length;
+            uint exifChunkSize = WebpConstants.ChunkHeaderSize + exifSize + (exifSize & 1);
+
+            return exifChunkSize;
+        }
+
         /// <summary>
         /// Writes the Exif profile to the stream.
         /// </summary>
@@ -97,12 +111,19 @@ protected void WriteExifProfile(Stream stream, byte[] exifBytes)
         {
             DebugGuard.NotNull(exifBytes, nameof(exifBytes));
 
-            Span<byte> buf = stackalloc byte[4];
+            uint size = (uint)exifBytes.Length;
+            Span<byte> buf = this.scratchBuffer.AsSpan(0, 4);
             BinaryPrimitives.WriteUInt32BigEndian(buf, (uint)WebpChunkType.Exif);
             stream.Write(buf);
-            BinaryPrimitives.WriteUInt32LittleEndian(buf, (uint)exifBytes.Length);
+            BinaryPrimitives.WriteUInt32LittleEndian(buf, size);
             stream.Write(buf);
             stream.Write(exifBytes);
+
+            // Add padding byte if needed.
+            if ((size & 1) == 1)
+            {
+                stream.WriteByte(0);
+            }
         }
 
         /// <summary>
@@ -112,16 +133,16 @@ protected void WriteExifProfile(Stream stream, byte[] exifBytes)
         /// <param name="exifProfile">A exif profile or null, if it does not exist.</param>
         /// <param name="width">The width of the image.</param>
         /// <param name="height">The height of the image.</param>
-        protected void WriteVp8XHeader(Stream stream, ExifProfile exifProfile, uint width, uint height)
+        /// <param name="hasAlpha">Flag indicating, if a alpha channel is present.</param>
+        protected void WriteVp8XHeader(Stream stream, ExifProfile exifProfile, uint width, uint height, bool hasAlpha)
         {
-            int maxDimension = 16777215;
-            if (width > maxDimension || height > maxDimension)
+            if (width > MaxDimension || height > MaxDimension)
             {
-                WebpThrowHelper.ThrowInvalidImageDimensions($"Image width or height exceeds maximum allowed dimension of {maxDimension}");
+                WebpThrowHelper.ThrowInvalidImageDimensions($"Image width or height exceeds maximum allowed dimension of {MaxDimension}");
             }
 
             // The spec states that the product of Canvas Width and Canvas Height MUST be at most 2^32 - 1.
-            if (width * height > 4294967295ul)
+            if (width * height > MaxCanvasPixels)
             {
                 WebpThrowHelper.ThrowInvalidImageDimensions("The product of image width and height MUST be at most 2^32 - 1");
             }
@@ -133,7 +154,13 @@ protected void WriteVp8XHeader(Stream stream, ExifProfile exifProfile, uint widt
                 flags |= 8;
             }
 
-            Span<byte> buf = stackalloc byte[4];
+            if (hasAlpha)
+            {
+                // Set alpha bit.
+                flags |= 16;
+            }
+
+            Span<byte> buf = this.scratchBuffer.AsSpan(0, 4);
             stream.Write(WebpConstants.Vp8XMagicBytes);
             BinaryPrimitives.WriteUInt32LittleEndian(buf, WebpConstants.Vp8XChunkSize);
             stream.Write(buf);

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

@@ -399,18 +399,25 @@ private void Flush()
             }
         }
 
-        /// <inheritdoc/>
-        public override void WriteEncodedImageToStream(Stream stream, ExifProfile exifProfile, uint width, uint height)
+        /// <summary>
+        /// Writes the encoded image to the stream.
+        /// </summary>
+        /// <param name="stream">The stream to write to.</param>
+        /// <param name="exifProfile">The exif 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>
+        public void WriteEncodedImageToStream(Stream stream, ExifProfile exifProfile, uint width, uint height, bool hasAlpha)
         {
             bool isVp8X = false;
             byte[] exifBytes = null;
             uint riffSize = 0;
             if (exifProfile != null)
             {
                 isVp8X = true;
-                riffSize += WebpConstants.ChunkHeaderSize + WebpConstants.Vp8XChunkSize;
+                riffSize += ExtendedFileChunkSize;
                 exifBytes = exifProfile.ToByteArray();
-                riffSize += WebpConstants.ChunkHeaderSize + (uint)exifBytes.Length;
+                riffSize += this.ExifChunkSize(exifBytes);
             }
 
             this.Finish();
@@ -433,7 +440,7 @@ public override void WriteEncodedImageToStream(Stream stream, ExifProfile exifPr
             riffSize += WebpConstants.TagSize + WebpConstants.ChunkHeaderSize + vp8Size;
 
             // Emit headers and partition #0
-            this.WriteWebpHeaders(stream, size0, vp8Size, riffSize, isVp8X, width, height, exifProfile);
+            this.WriteWebpHeaders(stream, size0, vp8Size, riffSize, isVp8X, width, height, exifProfile, hasAlpha);
             bitWriterPartZero.WriteToStream(stream);
 
             // Write the encoded image to the stream.
@@ -616,14 +623,14 @@ private void CodeIntraModes(Vp8BitWriter bitWriter)
             while (it.Next());
         }
 
-        private void WriteWebpHeaders(Stream stream, uint size0, uint vp8Size, uint riffSize, bool isVp8X, uint width, uint height, ExifProfile exifProfile)
+        private void WriteWebpHeaders(Stream stream, uint size0, uint vp8Size, uint riffSize, bool isVp8X, uint width, uint height, ExifProfile exifProfile, bool hasAlpha)
         {
             this.WriteRiffHeader(stream, riffSize);
 
             // Write VP8X, header if necessary.
             if (isVp8X)
             {
-                this.WriteVp8XHeader(stream, exifProfile, width, height);
+                this.WriteVp8XHeader(stream, exifProfile, width, height, hasAlpha);
             }
 
             this.WriteVp8Header(stream, vp8Size);

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

@@ -127,19 +127,25 @@ public override void Finish()
             this.used = 0;
         }
 
-        /// <inheritdoc/>
-        public override void WriteEncodedImageToStream(Stream stream, ExifProfile exifProfile, uint width, uint height)
+        /// <summary>
+        /// Writes the encoded image to the stream.
+        /// </summary>
+        /// <param name="stream">The stream to write to.</param>
+        /// <param name="exifProfile">The exif 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>
+        public void WriteEncodedImageToStream(Stream stream, ExifProfile exifProfile, uint width, uint height, bool hasAlpha)
         {
-            Span<byte> buffer = stackalloc byte[4];
             bool isVp8X = false;
             byte[] exifBytes = null;
             uint riffSize = 0;
             if (exifProfile != null)
             {
                 isVp8X = true;
-                riffSize += WebpConstants.ChunkHeaderSize + WebpConstants.Vp8XChunkSize;
+                riffSize += ExtendedFileChunkSize;
                 exifBytes = exifProfile.ToByteArray();
-                riffSize += WebpConstants.ChunkHeaderSize + (uint)exifBytes.Length;
+                riffSize += this.ExifChunkSize(exifBytes);
             }
 
             this.Finish();
@@ -154,15 +160,15 @@ public override void WriteEncodedImageToStream(Stream stream, ExifProfile exifPr
             // Write VP8X, header if necessary.
             if (isVp8X)
             {
-                this.WriteVp8XHeader(stream, exifProfile, width, height);
+                this.WriteVp8XHeader(stream, exifProfile, width, height, hasAlpha);
             }
 
             // Write magic bytes indicating its a lossless webp.
             stream.Write(WebpConstants.Vp8LMagicBytes);
 
             // Write Vp8 Header.
-            BinaryPrimitives.WriteUInt32LittleEndian(buffer, size);
-            stream.Write(buffer);
+            BinaryPrimitives.WriteUInt32LittleEndian(this.scratchBuffer, size);
+            stream.Write(this.scratchBuffer.AsSpan(0, 4));
             stream.WriteByte(WebpConstants.Vp8LHeaderMagicByte);
 
             // Write the encoded bytes of the image to the stream.

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

@@ -234,7 +234,7 @@ public void Encode<TPixel>(Image<TPixel> image, Stream stream)
             this.EncodeStream(image);
 
             // Write bytes from the bitwriter buffer to the stream.
-            this.bitWriter.WriteEncodedImageToStream(stream, image.Metadata.ExifProfile, (uint)width, (uint)height);
+            this.bitWriter.WriteEncodedImageToStream(stream, image.Metadata.ExifProfile, (uint)width, (uint)height, hasAlpha);
         }
 
         /// <summary>

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

@@ -317,6 +317,8 @@ public void Encode<TPixel>(Image<TPixel> image, Stream stream)
             this.bitWriter = new Vp8BitWriter(expectedSize, this);
 
             // TODO: EncodeAlpha();
+            bool hasAlpha = false;
+
             // Stats-collection loop.
             this.StatLoop(width, height, yStride, uvStride);
             it.Init();
@@ -348,7 +350,7 @@ public void Encode<TPixel>(Image<TPixel> image, Stream stream)
 
             // Write bytes from the bitwriter buffer to the stream.
             image.Metadata.SyncProfiles();
-            this.bitWriter.WriteEncodedImageToStream(stream, image.Metadata.ExifProfile, (uint)width, (uint)height);
+            this.bitWriter.WriteEncodedImageToStream(stream, image.Metadata.ExifProfile, (uint)width, (uint)height, hasAlpha);
         }
 
         /// <inheritdoc/>

src/ImageSharp/Formats/Webp/WebpEncoderCore.cs

@@ -4,11 +4,9 @@
 using System.IO;
 using System.Threading;
 using SixLabors.ImageSharp.Advanced;
-using SixLabors.ImageSharp.Formats.Bmp;
 using SixLabors.ImageSharp.Formats.Webp.Lossless;
 using SixLabors.ImageSharp.Formats.Webp.Lossy;
 using SixLabors.ImageSharp.Memory;
-using SixLabors.ImageSharp.Metadata;
 using SixLabors.ImageSharp.PixelFormats;
 
 namespace SixLabors.ImageSharp.Formats.Webp

tests/ImageSharp.Tests/Formats/WebP/WebpMetaDataTests.cs

@@ -63,6 +63,31 @@ public void IgnoreMetadata_ControlsWhetherIccpIsParsed<TPixel>(TestImageProvider
             }
         }
 
+        [Theory]
+        [InlineData(WebpFileFormatType.Lossy)]
+        [InlineData(WebpFileFormatType.Lossless)]
+        public void Encode_WritesExifWithPadding(WebpFileFormatType fileFormatType)
+        {
+            // arrange
+            using var input = new Image<Rgba32>(25, 25);
+            using var memoryStream = new MemoryStream();
+            var expectedExif = new ExifProfile();
+            string expectedSoftware = "ImageSharp";
+            expectedExif.SetValue(ExifTag.Software, expectedSoftware);
+            input.Metadata.ExifProfile = expectedExif;
+
+            // act
+            input.Save(memoryStream, new WebpEncoder() { FileFormat = fileFormatType });
+            memoryStream.Position = 0;
+
+            // assert
+            using var image = Image.Load<Rgba32>(memoryStream);
+            ExifProfile actualExif = image.Metadata.ExifProfile;
+            Assert.NotNull(actualExif);
+            Assert.Equal(expectedExif.Values.Count, actualExif.Values.Count);
+            Assert.Equal(expectedSoftware, actualExif.GetValue(ExifTag.Software).Value);
+        }
+
         [Theory]
         [WithFile(TestImages.Webp.Lossy.WithExif, PixelTypes.Rgba32)]
         public void EncodeLossyWebp_PreservesExif<TPixel>(TestImageProvider<TPixel> provider)