| Message ID | 20250703114225.2074071-7-paul.elder@ideasonboard.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Quoting Paul Elder (2025-07-03 12:42:21) > Add hooks into the CameraManager to call into the LayerManager on all > relevant public-facing interface of Camera. > > The entry point for each function into the LayerManager is has been > chosen based on the capabilities that we want to allow to Layer > implementations. > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > > --- > Changes in v2: > - move the LayerManager out of Camera into the CameraManager > - remove hooks for generateConfiguration() and streams() > - Camera now passes itself into the layer hooks, as it is required for > the LayerManager to fetch the closure based on the camera to pass to > each layer > --- > src/libcamera/camera.cpp | 40 ++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 38 insertions(+), 2 deletions(-) > > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp > index c3e656cab671..665fc5157349 100644 > --- a/src/libcamera/camera.cpp > +++ b/src/libcamera/camera.cpp > @@ -18,6 +18,7 @@ > #include <libcamera/base/log.h> > #include <libcamera/base/thread.h> > > +#include <libcamera/camera_manager.h> > #include <libcamera/color_space.h> > #include <libcamera/control_ids.h> > #include <libcamera/framebuffer_allocator.h> > @@ -27,6 +28,8 @@ > > #include "libcamera/internal/camera.h" > #include "libcamera/internal/camera_controls.h" > +#include "libcamera/internal/camera_manager.h" > +#include "libcamera/internal/layer_manager.h" > #include "libcamera/internal/pipeline_handler.h" > #include "libcamera/internal/request.h" > > @@ -741,18 +744,24 @@ void Camera::Private::setState(State state) > void Camera::Private::emitBufferCompleted(Request *request, FrameBuffer *buffer) > { > Camera *camera = _o<Camera>(); > + LayerManager *layerManager = pipe()->cameraManager()->_d()->layerManager(); > + layerManager->bufferCompleted(camera, request, buffer); I think we need some sort of Layers controller in the Camera class directly. It should be responsible for the specific instances of the Layers and the ordering/enablement from the (future) camera specific configuration. With that - these additions would be much 'cleaner' as well as we could expect: layerController->bufferCompleted(...) Which would be a part of the Camera::Private or something rather than having to indirectly obtain the layerManager each time. Otherwise - if I've mis-understood and the layerManager is already doing that functionality - perhaps we should have a reference/instance to it in the Camera::Private class so we don't have to grab it tyhrough a pipe, cameraManager and private instance each time.... > camera->bufferCompleted.emit(request, buffer); > } > > void Camera::Private::emitRequestCompleted(Request *request) > { > Camera *camera = _o<Camera>(); > + LayerManager *layerManager = pipe()->cameraManager()->_d()->layerManager(); > + layerManager->requestCompleted(camera, request); > camera->requestCompleted.emit(request); > } > > void Camera::Private::emitDisconnected() > { > Camera *camera = _o<Camera>(); > + LayerManager *layerManager = pipe()->cameraManager()->_d()->layerManager(); > + layerManager->disconnected(camera); > camera->disconnected.emit(); > } > > @@ -943,10 +952,14 @@ Camera::Camera(std::unique_ptr<Private> d, const std::string &id, > _d()->id_ = id; > _d()->streams_ = streams; > _d()->validator_ = std::make_unique<CameraControlValidator>(this); > + _d()->pipe()->cameraManager()->_d()->layerManager()->init(this, > + _d()->properties_, > + _d()->controlInfo_); > } > > Camera::~Camera() > { > + _d()->pipe()->cameraManager()->_d()->layerManager()->terminate(this); Somehow I'd call the Camera::Private 'layers' though so we could do a nice short: layers->terminate(this); > } > > /** > @@ -1032,6 +1045,9 @@ int Camera::acquire() > return -EBUSY; > } > > + LayerManager *layerManager = d->pipe()->cameraManager()->_d()->layerManager(); > + layerManager->acquire(this); > + > d->setState(Private::CameraAcquired); > > return 0; > @@ -1064,6 +1080,9 @@ int Camera::release() > d->pipe_->invokeMethod(&PipelineHandler::release, > ConnectionTypeBlocking, this); > > + LayerManager *layerManager = d->pipe()->cameraManager()->_d()->layerManager(); > + layerManager->release(this); > + > d->setState(Private::CameraAvailable); > > return 0; > @@ -1081,7 +1100,8 @@ int Camera::release() > */ > const ControlInfoMap &Camera::controls() const > { > - return _d()->controlInfo_; > + LayerManager *layerManager = _d()->pipe()->cameraManager()->_d()->layerManager(); > + return layerManager->controls(this); Somehow I'm surprised we're not passing in _d()->controlInfo() here ... return layerManager->controls(_d()->controlInfo_); Does the layer manager access the existing camera control Info directly somehow? > } > > /** > @@ -1094,7 +1114,8 @@ const ControlInfoMap &Camera::controls() const > */ > const ControlList &Camera::properties() const > { > - return _d()->properties_; > + LayerManager *layerManager = _d()->pipe()->cameraManager()->_d()->layerManager(); > + return layerManager->properties(this); Same thoughts as controls but perhaps it's just handled differently to how I expect and the answer is elsewhere. > } > > /** > @@ -1242,6 +1263,9 @@ int Camera::configure(CameraConfiguration *config) > d->activeStreams_.insert(stream); > } > > + LayerManager *layerManager = d->pipe()->cameraManager()->_d()->layerManager(); > + layerManager->configure(this, config, d->controlInfo_); > + > d->setState(Private::CameraConfigured); > > return 0; > @@ -1282,6 +1306,9 @@ std::unique_ptr<Request> Camera::createRequest(uint64_t cookie) > /* Associate the request with the pipeline handler. */ > d->pipe_->registerRequest(request.get()); > > + LayerManager *layerManager = d->pipe()->cameraManager()->_d()->layerManager(); > + layerManager->createRequest(this, cookie, request.get()); > + > return request; > } > > @@ -1366,6 +1393,9 @@ int Camera::queueRequest(Request *request) > } > } > > + LayerManager *layerManager = d->pipe()->cameraManager()->_d()->layerManager(); > + layerManager->queueRequest(this, request); > + > d->pipe_->invokeMethod(&PipelineHandler::queueRequest, > ConnectionTypeQueued, request); > > @@ -1402,6 +1432,9 @@ int Camera::start(const ControlList *controls) > > ASSERT(d->requestSequence_ == 0); > > + LayerManager *layerManager = d->pipe()->cameraManager()->_d()->layerManager(); > + layerManager->start(this, controls); > + > ret = d->pipe_->invokeMethod(&PipelineHandler::start, > ConnectionTypeBlocking, this, controls); > if (ret) > @@ -1446,6 +1479,9 @@ int Camera::stop() > > d->setState(Private::CameraStopping); > > + LayerManager *layerManager = d->pipe()->cameraManager()->_d()->layerManager(); > + layerManager->stop(this); > + I wonder if symetrically - we should stop the layers after stopping the camera, as part of the stop call on the camera will invoke completion(or cancellation?)? I'm not entirely certain on this one - but it's just that as a wrapper I think I'd expect: layers->start() camera->start() camera->stop() layers->stop ? > d->pipe_->invokeMethod(&PipelineHandler::stop, ConnectionTypeBlocking, > this); > > -- > 2.47.2 >
Quoting Kieran Bingham (2025-07-23 18:13:08) > Quoting Paul Elder (2025-07-03 12:42:21) > > Add hooks into the CameraManager to call into the LayerManager on all > > relevant public-facing interface of Camera. > > > > The entry point for each function into the LayerManager is has been > > chosen based on the capabilities that we want to allow to Layer > > implementations. > > > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > > > > --- > > Changes in v2: > > - move the LayerManager out of Camera into the CameraManager Oh - interesting. I think we do need a top level loader - but probably per-camera instances of the Layers... > > - remove hooks for generateConfiguration() and streams() > > - Camera now passes itself into the layer hooks, as it is required for > > the LayerManager to fetch the closure based on the camera to pass to > > each layer > > --- > > src/libcamera/camera.cpp | 40 ++++++++++++++++++++++++++++++++++++++-- > > 1 file changed, 38 insertions(+), 2 deletions(-) > > > > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp > > index c3e656cab671..665fc5157349 100644 > > --- a/src/libcamera/camera.cpp > > +++ b/src/libcamera/camera.cpp > > @@ -18,6 +18,7 @@ > > #include <libcamera/base/log.h> > > #include <libcamera/base/thread.h> > > > > +#include <libcamera/camera_manager.h> > > #include <libcamera/color_space.h> > > #include <libcamera/control_ids.h> > > #include <libcamera/framebuffer_allocator.h> > > @@ -27,6 +28,8 @@ > > > > #include "libcamera/internal/camera.h" > > #include "libcamera/internal/camera_controls.h" > > +#include "libcamera/internal/camera_manager.h" > > +#include "libcamera/internal/layer_manager.h" > > #include "libcamera/internal/pipeline_handler.h" > > #include "libcamera/internal/request.h" > > > > @@ -741,18 +744,24 @@ void Camera::Private::setState(State state) > > void Camera::Private::emitBufferCompleted(Request *request, FrameBuffer *buffer) > > { > > Camera *camera = _o<Camera>(); > > + LayerManager *layerManager = pipe()->cameraManager()->_d()->layerManager(); > > + layerManager->bufferCompleted(camera, request, buffer); > > I think we need some sort of Layers controller in the Camera class > directly. It should be responsible for the specific instances of the > Layers and the ordering/enablement from the (future) camera specific > configuration. > > With that - these additions would be much 'cleaner' as well as we could > expect: > > layerController->bufferCompleted(...) > > Which would be a part of the Camera::Private or something rather than > having to indirectly obtain the layerManager each time. > > Otherwise - if I've mis-understood and the layerManager is already doing > that functionality - perhaps we should have a reference/instance to it > in the Camera::Private class so we don't have to grab it tyhrough a > pipe, cameraManager and private instance each time.... > > > camera->bufferCompleted.emit(request, buffer); > > } > > > > void Camera::Private::emitRequestCompleted(Request *request) > > { > > Camera *camera = _o<Camera>(); > > + LayerManager *layerManager = pipe()->cameraManager()->_d()->layerManager(); > > + layerManager->requestCompleted(camera, request); > > camera->requestCompleted.emit(request); > > } > > > > void Camera::Private::emitDisconnected() > > { > > Camera *camera = _o<Camera>(); > > + LayerManager *layerManager = pipe()->cameraManager()->_d()->layerManager(); > > + layerManager->disconnected(camera); > > camera->disconnected.emit(); > > } > > > > @@ -943,10 +952,14 @@ Camera::Camera(std::unique_ptr<Private> d, const std::string &id, > > _d()->id_ = id; > > _d()->streams_ = streams; > > _d()->validator_ = std::make_unique<CameraControlValidator>(this); > > + _d()->pipe()->cameraManager()->_d()->layerManager()->init(this, > > + _d()->properties_, > > + _d()->controlInfo_); > > } > > > > Camera::~Camera() > > { > > + _d()->pipe()->cameraManager()->_d()->layerManager()->terminate(this); > > Somehow I'd call the Camera::Private 'layers' though so we could > do a nice short: > > layers->terminate(this); > > > } > > > > /** > > @@ -1032,6 +1045,9 @@ int Camera::acquire() > > return -EBUSY; > > } > > > > + LayerManager *layerManager = d->pipe()->cameraManager()->_d()->layerManager(); > > + layerManager->acquire(this); > > + > > d->setState(Private::CameraAcquired); > > > > return 0; > > @@ -1064,6 +1080,9 @@ int Camera::release() > > d->pipe_->invokeMethod(&PipelineHandler::release, > > ConnectionTypeBlocking, this); > > > > + LayerManager *layerManager = d->pipe()->cameraManager()->_d()->layerManager(); > > + layerManager->release(this); > > + > > d->setState(Private::CameraAvailable); > > > > return 0; > > @@ -1081,7 +1100,8 @@ int Camera::release() > > */ > > const ControlInfoMap &Camera::controls() const > > { > > - return _d()->controlInfo_; > > + LayerManager *layerManager = _d()->pipe()->cameraManager()->_d()->layerManager(); > > + return layerManager->controls(this); > > Somehow I'm surprised we're not passing in _d()->controlInfo() here ... > return layerManager->controls(_d()->controlInfo_); > > Does the layer manager access the existing camera control Info directly > somehow? Wait - now I read the previous review comments. It's because the layers aren't going to modify the camera state - so they get indirected/merged of course... I'd forgotten about that. > > > } > > > > /** > > @@ -1094,7 +1114,8 @@ const ControlInfoMap &Camera::controls() const > > */ > > const ControlList &Camera::properties() const > > { > > - return _d()->properties_; > > + LayerManager *layerManager = _d()->pipe()->cameraManager()->_d()->layerManager(); > > + return layerManager->properties(this); > > Same thoughts as controls but perhaps it's just handled differently to > how I expect and the answer is elsewhere. And same here of course ;-) > > > > } > > > > /** > > @@ -1242,6 +1263,9 @@ int Camera::configure(CameraConfiguration *config) > > d->activeStreams_.insert(stream); > > } > > > > + LayerManager *layerManager = d->pipe()->cameraManager()->_d()->layerManager(); > > + layerManager->configure(this, config, d->controlInfo_); > > + > > d->setState(Private::CameraConfigured); > > > > return 0; > > @@ -1282,6 +1306,9 @@ std::unique_ptr<Request> Camera::createRequest(uint64_t cookie) > > /* Associate the request with the pipeline handler. */ > > d->pipe_->registerRequest(request.get()); > > > > + LayerManager *layerManager = d->pipe()->cameraManager()->_d()->layerManager(); > > + layerManager->createRequest(this, cookie, request.get()); > > + > > return request; > > } > > > > @@ -1366,6 +1393,9 @@ int Camera::queueRequest(Request *request) > > } > > } > > > > + LayerManager *layerManager = d->pipe()->cameraManager()->_d()->layerManager(); > > + layerManager->queueRequest(this, request); > > + > > d->pipe_->invokeMethod(&PipelineHandler::queueRequest, > > ConnectionTypeQueued, request); > > > > @@ -1402,6 +1432,9 @@ int Camera::start(const ControlList *controls) > > > > ASSERT(d->requestSequence_ == 0); > > > > + LayerManager *layerManager = d->pipe()->cameraManager()->_d()->layerManager(); > > + layerManager->start(this, controls); > > + > > ret = d->pipe_->invokeMethod(&PipelineHandler::start, > > ConnectionTypeBlocking, this, controls); > > if (ret) > > @@ -1446,6 +1479,9 @@ int Camera::stop() > > > > d->setState(Private::CameraStopping); > > > > + LayerManager *layerManager = d->pipe()->cameraManager()->_d()->layerManager(); > > + layerManager->stop(this); > > + > > I wonder if symetrically - we should stop the layers after stopping the > camera, as part of the stop call on the camera will invoke completion(or > cancellation?)? > > I'm not entirely certain on this one - but it's just that as a wrapper > I think I'd expect: > > layers->start() > camera->start() > camera->stop() > layers->stop > ? > > > d->pipe_->invokeMethod(&PipelineHandler::stop, ConnectionTypeBlocking, > > this); > > > > -- > > 2.47.2 > >
Quoting Kieran Bingham (2025-07-24 02:30:38) > Quoting Kieran Bingham (2025-07-23 18:13:08) > > Quoting Paul Elder (2025-07-03 12:42:21) > > > Add hooks into the CameraManager to call into the LayerManager on all > > > relevant public-facing interface of Camera. > > > > > > The entry point for each function into the LayerManager is has been > > > chosen based on the capabilities that we want to allow to Layer > > > implementations. > > > > > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > > > > > > --- > > > Changes in v2: > > > - move the LayerManager out of Camera into the CameraManager > > Oh - interesting. I think we do need a top level loader - but probably Yeah. > per-camera instances of the Layers... Hm yeah I guess we do need that in order to support per-camera layer queues. Might as well do it now since it's a pretty core architectural change. > > > > - remove hooks for generateConfiguration() and streams() > > > - Camera now passes itself into the layer hooks, as it is required for > > > the LayerManager to fetch the closure based on the camera to pass to > > > each layer > > > --- > > > src/libcamera/camera.cpp | 40 ++++++++++++++++++++++++++++++++++++++-- > > > 1 file changed, 38 insertions(+), 2 deletions(-) > > > > > > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp > > > index c3e656cab671..665fc5157349 100644 > > > --- a/src/libcamera/camera.cpp > > > +++ b/src/libcamera/camera.cpp > > > @@ -18,6 +18,7 @@ > > > #include <libcamera/base/log.h> > > > #include <libcamera/base/thread.h> > > > > > > +#include <libcamera/camera_manager.h> > > > #include <libcamera/color_space.h> > > > #include <libcamera/control_ids.h> > > > #include <libcamera/framebuffer_allocator.h> > > > @@ -27,6 +28,8 @@ > > > > > > #include "libcamera/internal/camera.h" > > > #include "libcamera/internal/camera_controls.h" > > > +#include "libcamera/internal/camera_manager.h" > > > +#include "libcamera/internal/layer_manager.h" > > > #include "libcamera/internal/pipeline_handler.h" > > > #include "libcamera/internal/request.h" > > > > > > @@ -741,18 +744,24 @@ void Camera::Private::setState(State state) > > > void Camera::Private::emitBufferCompleted(Request *request, FrameBuffer *buffer) > > > { > > > Camera *camera = _o<Camera>(); > > > + LayerManager *layerManager = pipe()->cameraManager()->_d()->layerManager(); > > > + layerManager->bufferCompleted(camera, request, buffer); > > > > I think we need some sort of Layers controller in the Camera class > > directly. It should be responsible for the specific instances of the > > Layers and the ordering/enablement from the (future) camera specific > > configuration. > > > > With that - these additions would be much 'cleaner' as well as we could > > expect: > > > > layerController->bufferCompleted(...) > > > > Which would be a part of the Camera::Private or something rather than > > having to indirectly obtain the layerManager each time. > > > > Otherwise - if I've mis-understood and the layerManager is already doing > > that functionality - perhaps we should have a reference/instance to it > > in the Camera::Private class so we don't have to grab it tyhrough a > > pipe, cameraManager and private instance each time.... > > > > > camera->bufferCompleted.emit(request, buffer); > > > } > > > > > > void Camera::Private::emitRequestCompleted(Request *request) > > > { > > > Camera *camera = _o<Camera>(); > > > + LayerManager *layerManager = pipe()->cameraManager()->_d()->layerManager(); > > > + layerManager->requestCompleted(camera, request); > > > camera->requestCompleted.emit(request); > > > } > > > > > > void Camera::Private::emitDisconnected() > > > { > > > Camera *camera = _o<Camera>(); > > > + LayerManager *layerManager = pipe()->cameraManager()->_d()->layerManager(); > > > + layerManager->disconnected(camera); > > > camera->disconnected.emit(); > > > } > > > > > > @@ -943,10 +952,14 @@ Camera::Camera(std::unique_ptr<Private> d, const std::string &id, > > > _d()->id_ = id; > > > _d()->streams_ = streams; > > > _d()->validator_ = std::make_unique<CameraControlValidator>(this); > > > + _d()->pipe()->cameraManager()->_d()->layerManager()->init(this, > > > + _d()->properties_, > > > + _d()->controlInfo_); > > > } > > > > > > Camera::~Camera() > > > { > > > + _d()->pipe()->cameraManager()->_d()->layerManager()->terminate(this); > > > > Somehow I'd call the Camera::Private 'layers' though so we could > > do a nice short: > > > > layers->terminate(this); > > > > > } > > > > > > /** > > > @@ -1032,6 +1045,9 @@ int Camera::acquire() > > > return -EBUSY; > > > } > > > > > > + LayerManager *layerManager = d->pipe()->cameraManager()->_d()->layerManager(); > > > + layerManager->acquire(this); > > > + > > > d->setState(Private::CameraAcquired); > > > > > > return 0; > > > @@ -1064,6 +1080,9 @@ int Camera::release() > > > d->pipe_->invokeMethod(&PipelineHandler::release, > > > ConnectionTypeBlocking, this); > > > > > > + LayerManager *layerManager = d->pipe()->cameraManager()->_d()->layerManager(); > > > + layerManager->release(this); > > > + > > > d->setState(Private::CameraAvailable); > > > > > > return 0; > > > @@ -1081,7 +1100,8 @@ int Camera::release() > > > */ > > > const ControlInfoMap &Camera::controls() const > > > { > > > - return _d()->controlInfo_; > > > + LayerManager *layerManager = _d()->pipe()->cameraManager()->_d()->layerManager(); > > > + return layerManager->controls(this); > > > > Somehow I'm surprised we're not passing in _d()->controlInfo() here ... > > return layerManager->controls(_d()->controlInfo_); > > > > Does the layer manager access the existing camera control Info directly > > somehow? > > Wait - now I read the previous review comments. It's because the layers > aren't going to modify the camera state - so they get indirected/merged > of course... > > I'd forgotten about that. > > > > > > } > > > > > > /** > > > @@ -1094,7 +1114,8 @@ const ControlInfoMap &Camera::controls() const > > > */ > > > const ControlList &Camera::properties() const > > > { > > > - return _d()->properties_; > > > + LayerManager *layerManager = _d()->pipe()->cameraManager()->_d()->layerManager(); > > > + return layerManager->properties(this); > > > > Same thoughts as controls but perhaps it's just handled differently to > > how I expect and the answer is elsewhere. > > And same here of course ;-) > > > > > > > > > } > > > > > > /** > > > @@ -1242,6 +1263,9 @@ int Camera::configure(CameraConfiguration *config) > > > d->activeStreams_.insert(stream); > > > } > > > > > > + LayerManager *layerManager = d->pipe()->cameraManager()->_d()->layerManager(); > > > + layerManager->configure(this, config, d->controlInfo_); > > > + > > > d->setState(Private::CameraConfigured); > > > > > > return 0; > > > @@ -1282,6 +1306,9 @@ std::unique_ptr<Request> Camera::createRequest(uint64_t cookie) > > > /* Associate the request with the pipeline handler. */ > > > d->pipe_->registerRequest(request.get()); > > > > > > + LayerManager *layerManager = d->pipe()->cameraManager()->_d()->layerManager(); > > > + layerManager->createRequest(this, cookie, request.get()); > > > + > > > return request; > > > } > > > > > > @@ -1366,6 +1393,9 @@ int Camera::queueRequest(Request *request) > > > } > > > } > > > > > > + LayerManager *layerManager = d->pipe()->cameraManager()->_d()->layerManager(); > > > + layerManager->queueRequest(this, request); > > > + > > > d->pipe_->invokeMethod(&PipelineHandler::queueRequest, > > > ConnectionTypeQueued, request); > > > > > > @@ -1402,6 +1432,9 @@ int Camera::start(const ControlList *controls) > > > > > > ASSERT(d->requestSequence_ == 0); > > > > > > + LayerManager *layerManager = d->pipe()->cameraManager()->_d()->layerManager(); > > > + layerManager->start(this, controls); > > > + > > > ret = d->pipe_->invokeMethod(&PipelineHandler::start, > > > ConnectionTypeBlocking, this, controls); > > > if (ret) > > > @@ -1446,6 +1479,9 @@ int Camera::stop() > > > > > > d->setState(Private::CameraStopping); > > > > > > + LayerManager *layerManager = d->pipe()->cameraManager()->_d()->layerManager(); > > > + layerManager->stop(this); > > > + > > > > I wonder if symetrically - we should stop the layers after stopping the > > camera, as part of the stop call on the camera will invoke completion(or > > cancellation?)? > > > > I'm not entirely certain on this one - but it's just that as a wrapper > > I think I'd expect: > > > > layers->start() > > camera->start() > > camera->stop() > > layers->stop > > ? Good point, yeah. Thanks, Paul > > > > > d->pipe_->invokeMethod(&PipelineHandler::stop, ConnectionTypeBlocking, > > > this); > > > > > > -- > > > 2.47.2 > > >
diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index c3e656cab671..665fc5157349 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -18,6 +18,7 @@ #include <libcamera/base/log.h> #include <libcamera/base/thread.h> +#include <libcamera/camera_manager.h> #include <libcamera/color_space.h> #include <libcamera/control_ids.h> #include <libcamera/framebuffer_allocator.h> @@ -27,6 +28,8 @@ #include "libcamera/internal/camera.h" #include "libcamera/internal/camera_controls.h" +#include "libcamera/internal/camera_manager.h" +#include "libcamera/internal/layer_manager.h" #include "libcamera/internal/pipeline_handler.h" #include "libcamera/internal/request.h" @@ -741,18 +744,24 @@ void Camera::Private::setState(State state) void Camera::Private::emitBufferCompleted(Request *request, FrameBuffer *buffer) { Camera *camera = _o<Camera>(); + LayerManager *layerManager = pipe()->cameraManager()->_d()->layerManager(); + layerManager->bufferCompleted(camera, request, buffer); camera->bufferCompleted.emit(request, buffer); } void Camera::Private::emitRequestCompleted(Request *request) { Camera *camera = _o<Camera>(); + LayerManager *layerManager = pipe()->cameraManager()->_d()->layerManager(); + layerManager->requestCompleted(camera, request); camera->requestCompleted.emit(request); } void Camera::Private::emitDisconnected() { Camera *camera = _o<Camera>(); + LayerManager *layerManager = pipe()->cameraManager()->_d()->layerManager(); + layerManager->disconnected(camera); camera->disconnected.emit(); } @@ -943,10 +952,14 @@ Camera::Camera(std::unique_ptr<Private> d, const std::string &id, _d()->id_ = id; _d()->streams_ = streams; _d()->validator_ = std::make_unique<CameraControlValidator>(this); + _d()->pipe()->cameraManager()->_d()->layerManager()->init(this, + _d()->properties_, + _d()->controlInfo_); } Camera::~Camera() { + _d()->pipe()->cameraManager()->_d()->layerManager()->terminate(this); } /** @@ -1032,6 +1045,9 @@ int Camera::acquire() return -EBUSY; } + LayerManager *layerManager = d->pipe()->cameraManager()->_d()->layerManager(); + layerManager->acquire(this); + d->setState(Private::CameraAcquired); return 0; @@ -1064,6 +1080,9 @@ int Camera::release() d->pipe_->invokeMethod(&PipelineHandler::release, ConnectionTypeBlocking, this); + LayerManager *layerManager = d->pipe()->cameraManager()->_d()->layerManager(); + layerManager->release(this); + d->setState(Private::CameraAvailable); return 0; @@ -1081,7 +1100,8 @@ int Camera::release() */ const ControlInfoMap &Camera::controls() const { - return _d()->controlInfo_; + LayerManager *layerManager = _d()->pipe()->cameraManager()->_d()->layerManager(); + return layerManager->controls(this); } /** @@ -1094,7 +1114,8 @@ const ControlInfoMap &Camera::controls() const */ const ControlList &Camera::properties() const { - return _d()->properties_; + LayerManager *layerManager = _d()->pipe()->cameraManager()->_d()->layerManager(); + return layerManager->properties(this); } /** @@ -1242,6 +1263,9 @@ int Camera::configure(CameraConfiguration *config) d->activeStreams_.insert(stream); } + LayerManager *layerManager = d->pipe()->cameraManager()->_d()->layerManager(); + layerManager->configure(this, config, d->controlInfo_); + d->setState(Private::CameraConfigured); return 0; @@ -1282,6 +1306,9 @@ std::unique_ptr<Request> Camera::createRequest(uint64_t cookie) /* Associate the request with the pipeline handler. */ d->pipe_->registerRequest(request.get()); + LayerManager *layerManager = d->pipe()->cameraManager()->_d()->layerManager(); + layerManager->createRequest(this, cookie, request.get()); + return request; } @@ -1366,6 +1393,9 @@ int Camera::queueRequest(Request *request) } } + LayerManager *layerManager = d->pipe()->cameraManager()->_d()->layerManager(); + layerManager->queueRequest(this, request); + d->pipe_->invokeMethod(&PipelineHandler::queueRequest, ConnectionTypeQueued, request); @@ -1402,6 +1432,9 @@ int Camera::start(const ControlList *controls) ASSERT(d->requestSequence_ == 0); + LayerManager *layerManager = d->pipe()->cameraManager()->_d()->layerManager(); + layerManager->start(this, controls); + ret = d->pipe_->invokeMethod(&PipelineHandler::start, ConnectionTypeBlocking, this, controls); if (ret) @@ -1446,6 +1479,9 @@ int Camera::stop() d->setState(Private::CameraStopping); + LayerManager *layerManager = d->pipe()->cameraManager()->_d()->layerManager(); + layerManager->stop(this); + d->pipe_->invokeMethod(&PipelineHandler::stop, ConnectionTypeBlocking, this);
Add hooks into the CameraManager to call into the LayerManager on all relevant public-facing interface of Camera. The entry point for each function into the LayerManager is has been chosen based on the capabilities that we want to allow to Layer implementations. Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> --- Changes in v2: - move the LayerManager out of Camera into the CameraManager - remove hooks for generateConfiguration() and streams() - Camera now passes itself into the layer hooks, as it is required for the LayerManager to fetch the closure based on the camera to pass to each layer --- src/libcamera/camera.cpp | 40 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 38 insertions(+), 2 deletions(-)