diff --git a/src/ImageSharp/IO/BufferedReadStream.cs b/src/ImageSharp/IO/BufferedReadStream.cs index 0f6e9da1e5..0592e58cd1 100644 --- a/src/ImageSharp/IO/BufferedReadStream.cs +++ b/src/ImageSharp/IO/BufferedReadStream.cs @@ -1,4 +1,4 @@ -// Copyright (c) Six Labors and contributors. +// Copyright (c) Six Labors. // Licensed under the Apache License, Version 2.0. using System; @@ -223,7 +223,21 @@ namespace SixLabors.ImageSharp.IO this.stream.Seek(this.readerPosition, SeekOrigin.Begin); } - this.stream.Read(this.readBuffer, 0, BufferLength); + int n = this.stream.Read(this.readBuffer, 0, BufferLength); + + // Read doesn't always guarantee the full returned length so read a byte + // at a time until we get either our count or hit the end of the stream. + int i = 0; + while (n < BufferLength && i != -1) + { + i = this.stream.ReadByte(); + + if (i != -1) + { + this.readBuffer[n++] = (byte)i; + } + } + this.readBufferIndex = 0; } @@ -258,6 +272,20 @@ namespace SixLabors.ImageSharp.IO } int n = this.stream.Read(buffer, offset, count); + + // Read doesn't always guarantee the full returned length so read a byte + // at a time until we get either our count or hit the end of the stream. + int i = 0; + while (n < count && i != -1) + { + i = this.stream.ReadByte(); + + if (i != -1) + { + buffer[n++] = (byte)i; + } + } + this.Position += n; return n; diff --git a/src/ImageSharp/Image.Decode.cs b/src/ImageSharp/Image.Decode.cs index 5330782f22..c78c706bdb 100644 --- a/src/ImageSharp/Image.Decode.cs +++ b/src/ImageSharp/Image.Decode.cs @@ -59,7 +59,22 @@ namespace SixLabors.ImageSharp using (IManagedByteBuffer buffer = config.MemoryAllocator.AllocateManagedByteBuffer(headerSize, AllocationOptions.Clean)) { long startPosition = stream.Position; - stream.Read(buffer.Array, 0, headerSize); + + int n = stream.Read(buffer.Array, 0, headerSize); + + // Read doesn't always guarantee the full returned length so read a byte + // at a time until we get either our count or hit the end of the stream. + int i = 0; + while (n < headerSize && i != -1) + { + i = stream.ReadByte(); + + if (i != -1) + { + buffer.Array[n++] = (byte)i; + } + } + stream.Position = startPosition; // Does the given stream contain enough data to fit in the header for the format diff --git a/src/ImageSharp/Image.FromStream.cs b/src/ImageSharp/Image.FromStream.cs index e4b9e00937..1621724b0d 100644 --- a/src/ImageSharp/Image.FromStream.cs +++ b/src/ImageSharp/Image.FromStream.cs @@ -38,7 +38,7 @@ namespace SixLabors.ImageSharp /// The stream is not readable. /// The format type or null if none found. public static IImageFormat DetectFormat(Configuration configuration, Stream stream) - => WithSeekableStream(configuration, stream, false, s => InternalDetectFormat(s, configuration)); + => WithSeekableStream(configuration, stream, s => InternalDetectFormat(s, configuration), false); /// /// By reading the header on the provided stream this calculates the images format type. @@ -63,7 +63,8 @@ namespace SixLabors.ImageSharp => WithSeekableStreamAsync( configuration, stream, - s => InternalDetectFormatAsync(s, configuration)); + s => InternalDetectFormatAsync(s, configuration), + false); /// /// Reads the raw image information from the specified stream without fully decoding it. @@ -155,7 +156,7 @@ namespace SixLabors.ImageSharp /// public static IImageInfo Identify(Configuration configuration, Stream stream, out IImageFormat format) { - (IImageInfo ImageInfo, IImageFormat format) data = WithSeekableStream(configuration, stream, false, s => InternalIdentity(s, configuration ?? Configuration.Default)); + (IImageInfo ImageInfo, IImageFormat Format) data = WithSeekableStream(configuration, stream, s => InternalIdentity(s, configuration ?? Configuration.Default)); format = data.Format; return data.ImageInfo; @@ -291,7 +292,7 @@ namespace SixLabors.ImageSharp /// Image contains invalid content. /// A new . public static Image Load(Configuration configuration, Stream stream, IImageDecoder decoder) - => WithSeekableStream(configuration, stream, true, s => decoder.Decode(configuration, s)); + => WithSeekableStream(configuration, stream, s => decoder.Decode(configuration, s)); /// /// Decode a new instance of the class from the given stream. @@ -416,7 +417,7 @@ namespace SixLabors.ImageSharp /// A new . public static Image Load(Stream stream, IImageDecoder decoder) where TPixel : unmanaged, IPixel - => WithSeekableStream(Configuration.Default, stream, true, s => decoder.Decode(Configuration.Default, s)); + => WithSeekableStream(Configuration.Default, stream, s => decoder.Decode(Configuration.Default, s)); /// /// Create a new instance of the class from the given stream. @@ -451,7 +452,7 @@ namespace SixLabors.ImageSharp /// A new . public static Image Load(Configuration configuration, Stream stream, IImageDecoder decoder) where TPixel : unmanaged, IPixel - => WithSeekableStream(configuration, stream, true, s => decoder.Decode(configuration, s)); + => WithSeekableStream(configuration, stream, s => decoder.Decode(configuration, s)); /// /// Create a new instance of the class from the given stream. @@ -505,7 +506,7 @@ namespace SixLabors.ImageSharp public static Image Load(Configuration configuration, Stream stream, out IImageFormat format) where TPixel : unmanaged, IPixel { - (Image Image, IImageFormat Format) data = WithSeekableStream(configuration, stream, true, s => Decode(s, configuration)); + (Image Image, IImageFormat Format) data = WithSeekableStream(configuration, stream, s => Decode(s, configuration)); format = data.Format; @@ -632,7 +633,7 @@ namespace SixLabors.ImageSharp /// A new . public static Image Load(Configuration configuration, Stream stream, out IImageFormat format) { - (Image img, IImageFormat format) data = WithSeekableStream(configuration, stream, true, s => Decode(s, configuration)); + (Image img, IImageFormat format) data = WithSeekableStream(configuration, stream, s => Decode(s, configuration)); format = data.format; @@ -652,7 +653,24 @@ namespace SixLabors.ImageSharp throw new UnknownImageFormatException(sb.ToString()); } - private static T WithSeekableStream(Configuration configuration, Stream stream, bool buffer, Func action) + /// + /// Performs the given action against the stream ensuring that it is seekable. + /// + /// The type of object returned from the action. + /// The configuration. + /// The input stream. + /// The action to perform. + /// + /// Whether to buffer the input stream. + /// Short intial reads like do not require + /// the overhead of reading the stream to the buffer. Defaults to . + /// + /// The . + private static T WithSeekableStream( + Configuration configuration, + Stream stream, + Func action, + bool buffer = true) { Guard.NotNull(configuration, nameof(configuration)); Guard.NotNull(stream, nameof(stream)); @@ -684,14 +702,34 @@ namespace SixLabors.ImageSharp stream.CopyTo(memoryStream); memoryStream.Position = 0; + if (buffer) + { + using var bufferedStream = new BufferedReadStream(memoryStream); + return action(bufferedStream); + } + return action(memoryStream); } } + /// + /// Performs the given action asynchronously against the stream ensuring that it is seekable. + /// + /// The type of object returned from the action. + /// The configuration. + /// The input stream. + /// The action to perform. + /// + /// Whether to buffer the input stream. + /// Short intial reads like do not require + /// the overhead of reading the stream to the buffer. Defaults to . + /// + /// The . private static async Task WithSeekableStreamAsync( Configuration configuration, Stream stream, - Func> action) + Func> action, + bool buffer = true) { Guard.NotNull(configuration, nameof(configuration)); Guard.NotNull(stream, nameof(stream)); @@ -712,6 +750,12 @@ namespace SixLabors.ImageSharp stream.Position = 0; } + if (buffer) + { + using var bufferedStream = new BufferedReadStream(stream); + return await action(bufferedStream).ConfigureAwait(false); + } + return await action(stream).ConfigureAwait(false); } @@ -720,6 +764,12 @@ namespace SixLabors.ImageSharp await stream.CopyToAsync(memoryStream).ConfigureAwait(false); memoryStream.Position = 0; + if (buffer) + { + using var bufferedStream = new BufferedReadStream(memoryStream); + return await action(bufferedStream).ConfigureAwait(false); + } + return await action(memoryStream).ConfigureAwait(false); } } diff --git a/tests/ImageSharp.Benchmarks/General/IO/BufferedStreams.cs b/tests/ImageSharp.Benchmarks/General/IO/BufferedStreams.cs index 97028618f5..03cd9f087e 100644 --- a/tests/ImageSharp.Benchmarks/General/IO/BufferedStreams.cs +++ b/tests/ImageSharp.Benchmarks/General/IO/BufferedStreams.cs @@ -166,29 +166,41 @@ namespace SixLabors.ImageSharp.Benchmarks.IO } } - /* RESULTS (2019 April 24): - - BenchmarkDotNet=v0.11.5, OS=Windows 10.0.17763.437 (1809/October2018Update/Redstone5) - Intel Core i7-6600U CPU 2.60GHz (Skylake), 1 CPU, 4 logical and 2 physical cores - .NET Core SDK=2.2.202 - [Host] : .NET Core 2.1.9 (CoreCLR 4.6.27414.06, CoreFX 4.6.27415.01), 64bit RyuJIT - Clr : .NET Framework 4.7.2 (CLR 4.0.30319.42000), 64bit RyuJIT-v4.7.3362.0 - Core : .NET Core 2.1.9 (CoreCLR 4.6.27414.06, CoreFX 4.6.27415.01), 64bit RyuJIT + /* + BenchmarkDotNet=v0.12.0, OS=Windows 10.0.19041 + Intel Core i7-8650U CPU 1.90GHz (Kaby Lake R), 1 CPU, 8 logical and 4 physical cores + .NET Core SDK=3.1.301 + [Host] : .NET Core 3.1.5 (CoreCLR 4.700.20.26901, CoreFX 4.700.20.27001), X64 RyuJIT + Job-LKLBOT : .NET Framework 4.8 (4.8.4180.0), X64 RyuJIT + Job-RSTMKF : .NET Core 2.1.19 (CoreCLR 4.6.28928.01, CoreFX 4.6.28928.04), X64 RyuJIT + Job-PZIHIV : .NET Core 3.1.5 (CoreCLR 4.700.20.26901, CoreFX 4.700.20.27001), X64 RyuJIT IterationCount=3 LaunchCount=1 WarmupCount=3 - | Method | Job | Runtime | Mean | Error | StdDev | Ratio | RatioSD | Gen 0 | Gen 1 | Gen 2 | Allocated | - |----------------------------- |----- |-------- |---------:|-----------:|----------:|------:|--------:|------:|------:|------:|----------:| - | StandardStreamReadByte | Clr | Clr | 96.71 us | 5.9950 us | 0.3286 us | 1.00 | 0.00 | - | - | - | - | - | StandardStreamRead | Clr | Clr | 77.73 us | 5.2284 us | 0.2866 us | 0.80 | 0.00 | - | - | - | - | - | DoubleBufferedStreamReadByte | Clr | Clr | 23.17 us | 26.2354 us | 1.4381 us | 0.24 | 0.01 | - | - | - | - | - | DoubleBufferedStreamRead | Clr | Clr | 33.35 us | 3.4071 us | 0.1868 us | 0.34 | 0.00 | - | - | - | - | - | SimpleReadByte | Clr | Clr | 10.85 us | 0.4927 us | 0.0270 us | 0.11 | 0.00 | - | - | - | - | - | | | | | | | | | | | | | - | StandardStreamReadByte | Core | Core | 75.35 us | 12.9789 us | 0.7114 us | 1.00 | 0.00 | - | - | - | - | - | StandardStreamRead | Core | Core | 55.36 us | 1.4432 us | 0.0791 us | 0.73 | 0.01 | - | - | - | - | - | DoubleBufferedStreamReadByte | Core | Core | 21.47 us | 29.7076 us | 1.6284 us | 0.28 | 0.02 | - | - | - | - | - | DoubleBufferedStreamRead | Core | Core | 29.67 us | 2.5988 us | 0.1424 us | 0.39 | 0.00 | - | - | - | - | - | SimpleReadByte | Core | Core | 10.84 us | 0.7567 us | 0.0415 us | 0.14 | 0.00 | - | - | - | - | + | Method | Runtime | Mean | Error | StdDev | Ratio | RatioSD | Gen 0 | Gen 1 | Gen 2 | Allocated | + |------------------------------- |-------------- |----------:|-----------:|----------:|------:|--------:|------:|------:|------:|----------:| + | StandardStreamRead | .NET 4.7.2 | 126.07 us | 126.498 us | 6.934 us | 0.99 | 0.08 | - | - | - | - | + | BufferedReadStreamRead | .NET 4.7.2 | 118.08 us | 42.234 us | 2.315 us | 0.92 | 0.03 | - | - | - | - | + | BufferedReadStreamWrapRead | .NET 4.7.2 | 45.33 us | 22.833 us | 1.252 us | 0.35 | 0.00 | - | - | - | - | + | StandardStreamReadByte | .NET 4.7.2 | 128.17 us | 94.616 us | 5.186 us | 1.00 | 0.00 | - | - | - | - | + | BufferedReadStreamReadByte | .NET 4.7.2 | 143.60 us | 92.871 us | 5.091 us | 1.12 | 0.08 | - | - | - | - | + | BufferedReadStreamWrapReadByte | .NET 4.7.2 | 32.72 us | 53.708 us | 2.944 us | 0.26 | 0.02 | - | - | - | - | + | ArrayReadByte | .NET 4.7.2 | 19.40 us | 12.206 us | 0.669 us | 0.15 | 0.01 | - | - | - | - | + | | | | | | | | | | | | + | StandardStreamRead | .NET Core 2.1 | 84.82 us | 55.983 us | 3.069 us | 0.75 | 0.15 | - | - | - | - | + | BufferedReadStreamRead | .NET Core 2.1 | 49.62 us | 27.253 us | 1.494 us | 0.44 | 0.08 | - | - | - | - | + | BufferedReadStreamWrapRead | .NET Core 2.1 | 67.78 us | 87.546 us | 4.799 us | 0.60 | 0.10 | - | - | - | - | + | StandardStreamReadByte | .NET Core 2.1 | 115.81 us | 382.107 us | 20.945 us | 1.00 | 0.00 | - | - | - | - | + | BufferedReadStreamReadByte | .NET Core 2.1 | 16.32 us | 6.123 us | 0.336 us | 0.14 | 0.02 | - | - | - | - | + | BufferedReadStreamWrapReadByte | .NET Core 2.1 | 16.68 us | 4.616 us | 0.253 us | 0.15 | 0.03 | - | - | - | - | + | ArrayReadByte | .NET Core 2.1 | 15.13 us | 60.763 us | 3.331 us | 0.14 | 0.05 | - | - | - | - | + | | | | | | | | | | | | + | StandardStreamRead | .NET Core 3.1 | 92.44 us | 88.217 us | 4.835 us | 0.94 | 0.06 | - | - | - | - | + | BufferedReadStreamRead | .NET Core 3.1 | 36.41 us | 5.923 us | 0.325 us | 0.37 | 0.00 | - | - | - | - | + | BufferedReadStreamWrapRead | .NET Core 3.1 | 37.22 us | 9.628 us | 0.528 us | 0.38 | 0.01 | - | - | - | - | + | StandardStreamReadByte | .NET Core 3.1 | 98.67 us | 20.947 us | 1.148 us | 1.00 | 0.00 | - | - | - | - | + | BufferedReadStreamReadByte | .NET Core 3.1 | 41.36 us | 123.536 us | 6.771 us | 0.42 | 0.06 | - | - | - | - | + | BufferedReadStreamWrapReadByte | .NET Core 3.1 | 39.11 us | 93.894 us | 5.147 us | 0.40 | 0.05 | - | - | - | - | + | ArrayReadByte | .NET Core 3.1 | 18.84 us | 4.684 us | 0.257 us | 0.19 | 0.00 | - | - | - | - | */ } diff --git a/tests/ImageSharp.Tests/IO/BufferedReadStreamTests.cs b/tests/ImageSharp.Tests/IO/BufferedReadStreamTests.cs index 748550a541..c9ace8df29 100644 --- a/tests/ImageSharp.Tests/IO/BufferedReadStreamTests.cs +++ b/tests/ImageSharp.Tests/IO/BufferedReadStreamTests.cs @@ -1,4 +1,4 @@ -// Copyright (c) Six Labors and contributors. +// Copyright (c) Six Labors. // Licensed under the Apache License, Version 2.0. using System; @@ -221,7 +221,22 @@ namespace SixLabors.ImageSharp.Tests.IO var random = new Random(); random.NextBytes(buffer); - return new MemoryStream(buffer); + return new EvilStream(buffer); + } + + // Simulates a stream that can only return 1 byte at a time per read instruction. + // See https://github.com/SixLabors/ImageSharp/issues/1268 + private class EvilStream : MemoryStream + { + public EvilStream(byte[] buffer) + : base(buffer) + { + } + + public override int Read(byte[] buffer, int offset, int count) + { + return base.Read(buffer, offset, 1); + } } } }