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

TGA: RLE encoding #2056

Merged
merged 15 commits into from
Nov 26, 2023
Merged

TGA: RLE encoding #2056

merged 15 commits into from
Nov 26, 2023

Conversation

marvin-j97
Copy link
Contributor

@marvin-j97 marvin-j97 commented Nov 13, 2023

I license past and future contributions under the dual MIT/Apache-2.0 license,
allowing licensees to choose either at their option.


This is my first pass of RLE encoding support for TGA encoding. It currently supports ImageType::RunTrueColor and ImageType::RunGrayScale.

Here's a run down:

  • Modified Header::from_pixel_info to support an use_rle flag and set image_type accordingly
  • Add use_rle to enable RLE on a TgaEncoder
  • TgaEncoder::new defaults to rle = false
  • Add TgaEncoder::write_rle_encoded_packet to write a run-length encoded packet (high bit = 1) to a writer
  • Add TgaEncoder::run_length_encode which does the actual run-length encoding
  • Modified TgaEncoder::encode to check for the RLE flag and then use RLE
  • Moved old encoding tests to a test submodule, and added the equivalent RLE encoding tests
  • Added a test to check for saturating the run-length encoding counter (round_trip_bw), I added a test file inside the tests/ folder, I hope that's okay

Considerations

RunColorMap falls back to uncompressed encoding... should this throw an Unsupported error, because it does not fulfill user expectations?

src/codecs/tga/encoder.rs Outdated Show resolved Hide resolved
@fintelia
Copy link
Contributor

Looking at this code, I wonder whether it would make sense to do a prepass over the image to determine whether RLE compression would produce a smaller output than storing the pixels uncompressed. With that logic implemented, it might even make sense to not even expose the use_rel switch and just automatically compress if it would help

@marvin-j97
Copy link
Contributor Author

Looking at this code, I wonder whether it would make sense to do a prepass over the image to determine whether RLE compression would produce a smaller output than storing the pixels uncompressed. With that logic implemented, it might even make sense to not even expose the use_rel switch and just automatically compress if it would help

Even in run-length encoded TGA images there can be packets without RLE (high bit = 0).
So you can have a mix of run-length encoded packets and raw packets.
That would require some lookahead or buffer, basically as long as pixels are different, try to put them in raw packets, and when pixels are repeating, write a run-length packet instead.
Like the RLE packet, the raw packet can have 128 items max, so after 128 different pixels, it would need to be spilled to the writer and then continue with a new packet.

Could look like this:

[RLE packet header with counter = 4] PIXEL
[Raw packet header with counter = 50] PIXEL 1, PIXEL 2, ..., PIXEL 50

@fintelia
Copy link
Contributor

I think we should probably add raw packets then. Having your RGB image 0.3% larger because you enabled compression is probably OK, while having it become 33% larger is more frustrating

@marvin-j97
Copy link
Contributor Author

marvin-j97 commented Nov 15, 2023

Got it working, I think. My (very noisy) test image now only goes to 8.8 MB, instead of 10.8 MB.
If you want, RLE could become default, that would eliminate the old encoding code completely, and use_rle could be removed. Plus half the tests can be removed.

@fintelia
Copy link
Contributor

Could you make RLE the default but instead of removing uncompressed encoding, swap the builder method to be disable_rle? That way if anyone needs uncompressed they still have a way to get it, but calling DynamicImage::save_with_format will produce compressed TGAs.

@marvin-j97
Copy link
Contributor Author

This should be it!

I added another test to make sure RLE is used by default, and that disabling it increases the encoded size of an embarrassingly compressible example image.

src/codecs/tga/encoder.rs Show resolved Hide resolved
@fintelia fintelia merged commit 483d3b9 into image-rs:master Nov 26, 2023
35 checks passed
@marvin-j97 marvin-j97 deleted the feat/tga/rle-encoding branch November 26, 2023 22:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants