[libcamera-devel] [PATCH] android: Make libyaml dependency optional
Roman Stratiienko
r.stratiienko at gmail.com
Wed Dec 29 10:45:17 CET 2021
Hi Laurent,
вт, 28 дек. 2021 г. в 20:27, Laurent Pinchart
<laurent.pinchart at ideasonboard.com>:
>
> Hi Roman,
>
> On Tue, Dec 28, 2021 at 01:22:34PM +0200, Roman Stratiienko wrote:
> > вт, 28 дек. 2021 г., 13:04 Jacopo Mondi:
> > > Hi Roman,
> > >
> > > cc Hanlin Chen from ChromiumOS as he's planning to extend
> > > configuration file support for the ChromeOS Android HAL.
> >
> > > Before looking into the patch: is there any hope libyaml might become
> > > part of the standard AOSP distribution (or a process in place to request
> > > that)
> >
> > I am not aware of that, and I haven't initiated such a process. It will be
> > a very slow and bumpy road.
> >
> > Not sure what is planned for these properties in future, but now it is not
> > necessary to have it in libcamera to have fully functional hal. Also having
> > no configuration file available will encourage developers to add
> > corresponding records to dts, which is also a great thing to align with
> > linux world.
>
> Our goals align here. This is exactly why we have decided not to set the
> camera location in a configuration file for the libcamera core, but to
> isolate it in the HAL as a temporary (but in practice likely a long-term
> temporary) measure, as we have to support platforms that will take some
> time before a firmware (ACPI in this case) update can be deployed.
>
> This being said, we will introduce a configuration file for the
> libcamera core in the near future, and we also plan to use libyaml
> there. The tuning files for the IPU3 and RkISP1 IPA modules will also
> use YAML, and I'd like to move the Raspberry Pi tuning files from JSON
> to YAML for consistency.
It's a good idea to have a single file format. Currently we have to
ship boost to run raspberrypi cameras on Android.
> We could merge this patch in the meantime, but compilation on Android
> will likely break again soon. I wonder if we couldn't instead use a
> meson wrap to include libyaml as a subproject. This should solve the
> issue in the longer term.
At this moment we have libyaml with extra Android.bp, which can
satisfy libcamera needs.
But I really like to see it as an optional dependency only to help
fine-tuning things or for ACPI-only platforms.
> By the way, out of curiosity, how do you compile libcamera for Android ?
> We have a look at Soong (as the Makefile-based build system is being
> phased out), but blueprint files don't support all our needs (and would
> be annoying to maintain manually), and there's no way we found to
> generate Android.bp files from meson in a way that would integrate with
> Soong. This is still an unsolved problem, and I'm increasingly tempted
> to do the same as mesa3d and just document that libcamera should be
> built outside of AOSP, using the NDK.
We shipped libcamera in our GloDroid project v0.7.0 release for
raspberry pi about a month ago:
1. https://github.com/GloDroid/glodroid_manifest/releases/tag/v0.7.0
2. https://github.com/GloDroid/glodroid_forks/commits/libcamera-v0.7.0
For building using meson within AOSP we are using some tricks to
extract dependencies from AOSP makesystem and inject them into
meson cross compilation file.
First such kind of approach was integrated into mesa3d and replaced
existing Android.mk files (about 6 month ago).
It doesn't require continuous maintenance as previous Android.mk did
and has proven to be reliable.
It also enables FULL support of mesa3d for Android. Previously only
about 60% of mesa3d drivers were covered with Android.mk files.
Reworking the same approach for libcamera was pretty easy.
And yes, as an official approach for vendors NDK builds would be great
to support.
NDK approach and our meson.build within AOSP approach are 98% aligned.
>
> > > On Tue, Dec 28, 2021 at 11:51:53AM +0200, Roman Stratiienko wrote:
> > > > Mainline Android has no libyaml in-tree. Moreover camera_hal_config.cpp
> > > > uses std::filesystem class that is not permitted for AOSP VNDK vendor
> > > > libraries.
> > > >
> > > > External configuration file is no longer mandatory and DTS overlays can
> > > > be used to specify camera location and rotation.
> > > >
> > > > Do not require external configuration file for cases where meson.build
> > > > can't find libyaml. Assume that cameras without location specified
> > > > by V4L2 properties are always external.
> > > >
> > > > Signed-off-by: Roman Stratiienko <roman.o.stratiienko at globallogic.com>
> > > > ---
> > > > src/android/camera_hal_manager.cpp | 13 +++++++++++++
> > > > src/android/camera_hal_manager.h | 2 ++
> > > > src/android/meson.build | 10 ++++++++--
> > > > 3 files changed, 23 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/src/android/camera_hal_manager.cpp b/src/android/camera_hal_manager.cpp
> > > > index 5f7bfe265c71b..9e575bfa5204a 100644
> > > > --- a/src/android/camera_hal_manager.cpp
> > > > +++ b/src/android/camera_hal_manager.cpp
> > > > @@ -48,6 +48,7 @@ int CameraHalManager::init()
> > > > {
> > > > cameraManager_ = std::make_unique<CameraManager>();
> > > >
> > > > +#ifdef HAVE_LIBYAML
> > > > /*
> > > > * If the configuration file is not available the HAL only supports
> > > > * external cameras. If it exists but it's not valid then error out.
> > > > @@ -56,6 +57,7 @@ int CameraHalManager::init()
> > > > LOG(HAL, Error) << "HAL configuration file is not valid";
> > > > return -EINVAL;
> > > > }
> > > > +#endif
> > > >
> > > > /* Support camera hotplug. */
> > > > cameraManager_->cameraAdded.connect(this, &CameraHalManager::cameraAdded);
> > > > @@ -133,6 +135,7 @@ void CameraHalManager::cameraAdded(std::shared_ptr<Camera> cam)
> > > > }
> > > > }
> > > >
> > > > +#ifdef HAVE_LIBYAML
> > > > /*
> > > > * The configuration file must be valid, and contain a corresponding
> > > > * entry for internal cameras. External cameras can be initialized
> > > > @@ -145,6 +148,16 @@ void CameraHalManager::cameraAdded(std::shared_ptr<Camera> cam)
> > > > }
> > > >
> > > > const CameraConfigData *cameraConfigData = halConfig_.cameraConfigData(cam->id());
> > > > +#else
> > > > + /*
> > > > + * Assume that cameras without properties::Location specified are external.
> > > > + */
> > > > + const CameraConfigData *cameraConfigData = nullptr;
> > > > + if (!isCameraExternal && cameraLocation(cam.get()) < 0) {
> > > > + isCameraExternal = true;
> > > > + id = nextExternalCameraId_;
> > > > + }
> > > > +#endif
> > > >
> > > > /*
> > > > * Some cameras whose location is reported by libcamera as external may
> > > > diff --git a/src/android/camera_hal_manager.h b/src/android/camera_hal_manager.h
> > > > index a5f8b933a7902..056abbb0d4e54 100644
> > > > --- a/src/android/camera_hal_manager.h
> > > > +++ b/src/android/camera_hal_manager.h
> > > > @@ -56,7 +56,9 @@ private:
> > > > CameraDevice *cameraDeviceFromHalId(unsigned int id) LIBCAMERA_TSA_REQUIRES(mutex_);
> > > >
> > > > std::unique_ptr<libcamera::CameraManager> cameraManager_;
> > > > +#ifdef HAVE_LIBYAML
> > > > CameraHalConfig halConfig_;
> > > > +#endif
> > > >
> > > > const camera_module_callbacks_t *callbacks_;
> > > > std::vector<std::unique_ptr<CameraDevice>> cameras_ LIBCAMERA_TSA_GUARDED_BY(mutex_);
> > > > diff --git a/src/android/meson.build b/src/android/meson.build
> > > > index 75b4bf2070851..1e471c291a1e4 100644
> > > > --- a/src/android/meson.build
> > > > +++ b/src/android/meson.build
> > > > @@ -3,7 +3,6 @@
> > > > android_deps = [
> > > > dependency('libexif', required : get_option('android')),
> > > > dependency('libjpeg', required : get_option('android')),
> > > > - dependency('yaml-0.1', required : get_option('android')),
> > > > libcamera_private,
> > > > ]
> > > >
> > > > @@ -41,7 +40,6 @@ android_hal_sources = files([
> > > > 'camera3_hal.cpp',
> > > > 'camera_capabilities.cpp',
> > > > 'camera_device.cpp',
> > > > - 'camera_hal_config.cpp',
> > > > 'camera_hal_manager.cpp',
> > > > 'camera_metadata.cpp',
> > > > 'camera_ops.cpp',
> > > > @@ -56,6 +54,14 @@ android_hal_sources = files([
> > > >
> > > > android_cpp_args = []
> > > >
> > > > +libyaml_dep = dependency('yaml-0.1', required : false)
> > > > +if libyaml_dep.found()
> > > > + config_h.set('HAVE_LIBYAML', 1)
> > > > + android_hal_sources += files([
> > > > + 'camera_hal_config.cpp',
> > > > + ])
> > > > +endif
> > > > +
> > > > subdir('cros')
> > > > subdir('mm')
> > > >
>
> --
> Regards,
>
> Laurent Pinchart
Regards.
Roman
More information about the libcamera-devel
mailing list