[libcamera-devel] [RFC PATCH 10/17] libcamera: pipeline: simple: Migrate to Camera::Private
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Sat Jul 24 21:34:06 CEST 2021
Hi Niklas,
On Sat, Jul 24, 2021 at 09:24:45AM +0200, Niklas Söderlund wrote:
> On 2021-07-23 07:00:29 +0300, Laurent Pinchart wrote:
> > As part of the effort to remove the CameraData class, migrate the
> > pipeline handler-specific camera data from CameraData to the
> > Camera::Private class.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> > src/libcamera/pipeline/simple/simple.cpp | 25 ++++++++++++------------
> > 1 file changed, 13 insertions(+), 12 deletions(-)
> >
> > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> > index 43af3fafa475..81d342a3fa73 100644
> > --- a/src/libcamera/pipeline/simple/simple.cpp
> > +++ b/src/libcamera/pipeline/simple/simple.cpp
> > @@ -147,7 +147,7 @@ static const SimplePipelineInfo supportedDevices[] = {
> >
> > } /* namespace */
> >
> > -class SimpleCameraData : public CameraData
> > +class SimpleCameraData : public Camera::Private
> > {
> > public:
> > SimpleCameraData(SimplePipelineHandler *pipe,
> > @@ -213,7 +213,7 @@ private:
> > * reference to the camera data, store a new reference to the camera.
> > */
> > std::shared_ptr<Camera> camera_;
> > - const SimpleCameraData *data_;
> > + SimpleCameraData *data_;
>
> This looks odd, I can't figure out why const is dropped. Looking back
> should not Camera::Private::pipe() have a const version? It should be OK
> to get the pipe for RO operations.
The following changes end up being needed:
diff --git a/include/libcamera/internal/camera.h b/include/libcamera/internal/camera.h
index 44242332a52f..a20ace75fe8b 100644
--- a/include/libcamera/internal/camera.h
+++ b/include/libcamera/internal/camera.h
@@ -30,6 +30,7 @@ public:
~Private();
PipelineHandler *pipe() { return pipe_.get(); }
+ const PipelineHandler *pipe() const { return pipe_.get(); }
std::list<Request *> queuedRequests_;
ControlInfoMap controlInfo_;
diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
index 72a42dbeb3d3..dec4efbfe993 100644
--- a/src/libcamera/camera.cpp
+++ b/src/libcamera/camera.cpp
@@ -361,11 +361,15 @@ Camera::Private::~Private()
}
/**
- * \fn Camera::Private::pipe()
+ * \fn Camera::Private::pipe() const
* \brief Retrieve the pipeline handler related to this camera
* \return The pipeline handler for this camera
*/
+/**
+ * \fn Camera::Private::pipe()
+ * \copydoc Camera::Private::pipe() const
+ */
/**
* \var Camera::Private::queuedRequests_
* \brief The list of queued and not yet completed request
diff --git a/src/libcamera/pipeline/simple/converter.cpp b/src/libcamera/pipeline/simple/converter.cpp
index b5e34c4cd0c5..6a92e89a1867 100644
--- a/src/libcamera/pipeline/simple/converter.cpp
+++ b/src/libcamera/pipeline/simple/converter.cpp
@@ -278,7 +278,7 @@ SizeRange SimpleConverter::sizes(const Size &input)
std::tuple<unsigned int, unsigned int>
SimpleConverter::strideAndFrameSize(const PixelFormat &pixelFormat,
- const Size &size)
+ const Size &size) const
{
V4L2DeviceFormat format;
format.fourcc = m2m_->capture()->toV4L2PixelFormat(pixelFormat);
diff --git a/src/libcamera/pipeline/simple/converter.h b/src/libcamera/pipeline/simple/converter.h
index 276a2a291c21..960c102091be 100644
--- a/src/libcamera/pipeline/simple/converter.h
+++ b/src/libcamera/pipeline/simple/converter.h
@@ -40,7 +40,7 @@ public:
SizeRange sizes(const Size &input);
std::tuple<unsigned int, unsigned int>
- strideAndFrameSize(const PixelFormat &pixelFormat, const Size &size);
+ strideAndFrameSize(const PixelFormat &pixelFormat, const Size &size) const;
int configure(const StreamConfiguration &inputCfg,
const std::vector<std::reference_wrapper<StreamConfiguration>> &outputCfg);
diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
index 8594520f520c..84bc2723f42b 100644
--- a/src/libcamera/pipeline/simple/simple.cpp
+++ b/src/libcamera/pipeline/simple/simple.cpp
@@ -213,7 +213,7 @@ private:
* reference to the camera data, store a new reference to the camera.
*/
std::shared_ptr<Camera> camera_;
- SimpleCameraData *data_;
+ const SimpleCameraData *data_;
const SimpleCameraData::Configuration *pipeConfig_;
bool needConversion_;
@@ -239,6 +239,7 @@ public:
V4L2VideoDevice *video(const MediaEntity *entity);
V4L2Subdevice *subdev(const MediaEntity *entity);
SimpleConverter *converter() { return converter_.get(); }
+ const SimpleConverter *converter() const { return converter_.get(); }
protected:
int queueRequestDevice(Camera *camera, Request *request) override;
@@ -582,8 +583,8 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()
}
/* Adjust the requested streams. */
- SimplePipelineHandler *pipe = static_cast<SimplePipelineHandler *>(data_->pipe());
- SimpleConverter *converter = pipe->converter();
+ const SimplePipelineHandler *pipe = static_cast<const SimplePipelineHandler *>(data_->pipe());
+ const SimpleConverter *converter = pipe->converter();
/*
* Enable usage of the converter when producing multiple streams, as
I'm thinking about storing a pointer to the Camera in
CameraConfiguration, which will allow accessing the pipeline handler to
be accessed through Camera::Private. That's why I didn't want to
introduce this change yet.
> >
> > const SimpleCameraData::Configuration *pipeConfig_;
> > bool needConversion_;
> > @@ -246,10 +246,9 @@ protected:
> > private:
> > static constexpr unsigned int kNumInternalBuffers = 3;
> >
> > - SimpleCameraData *cameraData(const Camera *camera)
> > + SimpleCameraData *cameraData(Camera *camera)
> > {
> > - return static_cast<SimpleCameraData *>(
> > - PipelineHandler::cameraData(camera));
> > + return static_cast<SimpleCameraData *>(camera->_d());
> > }
> >
> > std::vector<MediaEntity *> locateSensors();
> > @@ -274,7 +273,7 @@ private:
> > SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe,
> > unsigned int numStreams,
> > MediaEntity *sensor)
> > - : CameraData(pipe), streams_(numStreams)
> > + : Camera::Private(pipe), streams_(numStreams)
> > {
> > int ret;
> >
> > @@ -355,7 +354,8 @@ SimpleCameraData::SimpleCameraData(SimplePipelineHandler *pipe,
> >
> > int SimpleCameraData::init()
> > {
> > - SimplePipelineHandler *pipe = static_cast<SimplePipelineHandler *>(pipe_);
> > + SimplePipelineHandler *pipe =
> > + static_cast<SimplePipelineHandler *>(this->pipe());
> > SimpleConverter *converter = pipe->converter();
> > int ret;
> >
> > @@ -480,7 +480,8 @@ int SimpleCameraData::setupLinks()
> > int SimpleCameraData::setupFormats(V4L2SubdeviceFormat *format,
> > V4L2Subdevice::Whence whence)
> > {
> > - SimplePipelineHandler *pipe = static_cast<SimplePipelineHandler *>(pipe_);
> > + SimplePipelineHandler *pipe =
> > + static_cast<SimplePipelineHandler *>(this->pipe());
> > int ret;
> >
> > /*
> > @@ -581,7 +582,7 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()
> > }
> >
> > /* Adjust the requested streams. */
> > - SimplePipelineHandler *pipe = static_cast<SimplePipelineHandler *>(data_->pipe_);
> > + SimplePipelineHandler *pipe = static_cast<SimplePipelineHandler *>(data_->pipe());
> > SimpleConverter *converter = pipe->converter();
> >
> > /*
> > @@ -1046,10 +1047,10 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)
> > std::inserter(streams, streams.end()),
> > [](Stream &stream) { return &stream; });
> >
> > + const std::string &id = data->sensor_->id();
> > std::shared_ptr<Camera> camera =
> > - Camera::create(new Camera::Private(this),
> > - data->sensor_->id(), streams);
> > - registerCamera(std::move(camera), std::move(data));
> > + Camera::create(data.release(), id, streams);
> > + registerCamera(std::move(camera), nullptr);
> > registered = true;
> > }
> >
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list