Browse Source

Fix 16-bit binary PBM sample byte order

The binary PGM and PPM formats store 16-bit samples most significant
byte first. The decoder and encoder read and wrote them in native
little-endian order, so every wide sample was byte swapped. This
corrects the swizzled colors reported for 16-bit PPM images.

- Reverse the sample byte order in BinaryDecoder and BinaryEncoder.
- Fix integer division in the max pixel value upscale factor.
- Add exact-value, reference-decoder, upscale, and round-trip tests.
- Add a 16-bit PPM test image and its Magick-anchored reference PNG.
- Document the binary decoder and encoder in full.
pull/3164/head
James Jackson-South 6 days ago
parent
commit
8ca712019e
  1. 88
      src/ImageSharp/Formats/Pbm/BinaryDecoder.cs
  2. 77
      src/ImageSharp/Formats/Pbm/BinaryEncoder.cs
  3. 6
      src/ImageSharp/Formats/Pbm/PbmDecoderCore.cs
  4. 62
      tests/ImageSharp.Tests/Formats/Pbm/PbmDecoderTests.cs
  5. 40
      tests/ImageSharp.Tests/Formats/Pbm/PbmEncoderTests.cs
  6. 34
      tests/ImageSharp.Tests/Formats/Pbm/PbmRoundTripTests.cs
  7. 1
      tests/ImageSharp.Tests/TestImages.cs
  8. 3
      tests/Images/External/ReferenceOutput/PbmDecoderTests/DecodeReferenceImage_Rgb48_rgb_binary_wide.png
  9. 3
      tests/Images/Input/Pbm/rgb_binary_wide.ppm

88
src/ImageSharp/Formats/Pbm/BinaryDecoder.cs

@ -2,6 +2,8 @@
// Licensed under the Six Labors Split License.
using System.Buffers;
using System.Buffers.Binary;
using System.Runtime.InteropServices;
using SixLabors.ImageSharp.IO;
using SixLabors.ImageSharp.Memory;
using SixLabors.ImageSharp.PixelFormats;
@ -13,21 +15,25 @@ namespace SixLabors.ImageSharp.Formats.Pbm;
/// </summary>
internal class BinaryDecoder
{
/// <summary>
/// The luminance value written for an unset bit in the black and white format.
/// </summary>
private static L8 white = new(255);
/// <summary>
/// The luminance value written for a set bit in the black and white format.
/// </summary>
private static L8 black = new(0);
/// <summary>
/// Decode the specified pixels.
/// </summary>
/// <typeparam name="TPixel">The type of pixel to encode to.</typeparam>
/// <typeparam name="TPixel">The type of pixel to decode to.</typeparam>
/// <param name="configuration">The configuration.</param>
/// <param name="pixels">The pixel array to encode into.</param>
/// <param name="pixels">The pixel buffer to decode into.</param>
/// <param name="stream">The stream to read the data from.</param>
/// <param name="colorType">The ColorType to decode.</param>
/// <param name="componentType">Data type of the pixles components.</param>
/// <exception cref="InvalidImageContentException">
/// Thrown if an invalid combination of setting is requested.
/// </exception>
/// <param name="colorType">The color type of the encoded pixels.</param>
/// <param name="componentType">The data type of the pixel components.</param>
public static void Process<TPixel>(Configuration configuration, Buffer2D<TPixel> pixels, BufferedReadStream stream, PbmColorType colorType, PbmComponentType componentType)
where TPixel : unmanaged, IPixel<TPixel>
{
@ -59,6 +65,15 @@ internal class BinaryDecoder
}
}
/// <summary>
/// Decodes 8-bit binary grayscale (PGM) pixel data.
/// Each pixel is a single byte that holds its luminance value.
/// When the stream ends early, the rows that were not read keep their default value.
/// </summary>
/// <typeparam name="TPixel">The type of pixel to decode to.</typeparam>
/// <param name="configuration">The configuration.</param>
/// <param name="pixels">The pixel buffer to decode into.</param>
/// <param name="stream">The stream to read the data from.</param>
private static void ProcessGrayscale<TPixel>(Configuration configuration, Buffer2D<TPixel> pixels, BufferedReadStream stream)
where TPixel : unmanaged, IPixel<TPixel>
{
@ -85,6 +100,15 @@ internal class BinaryDecoder
}
}
/// <summary>
/// Decodes 16-bit binary grayscale (PGM) pixel data.
/// Each pixel is one 16-bit sample, stored most significant byte first.
/// When the stream ends early, the rows that were not read keep their default value.
/// </summary>
/// <typeparam name="TPixel">The type of pixel to decode to.</typeparam>
/// <param name="configuration">The configuration.</param>
/// <param name="pixels">The pixel buffer to decode into.</param>
/// <param name="stream">The stream to read the data from.</param>
private static void ProcessWideGrayscale<TPixel>(Configuration configuration, Buffer2D<TPixel> pixels, BufferedReadStream stream)
where TPixel : unmanaged, IPixel<TPixel>
{
@ -102,6 +126,10 @@ internal class BinaryDecoder
return;
}
// The binary format stores 16-bit samples most significant byte first,
// but L16 expects native (little-endian) byte order.
SwapSampleBytes(rowSpan);
Span<TPixel> pixelSpan = pixels.DangerousGetRowSpan(y);
PixelOperations<TPixel>.Instance.FromL16Bytes(
configuration,
@ -111,6 +139,15 @@ internal class BinaryDecoder
}
}
/// <summary>
/// Decodes 8-bit binary color (PPM) pixel data.
/// Each pixel is three bytes in red, green, blue order.
/// When the stream ends early, the rows that were not read keep their default value.
/// </summary>
/// <typeparam name="TPixel">The type of pixel to decode to.</typeparam>
/// <param name="configuration">The configuration.</param>
/// <param name="pixels">The pixel buffer to decode into.</param>
/// <param name="stream">The stream to read the data from.</param>
private static void ProcessRgb<TPixel>(Configuration configuration, Buffer2D<TPixel> pixels, BufferedReadStream stream)
where TPixel : unmanaged, IPixel<TPixel>
{
@ -137,6 +174,15 @@ internal class BinaryDecoder
}
}
/// <summary>
/// Decodes 16-bit binary color (PPM) pixel data.
/// Each pixel is three 16-bit samples in red, green, blue order, stored most significant byte first.
/// When the stream ends early, the rows that were not read keep their default value.
/// </summary>
/// <typeparam name="TPixel">The type of pixel to decode to.</typeparam>
/// <param name="configuration">The configuration.</param>
/// <param name="pixels">The pixel buffer to decode into.</param>
/// <param name="stream">The stream to read the data from.</param>
private static void ProcessWideRgb<TPixel>(Configuration configuration, Buffer2D<TPixel> pixels, BufferedReadStream stream)
where TPixel : unmanaged, IPixel<TPixel>
{
@ -154,6 +200,10 @@ internal class BinaryDecoder
return;
}
// The binary format stores 16-bit samples most significant byte first,
// but Rgb48 expects native (little-endian) byte order.
SwapSampleBytes(rowSpan);
Span<TPixel> pixelSpan = pixels.DangerousGetRowSpan(y);
PixelOperations<TPixel>.Instance.FromRgb48Bytes(
configuration,
@ -163,6 +213,30 @@ internal class BinaryDecoder
}
}
/// <summary>
/// Reverses the byte order of each 16-bit sample in the given row when the host is little-endian.
/// The binary PGM and PPM formats store multi-byte samples most significant byte first.
/// </summary>
/// <param name="rowSpan">The row of big-endian sample data to convert in place.</param>
private static void SwapSampleBytes(Span<byte> rowSpan)
{
if (BitConverter.IsLittleEndian)
{
Span<ushort> samples = MemoryMarshal.Cast<byte, ushort>(rowSpan);
BinaryPrimitives.ReverseEndianness(samples, samples);
}
}
/// <summary>
/// Decodes binary black and white (PBM) pixel data.
/// Each byte holds eight pixels, most significant bit first, and a set bit means black.
/// Each row starts on a byte boundary, so the last byte of a row can hold unused bits.
/// When the stream ends early, the pixels that were not read keep their default value.
/// </summary>
/// <typeparam name="TPixel">The type of pixel to decode to.</typeparam>
/// <param name="configuration">The configuration.</param>
/// <param name="pixels">The pixel buffer to decode into.</param>
/// <param name="stream">The stream to read the data from.</param>
private static void ProcessBlackAndWhite<TPixel>(Configuration configuration, Buffer2D<TPixel> pixels, BufferedReadStream stream)
where TPixel : unmanaged, IPixel<TPixel>
{

77
src/ImageSharp/Formats/Pbm/BinaryEncoder.cs

@ -2,6 +2,8 @@
// Licensed under the Six Labors Split License.
using System.Buffers;
using System.Buffers.Binary;
using System.Runtime.InteropServices;
using SixLabors.ImageSharp.Memory;
using SixLabors.ImageSharp.PixelFormats;
@ -13,14 +15,14 @@ namespace SixLabors.ImageSharp.Formats.Pbm;
internal class BinaryEncoder
{
/// <summary>
/// Decode pixels into the PBM binary encoding.
/// Encode pixels into the PBM binary encoding.
/// </summary>
/// <typeparam name="TPixel">The type of input pixel.</typeparam>
/// <param name="configuration">The configuration.</param>
/// <param name="stream">The byte stream to write to.</param>
/// <param name="image">The input image.</param>
/// <param name="colorType">The ColorType to use.</param>
/// <param name="componentType">Data type of the pixels components.</param>
/// <param name="colorType">The color type to use.</param>
/// <param name="componentType">The data type of the pixel components.</param>
/// <param name="cancellationToken">The token to monitor for cancellation requests.</param>
/// <exception cref="ImageFormatException">
/// Thrown if an invalid combination of setting is requested.
@ -70,6 +72,15 @@ internal class BinaryEncoder
}
}
/// <summary>
/// Encodes 8-bit binary grayscale (PGM) pixel data.
/// Each pixel is written as a single byte that holds its luminance value.
/// </summary>
/// <typeparam name="TPixel">The type of input pixel.</typeparam>
/// <param name="configuration">The configuration.</param>
/// <param name="stream">The byte stream to write to.</param>
/// <param name="image">The input image.</param>
/// <param name="cancellationToken">The token to monitor for cancellation requests.</param>
private static void WriteGrayscale<TPixel>(
Configuration configuration,
Stream stream,
@ -100,6 +111,15 @@ internal class BinaryEncoder
}
}
/// <summary>
/// Encodes 16-bit binary grayscale (PGM) pixel data.
/// Each pixel is written as one 16-bit sample, most significant byte first.
/// </summary>
/// <typeparam name="TPixel">The type of input pixel.</typeparam>
/// <param name="configuration">The configuration.</param>
/// <param name="stream">The byte stream to write to.</param>
/// <param name="image">The input image.</param>
/// <param name="cancellationToken">The token to monitor for cancellation requests.</param>
private static void WriteWideGrayscale<TPixel>(
Configuration configuration,
Stream stream,
@ -127,10 +147,23 @@ internal class BinaryEncoder
rowSpan,
width);
// The binary format stores 16-bit samples most significant byte first,
// but ToL16Bytes produces native (little-endian) byte order.
SwapSampleBytes(rowSpan);
stream.Write(rowSpan);
}
}
/// <summary>
/// Encodes 8-bit binary color (PPM) pixel data.
/// Each pixel is written as three bytes in red, green, blue order.
/// </summary>
/// <typeparam name="TPixel">The type of input pixel.</typeparam>
/// <param name="configuration">The configuration.</param>
/// <param name="stream">The byte stream to write to.</param>
/// <param name="image">The input image.</param>
/// <param name="cancellationToken">The token to monitor for cancellation requests.</param>
private static void WriteRgb<TPixel>(
Configuration configuration,
Stream stream,
@ -162,6 +195,15 @@ internal class BinaryEncoder
}
}
/// <summary>
/// Encodes 16-bit binary color (PPM) pixel data.
/// Each pixel is written as three 16-bit samples in red, green, blue order, most significant byte first.
/// </summary>
/// <typeparam name="TPixel">The type of input pixel.</typeparam>
/// <param name="configuration">The configuration.</param>
/// <param name="stream">The byte stream to write to.</param>
/// <param name="image">The input image.</param>
/// <param name="cancellationToken">The token to monitor for cancellation requests.</param>
private static void WriteWideRgb<TPixel>(
Configuration configuration,
Stream stream,
@ -189,10 +231,39 @@ internal class BinaryEncoder
rowSpan,
width);
// The binary format stores 16-bit samples most significant byte first,
// but ToRgb48Bytes produces native (little-endian) byte order.
SwapSampleBytes(rowSpan);
stream.Write(rowSpan);
}
}
/// <summary>
/// Reverses the byte order of each 16-bit sample in the given row when the host is little-endian.
/// The binary PGM and PPM formats store multi-byte samples most significant byte first.
/// </summary>
/// <param name="rowSpan">The row of native-endian sample data to convert in place.</param>
private static void SwapSampleBytes(Span<byte> rowSpan)
{
if (BitConverter.IsLittleEndian)
{
Span<ushort> samples = MemoryMarshal.Cast<byte, ushort>(rowSpan);
BinaryPrimitives.ReverseEndianness(samples, samples);
}
}
/// <summary>
/// Encodes binary black and white (PBM) pixel data.
/// Each byte holds eight pixels, most significant bit first, and a set bit means black.
/// A pixel with a luminance value less than 128 is written as black.
/// Each row starts on a byte boundary, so the last byte of a row can hold unused bits.
/// </summary>
/// <typeparam name="TPixel">The type of input pixel.</typeparam>
/// <param name="configuration">The configuration.</param>
/// <param name="stream">The byte stream to write to.</param>
/// <param name="image">The input image.</param>
/// <param name="cancellationToken">The token to monitor for cancellation requests.</param>
private static void WriteBlackAndWhite<TPixel>(
Configuration
configuration,

6
src/ImageSharp/Formats/Pbm/PbmDecoderCore.cs

@ -53,9 +53,7 @@ internal sealed class PbmDecoderCore : ImageDecoderCore
/// <param name="options">The decoder options.</param>
public PbmDecoderCore(DecoderOptions options)
: base(options)
{
this.configuration = options.Configuration;
}
=> this.configuration = options.Configuration;
/// <inheritdoc/>
protected override Image<TPixel> Decode<TPixel>(BufferedReadStream stream, CancellationToken cancellationToken)
@ -205,7 +203,7 @@ internal sealed class PbmDecoderCore : ImageDecoderCore
where TPixel : unmanaged, IPixel<TPixel>
{
int maxAllocationValue = this.componentType == PbmComponentType.Short ? 65535 : 255;
float factor = maxAllocationValue / this.maxPixelValue;
float factor = maxAllocationValue / (float)this.maxPixelValue;
image.Mutate(x => x.Brightness(factor));
}

62
tests/ImageSharp.Tests/Formats/Pbm/PbmDecoderTests.cs

@ -7,6 +7,7 @@ using SixLabors.ImageSharp.Formats.Pbm;
using SixLabors.ImageSharp.PixelFormats;
using SixLabors.ImageSharp.Tests.TestUtilities;
using SixLabors.ImageSharp.Tests.TestUtilities.ImageComparison;
using SixLabors.ImageSharp.Tests.TestUtilities.ReferenceCodecs;
using static SixLabors.ImageSharp.Tests.TestImages.Pbm;
// ReSharper disable InconsistentNaming
@ -26,6 +27,7 @@ public class PbmDecoderTests
[InlineData(RgbPlain, PbmColorType.Rgb, PbmComponentType.Byte)]
[InlineData(RgbPlainMagick, PbmColorType.Rgb, PbmComponentType.Byte)]
[InlineData(RgbBinary, PbmColorType.Rgb, PbmComponentType.Byte)]
[InlineData(RgbBinaryWide, PbmColorType.Rgb, PbmComponentType.Short)]
public void ImageLoadCanDecode(string imagePath, PbmColorType expectedColorType, PbmComponentType expectedComponentType)
{
// Arrange
@ -91,6 +93,7 @@ public class PbmDecoderTests
[WithFile(RgbPlain, PixelTypes.Rgb24, "ppm")]
[WithFile(RgbPlainNormalized, PixelTypes.Rgb24, "ppm")]
[WithFile(RgbBinary, PixelTypes.Rgb24, "ppm")]
[WithFile(RgbBinaryWide, PixelTypes.Rgb48, "ppm")]
public void DecodeReferenceImage<TPixel>(TestImageProvider<TPixel> provider, string extension)
where TPixel : unmanaged, IPixel<TPixel>
{
@ -101,6 +104,65 @@ public class PbmDecoderTests
image.CompareToReferenceOutput(provider, grayscale: isGrayscale);
}
[Theory]
[WithFile(GrayscaleBinaryWide, PixelTypes.Rgb48)]
[WithFile(RgbBinaryWide, PixelTypes.Rgb48)]
public void Decode_WideBinary_MatchesReferenceDecoder<TPixel>(TestImageProvider<TPixel> provider)
where TPixel : unmanaged, IPixel<TPixel>
{
using Image<TPixel> image = provider.GetImage(PbmDecoder.Instance);
image.CompareToOriginal(provider, ImageComparer.Exact, new MagickReferenceDecoder(PbmFormat.Instance));
}
[Fact]
public void Decode_WideBinaryGrayscale_SamplesAreBigEndian()
{
// Per the Netpbm specification, 16-bit samples store the most significant byte first.
byte[] header = Encoding.ASCII.GetBytes("P5\n2 1\n65535\n");
byte[] samples = [0x80, 0x00, 0x00, 0x80];
byte[] data = [.. header, .. samples];
using Image<L16> image = Image.Load<L16>(data);
Assert.Equal(0x8000, image[0, 0].PackedValue);
Assert.Equal(0x0080, image[1, 0].PackedValue);
}
[Fact]
public void Decode_WideBinaryRgb_SamplesAreBigEndian()
{
// Per the Netpbm specification, 16-bit samples store the most significant byte first.
byte[] header = Encoding.ASCII.GetBytes("P6\n1 1\n65535\n");
byte[] samples = [0x81, 0xB5, 0x84, 0x91, 0x86, 0x71];
byte[] data = [.. header, .. samples];
using Image<Rgb48> image = Image.Load<Rgb48>(data);
Assert.Equal(new Rgb48(0x81B5, 0x8491, 0x8671), image[0, 0]);
}
[Fact]
public void Decode_NonStandardByteMaxPixelValue_UpscalesToFullRange()
{
byte[] data = Encoding.ASCII.GetBytes("P2\n1 1\n100\n100");
using Image<L8> image = Image.Load<L8>(data);
Assert.Equal(255, image[0, 0].PackedValue);
}
[Fact]
public void Decode_NonStandardShortMaxPixelValue_UpscalesToFullRange()
{
byte[] header = Encoding.ASCII.GetBytes("P5\n1 1\n1000\n");
byte[] samples = [0x03, 0xE8];
byte[] data = [.. header, .. samples];
using Image<L16> image = Image.Load<L16>(data);
Assert.Equal(65535, image[0, 0].PackedValue);
}
[Theory]
[WithFile(RgbPlain, PixelTypes.Rgb24)]
public void PbmDecoder_Decode_Resize<TPixel>(TestImageProvider<TPixel> provider)

40
tests/ImageSharp.Tests/Formats/Pbm/PbmEncoderTests.cs

@ -31,6 +31,7 @@ public class PbmEncoderTests
{ GrayscaleBinaryWide, PbmColorType.Grayscale },
{ GrayscalePlain, PbmColorType.Grayscale },
{ RgbBinary, PbmColorType.Rgb },
{ RgbBinaryWide, PbmColorType.Rgb },
{ RgbPlain, PbmColorType.Rgb },
};
@ -122,6 +123,45 @@ public class PbmEncoderTests
public void PbmEncoder_P6_Works<TPixel>(TestImageProvider<TPixel> provider)
where TPixel : unmanaged, IPixel<TPixel> => TestPbmEncoderCore(provider, PbmColorType.Rgb, PbmEncoding.Binary);
[Fact]
public void PbmEncoder_WideBinaryGrayscale_WritesBigEndianSamples()
{
// Per the Netpbm specification, 16-bit samples store the most significant byte first.
using Image<L16> image = new(2, 1);
image[0, 0] = new L16(0x8000);
image[1, 0] = new L16(0x1234);
using MemoryStream memStream = new();
image.Save(memStream, new PbmEncoder
{
ColorType = PbmColorType.Grayscale,
ComponentType = PbmComponentType.Short,
Encoding = PbmEncoding.Binary
});
byte[] encoded = memStream.ToArray();
Assert.Equal(new byte[] { 0x80, 0x00, 0x12, 0x34 }, encoded[^4..]);
}
[Fact]
public void PbmEncoder_WideBinaryRgb_WritesBigEndianSamples()
{
// Per the Netpbm specification, 16-bit samples store the most significant byte first.
using Image<Rgb48> image = new(1, 1);
image[0, 0] = new Rgb48(0x8000, 0x1234, 0x00FF);
using MemoryStream memStream = new();
image.Save(memStream, new PbmEncoder
{
ColorType = PbmColorType.Rgb,
ComponentType = PbmComponentType.Short,
Encoding = PbmEncoding.Binary
});
byte[] encoded = memStream.ToArray();
Assert.Equal(new byte[] { 0x80, 0x00, 0x12, 0x34, 0x00, 0xFF }, encoded[^6..]);
}
private static void TestPbmEncoderCore<TPixel>(
TestImageProvider<TPixel> provider,
PbmColorType colorType,

34
tests/ImageSharp.Tests/Formats/Pbm/PbmRoundTripTests.cs

@ -54,6 +54,40 @@ public class PbmRoundTripTests
ImageComparer.Exact.VerifySimilarity(originalImage, encodedImage);
}
[Theory]
[InlineData(GrayscaleBinaryWide)]
public void PbmWideGrayscaleImageCanRoundTrip(string imagePath)
{
// Arrange
TestFile testFile = TestFile.Create(imagePath);
using MemoryStream stream = new(testFile.Bytes, false);
// Act
using Image<L16> originalImage = Image.Load<L16>(stream);
using Image<L16> encodedImage = this.RoundTrip(originalImage);
// Assert
Assert.NotNull(encodedImage);
ImageComparer.Exact.VerifySimilarity(originalImage, encodedImage);
}
[Theory]
[InlineData(RgbBinaryWide)]
public void PbmWideColorImageCanRoundTrip(string imagePath)
{
// Arrange
TestFile testFile = TestFile.Create(imagePath);
using MemoryStream stream = new(testFile.Bytes, false);
// Act
using Image<Rgb48> originalImage = Image.Load<Rgb48>(stream);
using Image<Rgb48> encodedImage = this.RoundTrip(originalImage);
// Assert
Assert.NotNull(encodedImage);
ImageComparer.Exact.VerifySimilarity(originalImage, encodedImage);
}
private Image<TPixel> RoundTrip<TPixel>(Image<TPixel> originalImage)
where TPixel : unmanaged, IPixel<TPixel>
{

1
tests/ImageSharp.Tests/TestImages.cs

@ -1257,6 +1257,7 @@ public static class TestImages
public const string GrayscalePlainNormalized = "Pbm/grayscale_plain_normalized.pgm";
public const string GrayscalePlainMagick = "Pbm/grayscale_plain_magick.pgm";
public const string RgbBinary = "Pbm/00000_00000.ppm";
public const string RgbBinaryWide = "Pbm/rgb_binary_wide.ppm";
public const string RgbBinaryPrematureEof = "Pbm/00000_00000_premature_eof.ppm";
public const string RgbPlain = "Pbm/rgb_plain.ppm";
public const string RgbPlainNormalized = "Pbm/rgb_plain_normalized.ppm";

3
tests/Images/External/ReferenceOutput/PbmDecoderTests/DecodeReferenceImage_Rgb48_rgb_binary_wide.png

@ -0,0 +1,3 @@
version https://git-lfs.github.com/spec/v1
oid sha256:2bb443147c06e4d712421695f5cda1c9823c75ac8be7fbf6ce067c51d01445bf
size 5182

3
tests/Images/Input/Pbm/rgb_binary_wide.ppm

@ -0,0 +1,3 @@
version https://git-lfs.github.com/spec/v1
oid sha256:10cb6013ed7f17fd29b857189e864b11057fc34cdbf6d719c14470f7f2c34743
size 5235
Loading…
Cancel
Save