-
Notifications
You must be signed in to change notification settings - Fork 66
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
Silence warnings #16
base: master
Are you sure you want to change the base?
Silence warnings #16
Conversation
You may be interested in my fork which is updated to v1.0.8 and has ARM support. |
Can we get this merged in? @huydq189 @evanfarrar |
While waiting for this pull request to be merged, you can temporarily go get my fork at github.com/huydq189/goheif |
Hm getting compiler warning on your branch:
|
Check out my sample code at README.md of my fork. I don't see any warning. |
I would advise against silencing the warnings. @pxue That particular warning is revealing a problem that causes crashes at runtime. I was boggled as to why my application was crashing when compiled in a Windows CI environment and run on my local Windows box, but not when compiled and run on the same machine. Both CI and local were 64-bit Windows (amd64 / x86_64 / whatever you want to call it) and, sure enough, compiling the program the same way with the same environment on both systems, and then running it on the same machine, worked fine. When compiled on an Intel CPU and run on an AMD (Ryzen) CPU, the program crashed at runtime. It threw an overflow exception. This is extremely tricky because both chips are the same architecture and the the OSes were the same (Windows; although one was Windows Server 2022, and the other was Windows 11). It turns out that the implementation of the I submitted a PR here with a patch: strukturag/libde265#395 This patch eliminates these warnings and resolves the crashes, rather than simply ignoring the warnings like I did at first. |
@huydq189's fork silences warnings. I have not improved on it, but only removed the go-module name change. Feel free to ignore this PR and cherrypick those two commits.