[libcamera-devel] [PATCH v4 4/6] libcamera: camera: extend camera object to support streams
Niklas Söderlund
niklas.soderlund at ragnatech.se
Tue Jan 29 14:20:24 CET 2019
Hi Kieran,
Thanks for your feedback.
On 2019-01-29 10:05:42 +0000, Kieran Bingham wrote:
> Hi Niklas,
>
> On 29/01/2019 02:00, Niklas Söderlund wrote:
> > A camera consists of one or more video streams originating from the same
> > video source. The different streams could for example have access to
> > different hardware blocks in the video pipeline and therefore be able to
> > process the video source in different ways.
> >
> > All static information describing each stream need to be recorded at
> > camera creation. After a camera is created an application can retrieve
> > the static information about its stream at any time.
> >
> > Update all pipeline handlers to register one stream per camera, this
> > will be extended in the future for some of the pipelines.
> >
> > Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
>
> Just my usual Capitalisation comments of the Day here.
> The discussion point about reconnecting cameras is only that ... a
> discussion point.
I left it as is until we decide what to do.
>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>
> > ---
> > include/libcamera/camera.h | 8 ++++++-
> > src/libcamera/camera.cpp | 34 ++++++++++++++++++++++++----
> > src/libcamera/pipeline/ipu3/ipu3.cpp | 9 ++++++--
> > src/libcamera/pipeline/uvcvideo.cpp | 5 +++-
> > src/libcamera/pipeline/vimc.cpp | 5 +++-
> > 5 files changed, 52 insertions(+), 9 deletions(-)
> >
> > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h
> > index 7e358f8c0aa093cf..3ebb8e96cc63b98a 100644
> > --- a/include/libcamera/camera.h
> > +++ b/include/libcamera/camera.h
> > @@ -15,12 +15,14 @@
> > namespace libcamera {
> >
> > class PipelineHandler;
> > +class Stream;
> >
> > class Camera final
> > {
> > public:
> > static std::shared_ptr<Camera> create(PipelineHandler *pipe,
> > - const std::string &name);
> > + const std::string &name,
> > + const std::vector<Stream *> &streams);
> >
> > Camera(const Camera &) = delete;
> > void operator=(const Camera &) = delete;
> > @@ -32,6 +34,8 @@ public:
> > int acquire();
> > void release();
> >
> > + std::vector<Stream *> streams() const;
> > +
> > private:
> > Camera(PipelineHandler *pipe, const std::string &name);
> > ~Camera();
> > @@ -41,8 +45,10 @@ private:
> >
> > std::shared_ptr<PipelineHandler> pipe_;
> > std::string name_;
> > + std::vector<Stream *> streams_;
> >
> > bool acquired_;
> > + bool disconnected_;
> > };
> >
> > } /* namespace libcamera */
> > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> > index 500976b237bcbd2d..c8604ec8a8e6eaf4 100644
> > --- a/src/libcamera/camera.cpp
> > +++ b/src/libcamera/camera.cpp
> > @@ -6,6 +6,7 @@
> > */
> >
> > #include <libcamera/camera.h>
> > +#include <libcamera/stream.h>
> >
> > #include "log.h"
> > #include "pipeline_handler.h"
> > @@ -56,13 +57,15 @@ LOG_DECLARE_CATEGORY(Camera)
> > * \brief Create a camera instance
> > * \param[in] name The name of the camera device
> > * \param[in] pipe The pipeline handler responsible for the camera device
> > + * \param[in] streams Array of streams the camera provides
>
> "Streams" (except pluralisation is a pain in Doxygen)
>
> Here are some possible options to deal with this:
>
> A: Array of \link Stream streams \endlink provided by the Camera
> B: Array of streams (Stream) provided by the Camera
> C: Array of all Stream objects provided by the Camera
I think this adds value to our discussion, thanks for sharing your
findings.
>
>
>
> > *
> > * The caller is responsible for guaranteeing unicity of the camera name.
> > *
> > * \return A shared pointer to the newly created camera object
> > */
> > std::shared_ptr<Camera> Camera::create(PipelineHandler *pipe,
> > - const std::string &name)
> > + const std::string &name,
> > + const std::vector<Stream *> &streams)
> > {
> > struct Allocator : std::allocator<Camera> {
> > void construct(void *p, PipelineHandler *pipe,
> > @@ -76,7 +79,12 @@ std::shared_ptr<Camera> Camera::create(PipelineHandler *pipe,
> > }
> > };
> >
> > - return std::allocate_shared<Camera>(Allocator(), pipe, name);
> > + std::shared_ptr<Camera> camera =
> > + std::allocate_shared<Camera>(Allocator(), pipe, name);
> > +
> > + camera->streams_ = streams;
> > +
> > + return camera;
> > }
> >
> > /**
> > @@ -102,7 +110,8 @@ const std::string &Camera::name() const
> > */
> >
> > Camera::Camera(PipelineHandler *pipe, const std::string &name)
> > - : pipe_(pipe->shared_from_this()), name_(name), acquired_(false)
> > + : pipe_(pipe->shared_from_this()), name_(name), acquired_(false),
> > + disconnected_(false)
> > {
> > }
> >
> > @@ -125,7 +134,7 @@ void Camera::disconnect()
> > {
> > LOG(Camera, Debug) << "Disconnecting camera " << name_;
> >
> > - /** \todo Block API calls when they will be implemented. */
> > + disconnected_ = true;
> > disconnected.emit(this);
>
> Will we ever reconnect?
> Or will this object just be destroyed and a new one created?
>
> re-connecting an existing camera is tricky and requires keeping data
> around that perhaps we don't want (or maybe it would have a timeout?) -
> but it would mean an app handling a camera could automatically resume
> play in the event that a cable was pulled out and put back in again? (or
> a dodgy electrical link or such ... )
No, a camera will never be reconnected. If the media device returns a
new PipelineHandler will be created and new cameras created. The reason
we need this is so that an application can still access a Camera object
after the hardware is gone without accessing invalid pointer and instead
be notified by the camera object that it's in fact no longer usable.
>
>
> > }
> >
> > @@ -164,4 +173,21 @@ void Camera::release()
> > acquired_ = false;
> > }
> >
> > +/**
> > + * \brief Retrieve all the camera's stream information
> > + *
> > + * Retrieve all of the camera's static stream information. The static
> > + * information describes among other things how many streams the camera
> > + * support and each streams capabilities.
> > + *
> > + * \return An array of all the camera's streams valid streams.
> > + */
> > +std::vector<Stream *> Camera::streams() const
> > +{
> > + if (disconnected_)
> > + return std::vector<Stream *>{};
> > +
> > + return streams_;
> > +}
> > +
> > } /* namespace libcamera */
> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > index d74655d037728feb..f2029d17b4ad0273 100644
> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > @@ -9,6 +9,7 @@
> > #include <vector>
> >
> > #include <libcamera/camera.h>
> > +#include <libcamera/stream.h>
> >
> > #include "device_enumerator.h"
> > #include "log.h"
> > @@ -37,6 +38,7 @@ private:
> > : dev_(nullptr) {}
> > ~IPU3CameraData() { delete dev_; }
> > V4L2Device *dev_;
> > + Stream stream_;
> > };
> >
> > std::shared_ptr<MediaDevice> cio2_;
> > @@ -196,8 +198,11 @@ void PipelineHandlerIPU3::registerCameras()
> > if (link->setEnabled(true))
> > continue;
> >
> > + IPU3CameraData *data = new IPU3CameraData();
> > +
> > std::string cameraName = sensor->name() + " " + std::to_string(id);
> > - std::shared_ptr<Camera> camera = Camera::create(this, cameraName);
> > + std::vector<Stream *> streams{ &data->stream_ };
> > + std::shared_ptr<Camera> camera = Camera::create(this, cameraName, streams);
> >
> > /*
> > * If V4L2 device creation fails, the Camera instance won't be
> > @@ -209,10 +214,10 @@ void PipelineHandlerIPU3::registerCameras()
> > LOG(IPU3, Error)
> > << "Failed to register camera["
> > << numCameras << "] \"" << cameraName << "\"";
> > + delete data;
> > continue;
> > }
> >
> > - IPU3CameraData *data = new IPU3CameraData();
> > data->dev_ = videoDev;
> > setCameraData(camera.get(),
> > std::move(std::unique_ptr<IPU3CameraData>(data)));
> > diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
> > index c51e8bc1f2c2bf25..8ea7ac74d2a5395d 100644
> > --- a/src/libcamera/pipeline/uvcvideo.cpp
> > +++ b/src/libcamera/pipeline/uvcvideo.cpp
> > @@ -6,6 +6,7 @@
> > */
> >
> > #include <libcamera/camera.h>
> > +#include <libcamera/stream.h>
> >
> > #include "device_enumerator.h"
> > #include "media_device.h"
> > @@ -25,6 +26,7 @@ public:
> > private:
> > std::shared_ptr<MediaDevice> media_;
> > V4L2Device *video_;
> > + Stream stream_;
> > };
> >
> > PipelineHandlerUVC::PipelineHandlerUVC(CameraManager *manager)
> > @@ -64,7 +66,8 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)
> > return false;
> > }
> >
> > - std::shared_ptr<Camera> camera = Camera::create(this, media_->model());
> > + std::vector<Stream *> streams{ &stream_ };
> > + std::shared_ptr<Camera> camera = Camera::create(this, media_->model(), streams);
> > registerCamera(std::move(camera));
> > hotplugMediaDevice(media_.get());
> >
> > diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
> > index f58a97d51619515d..9e1cf11a20c7a7e3 100644
> > --- a/src/libcamera/pipeline/vimc.cpp
> > +++ b/src/libcamera/pipeline/vimc.cpp
> > @@ -6,6 +6,7 @@
> > */
> >
> > #include <libcamera/camera.h>
> > +#include <libcamera/stream.h>
> >
> > #include "device_enumerator.h"
> > #include "media_device.h"
> > @@ -23,6 +24,7 @@ public:
> >
> > private:
> > std::shared_ptr<MediaDevice> media_;
> > + Stream stream_;
> > };
> >
> > PipeHandlerVimc::PipeHandlerVimc(CameraManager *manager)
> > @@ -56,7 +58,8 @@ bool PipeHandlerVimc::match(DeviceEnumerator *enumerator)
> >
> > media_->acquire();
> >
> > - std::shared_ptr<Camera> camera = Camera::create(this, "Dummy VIMC Camera");
> > + std::vector<Stream *> streams{ &stream_ };
> > + std::shared_ptr<Camera> camera = Camera::create(this, "Dummy VIMC Camera", streams);
> > registerCamera(std::move(camera));
> >
> > return true;
> >
>
> --
> Regards
> --
> Kieran
--
Regards,
Niklas Söderlund
More information about the libcamera-devel
mailing list