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

OpenEXRCore.testHUF fails on armv7 #1135

Closed
ggardet opened this issue Aug 30, 2021 · 7 comments
Closed

OpenEXRCore.testHUF fails on armv7 #1135

ggardet opened this issue Aug 30, 2021 · 7 comments
Labels
Bug A bug in the source code

Comments

@ggardet
Copy link

ggardet commented Aug 30, 2021

With version 3.1.1, OpenEXRCore.testHUF fails on armv7 with:

[  205s]  37/111 Test  #40: OpenEXRCore.testHUF .......................Subprocess aborted***Exception:   0.01 sec
[  205s] tempDir = '/var/tmp/OpenEXR_bPWsKS/': 24
[  205s] 
[  205s] =======
[  205s] Running testHUF
[  205s] Core Test failed: esize == 65537 * (8 + 8 + 8 + 4)
[  205s]            file:/home/abuild/rpmbuild/BUILD/openexr-3.1.1/src/test/OpenEXRCoreTest/compression.cpp
[  205s]            line:1336
[  205s]        function:void testHUF(const string&)
@meshula meshula added the Bug A bug in the source code label Aug 30, 2021
kdt3rd added a commit to kdt3rd/openexr that referenced this issue Aug 30, 2021
…er size

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>
@ggardet
Copy link
Author

ggardet commented Aug 31, 2021

@cary-ilm thanks for your patch. With this patch applied, I get a new error, though:

[  221s]  37/111 Test  #40: OpenEXRCore.testHUF .......................Subprocess aborted***Exception:   0.01 sec
[  221s] tempDir = '/var/tmp/OpenEXR_pEWmDL/': 24
[  221s] 
[  221s] =======
[  221s] Running testHUF
[  221s] Core Test failed: dsize == (65537 * 8 + (1 << 14) * 16)
[  221s]            file:/home/abuild/rpmbuild/BUILD/openexr-3.1.1/src/test/OpenEXRCoreTest/compression.cpp
[  221s]            line:1337
[  221s]        function:void testHUF(const string&)

So, it seems that more fixes are needed.

@cary-ilm cary-ilm reopened this Aug 31, 2021
@cary-ilm
Copy link
Member

@kdt3rd, looks like sizeof(HufDec) on arm7 is 16, not 12. How much havoc does that wreak?

@kdt3rd
Copy link
Contributor

kdt3rd commented Sep 1, 2021

oh, drat. To confirm, it should be 12 on arm7 (32-bit), where it's 16 on a 64-bit, not the other way round, right?

sorry - my pi w/ the arm 7 is currently not booting, so I can't test this, so solving blindly. And I missed that HufDec has a pointer in it. And no, it does not cause issues to have different sizes. Those checks are mostly there to know if something unexpected changes since it's pre-allocating blocks to reduce malloc contention (so you want to make sure you are allocating enough). So these checks are there to ensure that, as well to ensure the api is fully exercised from a code-coverage standpoint.

@kdt3rd
Copy link
Contributor

kdt3rd commented Sep 1, 2021

so if we change that * 16 to a * (sizeof(void *) + 4 + 4), that will work

@cary-ilm
Copy link
Member

cary-ilm commented Sep 1, 2021

Yeah, sorry I had it backwards: the 16 in the test corresponds to sizeof(HufDec), which must be 12 on arm7. Might as well call it sizeof(uint32_t*) , since that's what it is.

@cary-ilm
Copy link
Member

cary-ilm commented Sep 9, 2021

Closing, this should be fixed by #1138.

@cary-ilm cary-ilm closed this as completed Sep 9, 2021
@ggardet
Copy link
Author

ggardet commented Sep 9, 2021

Closing, this should be fixed by #1138.

I confirm this is now fixed. Thanks!

cary-ilm pushed a commit to cary-ilm/openexr that referenced this issue Sep 23, 2021
cary-ilm pushed a commit that referenced this issue Sep 29, 2021
Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in the source code
Projects
None yet
Development

No branches or pull requests

4 participants