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 buildroot crosscompile #1

Merged

Conversation

pseiderer
Copy link
Contributor

Fix some findings while preparing a buildroot libcamera-apps package:

  • cmake: remove unsafe host include path for libdrm (use pkgconfig instead)
  • cmake: remove unrecognized gcc command-line option
  • libcamera_vid: fix getline call (needs size_t)
  • libcamera_still: fix getline call (needs size_t)
  • libcamera_app: fix int64_t constants
  • cmake: fix linking with exif, jpeg
  • cmake: add install target
  • cmake: no need to link against ipa_rpi.so

CMakeLists.txt Outdated
target_link_libraries(libcamera-jpeg common "${LIBCAMERA_LIBRARY}" exif jpeg ${Boost_LIBRARIES} pthread "${X11_LIBRARIES}" "${EPOXY_LIBRARY}" "${DRM_LIBRARY}")

install(TARGETS common encoders LIBRARY)
install(TARGETS libcamera-still libcamera-vid libcamera-hello libcamera-raw libcamera-jpeg RUNTIME)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, thanks for these changes! These install commands seem to give me:

CMake Error at CMakeLists.txt:69 (install): install TARGETS given no ARCHIVE DESTINATION for static library target "common".

CMake Error at CMakeLists.txt:70 (install):
install TARGETS given no RUNTIME DESTINATION for executable target
"libcamera-still".

Are you able to suggest a fix for that? Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to [1] the defaults depend on the cmake version, but the following should fix it (see [2]):

install(TARGETS common encoders LIBRARY DESTINATION lib ARCHIVE DESTINATION lib)
install(TARGETS libcamera-still libcamera-vid libcamera-hello libcamera-raw libcamera-jpeg RUNTIME DESTINATION bin)

[1] https://stackoverflow.com/questions/14990343/cmake-error-targets-given-no-library-destination-for-shared-library-target
[2] https://cmake.org/cmake/help/latest/command/install.html

@@ -13,7 +13,6 @@ set (CMAKE CXX_FLAGS "-Wall -Wextra -pedantic -Wno-unused-parameter -faligned-ne
add_definitions(-Wfatal-errors)
add_definitions(-Wno-psabi)
add_definitions(-DBOOST_LOG_DYN_LINK)
add_definitions(-mfpu=neon-fp-armv8 -ftree-vectorize)
Copy link
Collaborator

@davidplowman davidplowman Feb 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd quite like to leave these compile flags in place for platforms that support them. Do you know of a way to do that? Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, no easy way (and the buildroot view/preferred way is to avoid package specific heuristics to determine architecture specific compile flags - in buildroot the preferred way is build all packages specific to your selected target architecture/cpu etc. by a global configure/selection).

One problem is (for cross compiling) the detection of the target arch and/or the suitable/supported compiler flags (and not all cross-compiler available flags/features will be available on the target platform, specially for multi-arch/targets compilers).

The next/other problem is that gcc compiler distinguish between used/maximum feature set and optimization for target arch...

Do not know a perfect example but a quick search found e.g. [1] (did not check if it works for cross-compiling)...

But a simple solution is to implement one/some cmake option(s) to enable target specific flags/optimizations (easy to not to use and/or override with global/package specific compiler flags)...

[1] https://github.com/seredat/karbowanec/blob/master/CMakeLists.txt

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it work for you to have a TARGET variable which can default to (say) "armv8" but such that you can invoke cmake with "-DTARGET=other" or something like that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, should definitely work (would name it a little more specific like ENABLE_COMPILE_FLAGS_FOR_TARGET_ARCH?)...

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this do the trick for you?

IF (NOT ENABLE_COMPILE_FLAGS_FOR_TARGET)
  set(ENABLE_COMPILE_FLAGS_FOR_TARGET "armv8")
endif()
if ("${ENABLE_COMPILE_FLAGS_FOR_TARGET}" STREQUAL "armv8")
  add_definitions(-mfpu=neon-fp-armv8 -ftree-vectorize)
endif()

If you were ok to put the changes we've discussed into your branch then I'd be happy to test and merge if it all works fine for our "default Pi user". Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, works for the buildroot case, merge request updated...

@pseiderer pseiderer force-pushed the ps-devel-fix-buildroot-crosscompile-001 branch 2 times, most recently from a970d87 to 741272e Compare February 11, 2021 18:51
Add cmake option ENABLE_COMPILE_FLAGS_FOR_TARGET, defaulting to armv8
enabling the previous hard coded compiler options.

Run cmake with -DENABLE_COMPILE_FLAGS_FOR_TARGET=disabled for no extra
compiler options.

Fixes:

  aarch64-buildroot-linux-gnu-g++.br_real: error: unrecognized command-line option ‘-mfpu=neon-fp-armv8’

Signed-off-by: Peter Seiderer <[email protected]>
Fixes:

.../libcamera-apps-custom/libcamera_vid.cpp:39:26: error: cannot convert ‘unsigned int*’ to ‘size_t*’ {aka ‘long unsigned int*’}
   39 |    getline(&user_string, &len, stdin);
      |                          ^~~~
      |                          |
      |                          unsigned int*

Signed-off-by: Peter Seiderer <[email protected]>
Fixes:

.../build/libcamera-apps-custom/libcamera_still.cpp:152:26: error: cannot convert ‘unsigned int*’ to ‘size_t*’ {aka ‘long unsigned int*’}
  152 |    getline(&user_string, &len, stdin);
      |                          ^~~~
      |                          |
      |                          unsigned int*

Signed-off-by: Peter Seiderer <[email protected]>
Fixes:

.../host/aarch64-buildroot-linux-gnu/sysroot/usr/include/libcamera/libcamera/controls.h: In instantiation of ‘void libcamera::ControlList::set(const libcamera::Control<T>&, const std::initializer_list<_Up>&) [with T = libcamera::Span<const long int>; V = long long int]’:
.../libcamera_app.hpp:331:18:   required from ‘void LibcameraApp<OPTIONS>::StartCamera() [with OPTIONS = StillOptions]’
.../libcamera_jpeg.cpp:35:18:   required from here
.../host/aarch64-buildroot-linux-gnu/sysroot/usr/include/libcamera/libcamera/controls.h:399:14: error: no matching function for call to ‘libcamera::ControlValue::set<libcamera::Span<const long int, 18446744073709551615> >(libcamera::Span<const long long int, 18446744073709551615>)’
  399 |   val->set<T>(Span<const typename std::remove_cv_t<V>>{ value.begin(), value.size() });
      |   ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Signed-off-by: Peter Seiderer <[email protected]>
Fixes:

  [ 66%] Linking CXX executable libcamera-hello
  .../aarch64-buildroot-linux-gnu/bin/ld: libcommon.so: undefined reference to `jpeg_std_error'
  .../aarch64-buildroot-linux-gnu/bin/ld: libcommon.so: undefined reference to `exif_set_slong'

  [ 92%] Linking CXX executable libcamera-vid
  .../aarch64-buildroot-linux-gnu/bin/ld: libcommon.so: undefined reference to `exif_set_slong'

Signed-off-by: Peter Seiderer <[email protected]>
Signed-off-by: Peter Seiderer <[email protected]>
Fixes:
	$ libcamera-hello
	libcamera-hello: error while loading shared libraries: ipa_rpi.so: cannot open shared object file: No such file or directory

Signed-off-by: Peter Seiderer <[email protected]>
Fixes:

  .../libcamera-apps-b1ca8997530f1f0290b291a956de359f2d4ce3e2/circular_output.cpp: In member function ‘virtual void CircularOutput::outputBuffer(void*, size_t, int64_t, uint32_t)’:
  .../libcamera-apps-b1ca8997530f1f0290b291a956de359f2d4ce3e2/circular_output.cpp:72:62: warning: narrowing conversion of ‘size’ from ‘size_t {aka long unsigned int}’ to ‘unsigned int’ inside { } [-Wnarrowing]
    Header header = { size, flags & FLAG_KEYFRAME, timestamp_us };
                                                                ^

Signed-off-by: Peter Seiderer <[email protected]>
Fixes:

  .../libcamera-apps-b1ca8997530f1f0290b291a956de359f2d4ce3e2/egl_preview.cpp: In member function ‘void EglPreview::makeWindow(const char*)’:
  .../libcamera-apps-b1ca8997530f1f0290b291a956de359f2d4ce3e2/egl_preview.cpp:219:65: sorry, unimplemented: non-trivial designated initializers not supported
    XVisualInfo visTemplate = { .visualid = (long unsigned int)vid };
                                                                   ^

Signed-off-by: Peter Seiderer <[email protected]>
Introduce a custom jpeg_mem_len_t as the jpeg_mem_dest() signature
changed since jpegsrc.v9d from 'unsigned long * outsize' to
'size_t * outsize'.

Fixes:

  .../libcamera-apps-b1ca8997530f1f0290b291a956de359f2d4ce3e2/jpeg.cpp:270:41: error: invalid conversion from ‘long unsigned int*’ to ‘size_t*’ {aka ‘unsigned int*’} [-fpermissive]
    270 |     jpeg_mem_dest(&cinfo, &jpeg_buffer, &jpeg_len);
        |                                         ^~~~~~~~~
        |                                         |
        |                                         long unsigned int*

Signed-off-by: Peter Seiderer <[email protected]>
Fixes:

  .../libcamera-apps-b1ca8997530f1f0290b291a956de359f2d4ce3e2/mjpeg_encoder.cpp:57:22: error: cannot convert ‘bool’ to ‘boolean’ in assignment
     57 |  cinfo.raw_data_in = true;
        |                      ^~~~

Signed-off-by: Peter Seiderer <[email protected]>
Introduce a custom jpeg_mem_len_t as the jpeg_mem_dest() signature
changed since jpegsrc.v9d from 'unsigned long * outsize' to
'size_t * outsize'.

Fixes:

  .../libcamera-apps-b1ca8997530f1f0290b291a956de359f2d4ce3e2/mjpeg_encoder.cpp:61:44: error: invalid conversion from ‘long unsigned int*’ to ‘size_t*’ {aka ‘unsigned int*’} [-fpermissive]
     61 |     jpeg_mem_dest(&cinfo, &encoded_buffer, &buffer_len);
        |                                            ^~~~~~~~~~~
        |                                            |
        |                                            long unsigned int*

Signed-off-by: Peter Seiderer <[email protected]>
Fixes:

  .../libcamera-apps-b1ca8997530f1f0290b291a956de359f2d4ce3e2/mjpeg_encoder.cpp:57:22: error: cannot convert ‘bool’ to ‘boolean’ in assignment
     57 |  cinfo.raw_data_in = true;
        |                      ^~~~
  compilation terminated due to -Wfatal-errors.

Signed-off-by: Peter Seiderer <[email protected]>
@pseiderer pseiderer force-pushed the ps-devel-fix-buildroot-crosscompile-001 branch from 741272e to f70d2d3 Compare February 11, 2021 19:28
@davidplowman davidplowman merged commit b20dc09 into raspberrypi:main Feb 12, 2021
davidplowman added a commit to davidplowman/libcamera-apps that referenced this pull request Apr 9, 2021
Fixing up some type warnings had caused the jpeg length not to be
returned correctly from the encodeJPEG method.

Test script has also been altered to spot this.

Fixes: b20dc09 ("fix buildroot crosscompile (raspberrypi#1)")
Signed-off-by: David Plowman <[email protected]>
naushir pushed a commit that referenced this pull request Apr 12, 2021
Fixing up some type warnings had caused the jpeg length not to be
returned correctly from the encodeJPEG method.

Test script has also been altered to spot this.

Fixes: b20dc09 ("fix buildroot crosscompile (#1)")
Signed-off-by: David Plowman <[email protected]>
davidplowman added a commit that referenced this pull request Apr 12, 2021
Fixing up some type warnings had caused the jpeg length not to be
returned correctly from the encodeJPEG method.

Test script has also been altered to spot this.

Fixes: b20dc09 ("fix buildroot crosscompile (#1)")
Signed-off-by: David Plowman <[email protected]>
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.

2 participants