-
Notifications
You must be signed in to change notification settings - Fork 86
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
Wrong decoding with edge-condition parameters #13
Comments
problem occurs also in @lrq3000's fork (rev 7583ed) |
@SupraSummus Thank you for the detailed feedback, yes the codec is optimized for speed so it might not have all the checks to ensure that user's input is correct (actually it is assumed that the user should do the input sanitization). I will see if adding a length check is bothersome for performance or not :-) Or at least add something in the docs about this edge case. Keep in mind that RSCodec.encode() is meant to be used in a loop, so the less input checks we do, the faster it is! |
I didn't mean proposing to add a parameter sanity check. The arguments I provided seem valid to me - they describe RS codec with single-byte chunk and no error correction symbols. In my comprehension such codec should behave as identity for decoding and encoding operations. ... or I missed domain assertions for RS coding and Thank you for attention. |
@SupraSummus Thank you very much, I see what you mean. With the latest release, here is what we get now: >>> c.decode(b'aaa')
(bytearray(b''), bytearray(b'aaa'), bytearray(b'')) Which means that the decoded message, which should just result in identity, is not considered as the message but as ecc symbols. This indeed looks weird, I should check inside the code, but I will probably have to dig back to the maths to understand what's happening here, but it's certainly very counterintuitive. |
Ok I think I understand what's happening here: if nsym is 0, the algo as implemented here will still try to get at least one ecc symbol, hence the input message is considered as the symbols and the message is considered empty, whereas you would expect the opposite. I'll see how to best fix this issue, whether in the code or if we put a warning. (TODO for me: add a unit test for this case) |
I think it's a bug, but I'm not 100% sure, because I'm playing with RS codec for just few hours.
The text was updated successfully, but these errors were encountered: