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.

Sign up for GitHub

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

Undo horizontal prediction for each tile row in case of tiled tiff's #2878

Open
wants to merge 19 commits into
base: main
Choose a base branch
from
Open

Undo horizontal prediction for each tile row in case of tiled tiff's #2878

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
f72be91
Undo horizontal prediction for each tile row in case of tiled tiff's
brianpopow Feb 2, 2025
e186c52
Add test case for tiled tiff, deflate compressed with predictor
brianpopow Feb 3, 2025
787e04d
Add test case for tiled tiff, deflate compressed with predictor and c…
brianpopow Feb 4, 2025
e3c7471
Add test case for tiled gray tiff, deflate compressed with predictor
brianpopow Feb 6, 2025
f11fbc1
Add test cases for tiled 16 bit gray tiff's
brianpopow Feb 8, 2025
47c6755
Add test cases for tiled 32 bit gray tiff's
brianpopow Feb 9, 2025
97133fe
Add test cases for tiled tiff, deflate compressed with predictor and …
brianpopow Feb 9, 2025
f804ca2
Add test cases for tiled tiff, deflate compressed with predictor and …
brianpopow Feb 9, 2025
dacb713
Add test cases for tiled tiff, deflate compressed with predictor and …
brianpopow Feb 10, 2025
dec1dc1
Add test cases for tiled tiff, deflate compressed with predictor and …
brianpopow Feb 11, 2025
9954f50
Merge remote-tracking branch 'origin/main' into bp/Issue2877
brianpopow Feb 14, 2025
71f7d86
Add method UndoTile in HorizontalPredictor
brianpopow Feb 19, 2025
5cfa2bc
Enable 32 bits per channel decoding for MagickReferenceDecoder
brianpopow Feb 19, 2025
0a52ee7
Try fix build issue with ubuntu latest and net9.0
brianpopow Feb 20, 2025
7a5be72
Add test cases for tiled lzw compressed images
brianpopow Feb 20, 2025
8f97280
Add tiled rgba 16 bit each channel test files
brianpopow Feb 20, 2025
18f8de9
Do not ignore tileHeight when undoing horizontal predictor
brianpopow Feb 22, 2025
23cfdd2
Better test images for tiled tiff: tile width and height is not the same
brianpopow Feb 23, 2025
85c7ed2
Merge branch 'main' into bp/Issue2877
JimBobSquarePants Apr 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions src/ImageSharp/Formats/Tiff/Compression/Decompressors/DeflateTiffCompression.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ internal sealed class DeflateTiffCompression : TiffBaseDecompressor

private readonly TiffColorType colorType;

private readonly bool isTiled;

/// <summary>
/// Initializes a new instance of the <see cref="DeflateTiffCompression" /> class.
/// </summary>
Expand All @@ -31,11 +33,13 @@ internal sealed class DeflateTiffCompression : TiffBaseDecompressor
/// <param name="colorType">The color type of the pixel data.</param>
/// <param name="predictor">The tiff predictor used.</param>
/// <param name="isBigEndian">if set to <c>true</c> decodes the pixel data as big endian, otherwise as little endian.</param>
public DeflateTiffCompression(MemoryAllocator memoryAllocator, int width, int bitsPerPixel, TiffColorType colorType, TiffPredictor predictor, bool isBigEndian)
/// <param name="isTiled">Flag indicates, if the image is a tiled image.</param>
public DeflateTiffCompression(MemoryAllocator memoryAllocator, int width, int bitsPerPixel, TiffColorType colorType, TiffPredictor predictor, bool isBigEndian, bool isTiled)
: base(memoryAllocator, width, bitsPerPixel, predictor)
{
this.colorType = colorType;
this.isBigEndian = isBigEndian;
this.isTiled = isTiled;
}

/// <inheritdoc/>
Expand Down Expand Up @@ -68,7 +72,8 @@ protected override void Decompress(BufferedReadStream stream, int byteCount, int
}
}

if (this.Predictor == TiffPredictor.Horizontal)
// When the image is tiled, undoing the horizontal predictor will be done for each tile row in the DecodeTilesChunky() method.
if (this.Predictor == TiffPredictor.Horizontal && !this.isTiled)
{
HorizontalPredictor.Undo(buffer, this.Width, this.colorType, this.isBigEndian);
}
Expand Down
91 changes: 60 additions & 31 deletions src/ImageSharp/Formats/Tiff/Compression/HorizontalPredictor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,22 @@ public static void Undo(Span<byte> pixelBytes, int width, TiffColorType colorTyp
}
}

public static void UndoRow(Span<byte> pixelBytes, int width, int y, TiffColorType colorType)
{
// TODO: Implement missing colortypes, see above.
switch (colorType)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this switch not happen outside the loop?

{
case TiffColorType.Rgb888:
case TiffColorType.CieLab:
UndoRgb24BitRow(pixelBytes, width, y);
break;
case TiffColorType.Rgba8888:
case TiffColorType.Cmyk:
UndoRgba32BitRow(pixelBytes, width, y);
break;
}
}

public static void ApplyHorizontalPrediction(Span<byte> rows, int width, int bitsPerPixel)
{
if (bitsPerPixel == 8)
Expand Down Expand Up @@ -257,27 +273,56 @@ private static void UndoGray32Bit(Span<byte> pixelBytes, int width, bool isBigEn
}
}

private static void UndoRgb24BitRow(Span<byte> pixelBytes, int width, int y)
{
int rowBytesCount = width * 3;
Span<byte> rowBytes = pixelBytes.Slice(y * rowBytesCount, rowBytesCount);
Span<Rgb24> rowRgb = MemoryMarshal.Cast<byte, Rgb24>(rowBytes)[..width];
ref Rgb24 rowRgbBase = ref MemoryMarshal.GetReference(rowRgb);
byte r = rowRgbBase.R;
byte g = rowRgbBase.G;
byte b = rowRgbBase.B;

for (int x = 1; x < rowRgb.Length; x++)
{
ref Rgb24 pixel = ref rowRgb[x];
r += pixel.R;
g += pixel.G;
b += pixel.B;
pixel = new Rgb24(r, g, b);
}
}

private static void UndoRgb24Bit(Span<byte> pixelBytes, int width)
{
int rowBytesCount = width * 3;
int height = pixelBytes.Length / rowBytesCount;
for (int y = 0; y < height; y++)
{
Span<byte> rowBytes = pixelBytes.Slice(y * rowBytesCount, rowBytesCount);
Span<Rgb24> rowRgb = MemoryMarshal.Cast<byte, Rgb24>(rowBytes)[..width];
ref Rgb24 rowRgbBase = ref MemoryMarshal.GetReference(rowRgb);
byte r = rowRgbBase.R;
byte g = rowRgbBase.G;
byte b = rowRgbBase.B;
UndoRgb24BitRow(pixelBytes, width, y);
}
}

for (int x = 1; x < rowRgb.Length; x++)
{
ref Rgb24 pixel = ref rowRgb[x];
r += pixel.R;
g += pixel.G;
b += pixel.B;
pixel = new Rgb24(r, g, b);
}
private static void UndoRgba32BitRow(Span<byte> pixelBytes, int width, int y)
{
int rowBytesCount = width * 4;

Span<byte> rowBytes = pixelBytes.Slice(y * rowBytesCount, rowBytesCount);
Span<Rgba32> rowRgb = MemoryMarshal.Cast<byte, Rgba32>(rowBytes)[..width];
ref Rgba32 rowRgbBase = ref MemoryMarshal.GetReference(rowRgb);
byte r = rowRgbBase.R;
byte g = rowRgbBase.G;
byte b = rowRgbBase.B;
byte a = rowRgbBase.A;

for (int x = 1; x < rowRgb.Length; x++)
{
ref Rgba32 pixel = ref rowRgb[x];
r += pixel.R;
g += pixel.G;
b += pixel.B;
a += pixel.A;
pixel = new Rgba32(r, g, b, a);
}
}

Expand All @@ -287,23 +332,7 @@ private static void UndoRgba32Bit(Span<byte> pixelBytes, int width)
int height = pixelBytes.Length / rowBytesCount;
for (int y = 0; y < height; y++)
{
Span<byte> rowBytes = pixelBytes.Slice(y * rowBytesCount, rowBytesCount);
Span<Rgba32> rowRgb = MemoryMarshal.Cast<byte, Rgba32>(rowBytes)[..width];
ref Rgba32 rowRgbBase = ref MemoryMarshal.GetReference(rowRgb);
byte r = rowRgbBase.R;
byte g = rowRgbBase.G;
byte b = rowRgbBase.B;
byte a = rowRgbBase.A;

for (int x = 1; x < rowRgb.Length; x++)
{
ref Rgba32 pixel = ref rowRgb[x];
r += pixel.R;
g += pixel.G;
b += pixel.B;
a += pixel.A;
pixel = new Rgba32(r, g, b, a);
}
UndoRgba32BitRow(pixelBytes, width, y);
}
}

Expand Down
5 changes: 3 additions & 2 deletions src/ImageSharp/Formats/Tiff/Compression/TiffDecompressorsFactory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ public static TiffBaseDecompressor Create(
byte[] jpegTables,
uint oldJpegStartOfImageMarker,
TiffFillOrder fillOrder,
ByteOrder byteOrder)
ByteOrder byteOrder,
bool isTiled = false)
{
switch (method)
{
Expand All @@ -40,7 +41,7 @@ public static TiffBaseDecompressor Create(

case TiffDecoderCompressionType.Deflate:
DebugGuard.IsTrue(faxOptions == FaxCompressionOptions.None, "No fax compression options are expected");
return new DeflateTiffCompression(allocator, width, bitsPerPixel, colorType, predictor, byteOrder == ByteOrder.BigEndian);
return new DeflateTiffCompression(allocator, width, bitsPerPixel, colorType, predictor, byteOrder == ByteOrder.BigEndian, isTiled);

case TiffDecoderCompressionType.Lzw:
DebugGuard.IsTrue(faxOptions == FaxCompressionOptions.None, "No fax compression options are expected");
Expand Down
15 changes: 12 additions & 3 deletions src/ImageSharp/Formats/Tiff/TiffDecoderCore.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -683,7 +683,8 @@ private void DecodeTilesChunky<TPixel>(
Span<byte> tileBufferSpan = tileBuffer.GetSpan();
Span<byte> uncompressedPixelBufferSpan = uncompressedPixelBuffer.GetSpan();

using TiffBaseDecompressor decompressor = this.CreateDecompressor<TPixel>(frame.Width, bitsPerPixel);
bool isTiled = true;
using TiffBaseDecompressor decompressor = this.CreateDecompressor<TPixel>(frame.Width, bitsPerPixel, isTiled);
TiffBaseColorDecoder<TPixel> colorDecoder = this.CreateChunkyColorDecoder<TPixel>();

int tileIndex = 0;
Expand Down Expand Up @@ -712,6 +713,13 @@ private void DecodeTilesChunky<TPixel>(
{
Span<byte> uncompressedPixelRow = uncompressedPixelBufferSpan.Slice(uncompressedPixelBufferOffset, bytesToCopy);
tileBufferSpan.Slice(tileBufferOffset, bytesToCopy).CopyTo(uncompressedPixelRow);

// Undo the horziontal predictor for each tile row.
if (this.Predictor == TiffPredictor.Horizontal)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JimBobSquarePants I am not sure, if this is the best approach to undo the predictor here. I tried to do this in HorizontalPredictor.Undo(), but could not figure out a way without replicating the logic to iterate over the tiles in DecodeTilesChunky(). Maybe you have a better idea howto do this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, are you writing the row, then undoing the write?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It currently works like this:

  1. For each Tile: Decompress tile data
  2. For each row in tile: Undo horizontal predictor
  3. When done with all tiles: decode color

The horizontal predictor is used with lzw and deflate compression to achieve better compression. It assumes that neighboring pixel values do not change much and therefore only stores the difference between two consecutive pixels instead the actual pixel values.

Because it is tight to lzw and deflate compression, it feels to me this should be the concern of the decompressor and not be handled outside (which is the case for none tiled images).

I will try to gather more tiled image test cases for all the different color formats and then see if I can figure out a way how I can move the undoing of the predictor inside the uncompressing method.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I understand.

I'm just trying to read through TiffLibrary and LibTiff.NET to see how it works there as a reference. I agree the decompressor feels like it should be responsible though.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have moved undoing the predictor for tiles into the HorizontalPredictor class now with UndoTile(). Maybe this is better now. I am still open for any better suggestions.

I think it is ready for review. I have gathered as much different example files of tiled tiff I could.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @brianpopow I'll get on it ASAP

{
HorizontalPredictor.UndoRow(uncompressedPixelRow, tileLength, 0, this.ColorType);
}

tileBufferOffset += bytesPerTileRow;
uncompressedPixelBufferOffset += bytesPerRow;
}
Expand Down Expand Up @@ -750,7 +758,7 @@ private TiffBasePlanarColorDecoder<TPixel> CreatePlanarColorDecoder<TPixel>()
this.YcbcrSubSampling,
this.byteOrder);

private TiffBaseDecompressor CreateDecompressor<TPixel>(int frameWidth, int bitsPerPixel)
private TiffBaseDecompressor CreateDecompressor<TPixel>(int frameWidth, int bitsPerPixel, bool isTiled = false)
where TPixel : unmanaged, IPixel<TPixel> =>
TiffDecompressorsFactory.Create(
this.Options,
Expand All @@ -765,7 +773,8 @@ private TiffBaseDecompressor CreateDecompressor<TPixel>(int frameWidth, int bits
this.JpegTables,
this.OldJpegCompressionStartOfImageMarker.GetValueOrDefault(),
this.FillOrder,
this.byteOrder);
this.byteOrder,
isTiled);

private IMemoryOwner<ulong> ConvertNumbers(Array array, out Span<ulong> span)
{
Expand Down
2 changes: 1 addition & 1 deletion tests/ImageSharp.Tests/Formats/Tiff/Compression/DeflateTiffCompressionTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public void Compress_Decompress_Roundtrip_Works(byte[] data)
using BufferedReadStream stream = CreateCompressedStream(data);
byte[] buffer = new byte[data.Length];

using var decompressor = new DeflateTiffCompression(Configuration.Default.MemoryAllocator, 10, 8, TiffColorType.BlackIsZero8, TiffPredictor.None, false);
using var decompressor = new DeflateTiffCompression(Configuration.Default.MemoryAllocator, 10, 8, TiffColorType.BlackIsZero8, TiffPredictor.None, false, false);

decompressor.Decompress(stream, 0, (uint)stream.Length, 1, buffer, default);

Expand Down
Loading