[libcamera-devel] [PATCH v2] apps: Share common source between applications
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Mon Oct 24 22:11:34 CEST 2022
Hi Kieran,
On Mon, Oct 24, 2022 at 08:56:56PM +0100, Kieran Bingham wrote:
> Quoting Laurent Pinchart via libcamera-devel (2022-10-20 23:10:26)
> > Multiple source files in the src/apps/cam/ directory are used by cam,
> > qcam and lc-compliance. They are compiled separately for each
> > application. Move them to a new src/apps/common/ directory and compile
> > them in a static library to decrease the number of compilation
> > operations.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> > Changes since v1:
> >
> > - Move common code to src/apps/common/
> > ---
> > src/apps/cam/camera_session.cpp | 5 ++--
> > src/apps/cam/camera_session.h | 2 +-
> > src/apps/cam/drm.cpp | 2 +-
> > src/apps/cam/file_sink.cpp | 5 ++--
> > src/apps/cam/main.cpp | 7 +++---
> > src/apps/cam/meson.build | 14 ++---------
> > src/apps/cam/sdl_sink.cpp | 5 ++--
> > src/apps/cam/sdl_texture.h | 2 +-
> > src/apps/{cam => common}/dng_writer.cpp | 0
> > src/apps/{cam => common}/dng_writer.h | 0
> > src/apps/{cam => common}/event_loop.cpp | 0
> > src/apps/{cam => common}/event_loop.h | 0
> > src/apps/{cam => common}/image.cpp | 0
> > src/apps/{cam => common}/image.h | 0
> > src/apps/common/meson.build | 26 +++++++++++++++++++++
> > src/apps/{cam => common}/options.cpp | 0
> > src/apps/{cam => common}/options.h | 0
> > src/apps/{cam => common}/stream_options.cpp | 0
> > src/apps/{cam => common}/stream_options.h | 0
> > src/apps/lc-compliance/main.cpp | 3 ++-
> > src/apps/lc-compliance/meson.build | 3 +--
> > src/apps/lc-compliance/simple_capture.h | 2 +-
> > src/apps/meson.build | 2 ++
> > src/apps/qcam/format_converter.cpp | 2 +-
> > src/apps/qcam/main.cpp | 5 ++--
> > src/apps/qcam/main_window.cpp | 4 ++--
> > src/apps/qcam/main_window.h | 2 +-
> > src/apps/qcam/meson.build | 13 ++---------
> > src/apps/qcam/viewfinder_gl.cpp | 2 +-
> > src/apps/qcam/viewfinder_qt.cpp | 2 +-
> > 30 files changed, 61 insertions(+), 47 deletions(-)
> > rename src/apps/{cam => common}/dng_writer.cpp (100%)
> > rename src/apps/{cam => common}/dng_writer.h (100%)
> > rename src/apps/{cam => common}/event_loop.cpp (100%)
> > rename src/apps/{cam => common}/event_loop.h (100%)
> > rename src/apps/{cam => common}/image.cpp (100%)
> > rename src/apps/{cam => common}/image.h (100%)
> > create mode 100644 src/apps/common/meson.build
> > rename src/apps/{cam => common}/options.cpp (100%)
> > rename src/apps/{cam => common}/options.h (100%)
> > rename src/apps/{cam => common}/stream_options.cpp (100%)
> > rename src/apps/{cam => common}/stream_options.h (100%)
> >
> > diff --git a/src/apps/cam/camera_session.cpp b/src/apps/cam/camera_session.cpp
> > index 6b409c983b86..8fcec6304d66 100644
> > --- a/src/apps/cam/camera_session.cpp
> > +++ b/src/apps/cam/camera_session.cpp
> > @@ -13,9 +13,11 @@
> > #include <libcamera/control_ids.h>
> > #include <libcamera/property_ids.h>
> >
> > +#include "../common/event_loop.h"
> > +#include "../common/stream_options.h"
> > +
>
> I still dislike the includes from a relative parent path.
>
> Is it possible to fix with the dependency or static library apps_lib
> object? Or just something you didn't want to do?
I thought about it. It would involve -I, #include <...>, and the risk of
conflict with system includes. I think it's still a good idea, but I
didn't want to explore that rabbit hole right now. If someone wants to
send a patch on top, I'll be happy to review it :-)
> Anyway, with or without that, grouping this makes sense.
>
>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>
> > #include "camera_session.h"
> > #include "capture_script.h"
> > -#include "event_loop.h"
> > #include "file_sink.h"
> > #ifdef HAVE_KMS
> > #include "kms_sink.h"
> > @@ -24,7 +26,6 @@
> > #ifdef HAVE_SDL
> > #include "sdl_sink.h"
> > #endif
> > -#include "stream_options.h"
> >
> > using namespace libcamera;
> >
> > diff --git a/src/apps/cam/camera_session.h b/src/apps/cam/camera_session.h
> > index d562caae0794..0bab519f9efd 100644
> > --- a/src/apps/cam/camera_session.h
> > +++ b/src/apps/cam/camera_session.h
> > @@ -21,7 +21,7 @@
> > #include <libcamera/request.h>
> > #include <libcamera/stream.h>
> >
> > -#include "options.h"
> > +#include "../common/options.h"
> >
> > class CaptureScript;
> > class FrameSink;
> > diff --git a/src/apps/cam/drm.cpp b/src/apps/cam/drm.cpp
> > index 2e4d7985245d..8779a7137f80 100644
> > --- a/src/apps/cam/drm.cpp
> > +++ b/src/apps/cam/drm.cpp
> > @@ -24,7 +24,7 @@
> >
> > #include <libdrm/drm_mode.h>
> >
> > -#include "event_loop.h"
> > +#include "../common/event_loop.h"
> >
> > namespace DRM {
> >
> > diff --git a/src/apps/cam/file_sink.cpp b/src/apps/cam/file_sink.cpp
> > index 9d60c04e1cf4..4525fbada7c4 100644
> > --- a/src/apps/cam/file_sink.cpp
> > +++ b/src/apps/cam/file_sink.cpp
> > @@ -15,9 +15,10 @@
> >
> > #include <libcamera/camera.h>
> >
> > -#include "dng_writer.h"
> > +#include "../common/dng_writer.h"
> > +#include "../common/image.h"
> > +
> > #include "file_sink.h"
> > -#include "image.h"
> >
> > using namespace libcamera;
> >
> > diff --git a/src/apps/cam/main.cpp b/src/apps/cam/main.cpp
> > index d70130e2ab81..5d8a57bc14e5 100644
> > --- a/src/apps/cam/main.cpp
> > +++ b/src/apps/cam/main.cpp
> > @@ -14,11 +14,12 @@
> > #include <libcamera/libcamera.h>
> > #include <libcamera/property_ids.h>
> >
> > +#include "../common/event_loop.h"
> > +#include "../common/options.h"
> > +#include "../common/stream_options.h"
> > +
> > #include "camera_session.h"
> > -#include "event_loop.h"
> > #include "main.h"
> > -#include "options.h"
> > -#include "stream_options.h"
> >
> > using namespace libcamera;
> >
> > diff --git a/src/apps/cam/meson.build b/src/apps/cam/meson.build
> > index 297de64fbdd9..48c834ace71b 100644
> > --- a/src/apps/cam/meson.build
> > +++ b/src/apps/cam/meson.build
> > @@ -10,16 +10,12 @@ cam_enabled = true
> > cam_sources = files([
> > 'camera_session.cpp',
> > 'capture_script.cpp',
> > - 'event_loop.cpp',
> > 'file_sink.cpp',
> > 'frame_sink.cpp',
> > - 'image.cpp',
> > 'main.cpp',
> > - 'options.cpp',
> > - 'stream_options.cpp',
> > ])
> >
> > -cam_cpp_args = []
> > +cam_cpp_args = [apps_cpp_args]
> >
> > libdrm = dependency('libdrm', required : false)
> > libjpeg = dependency('libjpeg', required : false)
> > @@ -49,14 +45,8 @@ if libsdl2.found()
> > endif
> > endif
> >
> > -if libtiff.found()
> > - cam_cpp_args += ['-DHAVE_TIFF']
> > - cam_sources += files([
> > - 'dng_writer.cpp',
> > - ])
> > -endif
> > -
> > cam = executable('cam', cam_sources,
> > + link_with : apps_lib,
> > dependencies : [
> > libatomic,
> > libcamera_public,
> > diff --git a/src/apps/cam/sdl_sink.cpp b/src/apps/cam/sdl_sink.cpp
> > index ee177227dbca..a2f4abc1ec8c 100644
> > --- a/src/apps/cam/sdl_sink.cpp
> > +++ b/src/apps/cam/sdl_sink.cpp
> > @@ -19,8 +19,9 @@
> > #include <libcamera/camera.h>
> > #include <libcamera/formats.h>
> >
> > -#include "event_loop.h"
> > -#include "image.h"
> > +#include "../common/event_loop.h"
> > +#include "../common/image.h"
> > +
> > #ifdef HAVE_LIBJPEG
> > #include "sdl_texture_mjpg.h"
> > #endif
> > diff --git a/src/apps/cam/sdl_texture.h b/src/apps/cam/sdl_texture.h
> > index 6ccd85eab390..3993dd46ece7 100644
> > --- a/src/apps/cam/sdl_texture.h
> > +++ b/src/apps/cam/sdl_texture.h
> > @@ -11,7 +11,7 @@
> >
> > #include <SDL2/SDL.h>
> >
> > -#include "image.h"
> > +#include "../common/image.h"
> >
> > class SDLTexture
> > {
> > diff --git a/src/apps/cam/dng_writer.cpp b/src/apps/common/dng_writer.cpp
> > similarity index 100%
> > rename from src/apps/cam/dng_writer.cpp
> > rename to src/apps/common/dng_writer.cpp
> > diff --git a/src/apps/cam/dng_writer.h b/src/apps/common/dng_writer.h
> > similarity index 100%
> > rename from src/apps/cam/dng_writer.h
> > rename to src/apps/common/dng_writer.h
> > diff --git a/src/apps/cam/event_loop.cpp b/src/apps/common/event_loop.cpp
> > similarity index 100%
> > rename from src/apps/cam/event_loop.cpp
> > rename to src/apps/common/event_loop.cpp
> > diff --git a/src/apps/cam/event_loop.h b/src/apps/common/event_loop.h
> > similarity index 100%
> > rename from src/apps/cam/event_loop.h
> > rename to src/apps/common/event_loop.h
> > diff --git a/src/apps/cam/image.cpp b/src/apps/common/image.cpp
> > similarity index 100%
> > rename from src/apps/cam/image.cpp
> > rename to src/apps/common/image.cpp
> > diff --git a/src/apps/cam/image.h b/src/apps/common/image.h
> > similarity index 100%
> > rename from src/apps/cam/image.h
> > rename to src/apps/common/image.h
> > diff --git a/src/apps/common/meson.build b/src/apps/common/meson.build
> > new file mode 100644
> > index 000000000000..479326cdacbf
> > --- /dev/null
> > +++ b/src/apps/common/meson.build
> > @@ -0,0 +1,26 @@
> > +# SPDX-License-Identifier: CC0-1.0
> > +
> > +apps_sources = files([
> > + 'image.cpp',
> > + 'options.cpp',
> > + 'stream_options.cpp',
> > +])
> > +
> > +apps_cpp_args = []
> > +
> > +if libevent.found()
> > + apps_sources += files([
> > + 'event_loop.cpp',
> > + ])
> > +endif
> > +
> > +if libtiff.found()
> > + apps_cpp_args += ['-DHAVE_TIFF']
> > + apps_sources += files([
> > + 'dng_writer.cpp',
> > + ])
> > +endif
> > +
> > +apps_lib = static_library('apps', apps_sources,
> > + cpp_args : apps_cpp_args,
> > + dependencies : [libcamera_public])
> > diff --git a/src/apps/cam/options.cpp b/src/apps/common/options.cpp
> > similarity index 100%
> > rename from src/apps/cam/options.cpp
> > rename to src/apps/common/options.cpp
> > diff --git a/src/apps/cam/options.h b/src/apps/common/options.h
> > similarity index 100%
> > rename from src/apps/cam/options.h
> > rename to src/apps/common/options.h
> > diff --git a/src/apps/cam/stream_options.cpp b/src/apps/common/stream_options.cpp
> > similarity index 100%
> > rename from src/apps/cam/stream_options.cpp
> > rename to src/apps/common/stream_options.cpp
> > diff --git a/src/apps/cam/stream_options.h b/src/apps/common/stream_options.h
> > similarity index 100%
> > rename from src/apps/cam/stream_options.h
> > rename to src/apps/common/stream_options.h
> > diff --git a/src/apps/lc-compliance/main.cpp b/src/apps/lc-compliance/main.cpp
> > index 7eb52ae4c094..74e0d4df461b 100644
> > --- a/src/apps/lc-compliance/main.cpp
> > +++ b/src/apps/lc-compliance/main.cpp
> > @@ -14,8 +14,9 @@
> >
> > #include <libcamera/libcamera.h>
> >
> > +#include "../common/options.h"
> > +
> > #include "environment.h"
> > -#include "../cam/options.h"
> >
> > using namespace libcamera;
> >
> > diff --git a/src/apps/lc-compliance/meson.build b/src/apps/lc-compliance/meson.build
> > index 05d622be0a40..51d9075ac30b 100644
> > --- a/src/apps/lc-compliance/meson.build
> > +++ b/src/apps/lc-compliance/meson.build
> > @@ -11,8 +11,6 @@ endif
> > lc_compliance_enabled = true
> >
> > lc_compliance_sources = files([
> > - '../cam/event_loop.cpp',
> > - '../cam/options.cpp',
> > 'environment.cpp',
> > 'main.cpp',
> > 'simple_capture.cpp',
> > @@ -21,6 +19,7 @@ lc_compliance_sources = files([
> >
> > lc_compliance = executable('lc-compliance', lc_compliance_sources,
> > cpp_args : [ '-fexceptions' ],
> > + link_with : apps_lib,
> > dependencies : [
> > libatomic,
> > libcamera_public,
> > diff --git a/src/apps/lc-compliance/simple_capture.h b/src/apps/lc-compliance/simple_capture.h
> > index 9d31f7cb2e53..fd9d2a97fd8d 100644
> > --- a/src/apps/lc-compliance/simple_capture.h
> > +++ b/src/apps/lc-compliance/simple_capture.h
> > @@ -11,7 +11,7 @@
> >
> > #include <libcamera/libcamera.h>
> >
> > -#include "../cam/event_loop.h"
> > +#include "../common/event_loop.h"
> >
> > class SimpleCapture
> > {
> > diff --git a/src/apps/meson.build b/src/apps/meson.build
> > index b722a8f110d8..099876356bd1 100644
> > --- a/src/apps/meson.build
> > +++ b/src/apps/meson.build
> > @@ -12,6 +12,8 @@ endif
> >
> > libtiff = dependency('libtiff-4', required : false)
> >
> > +subdir('common')
> > +
> > subdir('lc-compliance')
> >
> > subdir('cam')
> > diff --git a/src/apps/qcam/format_converter.cpp b/src/apps/qcam/format_converter.cpp
> > index 9331da0ce7a3..de76b32c7ffd 100644
> > --- a/src/apps/qcam/format_converter.cpp
> > +++ b/src/apps/qcam/format_converter.cpp
> > @@ -14,7 +14,7 @@
> >
> > #include <libcamera/formats.h>
> >
> > -#include "../cam/image.h"
> > +#include "../common/image.h"
> >
> > #define RGBSHIFT 8
> > #ifndef MAX
> > diff --git a/src/apps/qcam/main.cpp b/src/apps/qcam/main.cpp
> > index d3f01a85f1fb..36cb93a53701 100644
> > --- a/src/apps/qcam/main.cpp
> > +++ b/src/apps/qcam/main.cpp
> > @@ -13,8 +13,9 @@
> >
> > #include <libcamera/camera_manager.h>
> >
> > -#include "../cam/options.h"
> > -#include "../cam/stream_options.h"
> > +#include "../common/options.h"
> > +#include "../common/stream_options.h"
> > +
> > #include "main_window.h"
> > #include "message_handler.h"
> >
> > diff --git a/src/apps/qcam/main_window.cpp b/src/apps/qcam/main_window.cpp
> > index f553ccb01805..fb2db4aad511 100644
> > --- a/src/apps/qcam/main_window.cpp
> > +++ b/src/apps/qcam/main_window.cpp
> > @@ -26,8 +26,8 @@
> > #include <QToolButton>
> > #include <QtDebug>
> >
> > -#include "../cam/dng_writer.h"
> > -#include "../cam/image.h"
> > +#include "../common/dng_writer.h"
> > +#include "../common/image.h"
> >
> > #include "cam_select_dialog.h"
> > #ifndef QT_NO_OPENGL
> > diff --git a/src/apps/qcam/main_window.h b/src/apps/qcam/main_window.h
> > index 95b64124336f..2e3e1b5c83c2 100644
> > --- a/src/apps/qcam/main_window.h
> > +++ b/src/apps/qcam/main_window.h
> > @@ -27,7 +27,7 @@
> > #include <QQueue>
> > #include <QTimer>
> >
> > -#include "../cam/stream_options.h"
> > +#include "../common/stream_options.h"
> >
> > #include "viewfinder.h"
> >
> > diff --git a/src/apps/qcam/meson.build b/src/apps/qcam/meson.build
> > index e298101e2c43..eb0712d91351 100644
> > --- a/src/apps/qcam/meson.build
> > +++ b/src/apps/qcam/meson.build
> > @@ -15,9 +15,6 @@ endif
> > qcam_enabled = true
> >
> > qcam_sources = files([
> > - '../cam/image.cpp',
> > - '../cam/options.cpp',
> > - '../cam/stream_options.cpp',
> > 'cam_select_dialog.cpp',
> > 'format_converter.cpp',
> > 'main.cpp',
> > @@ -36,14 +33,7 @@ qcam_resources = files([
> > 'assets/feathericons/feathericons.qrc',
> > ])
> >
> > -qt5_cpp_args = ['-DQT_NO_KEYWORDS']
> > -
> > -if libtiff.found()
> > - qt5_cpp_args += ['-DHAVE_TIFF']
> > - qcam_sources += files([
> > - '../cam/dng_writer.cpp',
> > - ])
> > -endif
> > +qt5_cpp_args = [apps_cpp_args, '-DQT_NO_KEYWORDS']
> >
> > if cxx.has_header_symbol('QOpenGLWidget', 'QOpenGLWidget',
> > dependencies : qt5_dep, args : '-fPIC')
> > @@ -73,6 +63,7 @@ resources = qt5.preprocess(moc_headers: qcam_moc_headers,
> >
> > qcam = executable('qcam', qcam_sources, resources,
> > install : true,
> > + link_with : apps_lib,
> > dependencies : [
> > libatomic,
> > libcamera_public,
> > diff --git a/src/apps/qcam/viewfinder_gl.cpp b/src/apps/qcam/viewfinder_gl.cpp
> > index 38ddad58e09e..f83b99ad6272 100644
> > --- a/src/apps/qcam/viewfinder_gl.cpp
> > +++ b/src/apps/qcam/viewfinder_gl.cpp
> > @@ -16,7 +16,7 @@
> >
> > #include <libcamera/formats.h>
> >
> > -#include "../cam/image.h"
> > +#include "../common/image.h"
> >
> > static const QList<libcamera::PixelFormat> supportedFormats{
> > /* YUV - packed (single plane) */
> > diff --git a/src/apps/qcam/viewfinder_qt.cpp b/src/apps/qcam/viewfinder_qt.cpp
> > index c20fd6bc8fc2..a7482bea6afd 100644
> > --- a/src/apps/qcam/viewfinder_qt.cpp
> > +++ b/src/apps/qcam/viewfinder_qt.cpp
> > @@ -20,7 +20,7 @@
> > #include <QPainter>
> > #include <QtDebug>
> >
> > -#include "../cam/image.h"
> > +#include "../common/image.h"
> >
> > #include "format_converter.h"
> >
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list