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

Silence warnings #16

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Silence warnings #16

wants to merge 2 commits into from

Conversation

evanfarrar
Copy link

@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.

@adrium
Copy link

adrium commented Mar 9, 2021

You may be interested in my fork which is updated to v1.0.8 and has ARM support.

@pxue
Copy link

pxue commented Feb 23, 2022

Can we get this merged in? @huydq189 @evanfarrar

@huydq189
Copy link

huydq189 commented Mar 1, 2022

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

@pxue
Copy link

pxue commented Mar 1, 2022

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:

../../../go/pkg/mod/github.com/huydq189/[email protected]/libde265/libde265/x86/sse-motion.cc:3530:68: warning: implicit conversion from 'int' to 'short' changes value from 65535 to -1 [-Wconstant-conversion]

@huydq189
Copy link

huydq189 commented Mar 2, 2022

Check out my sample code at README.md of my fork. I don't see any warning.

@mholt
Copy link

mholt commented Feb 23, 2023

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 _mm_set_epi16() function -- called on the lines mentioned by those very warnings -- differs between CPU brands.

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.

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

Successfully merging this pull request may close these issues.

None yet

7 participants