[libcamera-devel] [PATCH 10/16] libcamera/base: Move File to base library
Kieran Bingham
kieran.bingham at ideasonboard.com
Fri Jun 25 16:42:06 CEST 2021
Hi Laurent,
On 25/06/2021 13:38, Laurent Pinchart wrote:
> Hi Kieran,
>
> Thank you for the patch.
>
> On Fri, Jun 25, 2021 at 02:35:33AM +0100, Kieran Bingham wrote:
>> The File abstraction is a base helper and not part of the libcamera
>> API. Move it to the base library to allow usage when reading and
>> writing to files.
>>
>> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>> ---
>> include/libcamera/{internal => base}/file.h | 6 +++---
>> include/libcamera/base/meson.build | 1 +
>> include/libcamera/internal/meson.build | 1 -
>> src/ipa/vimc/vimc.cpp | 11 ++++-------
>> src/libcamera/{ => base}/file.cpp | 4 ++--
>> src/libcamera/base/meson.build | 1 +
>> src/libcamera/ipa_manager.cpp | 2 +-
>> src/libcamera/ipa_module.cpp | 2 +-
>> src/libcamera/meson.build | 1 -
>> src/libcamera/sysfs.cpp | 3 +--
>> test/file.cpp | 2 +-
>> test/hotplug-cameras.cpp | 3 +--
>> 12 files changed, 16 insertions(+), 21 deletions(-)
>> rename include/libcamera/{internal => base}/file.h (91%)
>> rename src/libcamera/{ => base}/file.cpp (99%)
>>
>> diff --git a/include/libcamera/internal/file.h b/include/libcamera/base/file.h
>> similarity index 91%
>> rename from include/libcamera/internal/file.h
>> rename to include/libcamera/base/file.h
>> index 44621ceb4c19..e8e4b76e1a4e 100644
>> --- a/include/libcamera/internal/file.h
>> +++ b/include/libcamera/base/file.h
>> @@ -4,8 +4,8 @@
>> *
>> * file.h - File I/O operations
>> */
>> -#ifndef __LIBCAMERA_INTERNAL_FILE_H__
>> -#define __LIBCAMERA_INTERNAL_FILE_H__
>> +#ifndef __LIBCAMERA_BASE_FILE_H__
>> +#define __LIBCAMERA_BASE_FILE_H__
>>
>> #include <map>
>> #include <string>
>> @@ -75,4 +75,4 @@ private:
>>
>> } /* namespace libcamera */
>>
>> -#endif /* __LIBCAMERA_INTERNAL_FILE_H__ */
>> +#endif /* __LIBCAMERA_BASE_FILE_H__ */
>> diff --git a/include/libcamera/base/meson.build b/include/libcamera/base/meson.build
>> index 7a858dcb6f1a..6fc6c138a5fd 100644
>> --- a/include/libcamera/base/meson.build
>> +++ b/include/libcamera/base/meson.build
>> @@ -7,6 +7,7 @@ libcamera_base_headers = files([
>> 'class.h',
>> 'event_dispatcher.h',
>> 'event_dispatcher_poll.h',
>> + 'file.h',
>> 'log.h',
>> 'message.h',
>> 'object.h',
>> diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build
>> index 64f50373a7fb..b10285edac27 100644
>> --- a/include/libcamera/internal/meson.build
>> +++ b/include/libcamera/internal/meson.build
>> @@ -23,7 +23,6 @@ libcamera_internal_headers = files([
>> 'device_enumerator_sysfs.h',
>> 'device_enumerator_udev.h',
>> 'event_notifier.h',
>> - 'file.h',
>> 'formats.h',
>> 'ipa_manager.h',
>> 'ipa_module.h',
>> diff --git a/src/ipa/vimc/vimc.cpp b/src/ipa/vimc/vimc.cpp
>> index 9ffd07f493a1..cb9414e7b97a 100644
>> --- a/src/ipa/vimc/vimc.cpp
>> +++ b/src/ipa/vimc/vimc.cpp
>> @@ -5,21 +5,18 @@
>> * ipa_vimc.cpp - Vimc Image Processing Algorithm module
>> */
>>
>> -#include <libcamera/ipa/vimc_ipa_interface.h>
>> -
>
> I think this was done on purpose to ensure self-contained compilation
> test of libcamera/ipa/vimc_ipa_interface.h.
>
Clang-format will keep an interface file 'first' if it's in "" as
opposed to <>. That said, I think it needs to be the same name as the
.cpp file to match too so even that might not work here.
I'm going to propose later changing includes which are required at the
top for interfaces to use "" to be able to handle this better.
I'll squash this one back for now.
>> #include <fcntl.h>
>> +#include <iostream>
>> #include <string.h>
>> #include <sys/stat.h>
>> #include <unistd.h>
>>
>> -#include <iostream>
>> +#include <libcamera/base/file.h>
>> +#include <libcamera/base/log.h>
>>
>> #include <libcamera/ipa/ipa_interface.h>
>> #include <libcamera/ipa/ipa_module_info.h>
>> -
>> -#include <libcamera/base/log.h>
>> -
>> -#include "libcamera/internal/file.h"
>> +#include <libcamera/ipa/vimc_ipa_interface.h>
>>
>> namespace libcamera {
>>
>> diff --git a/src/libcamera/file.cpp b/src/libcamera/base/file.cpp
>> similarity index 99%
>> rename from src/libcamera/file.cpp
>> rename to src/libcamera/base/file.cpp
>> index def0f60d044b..073666fa6f66 100644
>> --- a/src/libcamera/file.cpp
>> +++ b/src/libcamera/base/file.cpp
>> @@ -5,7 +5,7 @@
>> * file.cpp - File I/O operations
>> */
>>
>> -#include "libcamera/internal/file.h"
>> +#include <libcamera/base/file.h>
>>
>> #include <errno.h>
>> #include <fcntl.h>
>> @@ -17,7 +17,7 @@
>> #include <libcamera/base/log.h>
>>
>> /**
>> - * \file file.h
>> + * \file base/file.h
>> * \brief File I/O operations
>> */
>>
>> diff --git a/src/libcamera/base/meson.build b/src/libcamera/base/meson.build
>> index 7a19c67c51b8..fb8ed79acd8e 100644
>> --- a/src/libcamera/base/meson.build
>> +++ b/src/libcamera/base/meson.build
>> @@ -5,6 +5,7 @@ libcamera_base_sources = files([
>> 'bound_method.cpp',
>> 'event_dispatcher.cpp',
>> 'event_dispatcher_poll.cpp',
>> + 'file.cpp',
>> 'log.cpp',
>> 'message.cpp',
>> 'object.cpp',
>> diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp
>> index 35c7259801fa..b4606c6159e5 100644
>> --- a/src/libcamera/ipa_manager.cpp
>> +++ b/src/libcamera/ipa_manager.cpp
>> @@ -12,10 +12,10 @@
>> #include <string.h>
>> #include <sys/types.h>
>>
>> +#include <libcamera/base/file.h>
>> #include <libcamera/base/log.h>
>> #include <libcamera/base/utils.h>
>>
>> -#include "libcamera/internal/file.h"
>> #include "libcamera/internal/ipa_module.h"
>> #include "libcamera/internal/ipa_proxy.h"
>> #include "libcamera/internal/pipeline_handler.h"
>> diff --git a/src/libcamera/ipa_module.cpp b/src/libcamera/ipa_module.cpp
>> index 7ab5557916e7..984c1fed9bdb 100644
>> --- a/src/libcamera/ipa_module.cpp
>> +++ b/src/libcamera/ipa_module.cpp
>> @@ -23,10 +23,10 @@
>>
>> #include <libcamera/span.h>
>>
>> +#include <libcamera/base/file.h>
>> #include <libcamera/base/log.h>
>> #include <libcamera/base/utils.h>
>>
>> -#include "libcamera/internal/file.h"
>> #include "libcamera/internal/pipeline_handler.h"
>>
>> /**
>> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
>> index 86212cec0281..58eee14aed97 100644
>> --- a/src/libcamera/meson.build
>> +++ b/src/libcamera/meson.build
>> @@ -16,7 +16,6 @@ libcamera_sources = files([
>> 'device_enumerator.cpp',
>> 'device_enumerator_sysfs.cpp',
>> 'event_notifier.cpp',
>> - 'file.cpp',
>> 'file_descriptor.cpp',
>> 'formats.cpp',
>> 'framebuffer_allocator.cpp',
>> diff --git a/src/libcamera/sysfs.cpp b/src/libcamera/sysfs.cpp
>> index 4372b1348178..44c3331b4e1c 100644
>> --- a/src/libcamera/sysfs.cpp
>> +++ b/src/libcamera/sysfs.cpp
>> @@ -12,10 +12,9 @@
>> #include <sys/stat.h>
>> #include <sys/sysmacros.h>
>>
>> +#include <libcamera/base/file.h>
>> #include <libcamera/base/log.h>
>>
>> -#include "libcamera/internal/file.h"
>> -
>> /**
>> * \file sysfs.h
>> * \brief Miscellaneous utility functions to access sysfs
>> diff --git a/test/file.cpp b/test/file.cpp
>> index b80667ae5b2f..d768e3235b8c 100644
>> --- a/test/file.cpp
>> +++ b/test/file.cpp
>> @@ -13,7 +13,7 @@
>> #include <sys/types.h>
>> #include <unistd.h>
>>
>> -#include "libcamera/internal/file.h"
>> +#include <libcamera/base/file.h>
>>
>> #include "test.h"
>>
>> diff --git a/test/hotplug-cameras.cpp b/test/hotplug-cameras.cpp
>> index bb160537c5d5..df56040350c5 100644
>> --- a/test/hotplug-cameras.cpp
>> +++ b/test/hotplug-cameras.cpp
>> @@ -15,11 +15,10 @@
>> #include <libcamera/camera_manager.h>
>>
>> #include <libcamera/base/event_dispatcher.h>
>> +#include <libcamera/base/file.h>
>> #include <libcamera/base/thread.h>
>> #include <libcamera/base/timer.h>
>>
>> -#include "libcamera/internal/file.h"
>> -
>
> It's nice to see tests having less dependencies on internal headers. On
> top of this series we should move tests from internal_tests to
> public_tests where possible.
Yes, I suspect some 'base' tests should even move so they only have
visibility of the base library too, so there might be more that can be
done to re-organise the tests.
Though - using 'File' in this case means LIBCAMERA_BASE_PRIVATE is still
required ...
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>
>> #include "test.h"
>>
>> using namespace libcamera;
>
More information about the libcamera-devel
mailing list