[Buildroot] [PATCH 0/2] vlc autobuild fixes and opencv modules mess?

[Buildroot] [PATCH 0/2] vlc autobuild fixes and opencv modules mess?

[Samuel Martin]

All,


Here is a fix for some vlc autobuilder failures (caused by opencv) and
a cleanup for opencv.

That's all for the vlc fixes, now the opencv situation that I dug out
while I was investigating these failure, and on which I'd like to get
inputs from others developpers.

Along side this vlc fix, it appears the opencv situation is a bit
messy. Let me explain it.

OpenCV allows to enable/disable the selection of modules (a.k.a.
opencv libraries). These modules depend one from the others; these
dependencies are already handled by the build-system (CMake).

However, the way we handle them in Buildroot can be improved.
wever

The curreent situation:
For each opencv module, there is an option, so the corresponding cmake
config option is set accordingly:

OPENCV_CONF_OPT += \
  -DBUILD_opencv_core=$(if $(BR2_PACKAGE_OPENCV_LIB_CORE),ON,OFF) \
  -DBUILD_opencv_video=$(if $(BR2_PACKAGE_OPENCV_LIB_VIDEO),ON,OFF) \
  ...

The thing is that a module (e.g. video) won't be built if it is enable
while one of its dependencies (e.g. core) is disabled.
CMake behaves like this because we force (too?) many options, and
CMake, which is aware the dependency relations, solves them this way.

How can the situation be improved:
I see 2 ways to rework this:

1) Replicate the modules' dependency relations in the Config.in.
  This will:
  - make the Config.in even more longer;
  - make the bump a bit less easy (need to check the dependencies);
  - copy in Buildroot some of the intelligence put in the package's
    build-system itself. :-/

2) Set the CMake config option in a more permissive way:

  OPENCV_CONF_OPT += \
    $(if $(BR2_PACKAGE_OPENCV_LIB_CORE),-DBUILD_opencv_core=ON) \
    $(if $(BR2_PACKAGE_OPENCV_LIB_VIDEO),-DBUILD_opencv_video=ON) \
    ...

  This keep the intelligence in the build-system itself because if
  core is not enabled in Buildroot, its value in CMake is not forced
  by the config options on the command line. Thus the core module will
  be built if its defaults in the cmake code is ON, or if it is pulled
  by another module.
  However, this may result in bigger rootfs since most of the modules
  are enabled by default in the opencv's cmake code. :-/


So, we have 3 options, the 2 above ones, plus keeping things as they
are, relying on the Buildroot user and its knowledge of what he/she is
doing.

The more I think about this, the less I'm convinced about refactoring
this. Any input/comment/feeling is welcome.


Regards,
Samuel


Samuel Martin (2):
  vlc: explicitly disable opencv support
  opencv: always enable opencv_core module when opencv is enabled

 package/opencv/Config.in | 6 ------
 package/opencv/opencv.mk | 2 +-
 package/vlc/vlc.mk       | 3 ++-
 3 files changed, 3 insertions(+), 8 deletions(-)

--
1.8.5.4

[Buildroot] [PATCH 1/2] vlc: explicitly disable opencv support

[Samuel Martin]

vlc can use opencv in video filters, but it requires a couple of
features enabled in opencv; so, simply force disabling opencv support.

Note that vlc does includes the legacy opencv/cv.h header, which pulls a
number of opencv's features (core, video, imgproc, ...).

Fixes:
  http://autobuild.buildroot.org/results/39c/39c77ffb5a5599d0b09422433c747b2bac185c4f/
  http://autobuild.buildroot.org/results/a79/a79b055da09e4d8ede263251df8461bc8a64569a/

Signed-off-by: Samuel Martin <s.martin49@gmail.com>
---
 package/vlc/vlc.mk | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/package/vlc/vlc.mk b/package/vlc/vlc.mk
index f235ff8..db99a3b 100644
--- a/package/vlc/vlc.mk
+++ b/package/vlc/vlc.mk
@@ -30,7 +30,8 @@ VLC_CONF_OPT += \
 	--disable-goom \
 	--disable-projectm \
 	--disable-vsxu \
-	--disable-mtp
+	--disable-mtp \
+	--without-opencv
 
 # Set powerpc altivec appropriately
 ifeq ($(BR2_powerpc_7400)$(BR2_powerpc_7450)$(BR2_powerpc_970),y)
-- 
1.8.5.4

[Buildroot] [PATCH 2/2] opencv: always enable opencv_core module when opencv is enabled

[Samuel Martin]

It does not make much sense enabling opencv without its core module.

This configuration leads to build nothing (since all modules depend on
the core one), but install the configuration files (*.pc and *.cmake)
anyway.

This absurd situation may break the build-system of other packages
that would correctly find the *.pc (but does not check for the modules
they actually use), but would not build because of missing headers and
libraries.

Signed-off-by: Samuel Martin <s.martin49@gmail.com>
---
 package/opencv/Config.in | 6 ------
 package/opencv/opencv.mk | 2 +-
 2 files changed, 1 insertion(+), 7 deletions(-)

diff --git a/package/opencv/Config.in b/package/opencv/Config.in
index ccde5cb..217243d 100644
--- a/package/opencv/Config.in
+++ b/package/opencv/Config.in
@@ -26,12 +26,6 @@ config BR2_PACKAGE_OPENCV_LIB_CONTRIB
 	help
 	  Include opencv_contrib module into the OpenCV build.
 
-config BR2_PACKAGE_OPENCV_LIB_CORE
-	bool "core"
-	default y
-	help
-	  Include opencv_core module into the OpenCV build.
-
 config BR2_PACKAGE_OPENCV_LIB_FEATURES2D
 	bool "features2d"
 	default y
diff --git a/package/opencv/opencv.mk b/package/opencv/opencv.mk
index 732bc0c..42f9b0a 100644
--- a/package/opencv/opencv.mk
+++ b/package/opencv/opencv.mk
@@ -34,7 +34,7 @@ OPENCV_CONF_OPT += \
 	-DBUILD_opencv_androidcamera=OFF                                        \
 	-DBUILD_opencv_calib3d=$(if $(BR2_PACKAGE_OPENCV_LIB_CALIB3D),ON,OFF)   \
 	-DBUILD_opencv_contrib=$(if $(BR2_PACKAGE_OPENCV_LIB_CONTRIB),ON,OFF)   \
-	-DBUILD_opencv_core=$(if $(BR2_PACKAGE_OPENCV_LIB_CORE),ON,OFF)         \
+	-DBUILD_opencv_core=ON                                                  \
 	-DBUILD_opencv_features2d=$(if $(BR2_PACKAGE_OPENCV_LIB_FEATURES2D),ON,OFF) \
 	-DBUILD_opencv_flann=$(if $(BR2_PACKAGE_OPENCV_LIB_FLANN),ON,OFF)       \
 	-DBUILD_opencv_gpu=$(if $(BR2_PACKAGE_OPENCV_LIB_GPU),ON,OFF)           \
-- 
1.8.5.4

[Buildroot] [PATCH 0/2] vlc autobuild fixes and opencv modules mess?

[Peter Korsgaard]

>>>>> "Samuel" == Samuel Martin <s.martin49@gmail.com> writes:

 > All,
 > Here is a fix for some vlc autobuilder failures (caused by opencv) and
 > a cleanup for opencv.

 > That's all for the vlc fixes, now the opencv situation that I dug out
 > while I was investigating these failure, and on which I'd like to get
 > inputs from others developpers.

 > Along side this vlc fix, it appears the opencv situation is a bit
 > messy. Let me explain it.

 > OpenCV allows to enable/disable the selection of modules (a.k.a.
 > opencv libraries). These modules depend one from the others; these
 > dependencies are already handled by the build-system (CMake).

 > However, the way we handle them in Buildroot can be improved.
 > wever

 > The curreent situation:
 > For each opencv module, there is an option, so the corresponding cmake
 > config option is set accordingly:

 > OPENCV_CONF_OPT += \
 >   -DBUILD_opencv_core=$(if $(BR2_PACKAGE_OPENCV_LIB_CORE),ON,OFF) \
 >   -DBUILD_opencv_video=$(if $(BR2_PACKAGE_OPENCV_LIB_VIDEO),ON,OFF) \
 >   ...

 > The thing is that a module (e.g. video) won't be built if it is enable
 > while one of its dependencies (e.g. core) is disabled.
 > CMake behaves like this because we force (too?) many options, and
 > CMake, which is aware the dependency relations, solves them this way.

 > How can the situation be improved:
 > I see 2 ways to rework this:

 > 1) Replicate the modules' dependency relations in the Config.in.
 >   This will:
 >   - make the Config.in even more longer;
 >   - make the bump a bit less easy (need to check the dependencies);
 >   - copy in Buildroot some of the intelligence put in the package's
 >     build-system itself. :-/

I don't really think we have any way around that. If people enable a
certain opencv option in menuconfig they rightly expect it to get
enabled without having to study the opencv build system to figure out
what other options to enable.

So this is my preference.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH 1/2] vlc: explicitly disable opencv support

[Peter Korsgaard]

>>>>> "Samuel" == Samuel Martin <s.martin49@gmail.com> writes:

 > vlc can use opencv in video filters, but it requires a couple of
 > features enabled in opencv; so, simply force disabling opencv support.

 > Note that vlc does includes the legacy opencv/cv.h header, which pulls a
 > number of opencv's features (core, video, imgproc, ...).

Is this meant as a short term fix for 2014.02? Presumably the user CAN
enable the needed opencv option so it could work? - E.G. we could come
up with the needed test like:

ifeq ($(BR2_PACKAGE_OPENCV_FOO)$(BR2_PACKAGE_OPENCV_BLAH)...,yyy)
VLC_CONF_OPT += --with-opencv
VLC_DEPENDENCIES += opencv
else
VLC_CONF_OPT += --without-opencv
endif

 > Fixes:
 >   http://autobuild.buildroot.org/results/39c/39c77ffb5a5599d0b09422433c747b2bac185c4f/
 >   http://autobuild.buildroot.org/results/a79/a79b055da09e4d8ede263251df8461bc8a64569a/

 > Signed-off-by: Samuel Martin <s.martin49@gmail.com>
 > ---
 >  package/vlc/vlc.mk | 3 ++-
 >  1 file changed, 2 insertions(+), 1 deletion(-)

 > diff --git a/package/vlc/vlc.mk b/package/vlc/vlc.mk
 > index f235ff8..db99a3b 100644
 > --- a/package/vlc/vlc.mk
 > +++ b/package/vlc/vlc.mk
 > @@ -30,7 +30,8 @@ VLC_CONF_OPT += \
 >  	--disable-goom \
 >  	--disable-projectm \
 >  	--disable-vsxu \
 > -	--disable-mtp
 > +	--disable-mtp \
 > +	--without-opencv
 
 >  # Set powerpc altivec appropriately
 >  ifeq ($(BR2_powerpc_7400)$(BR2_powerpc_7450)$(BR2_powerpc_970),y)
 > -- 
 > 1.8.5.4

 > _______________________________________________
 > buildroot mailing list
 > buildroot at busybox.net
 > http://lists.busybox.net/mailman/listinfo/buildroot


-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH 2/2] opencv: always enable opencv_core module when opencv is enabled

[Peter Korsgaard]

>>>>> "Samuel" == Samuel Martin <s.martin49@gmail.com> writes:

 > It does not make much sense enabling opencv without its core module.
 > This configuration leads to build nothing (since all modules depend on
 > the core one), but install the configuration files (*.pc and *.cmake)
 > anyway.

 > This absurd situation may break the build-system of other packages
 > that would correctly find the *.pc (but does not check for the modules
 > they actually use), but would not build because of missing headers and
 > libraries.

 > Signed-off-by: Samuel Martin <s.martin49@gmail.com>

Committed, thanks.

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH 1/2] vlc: explicitly disable opencv support

[Samuel Martin]

Hi Peter,

On Sun, Feb 16, 2014 at 9:17 AM, Peter Korsgaard <jacmet@uclibc.org> wrote:
>>>>>> "Samuel" == Samuel Martin <s.martin49@gmail.com> writes:
>
>  > vlc can use opencv in video filters, but it requires a couple of
>  > features enabled in opencv; so, simply force disabling opencv support.
>
>  > Note that vlc does includes the legacy opencv/cv.h header, which pulls a
>  > number of opencv's features (core, video, imgproc, ...).
>
> Is this meant as a short term fix for 2014.02?

Yes, it is a short term fix for the 2014.02 release, also because I
don't think vlc package was added to buildroot with opencv support in
mind... ;-)

I just need some time to find all opencv's features used/required by vlc.

> Presumably the user CAN
> enable the needed opencv option so it could work? - E.G. we could come
> up with the needed test like:
>
> ifeq ($(BR2_PACKAGE_OPENCV_FOO)$(BR2_PACKAGE_OPENCV_BLAH)...,yyy)
> VLC_CONF_OPT += --with-opencv
> VLC_DEPENDENCIES += opencv
> else
> VLC_CONF_OPT += --without-opencv
> endif
>

Regards,

-- 
Samuel

[Buildroot] [PATCH 1/2] vlc: explicitly disable opencv support

[Peter Korsgaard]

>>>>> "Samuel" == Samuel Martin <s.martin49@gmail.com> writes:

 > vlc can use opencv in video filters, but it requires a couple of
 > features enabled in opencv; so, simply force disabling opencv support.

 > Note that vlc does includes the legacy opencv/cv.h header, which pulls a
 > number of opencv's features (core, video, imgproc, ...).

 > Fixes:
 >   http://autobuild.buildroot.org/results/39c/39c77ffb5a5599d0b09422433c747b2bac185c4f/
 >   http://autobuild.buildroot.org/results/a79/a79b055da09e4d8ede263251df8461bc8a64569a/

 > Signed-off-by: Samuel Martin <s.martin49@gmail.com>

Committed, thanks.

-- 
Bye, Peter Korsgaard