-
Notifications
You must be signed in to change notification settings - Fork 335
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
Color index array as FIFO #94
Comments
Using hash_pos during encode is a neat solution because AFAICT it's scalable, if encoder memory is very tight hash_pos can be some lower multiple of index size which only mildly decreases efficiency due to more collisions in hash_pos. Some higher multiple seems fine for normal use, and maximal compressors can always use linear search if they wish. Eliminating the hash entirely from the decode path seems very beneficial, IMO it's far more critical that the decode path be as simple as possible. |
On x86, I like the simpler and faster decoder and the option to trade encoding time for improved compression ratio. As I understand, FIFO can have better compression ratio for every possible input? Is your FIFO encoder slower? How much? |
No. Even with exhaustive search, some images encode slightly worse compared to master. I assume this is because it's beneficial for these images to refer to "older" pixels, further back then 64. (lfifo is linear search, hfifo is the current hash impl in the fifo-branch)
In the majority of cases, fifo compresses better, though. Sometimes substantially.
Totals:
As you can see, encoding is a bit faster with the fifo (hash impl) compared to master, for reasons I don't understand. |
Those numbers look good for the fifo approach and the source code only looks slightly more complicated. I do agree that it is somewhat harder to explain how it works. |
I think i'd still prefer the hash variant with 35711, as it's easier to implement in general and easier to explain. For performance comparison, one would need to implement a nice vectorized lookup function for the fifo, as this is something that can nicely be vectorized, so it might not have much impact on encoding speed. I personally would probably stay with the hash impl |
I think a "SIMD FIFO" encoder would be possible, with 64-entry Your only cons are implementation complexity, but QOI remains straightforward compared to JPEG, PNG and even GIF. |
After careful consideration, I have decided against this. It doesn't feel right. QOI emphasizes simplicity (and explaining how it works is part of that) so much everywhere else that it would be weird to sacrifice it here for slight gains in compression and throughput. This makes the current specification in master final. Thanks everyone! |
As proposed by @nigeltao and advocated for in #48, the fifo-branch implements the color index as a FIFO, instead of using a hash. It is up to implementers how to search through this FIFO in the encoder. The naive approach would be a linear search. The implementation in the fifo-branch uses a secondary hash table.
master:
fifo:
Pros:
Cons:
Compression ratio with the FIFO is only slightly better in total (even when using an exhaustive linear search). Slightly worse with some particular images.
I'm still undecided. In theory it's cool to not specify the hash function, but in practice we don't gain much here and (imho) make it more complicated. I'm curious to hear if there are any objections, specifically from other implementers. @pfusik, @MasterQ32?
The text was updated successfully, but these errors were encountered: