goheif icon indicating copy to clipboard operation
goheif copied to clipboard

Published 20 hours ago • verified publisher icon jdeng

Silence warnings

Open evanfarrar opened this issue 3 years ago • 6 comments

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

evanfarrar avatar Mar 08 '21 01:03 evanfarrar

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

adrium avatar Mar 09 '21 20:03 adrium

Can we get this merged in? @huydq189 @evanfarrar

pxue avatar Feb 23 '22 22:02 pxue

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

huydq189 avatar Mar 01 '22 08:03 huydq189

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]

pxue avatar Mar 01 '22 16:03 pxue

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

huydq189 avatar Mar 02 '22 04:03 huydq189

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: https://github.com/strukturag/libde265/pull/395

This patch eliminates these warnings and resolves the crashes, rather than simply ignoring the warnings like I did at first.

mholt avatar Feb 23 '23 20:02 mholt