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

Fix some more warnings (a question) #2265

Open
Nomis101 opened this issue Aug 19, 2019 · 7 comments · Fixed by #2281

Comments

@Nomis101
Copy link
Contributor

commented Aug 19, 2019

Description of the problem

I would like to fix some more warnings I see while building HandBrake. Currently the easy to fix ones. But I would have some questions.

The first warning I would like to fix is

  : src/opus_decoder.c:37:10: warning: You appear to be compiling without optimization, if so opus will be very slow. [-W#pragma-messages]
  : # pragma message "You appear to be compiling without optimization, if so opus will be very slow."
  : 
  :          ^
  : 1 warning generated.
  :   CC       src/opus_encoder.lo

You see this warning if Opus is build in debug mode, see https://bugs.chromium.org/p/chromium/issues/detail?id=355440
Which seems a bit strange for a release HandBrake build. This warning can be silenced by adding LIBOPUS.GCC.args.extra += -DOPUS_WILL_BE_SLOW in module.defs. But, I'm not sure, if silencing is the correct way to fix this. Because, this warning also goes away if I e.g. add LIBOPUS.GCC.args.extra += -O3. So, it seems to me opus is really build without optimization in the release builds. But, if this is the case for so long time, maybe its fine? I'm a bit unsure what to do here.

The second warning I would like to fix and I have a question, is:

configure: WARNING: unrecognized options: --disable-dependency-tracking

This warning comes from this line:
https://github.com/HandBrake/HandBrake/blob/master/make/include/contrib.defs#L96
What is this actually needed for, if it seems to be an unrecognized option?

HandBrake version (e.g., 1.0.0)

master

Operating system and version (e.g., Ubuntu 18.04 LTS, macOS 10.14 Mojave, Windows 10 1809)

macOS 10.14 Mojave

@sr55

This comment has been minimized.

Copy link
Contributor

commented Sep 7, 2019

But, if this is the case for so long time, maybe its fine? I'm a bit unsure what to do here.

Probably unnoticed due to so few people using it. Feel free to submit a PR to fix.

@sr55 sr55 added the Bug label Sep 7, 2019

@sr55 sr55 added this to the 1.3.0 milestone Sep 7, 2019

@Nomis101

This comment has been minimized.

Copy link
Contributor Author

commented Sep 7, 2019

But, if this is the case for so long time, maybe its fine? I'm a bit unsure what to do here.

Probably unnoticed due to so few people using it. Feel free to submit a PR to fix.

I did already. :-)

@bradleysepos

This comment has been minimized.

Copy link
Contributor

commented Sep 7, 2019

I'll try to make some time to review. Been pretty swamped here.

@bradleysepos

This comment has been minimized.

Copy link
Contributor

commented Sep 8, 2019

--disable-dependency-tracking is an argument to GNU configure: https://www.gnu.org/software/automake/manual/html_node/Dependency-Tracking.html

It would be helpful if you could figure out building which contribs causes this warning. I suspect they are using something else, and we should avoid passing this if possible.

@bradleysepos

This comment has been minimized.

Copy link
Contributor

commented Sep 8, 2019

Passing --enable-asm to libopus' configure also silences the warning, with no effect on the configuration. In either case, configure reports the following on my w3680 CPU:

      Intrinsics Optimizations: ...... x86 SSE SSE2 SSE4.1 AVX
      Run-time CPU detection: ........ x86 SSE4.1 AVX

Optimizations are clearly enabled. AVX will be disabled on this CPU at run time since it's not available. This appears to be correct.

Since there is no inline assembly in libopus for platforms other than arm, I'm guessing configure treats this condition as though --disable-asm was passed and prints the warning even though it's irrelevant on x86(_64). Your solution to silence the warning directly seems to be the correct workaround to their minor configure bug. I've already pushed it. Thanks!

@bradleysepos

This comment has been minimized.

Copy link
Contributor

commented Sep 8, 2019

Reopening pending further investigation regarding contribs affected by --disable-dependency-tracking.

Aside, @Nomis101, if you'd like to test cross building for Windows, you can do this within macOS. See scripts/mingw-w64-build, which by default installs in userspace to ~/toolchains.

@bradleysepos bradleysepos reopened this Sep 8, 2019

@bradleysepos bradleysepos self-assigned this Sep 8, 2019

@Nomis101

This comment has been minimized.

Copy link
Contributor Author

commented Sep 8, 2019

Aside, @Nomis101, if you'd like to test cross building for Windows, you can do this within macOS. See scripts/mingw-w64-build, which by default installs in userspace to ~/toolchains.

Thanks. Will give this a try.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants
You can’t perform that action at this time.