[libcamera-devel] [PATCH v2 02/11] libcamera: pipeline_handler: Move CameraData members to Camera::Private
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Thu Aug 5 19:58:39 CEST 2021
With pipeline handlers now being able to subclass Camera::Private, start
the migration from CameraData to Camera::Private by moving the members
of the base CameraData class. The controlInfo_, properties_ and pipe_
members are duplicated for now, to allow migrating pipeline handlers one
by one.
The Camera::Private class is now properly documented, don't exclude it
from documentation generation.
Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
---
Changes since v1:
- Add \todo comment for controlInfo_
---
Documentation/Doxyfile.in | 1 -
include/libcamera/internal/camera.h | 8 +++
include/libcamera/internal/pipeline_handler.h | 5 +-
src/libcamera/camera.cpp | 65 ++++++++++++++++++-
src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 +-
src/libcamera/pipeline_handler.cpp | 45 +++++--------
6 files changed, 88 insertions(+), 38 deletions(-)
diff --git a/Documentation/Doxyfile.in b/Documentation/Doxyfile.in
index fb321ad25f6f..e3b77428cd4f 100644
--- a/Documentation/Doxyfile.in
+++ b/Documentation/Doxyfile.in
@@ -882,7 +882,6 @@ EXCLUDE_SYMBOLS = libcamera::BoundMethodArgs \
libcamera::BoundMethodPack \
libcamera::BoundMethodPackBase \
libcamera::BoundMethodStatic \
- libcamera::Camera::Private \
libcamera::CameraManager::Private \
libcamera::SignalBase \
*::details \
diff --git a/include/libcamera/internal/camera.h b/include/libcamera/internal/camera.h
index 9ec8321a9a21..44242332a52f 100644
--- a/include/libcamera/internal/camera.h
+++ b/include/libcamera/internal/camera.h
@@ -29,6 +29,14 @@ public:
Private(PipelineHandler *pipe);
~Private();
+ PipelineHandler *pipe() { return pipe_.get(); }
+
+ std::list<Request *> queuedRequests_;
+ ControlInfoMap controlInfo_;
+ ControlList properties_;
+
+ uint32_t requestSequence_;
+
private:
enum State {
CameraAvailable,
diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h
index 9e2d65d6f2c5..86727f90bb65 100644
--- a/include/libcamera/internal/pipeline_handler.h
+++ b/include/libcamera/internal/pipeline_handler.h
@@ -39,18 +39,15 @@ class CameraData
{
public:
explicit CameraData(PipelineHandler *pipe)
- : pipe_(pipe), requestSequence_(0)
+ : pipe_(pipe)
{
}
virtual ~CameraData() = default;
PipelineHandler *pipe_;
- std::list<Request *> queuedRequests_;
ControlInfoMap controlInfo_;
ControlList properties_;
- uint32_t requestSequence_;
-
private:
LIBCAMERA_DISABLE_COPY(CameraData)
};
diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
index a5bb60c698bc..f0893f89a1b3 100644
--- a/src/libcamera/camera.cpp
+++ b/src/libcamera/camera.cpp
@@ -332,13 +332,25 @@ std::size_t CameraConfiguration::size() const
* \brief The vector of stream configurations
*/
+/**
+ * \class Camera::Private
+ * \brief Base class for camera private data
+ *
+ * The Camera::Private class stores all private data associated with a camera.
+ * In addition to hiding core Camera data from the public API, it is expected to
+ * be subclassed by pipeline handlers to store pipeline-specific data.
+ *
+ * Pipeline handlers can obtain the Camera::Private instance associated with a
+ * camera by calling Camera::_d().
+ */
+
/**
* \brief Construct a Camera::Private instance
* \param[in] pipe The pipeline handler responsible for the camera device
*/
Camera::Private::Private(PipelineHandler *pipe)
- : pipe_(pipe->shared_from_this()), disconnected_(false),
- state_(CameraAvailable)
+ : requestSequence_(0), pipe_(pipe->shared_from_this()),
+ disconnected_(false), state_(CameraAvailable)
{
}
@@ -348,6 +360,55 @@ Camera::Private::~Private()
LOG(Camera, Error) << "Removing camera while still in use";
}
+/**
+ * \fn Camera::Private::pipe()
+ * \brief Retrieve the pipeline handler related to this camera
+ * \return The pipeline handler for this camera
+ */
+
+/**
+ * \var Camera::Private::queuedRequests_
+ * \brief The list of queued and not yet completed request
+ *
+ * The list of queued request is used to track requests queued in order to
+ * ensure completion of all requests when the pipeline handler is stopped.
+ *
+ * \sa PipelineHandler::queueRequest(), PipelineHandler::stop(),
+ * PipelineHandler::completeRequest()
+ */
+
+/**
+ * \var Camera::Private::controlInfo_
+ * \brief The set of controls supported by the camera
+ *
+ * The control information shall be initialised by the pipeline handler when
+ * creating the camera.
+ *
+ * \todo This member was initially meant to stay constant after the camera is
+ * created. Several pipeline handlers are already updating it when the camera
+ * is configured. Update the documentation accordingly, and possibly the API as
+ * well, when implementing official support for control info updates.
+ */
+
+/**
+ * \var Camera::Private::properties_
+ * \brief The list of properties supported by the camera
+ *
+ * The list of camera properties shall be initialised by the pipeline handler
+ * when creating the camera, and shall not be modified afterwards.
+ */
+
+/**
+ * \var Camera::Private::requestSequence_
+ * \brief The queuing sequence of the request
+ *
+ * When requests are queued, they are given a per-camera sequence number to
+ * facilitate debugging of internal request usage.
+ *
+ * The requestSequence_ tracks the number of requests queued to a camera
+ * over its lifetime.
+ */
+
static const char *const camera_state_names[] = {
"Available",
"Acquired",
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index 4a8ac97d5ef0..cc279ce733ef 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -847,7 +847,7 @@ void PipelineHandlerRkISP1::stop(Camera *camera)
LOG(RkISP1, Warning)
<< "Failed to stop parameters for " << camera->id();
- ASSERT(data->queuedRequests_.empty());
+ ASSERT(camera->_d()->queuedRequests_.empty());
data->frameInfo_.clear();
freeBuffers(camera);
diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
index c9928d444b04..1f1de19d81c5 100644
--- a/src/libcamera/pipeline_handler.cpp
+++ b/src/libcamera/pipeline_handler.cpp
@@ -16,6 +16,7 @@
#include <libcamera/camera_manager.h>
#include <libcamera/framebuffer.h>
+#include "libcamera/internal/camera.h"
#include "libcamera/internal/device_enumerator.h"
#include "libcamera/internal/media_device.h"
#include "libcamera/internal/tracepoints.h"
@@ -71,17 +72,6 @@ LOG_DEFINE_CATEGORY(Pipeline)
* and remains valid until the instance is destroyed.
*/
-/**
- * \var CameraData::queuedRequests_
- * \brief The list of queued and not yet completed request
- *
- * The list of queued request is used to track requests queued in order to
- * ensure completion of all requests when the pipeline handler is stopped.
- *
- * \sa PipelineHandler::queueRequest(), PipelineHandler::stop(),
- * PipelineHandler::completeRequest()
- */
-
/**
* \var CameraData::controlInfo_
* \brief The set of controls supported by the camera
@@ -98,17 +88,6 @@ LOG_DEFINE_CATEGORY(Pipeline)
* when creating the camera, and shall not be modified afterwards.
*/
-/**
- * \var CameraData::requestSequence_
- * \brief The queuing sequence of the request
- *
- * When requests are queued, they are given a per-camera sequence number to
- * facilitate debugging of internal request usage.
- *
- * The requestSequence_ tracks the number of requests queued to a camera
- * over its lifetime.
- */
-
/**
* \class PipelineHandler
* \brief Create and manage cameras based on a set of media devices
@@ -254,8 +233,7 @@ void PipelineHandler::unlock()
*/
const ControlInfoMap &PipelineHandler::controls(const Camera *camera) const
{
- const CameraData *data = cameraData(camera);
- return data->controlInfo_;
+ return camera->_d()->controlInfo_;
}
/**
@@ -265,8 +243,7 @@ const ControlInfoMap &PipelineHandler::controls(const Camera *camera) const
*/
const ControlList &PipelineHandler::properties(const Camera *camera) const
{
- const CameraData *data = cameraData(camera);
- return data->properties_;
+ return camera->_d()->properties_;
}
/**
@@ -380,8 +357,7 @@ const ControlList &PipelineHandler::properties(const Camera *camera) const
*/
bool PipelineHandler::hasPendingRequests(const Camera *camera) const
{
- const CameraData *data = cameraData(camera);
- return !data->queuedRequests_.empty();
+ return !camera->_d()->queuedRequests_.empty();
}
/**
@@ -406,7 +382,7 @@ void PipelineHandler::queueRequest(Request *request)
LIBCAMERA_TRACEPOINT(request_queue, request);
Camera *camera = request->camera_;
- CameraData *data = cameraData(camera);
+ Camera::Private *data = camera->_d();
data->queuedRequests_.push_back(request);
request->sequence_ = data->requestSequence_++;
@@ -479,7 +455,7 @@ void PipelineHandler::completeRequest(Request *request)
request->complete();
- CameraData *data = cameraData(camera);
+ Camera::Private *data = camera->_d();
while (!data->queuedRequests_.empty()) {
Request *req = data->queuedRequests_.front();
@@ -507,6 +483,15 @@ void PipelineHandler::completeRequest(Request *request)
void PipelineHandler::registerCamera(std::shared_ptr<Camera> camera,
std::unique_ptr<CameraData> data)
{
+ /*
+ * To be removed once all pipeline handlers will have migrated from
+ * CameraData to Camera::Private.
+ */
+ if (data) {
+ camera->_d()->controlInfo_ = std::move(data->controlInfo_);
+ camera->_d()->properties_ = std::move(data->properties_);
+ }
+
cameraData_[camera.get()] = std::move(data);
cameras_.push_back(camera);
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list