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

cleanup configure #430

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

cleanup configure #430

wants to merge 15 commits into from

Conversation

ulmus-scott
Copy link
Contributor

@ulmus-scott ulmus-scott commented Dec 13, 2021

This should be a better way of setting the C++ optimization flag.

TODO

@ulmus-scott
Copy link
Contributor Author

Some of the macOS/Darwin conditional code probably needs to be tested to see if it is still needed, e.g.

#ifdef Q_OS_MAC
// Qt 4.4 has window-focus problems
gCoreContext->OverrideSettingForSession("RunFrontendInWindow", "1");
#endif

@ulmus-scott ulmus-scott marked this pull request as ready for review December 24, 2021 23:39
@garybuhrmaster
Copy link
Contributor

Maybe I misinterpreted the discussion on IRC, but as I understand it, the plan was to transition the (legacy) configure (derived from the original ffmpeg configure) to CMAKE as soon as V32 is released. So unless this work is helping that transition, this seems to all just be rearranging the deck chairs on the Titanic.

@ulmus-scott
Copy link
Contributor Author

The first few commits make setting the optimization flag easier.

Most of the rest of the commits are replacing defined macros from configure with Qt macros (and updating the Qt macros for Darwin/macOS).

git grep -nE "[^A-Z]ARCH_" -- :^platform/** :^*/FFmpeg/**
only in:
configure
libmpeg2 (which will be removed once mythtranscode is updated)

mythtv/libs/libmyth/audio/audiooutputbase.cpp:321:#if ARCH_ARM || defined(Q_OS_ANDROID)
mythtv/libs/libmythtv/mythavutil.cpp:204:#if ARCH_ARM
mythtv/libs/libmythtv/mythdeinterlacer.cpp:13:#if (HAVE_SSE2 && ARCH_X86_64)
mythtv/libs/libmythtv/mythdeinterlacer.cpp:18:#if ARCH_AARCH64
mythtv/libs/libmythtv/mythdeinterlacer.cpp:20:#elif ARCH_ARM
mythtv/libs/libmythtv/mythdeinterlacer.cpp:517:#if (HAVE_SSE2 && ARCH_X86_64) || HAVE_INTRINSICS_NEON
mythtv/libs/libmythtv/mythdeinterlacer.cpp:550:#if (HAVE_SSE2 && ARCH_X86_64)
mythtv/libs/libmythtv/mythdeinterlacer.cpp:605:#if (HAVE_SSE2 && ARCH_X86_64)
mythtv/libs/libmythtv/mythdeinterlacer.cpp:660:#if (HAVE_SSE2 && ARCH_X86_64) || HAVE_INTRINSICS_NEON
mythtv/libs/libmythtv/visualisations/goom/zoom_filter_xmmx.cpp:5:#if defined(MMX) && !defined(ARCH_X86_64)
mythtv/settings.pro:286:release:contains( ARCH_POWERPC, yes ) {

https://doc.qt.io/qt-5/qtglobal.html#Q_PROCESSOR_ARM no macro for aarch64, unfortunately.

git grep -nE "[^_A-Z]CONFIG_" -- :^platform/** :^*/FFmpeg/** very few unique defines
git grep -nE "HAVE_" -- :^platform/** :^*/FFmpeg/** not very many unique defines either

@ulmus-scott
Copy link
Contributor Author

ulmus-scott commented Dec 30, 2021

I will split this into a few orthogonal pull requests.

They were from FFmpeg, but unused by MythTV.
They already were, but make it clearer.
Removed duplicate test for non-zero length optflags, since it was already checked
if zero length.
This is clearer than my first change and easier to search for.
Add help text, note neither optimization flag settings are confirmed to work,
it will silently not add them instead.  (Or log the error for C++)

Rename C++ opt flags to match the C ones and add g++ flags.
Note:
noopt is not actually no optimization.  That would be -O0 or -Og for debugging.

Print supplied or selected flags at the end.
Added with optimization flags
copies ffmpeg_optenable but removes the else branch
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

2 participants