@eapache - I don't know if you are interested in having these changes upstream?

Hmm, that's a surprisingly good question :)

It sounds like there are some bug fixes and some feature improvements mixed together here? Though it's not clear to me at a glance whether the bug fixes are just side-effects of the switch to S2, or whether the framing code in xerial.go actually changed meaningfully as well?

The last time I touched the xerial-snappy package was January, ineapache/go-xerial-snappy#9 -@dnwe and I had a chat in that PR (which you are welcome to read for context) about the future of the package and its primary user (https://github.com/IBM/sarama/), given I'm not really involved in any of the related projects anymore.

klauspost/compress certainly seems like a much more active and maintained project, so if you want to subsume the xerial decoder then I would be quite happy to EOL/archiveeapache/go-xerial-snappy and redirect folks to this package instead. But that puts some onus on@dnwe to migrate Sarama over to this package - I think that seems reasonable but really that's up to him. (I'm assuming based on the docs/readme here that it would be mostly a drop-in replacement, but I am coming at this with very little context).

To answer the original question though - I'm certainly happy to accept small bug fixes and improvements upstream if you'd like to contribute them there instead / as well / depending on the discussion with Dom. Swapping the underlying library for S2 is not something that I would feel comfortable accepting just because it's a major change and I don't have the setup or time to verify for myself that it actually maintains compatibility, wouldn't break Sarama, etc.

@eapache I can send the fixes for the decoding edge cases. That'll give me an excuse to add the test cases outside of the fuzz tests.

I understand that you aren't looking to test replacing the encoder - I had a hunch about that, which is why I forked it. It is of course safe, but I wouldn't take such a change without spending time investigating myself.

Yes, it would be drop-in, exceptEncodeStream is now justEncode. Nothing in the API functionality changed.

Sidenote: It took me a while to understand how the "append" mechanic of EncodeStream works. I think I will add some more documentation on that.

I will send a PR with the fixes ASAP. You can then decide on what the fate of your package will be :)