Skip to content

Commit 529d120

Browse files
JimBobSquarePantsJimBobSquarePants
committed
Handle slow streams. Fix #1268 
1 parent 889dfd5 commit 529d120

5 files changed

Lines changed: 156 additions & 36 deletions

File tree

‎src/ImageSharp/IO/BufferedReadStream.cs‎

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright (c) Six Labors and contributors.
1+
// Copyright (c) Six Labors.
22
// Licensed under the Apache License, Version 2.0.
33

44
using System;
@@ -223,7 +223,21 @@ private void FillReadBuffer()
223223
this.stream.Seek(this.readerPosition, SeekOrigin.Begin);
224224
}
225225

226-
this.stream.Read(this.readBuffer, 0, BufferLength);
226+
int n = this.stream.Read(this.readBuffer, 0, BufferLength);
227+
228+
// Read doesn't always guarantee the full returned length so read a byte
229+
// at a time until we get either our count or hit the end of the stream.
230+
int i = 0;
231+
while (n < BufferLength && i != -1)
232+
{
233+
i = this.stream.ReadByte();
234+
235+
if (i != -1)
236+
{
237+
this.readBuffer[n++] = (byte)i;
238+
}
239+
}
240+
227241
this.readBufferIndex = 0;
228242
}
229243

@@ -258,6 +272,20 @@ private int ReadToBufferDirectSlow(byte[] buffer, int offset, int count)
258272
}
259273

260274
int n = this.stream.Read(buffer, offset, count);
275+
276+
// Read doesn't always guarantee the full returned length so read a byte
277+
// at a time until we get either our count or hit the end of the stream.
278+
int i = 0;
279+
while (n < count && i != -1)
280+
{
281+
i = this.stream.ReadByte();
282+
283+
if (i != -1)
284+
{
285+
buffer[n++] = (byte)i;
286+
}
287+
}
288+
261289
this.Position += n;
262290

263291
return n;

‎src/ImageSharp/Image.Decode.cs‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,22 @@ private static IImageFormat InternalDetectFormat(Stream stream, Configuration co
5959
using (IManagedByteBuffer buffer = config.MemoryAllocator.AllocateManagedByteBuffer(headerSize, AllocationOptions.Clean))
6060
{
6161
long startPosition = stream.Position;
62-
stream.Read(buffer.Array, 0, headerSize);
62+
63+
int n = stream.Read(buffer.Array, 0, headerSize);
64+
65+
// Read doesn't always guarantee the full returned length so read a byte
66+
// at a time until we get either our count or hit the end of the stream.
67+
int i = 0;
68+
while (n < headerSize && i != -1)
69+
{
70+
i = stream.ReadByte();
71+
72+
if (i != -1)
73+
{
74+
buffer.Array[n++] = (byte)i;
75+
}
76+
}
77+
6378
stream.Position = startPosition;
6479

6580
// Does the given stream contain enough data to fit in the header for the format

‎src/ImageSharp/Image.FromStream.cs‎

Lines changed: 60 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ public static IImageFormat DetectFormat(Stream stream)
3838
/// <exception cref="NotSupportedException">The stream is not readable.</exception>
3939
/// <returns>The format type or null if none found.</returns>
4040
public static IImageFormat DetectFormat(Configuration configuration, Stream stream)
41-
=> WithSeekableStream(configuration, stream, false, s => InternalDetectFormat(s, configuration));
41+
=> WithSeekableStream(configuration, stream, s => InternalDetectFormat(s, configuration), false);
4242

4343
/// <summary>
4444
/// By reading the header on the provided stream this calculates the images format type.
@@ -63,7 +63,8 @@ public static Task<IImageFormat> DetectFormatAsync(Configuration configuration,
6363
=> WithSeekableStreamAsync(
6464
configuration,
6565
stream,
66-
s => InternalDetectFormatAsync(s, configuration));
66+
s => InternalDetectFormatAsync(s, configuration),
67+
false);
6768

6869
/// <summary>
6970
/// Reads the raw image information from the specified stream without fully decoding it.
@@ -155,7 +156,7 @@ public static async Task<IImageInfo> IdentifyAsync(Configuration configuration,
155156
/// </returns>
156157
public static IImageInfo Identify(Configuration configuration, Stream stream, out IImageFormat format)
157158
{
158-
(IImageInfo ImageInfo, IImageFormat format) data = WithSeekableStream(configuration, stream, false, s => InternalIdentity(s, configuration ?? Configuration.Default));
159+
(IImageInfo ImageInfo, IImageFormat Format) data = WithSeekableStream(configuration, stream, s => InternalIdentity(s, configuration ?? Configuration.Default));
159160

160161
format = data.Format;
161162
return data.ImageInfo;
@@ -291,7 +292,7 @@ public static Task<Image> LoadAsync(Stream stream, IImageDecoder decoder)
291292
/// <exception cref="InvalidImageContentException">Image contains invalid content.</exception>
292293
/// <returns>A new <see cref="Image"/>.</returns>
293294
public static Image Load(Configuration configuration, Stream stream, IImageDecoder decoder)
294-
=> WithSeekableStream(configuration, stream, true, s => decoder.Decode(configuration, s));
295+
=> WithSeekableStream(configuration, stream, s => decoder.Decode(configuration, s));
295296

296297
/// <summary>
297298
/// Decode a new instance of the <see cref="Image"/> class from the given stream.
@@ -416,7 +417,7 @@ public static Image<TPixel> Load<TPixel>(Stream stream, out IImageFormat format)
416417
/// <returns>A new <see cref="Image{TPixel}"/>.</returns>
417418
public static Image<TPixel> Load<TPixel>(Stream stream, IImageDecoder decoder)
418419
where TPixel : unmanaged, IPixel<TPixel>
419-
=> WithSeekableStream(Configuration.Default, stream, true, s => decoder.Decode<TPixel>(Configuration.Default, s));
420+
=> WithSeekableStream(Configuration.Default, stream, s => decoder.Decode<TPixel>(Configuration.Default, s));
420421

421422
/// <summary>
422423
/// Create a new instance of the <see cref="Image{TPixel}"/> class from the given stream.
@@ -451,7 +452,7 @@ public static Task<Image<TPixel>> LoadAsync<TPixel>(Stream stream, IImageDecoder
451452
/// <returns>A new <see cref="Image{TPixel}"/>.</returns>
452453
public static Image<TPixel> Load<TPixel>(Configuration configuration, Stream stream, IImageDecoder decoder)
453454
where TPixel : unmanaged, IPixel<TPixel>
454-
=> WithSeekableStream(configuration, stream, true, s => decoder.Decode<TPixel>(configuration, s));
455+
=> WithSeekableStream(configuration, stream, s => decoder.Decode<TPixel>(configuration, s));
455456

456457
/// <summary>
457458
/// Create a new instance of the <see cref="Image{TPixel}"/> class from the given stream.
@@ -505,7 +506,7 @@ public static Image<TPixel> Load<TPixel>(Configuration configuration, Stream str
505506
public static Image<TPixel> Load<TPixel>(Configuration configuration, Stream stream, out IImageFormat format)
506507
where TPixel : unmanaged, IPixel<TPixel>
507508
{
508-
(Image<TPixel> Image, IImageFormat Format) data = WithSeekableStream(configuration, stream, true, s => Decode<TPixel>(s, configuration));
509+
(Image<TPixel> Image, IImageFormat Format) data = WithSeekableStream(configuration, stream, s => Decode<TPixel>(s, configuration));
509510

510511
format = data.Format;
511512

@@ -632,7 +633,7 @@ public static async Task<Image<TPixel>> LoadAsync<TPixel>(Configuration configur
632633
/// <returns>A new <see cref="Image{TPixel}"/>.</returns>
633634
public static Image Load(Configuration configuration, Stream stream, out IImageFormat format)
634635
{
635-
(Image img, IImageFormat format) data = WithSeekableStream(configuration, stream, true, s => Decode(s, configuration));
636+
(Image img, IImageFormat format) data = WithSeekableStream(configuration, stream, s => Decode(s, configuration));
636637

637638
format = data.format;
638639

@@ -652,7 +653,24 @@ public static Image Load(Configuration configuration, Stream stream, out IImageF
652653
throw new UnknownImageFormatException(sb.ToString());
653654
}
654655

655-
private static T WithSeekableStream<T>(Configuration configuration, Stream stream, bool buffer, Func<Stream, T> action)
656+
/// <summary>
657+
/// Performs the given action against the stream ensuring that it is seekable.
658+
/// </summary>
659+
/// <typeparam name="T">The type of object returned from the action.</typeparam>
660+
/// <param name="configuration">The configuration.</param>
661+
/// <param name="stream">The input stream.</param>
662+
/// <param name="action">The action to perform.</param>
663+
/// <param name="buffer">
664+
/// Whether to buffer the input stream.
665+
/// Short intial reads like <see cref="DetectFormat(Configuration, Stream)"/> do not require
666+
/// the overhead of reading the stream to the buffer. Defaults to <see langword="true"/>.
667+
/// </param>
668+
/// <returns>The <typeparamref name="T"/>.</returns>
669+
private static T WithSeekableStream<T>(
670+
Configuration configuration,
671+
Stream stream,
672+
Func<Stream, T> action,
673+
bool buffer = true)
656674
{
657675
Guard.NotNull(configuration, nameof(configuration));
658676
Guard.NotNull(stream, nameof(stream));
@@ -684,14 +702,34 @@ private static T WithSeekableStream<T>(Configuration configuration, Stream strea
684702
stream.CopyTo(memoryStream);
685703
memoryStream.Position = 0;
686704

705+
if (buffer)
706+
{
707+
using var bufferedStream = new BufferedReadStream(memoryStream);
708+
return action(bufferedStream);
709+
}
710+
687711
return action(memoryStream);
688712
}
689713
}
690714

715+
/// <summary>
716+
/// Performs the given action asynchronously against the stream ensuring that it is seekable.
717+
/// </summary>
718+
/// <typeparam name="T">The type of object returned from the action.</typeparam>
719+
/// <param name="configuration">The configuration.</param>
720+
/// <param name="stream">The input stream.</param>
721+
/// <param name="action">The action to perform.</param>
722+
/// <param name="buffer">
723+
/// Whether to buffer the input stream.
724+
/// Short intial reads like <see cref="DetectFormat(Configuration, Stream)"/> do not require
725+
/// the overhead of reading the stream to the buffer. Defaults to <see langword="true"/>.
726+
/// </param>
727+
/// <returns>The <see cref="Task{T}"/>.</returns>
691728
private static async Task<T> WithSeekableStreamAsync<T>(
692729
Configuration configuration,
693730
Stream stream,
694-
Func<Stream, Task<T>> action)
731+
Func<Stream, Task<T>> action,
732+
bool buffer = true)
695733
{
696734
Guard.NotNull(configuration, nameof(configuration));
697735
Guard.NotNull(stream, nameof(stream));
@@ -712,6 +750,12 @@ private static async Task<T> WithSeekableStreamAsync<T>(
712750
stream.Position = 0;
713751
}
714752

753+
if (buffer)
754+
{
755+
using var bufferedStream = new BufferedReadStream(stream);
756+
return await action(bufferedStream).ConfigureAwait(false);
757+
}
758+
715759
return await action(stream).ConfigureAwait(false);
716760
}
717761

@@ -720,6 +764,12 @@ private static async Task<T> WithSeekableStreamAsync<T>(
720764
await stream.CopyToAsync(memoryStream).ConfigureAwait(false);
721765
memoryStream.Position = 0;
722766

767+
if (buffer)
768+
{
769+
using var bufferedStream = new BufferedReadStream(memoryStream);
770+
return await action(bufferedStream).ConfigureAwait(false);
771+
}
772+
723773
return await action(memoryStream).ConfigureAwait(false);
724774
}
725775
}

‎tests/ImageSharp.Benchmarks/General/IO/BufferedStreams.cs‎

Lines changed: 33 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -166,29 +166,41 @@ private static byte[] CreateTestBytes()
166166
}
167167
}
168168

169-
/* RESULTS (2019 April 24):
170-
171-
BenchmarkDotNet=v0.11.5, OS=Windows 10.0.17763.437 (1809/October2018Update/Redstone5)
172-
Intel Core i7-6600U CPU 2.60GHz (Skylake), 1 CPU, 4 logical and 2 physical cores
173-
.NET Core SDK=2.2.202
174-
[Host] : .NET Core 2.1.9 (CoreCLR 4.6.27414.06, CoreFX 4.6.27415.01), 64bit RyuJIT
175-
Clr : .NET Framework 4.7.2 (CLR 4.0.30319.42000), 64bit RyuJIT-v4.7.3362.0
176-
Core : .NET Core 2.1.9 (CoreCLR 4.6.27414.06, CoreFX 4.6.27415.01), 64bit RyuJIT
169+
/*
170+
BenchmarkDotNet=v0.12.0, OS=Windows 10.0.19041
171+
Intel Core i7-8650U CPU 1.90GHz (Kaby Lake R), 1 CPU, 8 logical and 4 physical cores
172+
.NET Core SDK=3.1.301
173+
[Host] : .NET Core 3.1.5 (CoreCLR 4.700.20.26901, CoreFX 4.700.20.27001), X64 RyuJIT
174+
Job-LKLBOT : .NET Framework 4.8 (4.8.4180.0), X64 RyuJIT
175+
Job-RSTMKF : .NET Core 2.1.19 (CoreCLR 4.6.28928.01, CoreFX 4.6.28928.04), X64 RyuJIT
176+
Job-PZIHIV : .NET Core 3.1.5 (CoreCLR 4.700.20.26901, CoreFX 4.700.20.27001), X64 RyuJIT
177177
178178
IterationCount=3 LaunchCount=1 WarmupCount=3
179179
180-
| Method | Job | Runtime | Mean | Error | StdDev | Ratio | RatioSD | Gen 0 | Gen 1 | Gen 2 | Allocated |
181-
|----------------------------- |----- |-------- |---------:|-----------:|----------:|------:|--------:|------:|------:|------:|----------:|
182-
| StandardStreamReadByte | Clr | Clr | 96.71 us | 5.9950 us | 0.3286 us | 1.00 | 0.00 | - | - | - | - |
183-
| StandardStreamRead | Clr | Clr | 77.73 us | 5.2284 us | 0.2866 us | 0.80 | 0.00 | - | - | - | - |
184-
| DoubleBufferedStreamReadByte | Clr | Clr | 23.17 us | 26.2354 us | 1.4381 us | 0.24 | 0.01 | - | - | - | - |
185-
| DoubleBufferedStreamRead | Clr | Clr | 33.35 us | 3.4071 us | 0.1868 us | 0.34 | 0.00 | - | - | - | - |
186-
| SimpleReadByte | Clr | Clr | 10.85 us | 0.4927 us | 0.0270 us | 0.11 | 0.00 | - | - | - | - |
187-
| | | | | | | | | | | | |
188-
| StandardStreamReadByte | Core | Core | 75.35 us | 12.9789 us | 0.7114 us | 1.00 | 0.00 | - | - | - | - |
189-
| StandardStreamRead | Core | Core | 55.36 us | 1.4432 us | 0.0791 us | 0.73 | 0.01 | - | - | - | - |
190-
| DoubleBufferedStreamReadByte | Core | Core | 21.47 us | 29.7076 us | 1.6284 us | 0.28 | 0.02 | - | - | - | - |
191-
| DoubleBufferedStreamRead | Core | Core | 29.67 us | 2.5988 us | 0.1424 us | 0.39 | 0.00 | - | - | - | - |
192-
| SimpleReadByte | Core | Core | 10.84 us | 0.7567 us | 0.0415 us | 0.14 | 0.00 | - | - | - | - |
180+
| Method | Runtime | Mean | Error | StdDev | Ratio | RatioSD | Gen 0 | Gen 1 | Gen 2 | Allocated |
181+
|------------------------------- |-------------- |----------:|-----------:|----------:|------:|--------:|------:|------:|------:|----------:|
182+
| StandardStreamRead | .NET 4.7.2 | 126.07 us | 126.498 us | 6.934 us | 0.99 | 0.08 | - | - | - | - |
183+
| BufferedReadStreamRead | .NET 4.7.2 | 118.08 us | 42.234 us | 2.315 us | 0.92 | 0.03 | - | - | - | - |
184+
| BufferedReadStreamWrapRead | .NET 4.7.2 | 45.33 us | 22.833 us | 1.252 us | 0.35 | 0.00 | - | - | - | - |
185+
| StandardStreamReadByte | .NET 4.7.2 | 128.17 us | 94.616 us | 5.186 us | 1.00 | 0.00 | - | - | - | - |
186+
| BufferedReadStreamReadByte | .NET 4.7.2 | 143.60 us | 92.871 us | 5.091 us | 1.12 | 0.08 | - | - | - | - |
187+
| BufferedReadStreamWrapReadByte | .NET 4.7.2 | 32.72 us | 53.708 us | 2.944 us | 0.26 | 0.02 | - | - | - | - |
188+
| ArrayReadByte | .NET 4.7.2 | 19.40 us | 12.206 us | 0.669 us | 0.15 | 0.01 | - | - | - | - |
189+
| | | | | | | | | | | |
190+
| StandardStreamRead | .NET Core 2.1 | 84.82 us | 55.983 us | 3.069 us | 0.75 | 0.15 | - | - | - | - |
191+
| BufferedReadStreamRead | .NET Core 2.1 | 49.62 us | 27.253 us | 1.494 us | 0.44 | 0.08 | - | - | - | - |
192+
| BufferedReadStreamWrapRead | .NET Core 2.1 | 67.78 us | 87.546 us | 4.799 us | 0.60 | 0.10 | - | - | - | - |
193+
| StandardStreamReadByte | .NET Core 2.1 | 115.81 us | 382.107 us | 20.945 us | 1.00 | 0.00 | - | - | - | - |
194+
| BufferedReadStreamReadByte | .NET Core 2.1 | 16.32 us | 6.123 us | 0.336 us | 0.14 | 0.02 | - | - | - | - |
195+
| BufferedReadStreamWrapReadByte | .NET Core 2.1 | 16.68 us | 4.616 us | 0.253 us | 0.15 | 0.03 | - | - | - | - |
196+
| ArrayReadByte | .NET Core 2.1 | 15.13 us | 60.763 us | 3.331 us | 0.14 | 0.05 | - | - | - | - |
197+
| | | | | | | | | | | |
198+
| StandardStreamRead | .NET Core 3.1 | 92.44 us | 88.217 us | 4.835 us | 0.94 | 0.06 | - | - | - | - |
199+
| BufferedReadStreamRead | .NET Core 3.1 | 36.41 us | 5.923 us | 0.325 us | 0.37 | 0.00 | - | - | - | - |
200+
| BufferedReadStreamWrapRead | .NET Core 3.1 | 37.22 us | 9.628 us | 0.528 us | 0.38 | 0.01 | - | - | - | - |
201+
| StandardStreamReadByte | .NET Core 3.1 | 98.67 us | 20.947 us | 1.148 us | 1.00 | 0.00 | - | - | - | - |
202+
| BufferedReadStreamReadByte | .NET Core 3.1 | 41.36 us | 123.536 us | 6.771 us | 0.42 | 0.06 | - | - | - | - |
203+
| BufferedReadStreamWrapReadByte | .NET Core 3.1 | 39.11 us | 93.894 us | 5.147 us | 0.40 | 0.05 | - | - | - | - |
204+
| ArrayReadByte | .NET Core 3.1 | 18.84 us | 4.684 us | 0.257 us | 0.19 | 0.00 | - | - | - | - |
193205
*/
194206
}

‎tests/ImageSharp.Tests/IO/BufferedReadStreamTests.cs‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright (c) Six Labors and contributors.
1+
// Copyright (c) Six Labors.
22
// Licensed under the Apache License, Version 2.0.
33

44
using System;
@@ -221,7 +221,22 @@ private MemoryStream CreateTestStream(int length = BufferedReadStream.BufferLeng
221221
var random = new Random();
222222
random.NextBytes(buffer);
223223

224-
return new MemoryStream(buffer);
224+
return new EvilStream(buffer);
225+
}
226+
227+
// Simulates a stream that can only return 1 byte at a time per read instruction.
228+
// See https://github.com/SixLabors/ImageSharp/issues/1268
229+
private class EvilStream : MemoryStream
230+
{
231+
public EvilStream(byte[] buffer)
232+
: base(buffer)
233+
{
234+
}
235+
236+
public override int Read(byte[] buffer, int offset, int count)
237+
{
238+
return base.Read(buffer, offset, 1);
239+
}
225240
}
226241
}
227242
}

0 commit comments

Comments
 (0)