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

Docs on how to upgrade from v1 to v2 api? #62

Open
ghost opened this issue May 28, 2021 · 7 comments
Open

Docs on how to upgrade from v1 to v2 api? #62

ghost opened this issue May 28, 2021 · 7 comments

Comments

@ghost
Copy link

ghost commented May 28, 2021

No description provided.

@0-wiz-0
Copy link

0-wiz-0 commented May 28, 2021

I tried this in my code:

-use crc::{crc32, Hasher32};
+use crc::{Crc, CRC_32_BZIP2};
...
-    let mut crc = crc32::Digest::new(crc32::IEEE);
+    let crc = Crc::<u32>::new(&CRC_32_BZIP2);
+    let mut digest = crc.digest();
...
-       crc.write(&buffer[..n]);
+       digest.update(&buffer[..n]);
...
-    Ok(format!("crc {:08x}", crc.sum32())
+    Ok(format!("crc {:08x}", digest.finalize())

but then I get different output for my test files, so this is wrong. What did I do wrong?

@akhilles
Copy link
Collaborator

@0-wiz-0 CRC_32_BZIP2 is a different algorithm from CRC_32_IEEE, try CRC_32_ISO_HDLC instead. Other than that your diff is correct, and should work. One performance nitpick: use const crc: Crc<u32> = Crc::<u32>::new(&CRC_32_BZIP2); instead to generate the CRC table at compile-time instead of runtime.

It's unfortunate that these CRC polynomials have multiple names (https://reveng.sourceforge.io/crc-catalogue/all.htm is a pretty good reference).

@0-wiz-0
Copy link

0-wiz-0 commented May 28, 2021

Thanks, @akhilles , that worked!

@jszwedko
Copy link

jszwedko commented Jul 8, 2021

I also have a question about this. We were using:

crc::crc64::checksum_ecma(&buf[..])

It seems like this should be replace by:

Crc::<u64>::new(&crc::CRC_64_ECMA_182).checksum(&buf[..])

But this is calculating a different checksum value. Is there something equivalent to the old crc::crc64::checksum_ecma()?

jszwedko added a commit to vectordotdev/vector that referenced this issue Jul 8, 2021
0.14.0 included an upgrade to the `crc` crate, with associated updates
to use the new API, however it appears to calculate different checksums.

I put a comment here asking about how to calculate an equivalent
checksum using the new API:
mrhooray/crc-rs#62 (comment)

This also adds an alias to ensure that checkpoints written by 0.14.0
will be able to be read by 0.15.0 when it is released.

Fixes: #8182

Signed-off-by: Jesse Szwedko <jesse@szwedko.me>
@akhilles
Copy link
Collaborator

akhilles commented Jul 8, 2021

I also have a question about this. We were using:

crc::crc64::checksum_ecma(&buf[..])

It seems like this should be replace by:

Crc::<u64>::new(&crc::CRC_64_ECMA_182).checksum()

But this is calculating a different checksum value. Is there something equivalent to the old crc::crc64::checksum_ecma()?

Could you try crc::CRC_64_XZ? According to https://reveng.sourceforge.io/crc-catalogue/all.htm, it's an algorithm commonly misidentified as ECMA.

@jszwedko
Copy link

jszwedko commented Jul 9, 2021

Aha, yep, that appears to be it. Thanks @akhilles !

gstreamer-github pushed a commit to sdroege/gst-plugin-rs that referenced this issue Sep 3, 2021
This was broken when porting to crc 2, based on:

mrhooray/crc-rs#62 (comment)

> CRC_32_BZIP2 is a different algorithm from CRC_32_IEEE, try CRC_32_ISO_HDLC instead.

The correct algorithm for replacing checksum_ieee is not CRC_32_BZIP2.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-rs/-/merge_requests/555>
@jimvdl
Copy link

jimvdl commented Nov 9, 2021

I upgraded from v1.8 to v2 and it was pretty straightforward and painless. I used the reference that was in the documentation (this link: https://reveng.sourceforge.io/crc-catalogue/all.htm) to figure out what constant I needed (for me this was from IEEE to CRC_32_ISO_HDLC). It took me about 3 minutes to upgrade. The tip about generating the CRC table at compile-time was also very handy: const crc: Crc<u32> = Crc::<u32>::new(&CRC_32_BZIP2);

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

No branches or pull requests

4 participants