[libcamera-devel] [PATCH v3 3/5] libcamera: camera: Generate camera name from a CameraSensor
Jacopo Mondi
jacopo at jmondi.org
Wed Jul 29 10:07:43 CEST 2020
Hi Niklas,
On Wed, Jul 29, 2020 at 01:42:23AM +0200, Niklas Söderlund wrote:
> Add a create() that generates a camera name from information in a
> CameraSensor. The intention is to help pipeline handlers that already
> uses CameraSensor to not have to worry about how generate unique and
s/how generate/how to generate/
> persistent name for cameras.
or "a unique and persistent name" or "unique and persistent names".
Source: it 'sound' more correct to me, but as a non-native speaker it
might just be me.
>
> All pipelines that implements a CameraSensor (all but UVC) are updated
s/implements/implement
And I would say "use" not implement.
> to make use of this new function. All names of cameras created by these
> updated pipelines are modified.
>
> Before this change example of camera names:
>
> * OF based systems
> ov5695 7-0036
> ov2685 7-003c
>
> * ACPI based systems
> ov13858 8-0010
> ov5670 10-0036
>
> After this change the same cameras are:
>
> * OF based systems
> base/i2c at ff160000/camera at 36
> base/i2c at ff160000/camera at 3c
>
> * ACPI based systems
> \_SB_.PCI0.I2C2.CAM0
> \_SB_.PCI0.I2C4.CAM1
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
> include/libcamera/camera.h | 5 +++++
> src/libcamera/camera.cpp | 19 +++++++++++++++++++
> src/libcamera/pipeline/ipu3/ipu3.cpp | 12 +++++-------
> .../pipeline/raspberrypi/raspberrypi.cpp | 3 ++-
> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 +-
> src/libcamera/pipeline/simple/simple.cpp | 2 +-
> src/libcamera/pipeline/vimc/vimc.cpp | 4 ++--
> 7 files changed, 35 insertions(+), 12 deletions(-)
>
> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h
> index 4d1a4a9f52ec0fac..784510c9a79d44b9 100644
> --- a/include/libcamera/camera.h
> +++ b/include/libcamera/camera.h
> @@ -19,6 +19,7 @@
>
> namespace libcamera {
>
> +class CameraSensor;
> class FrameBuffer;
> class FrameBufferAllocator;
> class PipelineHandler;
> @@ -73,6 +74,10 @@ public:
> const std::string &name,
> const std::set<Stream *> &streams);
>
> + static std::shared_ptr<Camera> create(PipelineHandler *pipe,
> + const CameraSensor *sensor,
Should this be a const & ? Or is this because most pipeline handlers
have pointers and it's nicer to pass the pointer without having to
dereference it ?
> + const std::set<Stream *> &streams);
> +
> Camera(const Camera &) = delete;
> Camera &operator=(const Camera &) = delete;
>
> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> index 69a1b4428e3f4eca..6ad746bc2ed4ee0a 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -11,9 +11,11 @@
> #include <iomanip>
>
> #include <libcamera/framebuffer_allocator.h>
> +#include <libcamera/property_ids.h>
Why property_ids.h ?
The rest looks good
Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
Thanks
j
> #include <libcamera/request.h>
> #include <libcamera/stream.h>
>
> +#include "libcamera/internal/camera_sensor.h"
> #include "libcamera/internal/log.h"
> #include "libcamera/internal/pipeline_handler.h"
> #include "libcamera/internal/utils.h"
> @@ -473,6 +475,23 @@ std::shared_ptr<Camera> Camera::create(PipelineHandler *pipe,
> return std::shared_ptr<Camera>(camera, Deleter());
> }
>
> +/**
> + * \brief Create a camera instance
> + * \param[in] pipe The pipeline handler responsible for the camera device
> + * \param[in] sensor The sensor of the camera device
> + * \param[in] streams Array of streams the camera provides
> + *
> + * Create a camera with name generated from \a sensor.
> + *
> + * \return A shared pointer to the newly created camera object
> + */
> +std::shared_ptr<Camera> Camera::create(PipelineHandler *pipe,
> + const CameraSensor *sensor,
> + const std::set<Stream *> &streams)
> +{
> + return Camera::create(pipe, sensor->id(), streams);
> +}
> +
> /**
> * \brief Retrieve the name of the camera
> * \context This function is \threadsafe.
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index eb00eecfd10a89e4..72da6ed62a7f0de5 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -813,18 +813,16 @@ int PipelineHandlerIPU3::registerCameras()
> &IPU3CameraData::imguOutputBufferReady);
>
> /* Create and register the Camera instance. */
> - std::string cameraName = cio2->sensor()->entity()->name();
> - std::shared_ptr<Camera> camera = Camera::create(this,
> - cameraName,
> - streams);
> -
> - registerCamera(std::move(camera), std::move(data));
> + std::shared_ptr<Camera> camera =
> + Camera::create(this, cio2->sensor(), streams);
>
> LOG(IPU3, Info)
> << "Registered Camera[" << numCameras << "] \""
> - << cameraName << "\""
> + << camera->name() << "\""
> << " connected to CSI-2 receiver " << id;
>
> + registerCamera(std::move(camera), std::move(data));
> +
> numCameras++;
> }
>
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index 82a0a4dfd6824fce..a62dd24b1ab76b87 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -972,7 +972,8 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)
> streams.insert(&data->isp_[Isp::Stats]);
>
> /* Create and register the camera. */
> - std::shared_ptr<Camera> camera = Camera::create(this, data->sensor_->model(), streams);
> + std::shared_ptr<Camera> camera =
> + Camera::create(this, data->sensor_, streams);
> registerCamera(std::move(camera), std::move(data));
>
> return true;
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 52a0d862417cc4ec..663e45b109aae9eb 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -971,7 +971,7 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)
>
> std::set<Stream *> streams{ &data->stream_ };
> std::shared_ptr<Camera> camera =
> - Camera::create(this, sensor->name(), streams);
> + Camera::create(this, data->sensor_, streams);
> registerCamera(std::move(camera), std::move(data));
>
> return 0;
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index 0bab5af86f05d63c..1258f81284590060 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -815,7 +815,7 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)
> continue;
>
> std::shared_ptr<Camera> camera =
> - Camera::create(this, data->sensor_->entity()->name(),
> + Camera::create(this, data->sensor_.get(),
> data->streams());
> registerCamera(std::move(camera), std::move(data));
> }
> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
> index 4f461b928514022d..e1fc087f111b0bc4 100644
> --- a/src/libcamera/pipeline/vimc/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc/vimc.cpp
> @@ -432,9 +432,9 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)
> return false;
>
> /* Create and register the camera. */
> - std::string name{ "VIMC " + data->sensor_->model() };
> std::set<Stream *> streams{ &data->stream_ };
> - std::shared_ptr<Camera> camera = Camera::create(this, name, streams);
> + std::shared_ptr<Camera> camera
> + = Camera::create(this, data->sensor_, streams);
> registerCamera(std::move(camera), std::move(data));
>
> return true;
> --
> 2.27.0
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
More information about the libcamera-devel
mailing list