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

CMakeLists.txt assumes backscrub build directory layout #3

Open
ali1234 opened this issue Aug 30, 2021 · 7 comments
Open

CMakeLists.txt assumes backscrub build directory layout #3

ali1234 opened this issue Aug 30, 2021 · 7 comments

Comments

@ali1234
Copy link
Contributor

ali1234 commented Aug 30, 2021

The CMakeLists.txt allows to override the BACKSCRUB variable to specify the path to the backscrub repository:

if(NOT DEFINED BACKSCRUB)
set(BACKSCRUB ../backscrub)
endif()

Then it does two things with this variable. It sets LIBBACKSRUB_INCLUDE_DIR, which needs to point to the source repository:

set(LIBBACKSCRUB_INCLUDE_DIR ${BACKSCRUB})

and also it includes a file from the build directory:

include(${BACKSCRUB}/build/BackscrubTargets.cmake)

Here the build directory is assumed to be within the source repository with no way to override. This is not necessarily the case. The cmake build directory can be anywhere, and in the case of building a snap package, it will be along side the source, not inside it.

ali1234 added a commit to ali1234/obs-backscrub that referenced this issue Aug 31, 2021
After patching backscrub to install its header into the prefix,
this patch makes obs-backscrub get the header from its installed
location.

The link targets are still fetched from the backscrub build dir
and that variable is renamed to indicate that it is only used
for that purpose.

Both variables can be overridden separately.

See ali1234/backscrub@9dea13b
and phlash#3
@phlash
Copy link
Owner

phlash commented Aug 31, 2021

Good point. I'm not sure I want to insist on installing backscrub before it can be used in another project (and installing backscrub currently doesn't install the dependencies list file BackscrubTargets.cmake, this may be a bug!). I think I would rather be able to pull in backscrub as a sub-project via cmake's add_subdirectory().

Right now I do neither and have this fragile dependency between my projects because I'm lazy and wanted to avoid re-compiling Tensorflow Lite everywhere. I'll try using backscrub as a proper sub-project, if I can avoid the TFLite compile that would be a bonus..

@ali1234
Copy link
Contributor Author

ali1234 commented Aug 31, 2021

It should be possible to make backscrub work as either a sub-project or installed, but unfortunately it is currently beyond my understanding of cmake. I'm looking in to it though. What I found so far is that the BackscrubTargets.cmake cannot be installed because it refers to the build directory. You have to make a BacksrubConfig.cmake and that is a bit more complicated. No reason you can't have both though.

@ali1234
Copy link
Contributor Author

ali1234 commented Aug 31, 2021

Here is obs-backscrub reworked to have backscrub as a submodule:

https://github.com/ali1234/obs-backscrub/tree/bs-submodule

Note that it points to my fork of backscrub as I have made changes to both CMakeLists.txt to facilitate this.

Also note that I have patched the default model path in obs-backscrub so that it points to the location needed for a snap. That definitely won't work for you.

What has been done in backscrub:

  • The executable deepseg is renamed to backscrub.
  • The library backscrub is renamed to libbackscrub.
  • The cmake rules for building the executable are moved into a subdirectory.
  • The entire videoio subdirectory is moved into the app subdirectory, because it is only needed for the app.
  • The entire app directory is only added if we aren't on WIN32.

The above changes are not strictly necessary for this but they make the layout easier to understand, as a large amount of code can be removed from the top level CMakeLists.txt. Then:

  • The libbackscrub library is marked STATIC so it gets installed.
  • The header backscrub.h is marked PUBLIC_HEADER so it gets installed.

Again, these aren't strictly necessary and only affect installation which we don't use when backscrub is a submodule. Then finally:

  • The lib directory is added as a PUBLIC include directory to the libbackscrub target.

The effect of this is that when you add backscrub as a subdirectory and add libbackscrub as a dependency on obs-backscrub, cmake automatically knows to add the dependency's include directories. So now you don't need those path variables. Or BackscrubTargets.cmake, because you already have the actual target from the real CMakeLists.txt.

So after all this, you can now make one build directory and call cmake on the top of the obs-backscrub project, and it will build everything and optionally install it. The following will be installed in CMAKE_INSTALL_PREFIX:

lib/libbackscrub.a
include/libbackscrub.h
share/backscrub/models
share/backscrub/models/body-pix-float-050-8.tflite
share/backscrub/models/deeplabv3_257_mv_gpu.tflite
share/backscrub/models/segm_lite_v681.tflite
share/backscrub/models/retrain.md
share/backscrub/models/selfiesegmentation_mlkit-256x256-2021_01_19-v1215.f16.tflite
share/backscrub/models/body-pix
share/backscrub/models/segm_full_v679.tflite
bin/backscrub
lib/obs-plugins/obs-backscrub.so

All of this builds, the backscrub executable works, and the OBS plugin works too as long as it can find the model.

The next step would be to create an installable BackscrubConfig.cmake so that the install libraries and headers are usable without needing backscrub as a submodule, and also fix up both projects so that they check /usr/share/backscrub/models for the model files.

@phlash
Copy link
Owner

phlash commented Aug 31, 2021

Good work! I've just been doing similar in my tree, but didn't want to munge the existing backscrub system about (that's a separate challenge that needs agreement from Ben & Florian), so I've created a cmake find-module (FindBackscrub.cmake) for backscrub, pushed to my experimental fork.

This finds and pulls in the BackscrubTargets.cmake export file (which defines all the targets and dependent libs) it then sets two variables as per normal find modules: BACKSCRUB_INCLUDES and BACKSCRUB_LIBS that are used to build super-projects.

Here I have just pushed a modified CMakeLists.txt that uses this find module to remove internal knowledge of backscrub.

@ali1234
Copy link
Contributor Author

ali1234 commented Aug 31, 2021

As long as you know where the backscrub source is you can just add it directly like this:

ali1234@2f7ef5c

This will work against backscrub main. You don't need to build it, just set BACKSCRUB to wherever the source is. The standalone app won't be built and install will do nothing, due to the EXCLUDE_FROM_ALL directive.

You do not need to find and use BackscrubTargets.cmake at all.

@phlash
Copy link
Owner

phlash commented Sep 1, 2021

Yep, I'm aware that using backscrub as a sub-project will build everything and get all the dependencies correct, it also means a whole Tensorflow download/build which is several minutes (on my laptop anyhow!) and 100s of MB. Using backscrub as an external (pre-built) package avoids this in my situation where I work on backscrub as a separate project from this one, and simply want to re-link this occasionally. Supporting both mechanisms could be an option via a cmake config variable?

Using a git sub-project reference to pull in backscrub is also how we reference Tensorflow in the backscrub build, and would work similarly here, hopefully with the right cmake voodoo we can also avoid a checkout if referring to an external backscrub build instead.. I'll have a play!

@phlash
Copy link
Owner

phlash commented Sep 1, 2021

OK! The top-level CMakeLists.txt now builds with backscrub as a sub-module by default, or as an out-of-tree external library if you provide a location (-DBACKSCRUB=<path>).

Some tidy-up to export the include path(s) in the backscrub project would be helpful but I think this is a good start?

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

No branches or pull requests

2 participants