Conversation
# Conflicts: # src/ImageSharp/Configuration.cs
# Conflicts: # src/ImageSharp/Formats/Bmp/BmpConstants.cs # src/ImageSharp/Formats/Bmp/BmpDecoder.cs # src/ImageSharp/Formats/Bmp/BmpFormat.cs
# Conflicts: # src/ImageSharp/Configuration.cs # src/ImageSharp/Formats/Bmp/BmpArrayFileHeader.cs # src/ImageSharp/Formats/Bmp/BmpConstants.cs # src/ImageSharp/Formats/Bmp/BmpFormat.cs # src/ImageSharp/Formats/Bmp/BmpImageFormatDetector.cs # src/ImageSharp/Formats/ImageExtensions.Save.cs # src/ImageSharp/Formats/Pbm/BufferedReadStreamExtensions.cs # tests/ImageSharp.Tests/ConfigurationTests.cs # tests/ImageSharp.Tests/Formats/GeneralFormatTests.cs
# Conflicts: # src/ImageSharp/Configuration.cs # src/ImageSharp/Formats/Bmp/BmpArrayFileHeader.cs # src/ImageSharp/Formats/Bmp/BmpConstants.cs # src/ImageSharp/Formats/Bmp/BmpDecoder.cs # src/ImageSharp/Formats/Bmp/BmpDecoderCore.cs # src/ImageSharp/Formats/Bmp/BmpFileHeader.cs # src/ImageSharp/Formats/Bmp/BmpFormat.cs # src/ImageSharp/Formats/Bmp/BmpImageFormatDetector.cs # src/ImageSharp/Formats/Bmp/BmpInfoHeader.cs # src/ImageSharp/Formats/ImageDecoderUtilities.cs # src/ImageSharp/Formats/ImageExtensions.Save.cs # src/ImageSharp/Formats/Pbm/BufferedReadStreamExtensions.cs # src/ImageSharp/Image.Decode.cs # tests/ImageSharp.Tests/ConfigurationTests.cs # tests/ImageSharp.Tests/Formats/GeneralFormatTests.cs
# Conflicts: # src/ImageSharp/Formats/Bmp/BmpFormat.cs # src/ImageSharp/Formats/ImageDecoderUtilities.cs # src/ImageSharp/Formats/ImageExtensions.Save.cs # src/ImageSharp/Formats/OpenExr/ExrDecoderCore.cs
# Conflicts: # src/ImageSharp/Configuration.cs # src/ImageSharp/Formats/ImageDecoderUtilities.cs # tests/ImageSharp.Tests/TestUtilities/TestEnvironment.Formats.cs
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
src/ImageSharp/Formats/Exr/Compression/Decompressors/B44ExrCompression.cs
Fixed
Show fixed
Hide fixed
|
@brianpopow I'll review this asap. |
| /// <param name="component">The 32 bit component value.</param> | ||
| /// <returns>The <see cref="byte"/> value.</returns> | ||
| [MethodImpl(MethodImplOptions.AggressiveInlining)] | ||
| public static byte From32BitTo8Bit(uint component) => (byte)(component * Scale32Bit); |
There was a problem hiding this comment.
Perf is maybe not a primary concern at this point but I wonder if there is a cheaper way to correctly calculate this that avoids FP conversion? How does the reference implementation calculate this (if they care about decoding into lower res format)?
| public float ReadSingle(byte[] data) | ||
| { | ||
| int offset = 0; | ||
| int bytesToRead = 4; | ||
| while (bytesToRead > 0) | ||
| { | ||
| int bytesRead = this.Read(data, offset, bytesToRead); | ||
| if (bytesRead == 0) | ||
| { | ||
| throw new ImageFormatException("Not enough data to read a float value from the stream"); |
There was a problem hiding this comment.
BufferedReadStream doesn't expose such methods and it is especially odd to throw ImageFormatException here. Wouldn't it be better to keep this in the codec implementation?
There was a problem hiding this comment.
I see now that there are similar methods in the codec implementation, but they don't seem to iterate. Does it make sense to do it here? Doesn't the BufferedReadStream logic make it a very rare case that we can't read such small buffers immediately? What is our standard way of handling this in other codecs?
There was a problem hiding this comment.
We don't have any extensions on BufferedReadStream like this elsewhere. There was one attempted in this PR but I've already said no to that because it is dangerous.
Personally, I would avoid them because we then start introducing endianess.
|
|
||
| /// <inheritdoc/> | ||
| public override void Decompress(BufferedReadStream stream, uint compressedBytes, Span<byte> buffer) | ||
| => stream.Read(buffer, 0, Math.Min(buffer.Length, (int)this.BytesPerBlock)); |
There was a problem hiding this comment.
Is it safe to omit checking the return value here?
| stream.ReadByte(); | ||
| stream.ReadByte(); |
There was a problem hiding this comment.
Return value checks omitted here
| } | ||
|
|
||
| // Last byte should be a null byte. | ||
| stream.ReadByte(); |
| byte pLinear = (byte)stream.ReadByte(); | ||
|
|
||
| // Next 3 bytes are reserved bytes and not use. | ||
| stream.ReadByte(); | ||
| stream.ReadByte(); | ||
| stream.ReadByte(); |
There was a problem hiding this comment.
Also here. Would it make sense to read 4 bytes to this.buffer in this case?
3 participants
Prerequisites
Description
This PR adds support for decoding and encoding of images in the OpenEXR format. OpenEXR is a HDR image format, therefore I have added two new pixel formats
Rgb96andRgba128which store each color channel asUInt32.The specification can be found here: https://openexr.com/en/latest/index.html#openexr