Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

System.ArgumentOutOfRangeException (Parameter name: minimumLength) when loading moderate size jpg image #805

Closed
4 tasks done
sardok opened this issue Jan 8, 2019 · 20 comments · Fixed by #1109
Closed
4 tasks done

Comments

@sardok
Copy link

sardok commented Jan 8, 2019

Prerequisites

  • I have written a descriptive issue title
  • I have verified that I am running the latest version of ImageSharp
  • I have verified if the problem exist in both DEBUG and RELEASE mode
  • I have searched open and closed issues to ensure it has not already been reported

Description

Trying to load a jpeg image (1024x36000) fails with following stack trace.

Unhandled Exception: System.ArgumentOutOfRangeException: Specified argument was out of the range of valid values.
Parameter name: minimumLength
   at System.Buffers.ConfigurableArrayPool`1.Rent(Int32 minimumLength)
   at SixLabors.Memory.ArrayPoolMemoryAllocator.Allocate[T](Int32 length, AllocationOptions options)
   at SixLabors.ImageSharp.Memory.MemoryAllocatorExtensions.Allocate2D[T](MemoryAllocator memoryAllocator, Int32 width, Int32 height, AllocationOptions options)
   at SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder.JpegComponent.Init()
   at SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder.JpegFrame.InitComponents()
   at SixLabors.ImageSharp.Formats.Jpeg.JpegDecoderCore.ProcessStartOfFrameMarker(Int32 remaining, JpegFileMarker& frameMarker, Boolean metadataOnly)
   at SixLabors.ImageSharp.Formats.Jpeg.JpegDecoderCore.ParseStream(Stream stream, Boolean metadataOnly)
   at SixLabors.ImageSharp.Formats.Jpeg.JpegDecoderCore.Decode[TPixel](Stream stream)
   at SixLabors.ImageSharp.Formats.Jpeg.JpegDecoder.Decode[TPixel](Configuration configuration, Stream stream)
   at SixLabors.ImageSharp.Image.Decode[TPixel](Stream stream, Configuration config)
   at SixLabors.ImageSharp.Image.Load[TPixel](Configuration config, Stream stream, IImageFormat& format)
   at SixLabors.ImageSharp.Image.Load[TPixel](Configuration config, String path)
   at dotnet_sample.Program.Main(String[] args) in /home/sinan/tmp/dotnet-sample/Program.cs:line 15

I believe the size of the image does not hit to 2GB or 4GB buffer limits which are mentioned in some other issues.

Steps to Reproduce

  • Open GIMP and create an image in 1024x36000 width x height.
  • Export as jpeg.
  • Try to load the jpeg via Image.Load.

System Configuration

  • ImageSharp version: 1.0.0-dev002315
  • Other ImageSharp packages and versions: SixLabors.Core: 1.0.0-dev000096
  • Environment (Operating system, version and so on): Ubuntu 18.04
  • .NET Framework version: 2.1 (also tried with 2.2)
  • Additional information:
    Test file: sample.zip
@JimBobSquarePants
Copy link
Member

JimBobSquarePants commented Jan 8, 2019

Hi @sardok

1024x36000

That's not a moderate image, that's a massive one!! Can we have it for testing against please?

I believe the size of the image does not hit to 2GB or 4GB buffer limits which are mentioned in some other issues.

When we see these errors it's actually due to multiplication, I'll try to explain.

We request everything from a single pool as bytes to prevent having to maintain multiple pools. This means that we request the struct as bytes and then cast to the appropriate layout. Unfortunately this means when processing large collections of larger structs we can make requests over the maximum allowable length.

We're going to need to have a look at specific areas of the library. I've identified them previously as jpeg decoding and image resizing. @antonfirsov has already done some work to reduce the overheads in resizing but we'll have to see what we can do in the decoder also.

@sardok
Copy link
Author

sardok commented Jan 9, 2019

Thanks @JimBobSquarePants for clarification. I've added a test file.

@JimBobSquarePants
Copy link
Member

@sardok Excellent. Thanks!

@HughPH
Copy link

HughPH commented Apr 1, 2019

I'm getting the same when trying to create a massive image. My image is 65536 x 55296

Failing on new Image<Rgb24>(Width, Height);

Version 1.0.0-beta0006
Running on Ubuntu 19.04 and targeting .NETCoreApp 3 (I'm adventurous)

@antonfirsov
Copy link
Member

@HughPH current theoretical image size limit is sqrt(int.MaxValue / sizeof(TPixel) ) x sqrt(int.MaxValue / sizeof(TPixel) ).

For TPixel = Rgb24 (sizeof(Rgb24) = 3) it's around 26754 x 26754. However if you try to resize that image the pool will be exceeded, and the same exception will be raised.

@HughPH the size of that image is 11 GB in memory. I wonder what is your use case? (Including execution environment / HW) Do you really need to handle those monsters in-memory?

@JimBobSquarePants
Copy link
Member

@HughPH Adventurous indeed!

Yeah, you'll get that. System.Drawing can't even open that file, only WIC can.

https://twitter.com/rickbrewPDN/status/1112135533348552704

@HughPH
Copy link

HughPH commented Apr 2, 2019

Thanks both!

Actually WIC can't quite reach the size I wanted to get to, but it's probably close enough. 10 years ago I rendered a Buddhabrot at 20480 x 17280 so I wanted to go a bit bigger this time. I'm also not sure WIC is going to be supported on Linux...

I guess I could render it to tiles. It just makes the image a bit more unwieldy... (The printers struggled to get the original to load, but they did manage it in the end, and it's still very detailed on a 4ft x 3ft canvas.)

@JimBobSquarePants
Copy link
Member

I'm also not sure WIC is going to be supported on Linux...

Nah forever windows I'm afraid. Tied intrinsically to the OS. I'd love to one day be able to support full resolution images, we're limited by .NET itself as far as I see as it is though.

@HughPH
Copy link

HughPH commented Apr 3, 2019

You can create an array up to 2bn x 2bn regardless of the size of the elements. Then it's just a case of encoding that data. Bitmap is a piece of cake. If the PDN statement is correct, PNG has a builtin limitation, although I'd like to explore that for myself. The "only" challenges then are in coding custom encoders. And then it's likely you could find implementations that can be tweaked for the extra resolution. .net Core 3 has intrinsics that will help speed up stuff like Vector4, and System.Math already goes direct to the FPU where it can.

@JimBobSquarePants
Copy link
Member

JimBobSquarePants commented Apr 3, 2019

@HughPH

Png is limited to in.MaxValue in either dimension as the physical dimensions are stored in 4 bytes.

We're currently limited by the 2GB size limit of Span<T> and Memory<T>. If we could work around that and use an ArrayPool<long> instead of byte then yes we could theoretically work with very large files.

How feasible any of that is I don't know.

Ping @antonfirsov

Edit.. Reading up it looks like it's impossible for now.

@HughPH
Copy link

HughPH commented Apr 3, 2019

So other than speed, why do you need to use Span and/or Memory? And why would you need to use ArrayPool other than possible LOH/GC hangs? For the special case of Incredibly Large Images (well over my now-modest-looking 65536 pixels) one would imagine the code running in an environment well endowed with memory. Maybe it can be "solved" by checking the size, and over certain sizes (say, 4GB) raise an exception to the user explaining the issues and risks, and providing another (Obsolete-marked) method or method override for that special case?

@JimBobSquarePants
Copy link
Member

Well I'll be damned! Span and Memory are limited to 2Bn elements not 2GB!

https://twitter.com/ben_a_adams/status/1113418885376741377

That means my ArrayPool<long> idea might just work!

P.S zero point even attempting a library like this without slicing. It would be so inefficient memory-wise that it would be unusable.

@HughPH
Copy link

HughPH commented Apr 3, 2019

By "slicing" you refer to slicing up the image in memory? I was also thinking about the slicing of the encoded image - making it possible to save a 65k squared PNG is a bit of a pointless exercise if nothing else on the planet can actually load it... (Krita crashed trying to create an empty image of my original size, let alone saving it.)

@JimBobSquarePants
Copy link
Member

I mean slicing buffers and operating on those slices. Just thought of a flaw in my plan... alignment!

@antonfirsov
Copy link
Member

antonfirsov commented Apr 3, 2019

ReadOnlySequence<T> is the new standard solution to achieve this "slicing". [EDIT: We need to define our own primitive.] May result in breaking API changes however. Not in scope for 1.0, but it should be definitely on the roadmap!

We need to throw more meaningful exceptions though. It also might be possible to extend the current theoretical limits using some unsafe magic.

@HughPH
Copy link

HughPH commented Apr 3, 2019

I'm a big fan of meaningful exceptions, especially ones that offer a solution.

@JimBobSquarePants
Copy link
Member

ReadOnlySequence<T> is the new standard solution to achieve this "slicing". May result in breaking API changes however. Not in scope for 1.0, but it should be definitely on the roadmap!

if we can get this in the roadmap for V1 It would be really good. I'd hate to have to break the APIs in the future.

@antonfirsov
Copy link
Member

I think we can make it. It would be much easier to experiment when #1002 is done.

@JimBobSquarePants JimBobSquarePants modified the milestones: Future, 1.0.0 Oct 25, 2019
@JimBobSquarePants
Copy link
Member

JimBobSquarePants commented Jan 8, 2020

@antonfirsov

If we don't get the opportunity to use ReadOnlySequence<T> I propose moving our backing store to int or long and dropping the IManagedByteBuffer.Array property in favor of polyfilling Read/Write with Span<byte> for streams.

The polyfills are easy enough to do. source and we're patching one already.

This would be fairly trivial to implement and can be done when we merge Core into ImageSharp.

@antonfirsov
Copy link
Member

It turns out I terribly misunderstood the purpose of ReadOnlySequence<T> 😅 We need to define our own primitive, since there is no standard solution for what we need as far as I'm aware. The primitive type itself doesn't seem too hard actually, the consequences of the integration process are more difficult.

I propose moving our backing store to int or long [...]

I'm afraid it won't help too much, apart from replacing ArgumentOutOfRangeException with OOM on many systems.

We should probably fix the unexpected/undocumented ArgumentOutOfRangeException first, close this issue, and continue discussion on #898 for the rest.

@antonfirsov antonfirsov modified the milestones: 1.0.0, 1.0.0-rc1 Jan 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants