| Message ID | 20201105001546.1690179-9-niklas.soderlund@ragnatech.se |
|---|---|
| State | Superseded |
| Delegated to: | Niklas Söderlund |
| Headers | show |
| Series |
|
| Related | show |
Hi Niklas, On Thu, Nov 05, 2020 at 01:15:43AM +0100, Niklas Söderlund wrote: > Attache to an IPA and allow it to push V4L2 controls that applies on the > camera sensor. The IPA is not fully integrated in the pipeline as > statistics and parameters buffers are not yet allocated, processed by > the IPA nor queued to the hardware. > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > src/libcamera/pipeline/ipu3/ipu3.cpp | 83 +++++++++++++++++++++++++++- > 1 file changed, 82 insertions(+), 1 deletion(-) > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > index 62e99a308a6136a7..d161c2e68c0db0f2 100644 > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > @@ -14,11 +14,13 @@ > #include <libcamera/camera.h> > #include <libcamera/control_ids.h> > #include <libcamera/formats.h> > +#include <libcamera/ipa/ipu3.h> > #include <libcamera/request.h> > #include <libcamera/stream.h> > > #include "libcamera/internal/camera_sensor.h" > #include "libcamera/internal/device_enumerator.h" > +#include "libcamera/internal/ipa_manager.h" > #include "libcamera/internal/log.h" > #include "libcamera/internal/media_device.h" > #include "libcamera/internal/pipeline_handler.h" > @@ -49,10 +51,12 @@ class IPU3CameraData : public CameraData > { > public: > IPU3CameraData(PipelineHandler *pipe) > - : CameraData(pipe) > + : CameraData(pipe), delayedCtrls_(nullptr) > { > } > > + int loadIPA(); > + > void imguOutputBufferReady(FrameBuffer *buffer); > void cio2BufferReady(FrameBuffer *buffer); > > @@ -62,6 +66,11 @@ public: > Stream outStream_; > Stream vfStream_; > Stream rawStream_; > + > + DelayedControls *delayedCtrls_; > + > +private: > + void actOnIpa(unsigned int id, const IPAOperationData &op); > }; > > class IPU3CameraConfiguration : public CameraConfiguration > @@ -590,6 +599,12 @@ int PipelineHandlerIPU3::start(Camera *camera) > IPU3CameraData *data = cameraData(camera); > CIO2Device *cio2 = &data->cio2_; > ImgUDevice *imgu = data->imgu_; > + > + CameraSensorInfo sensorInfo = {}; > + std::map<unsigned int, IPAStream> streamConfig; > + std::map<unsigned int, const ControlInfoMap &> entityControls; > + IPAOperationData ipaConfig; > + > int ret; > > /* Allocate buffers for internal pipeline usage. */ > @@ -597,6 +612,10 @@ int PipelineHandlerIPU3::start(Camera *camera) > if (ret) > return ret; > > + ret = data->ipa_->start(); > + if (ret) > + goto error; > + > /* > * Start the ImgU video devices, buffers will be queued to the > * ImgU output and viewfinder when requests will be queued. > @@ -612,9 +631,33 @@ int PipelineHandlerIPU3::start(Camera *camera) > goto error; > } > > + /* Inform IPA of stream configuration and sensor controls. */ > + ret = data->cio2_.sensor()->sensorInfo(&sensorInfo); > + if (ret) { > + /* \todo Turn to hard failure once sensors info is mandatory. */ > + LOG(IPU3, Warning) << "Camera sensor information not available"; > + sensorInfo = {}; > + ret = 0; > + } > + > + streamConfig[0] = { > + .pixelFormat = data->outStream_.configuration().pixelFormat, > + .size = data->outStream_.configuration().size, > + }; > + streamConfig[0] = { > + .pixelFormat = data->vfStream_.configuration().pixelFormat, > + .size = data->vfStream_.configuration().size, > + }; Why are you overwriting streamConfig[0] here? Paul > + > + entityControls.emplace(0, data->cio2_.sensor()->controls()); > + > + data->ipa_->configure(sensorInfo, streamConfig, entityControls, > + ipaConfig, nullptr); > + > return 0; > > error: > + data->ipa_->stop(); > freeBuffers(camera); > LOG(IPU3, Error) << "Failed to start camera " << camera->id(); > > @@ -631,6 +674,8 @@ void PipelineHandlerIPU3::stop(Camera *camera) > if (ret) > LOG(IPU3, Warning) << "Failed to stop camera " << camera->id(); > > + data->ipa_->stop(); > + > freeBuffers(camera); > } > > @@ -762,12 +807,20 @@ int PipelineHandlerIPU3::registerCameras() > if (ret) > continue; > > + ret = data->loadIPA(); > + if (ret) > + continue; > + > /* Initialize the camera properties. */ > data->properties_ = cio2->sensor()->properties(); > > /* Initialze the camera controls. */ > data->controlInfo_ = IPU3Controls; > > + data->delayedCtrls_ = cio2->sensor()->delayedContols(); > + data->cio2_.frameStart().connect(data->delayedCtrls_, > + &DelayedControls::frameStart); > + > /** > * \todo Dynamically assign ImgU and output devices to each > * stream and camera; as of now, limit support to two cameras > @@ -811,6 +864,34 @@ int PipelineHandlerIPU3::registerCameras() > return numCameras ? 0 : -ENODEV; > } > > +int IPU3CameraData::loadIPA() > +{ > + ipa_ = IPAManager::createIPA(pipe_, 1, 1); > + if (!ipa_) > + return -ENOENT; > + > + ipa_->queueFrameAction.connect(this, &IPU3CameraData::actOnIpa); > + > + ipa_->init(IPASettings{}); > + > + return 0; > +} > + > +void IPU3CameraData::actOnIpa([[maybe_unused]] unsigned int id, > + const IPAOperationData &action) > +{ > + switch (action.operation) { > + case IPU3_IPA_ACTION_SET_SENSOR_CONTROLS: { > + const ControlList &controls = action.controls[0]; > + delayedCtrls_->push(controls); > + break; > + } > + default: > + LOG(IPU3, Error) << "Unknown action " << action.operation; > + break; > + } > +} > + > /* ----------------------------------------------------------------------------- > * Buffer Ready slots > */ > -- > 2.29.2 > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Niklas, On 05/11/2020 01:15, Niklas Söderlund wrote: > Attache to an IPA and allow it to push V4L2 controls that applies on the > camera sensor. The IPA is not fully integrated in the pipeline as > statistics and parameters buffers are not yet allocated, processed by > the IPA nor queued to the hardware. > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > src/libcamera/pipeline/ipu3/ipu3.cpp | 83 +++++++++++++++++++++++++++- > 1 file changed, 82 insertions(+), 1 deletion(-) > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > index 62e99a308a6136a7..d161c2e68c0db0f2 100644 > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > @@ -14,11 +14,13 @@ > #include <libcamera/camera.h> > #include <libcamera/control_ids.h> > #include <libcamera/formats.h> > +#include <libcamera/ipa/ipu3.h> > #include <libcamera/request.h> > #include <libcamera/stream.h> > > #include "libcamera/internal/camera_sensor.h" > #include "libcamera/internal/device_enumerator.h" > +#include "libcamera/internal/ipa_manager.h" > #include "libcamera/internal/log.h" > #include "libcamera/internal/media_device.h" > #include "libcamera/internal/pipeline_handler.h" > @@ -49,10 +51,12 @@ class IPU3CameraData : public CameraData > { > public: > IPU3CameraData(PipelineHandler *pipe) > - : CameraData(pipe) > + : CameraData(pipe), delayedCtrls_(nullptr) > { > } > > + int loadIPA(); > + > void imguOutputBufferReady(FrameBuffer *buffer); > void cio2BufferReady(FrameBuffer *buffer); > > @@ -62,6 +66,11 @@ public: > Stream outStream_; > Stream vfStream_; > Stream rawStream_; > + > + DelayedControls *delayedCtrls_; > + > +private: > + void actOnIpa(unsigned int id, const IPAOperationData &op); > }; > > class IPU3CameraConfiguration : public CameraConfiguration > @@ -590,6 +599,12 @@ int PipelineHandlerIPU3::start(Camera *camera) > IPU3CameraData *data = cameraData(camera); > CIO2Device *cio2 = &data->cio2_; > ImgUDevice *imgu = data->imgu_; > + > + CameraSensorInfo sensorInfo = {}; > + std::map<unsigned int, IPAStream> streamConfig; > + std::map<unsigned int, const ControlInfoMap &> entityControls; > + IPAOperationData ipaConfig; > + > int ret; > > /* Allocate buffers for internal pipeline usage. */ > @@ -597,6 +612,10 @@ int PipelineHandlerIPU3::start(Camera *camera) > if (ret) > return ret; > > + ret = data->ipa_->start(); > + if (ret) > + goto error; > + > /* > * Start the ImgU video devices, buffers will be queued to the > * ImgU output and viewfinder when requests will be queued. > @@ -612,9 +631,33 @@ int PipelineHandlerIPU3::start(Camera *camera) > goto error; > } > > + /* Inform IPA of stream configuration and sensor controls. */ > + ret = data->cio2_.sensor()->sensorInfo(&sensorInfo); > + if (ret) { > + /* \todo Turn to hard failure once sensors info is mandatory. */ > + LOG(IPU3, Warning) << "Camera sensor information not available"; > + sensorInfo = {}; > + ret = 0; > + } I tried this patch series on my Surface Go 2 and I have some issues with it. The most critical is that I get a core dump :-) : [0:08:57.210851579] [4959] WARN IPU3 ipu3.cpp:646 Camera sensor information not available [0:08:57.211150487] [4959] ERROR IPAIPU3 ipu3.cpp:88 Can't find exposure control terminate called after throwing an instance of 'std::out_of_range' what(): _Map_base::at Aborted (core dumped) Thanks, JM
Hi Niklas, On 05/11/2020 01:15, Niklas Söderlund wrote: > Attache to an IPA and allow it to push V4L2 controls that applies on the > camera sensor. The IPA is not fully integrated in the pipeline as > statistics and parameters buffers are not yet allocated, processed by > the IPA nor queued to the hardware. > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > src/libcamera/pipeline/ipu3/ipu3.cpp | 83 +++++++++++++++++++++++++++- > 1 file changed, 82 insertions(+), 1 deletion(-) > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > index 62e99a308a6136a7..d161c2e68c0db0f2 100644 > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > @@ -14,11 +14,13 @@ > #include <libcamera/camera.h> > #include <libcamera/control_ids.h> > #include <libcamera/formats.h> > +#include <libcamera/ipa/ipu3.h> > #include <libcamera/request.h> > #include <libcamera/stream.h> > > #include "libcamera/internal/camera_sensor.h" > #include "libcamera/internal/device_enumerator.h" > +#include "libcamera/internal/ipa_manager.h" > #include "libcamera/internal/log.h" > #include "libcamera/internal/media_device.h" > #include "libcamera/internal/pipeline_handler.h" > @@ -49,10 +51,12 @@ class IPU3CameraData : public CameraData > { > public: > IPU3CameraData(PipelineHandler *pipe) > - : CameraData(pipe) > + : CameraData(pipe), delayedCtrls_(nullptr) > { > } > > + int loadIPA(); > + > void imguOutputBufferReady(FrameBuffer *buffer); > void cio2BufferReady(FrameBuffer *buffer); > > @@ -62,6 +66,11 @@ public: > Stream outStream_; > Stream vfStream_; > Stream rawStream_; > + > + DelayedControls *delayedCtrls_; > + > +private: > + void actOnIpa(unsigned int id, const IPAOperationData &op); > }; > > class IPU3CameraConfiguration : public CameraConfiguration > @@ -590,6 +599,12 @@ int PipelineHandlerIPU3::start(Camera *camera) > IPU3CameraData *data = cameraData(camera); > CIO2Device *cio2 = &data->cio2_; > ImgUDevice *imgu = data->imgu_; > + > + CameraSensorInfo sensorInfo = {}; > + std::map<unsigned int, IPAStream> streamConfig; > + std::map<unsigned int, const ControlInfoMap &> entityControls; > + IPAOperationData ipaConfig; > + > int ret; > > /* Allocate buffers for internal pipeline usage. */ > @@ -597,6 +612,10 @@ int PipelineHandlerIPU3::start(Camera *camera) > if (ret) > return ret; > > + ret = data->ipa_->start(); > + if (ret) > + goto error; > + > /* > * Start the ImgU video devices, buffers will be queued to the > * ImgU output and viewfinder when requests will be queued. > @@ -612,9 +631,33 @@ int PipelineHandlerIPU3::start(Camera *camera) > goto error; > } > > + /* Inform IPA of stream configuration and sensor controls. */ > + ret = data->cio2_.sensor()->sensorInfo(&sensorInfo); > + if (ret) { > + /* \todo Turn to hard failure once sensors info is mandatory. */ > + LOG(IPU3, Warning) << "Camera sensor information not available"; > + sensorInfo = {}; > + ret = 0; > + } > + > + streamConfig[0] = { > + .pixelFormat = data->outStream_.configuration().pixelFormat, > + .size = data->outStream_.configuration().size, > + }; > + streamConfig[0] = { > + .pixelFormat = data->vfStream_.configuration().pixelFormat, > + .size = data->vfStream_.configuration().size, > + }; > + > + entityControls.emplace(0, data->cio2_.sensor()->controls()); > + > + data->ipa_->configure(sensorInfo, streamConfig, entityControls, > + ipaConfig, nullptr); > + > return 0; There is an issue there, if no controls are found (not implemented) then you will go on without really nowing... > > error: > + data->ipa_->stop(); > freeBuffers(camera); > LOG(IPU3, Error) << "Failed to start camera " << camera->id(); > > @@ -631,6 +674,8 @@ void PipelineHandlerIPU3::stop(Camera *camera) > if (ret) > LOG(IPU3, Warning) << "Failed to stop camera " << camera->id(); > > + data->ipa_->stop(); > + > freeBuffers(camera); > } > > @@ -762,12 +807,20 @@ int PipelineHandlerIPU3::registerCameras() > if (ret) > continue; > > + ret = data->loadIPA(); > + if (ret) > + continue; > + > /* Initialize the camera properties. */ > data->properties_ = cio2->sensor()->properties(); > > /* Initialze the camera controls. */ > data->controlInfo_ = IPU3Controls; > > + data->delayedCtrls_ = cio2->sensor()->delayedContols(); > + data->cio2_.frameStart().connect(data->delayedCtrls_, > + &DelayedControls::frameStart); > + > /** > * \todo Dynamically assign ImgU and output devices to each > * stream and camera; as of now, limit support to two cameras > @@ -811,6 +864,34 @@ int PipelineHandlerIPU3::registerCameras() > return numCameras ? 0 : -ENODEV; > } > > +int IPU3CameraData::loadIPA() > +{ > + ipa_ = IPAManager::createIPA(pipe_, 1, 1); > + if (!ipa_) > + return -ENOENT; > + > + ipa_->queueFrameAction.connect(this, &IPU3CameraData::actOnIpa); > + > + ipa_->init(IPASettings{}); > + > + return 0; > +} > + > +void IPU3CameraData::actOnIpa([[maybe_unused]] unsigned int id, > + const IPAOperationData &action) > +{ > + switch (action.operation) { > + case IPU3_IPA_ACTION_SET_SENSOR_CONTROLS: { > + const ControlList &controls = action.controls[0]; > + delayedCtrls_->push(controls); And you will push an empty list... which leads to "terminate called after throwing an instance of 'std::out_of_range'" It should not abort but deal with it ? Thanks, JM
Hi Niklas On Thu, Nov 05, 2020 at 01:15:43AM +0100, Niklas Söderlund wrote: > Attache to an IPA and allow it to push V4L2 controls that applies on the Attach to the IPA The question on which control identifiers to use is the same as in the previous patches. Do we want the IPA and pipeline to speak V4L2_CID_ or should we use libcamera controls where possible and let the CameraSensor do the translation ? > camera sensor. The IPA is not fully integrated in the pipeline as > statistics and parameters buffers are not yet allocated, processed by > the IPA nor queued to the hardware. > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > src/libcamera/pipeline/ipu3/ipu3.cpp | 83 +++++++++++++++++++++++++++- > 1 file changed, 82 insertions(+), 1 deletion(-) > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > index 62e99a308a6136a7..d161c2e68c0db0f2 100644 > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > @@ -14,11 +14,13 @@ > #include <libcamera/camera.h> > #include <libcamera/control_ids.h> > #include <libcamera/formats.h> > +#include <libcamera/ipa/ipu3.h> > #include <libcamera/request.h> > #include <libcamera/stream.h> > > #include "libcamera/internal/camera_sensor.h" > #include "libcamera/internal/device_enumerator.h" > +#include "libcamera/internal/ipa_manager.h" > #include "libcamera/internal/log.h" > #include "libcamera/internal/media_device.h" > #include "libcamera/internal/pipeline_handler.h" > @@ -49,10 +51,12 @@ class IPU3CameraData : public CameraData > { > public: > IPU3CameraData(PipelineHandler *pipe) > - : CameraData(pipe) > + : CameraData(pipe), delayedCtrls_(nullptr) > { > } > > + int loadIPA(); > + > void imguOutputBufferReady(FrameBuffer *buffer); > void cio2BufferReady(FrameBuffer *buffer); > > @@ -62,6 +66,11 @@ public: > Stream outStream_; > Stream vfStream_; > Stream rawStream_; > + > + DelayedControls *delayedCtrls_; > + > +private: > + void actOnIpa(unsigned int id, const IPAOperationData &op); > }; > > class IPU3CameraConfiguration : public CameraConfiguration > @@ -590,6 +599,12 @@ int PipelineHandlerIPU3::start(Camera *camera) > IPU3CameraData *data = cameraData(camera); > CIO2Device *cio2 = &data->cio2_; > ImgUDevice *imgu = data->imgu_; > + > + CameraSensorInfo sensorInfo = {}; > + std::map<unsigned int, IPAStream> streamConfig; > + std::map<unsigned int, const ControlInfoMap &> entityControls; > + IPAOperationData ipaConfig; > + > int ret; > > /* Allocate buffers for internal pipeline usage. */ > @@ -597,6 +612,10 @@ int PipelineHandlerIPU3::start(Camera *camera) > if (ret) > return ret; > > + ret = data->ipa_->start(); > + if (ret) > + goto error; > + > /* > * Start the ImgU video devices, buffers will be queued to the > * ImgU output and viewfinder when requests will be queued. > @@ -612,9 +631,33 @@ int PipelineHandlerIPU3::start(Camera *camera) > goto error; > } > > + /* Inform IPA of stream configuration and sensor controls. */ > + ret = data->cio2_.sensor()->sensorInfo(&sensorInfo); > + if (ret) { > + /* \todo Turn to hard failure once sensors info is mandatory. */ > + LOG(IPU3, Warning) << "Camera sensor information not available"; > + sensorInfo = {}; > + ret = 0; > + } > + > + streamConfig[0] = { > + .pixelFormat = data->outStream_.configuration().pixelFormat, > + .size = data->outStream_.configuration().size, > + }; > + streamConfig[0] = { Overwriting streamConfig[0] ? It's anyway so far ignored in the IPA > + .pixelFormat = data->vfStream_.configuration().pixelFormat, > + .size = data->vfStream_.configuration().size, > + }; > + > + entityControls.emplace(0, data->cio2_.sensor()->controls()); > + > + data->ipa_->configure(sensorInfo, streamConfig, entityControls, > + ipaConfig, nullptr); > + > return 0; > > error: > + data->ipa_->stop(); > freeBuffers(camera); > LOG(IPU3, Error) << "Failed to start camera " << camera->id(); > > @@ -631,6 +674,8 @@ void PipelineHandlerIPU3::stop(Camera *camera) > if (ret) > LOG(IPU3, Warning) << "Failed to stop camera " << camera->id(); > > + data->ipa_->stop(); > + > freeBuffers(camera); > } > > @@ -762,12 +807,20 @@ int PipelineHandlerIPU3::registerCameras() > if (ret) > continue; > > + ret = data->loadIPA(); > + if (ret) > + continue; > + > /* Initialize the camera properties. */ > data->properties_ = cio2->sensor()->properties(); > > /* Initialze the camera controls. */ > data->controlInfo_ = IPU3Controls; > > + data->delayedCtrls_ = cio2->sensor()->delayedContols(); > + data->cio2_.frameStart().connect(data->delayedCtrls_, > + &DelayedControls::frameStart); > + > /** > * \todo Dynamically assign ImgU and output devices to each > * stream and camera; as of now, limit support to two cameras > @@ -811,6 +864,34 @@ int PipelineHandlerIPU3::registerCameras() > return numCameras ? 0 : -ENODEV; > } > > +int IPU3CameraData::loadIPA() > +{ > + ipa_ = IPAManager::createIPA(pipe_, 1, 1); > + if (!ipa_) > + return -ENOENT; > + > + ipa_->queueFrameAction.connect(this, &IPU3CameraData::actOnIpa); > + > + ipa_->init(IPASettings{}); > + > + return 0; > +} > + > +void IPU3CameraData::actOnIpa([[maybe_unused]] unsigned int id, > + const IPAOperationData &action) > +{ > + switch (action.operation) { > + case IPU3_IPA_ACTION_SET_SENSOR_CONTROLS: { > + const ControlList &controls = action.controls[0]; > + delayedCtrls_->push(controls); Do we want to have a CameraSensor::setDelayedControls() ? This direct handling of delayed controls bypasses the CameraSensor completely pushing to each pipeline handler the controls translation burden > + break; > + } > + default: > + LOG(IPU3, Error) << "Unknown action " << action.operation; > + break; > + } > +} > + > /* ----------------------------------------------------------------------------- > * Buffer Ready slots > */ > -- > 2.29.2 > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Jacopo, On Wed, Nov 18, 2020 at 04:48:28PM +0100, Jacopo Mondi wrote: > Hi Niklas > > On Thu, Nov 05, 2020 at 01:15:43AM +0100, Niklas Söderlund wrote: > > Attache to an IPA and allow it to push V4L2 controls that applies on the > > Attach > to the IPA > > The question on which control identifiers to use is the same as in the > previous patches. Do we want the IPA and pipeline to speak V4L2_CID_ > or should we use libcamera controls where possible and let the > CameraSensor do the translation ? The latter, possible with CameraSensor controls instead of libcamera controls (at least a separate ControlInfoMap, to report the right limits). That requires work on the CameraSensor side and I don't think this series needs to depend on that, as the RPi IPA already uses V4L2 controls. > > camera sensor. The IPA is not fully integrated in the pipeline as > > statistics and parameters buffers are not yet allocated, processed by > > the IPA nor queued to the hardware. > > > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > > --- > > src/libcamera/pipeline/ipu3/ipu3.cpp | 83 +++++++++++++++++++++++++++- > > 1 file changed, 82 insertions(+), 1 deletion(-) > > > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > > index 62e99a308a6136a7..d161c2e68c0db0f2 100644 > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > > @@ -14,11 +14,13 @@ > > #include <libcamera/camera.h> > > #include <libcamera/control_ids.h> > > #include <libcamera/formats.h> > > +#include <libcamera/ipa/ipu3.h> > > #include <libcamera/request.h> > > #include <libcamera/stream.h> > > > > #include "libcamera/internal/camera_sensor.h" > > #include "libcamera/internal/device_enumerator.h" > > +#include "libcamera/internal/ipa_manager.h" > > #include "libcamera/internal/log.h" > > #include "libcamera/internal/media_device.h" > > #include "libcamera/internal/pipeline_handler.h" > > @@ -49,10 +51,12 @@ class IPU3CameraData : public CameraData > > { > > public: > > IPU3CameraData(PipelineHandler *pipe) > > - : CameraData(pipe) > > + : CameraData(pipe), delayedCtrls_(nullptr) > > { > > } > > > > + int loadIPA(); > > + > > void imguOutputBufferReady(FrameBuffer *buffer); > > void cio2BufferReady(FrameBuffer *buffer); > > > > @@ -62,6 +66,11 @@ public: > > Stream outStream_; > > Stream vfStream_; > > Stream rawStream_; > > + > > + DelayedControls *delayedCtrls_; > > + > > +private: > > + void actOnIpa(unsigned int id, const IPAOperationData &op); > > }; > > > > class IPU3CameraConfiguration : public CameraConfiguration > > @@ -590,6 +599,12 @@ int PipelineHandlerIPU3::start(Camera *camera) > > IPU3CameraData *data = cameraData(camera); > > CIO2Device *cio2 = &data->cio2_; > > ImgUDevice *imgu = data->imgu_; > > + > > + CameraSensorInfo sensorInfo = {}; > > + std::map<unsigned int, IPAStream> streamConfig; > > + std::map<unsigned int, const ControlInfoMap &> entityControls; > > + IPAOperationData ipaConfig; > > + > > int ret; > > > > /* Allocate buffers for internal pipeline usage. */ > > @@ -597,6 +612,10 @@ int PipelineHandlerIPU3::start(Camera *camera) > > if (ret) > > return ret; > > > > + ret = data->ipa_->start(); > > + if (ret) > > + goto error; > > + > > /* > > * Start the ImgU video devices, buffers will be queued to the > > * ImgU output and viewfinder when requests will be queued. > > @@ -612,9 +631,33 @@ int PipelineHandlerIPU3::start(Camera *camera) > > goto error; > > } > > > > + /* Inform IPA of stream configuration and sensor controls. */ > > + ret = data->cio2_.sensor()->sensorInfo(&sensorInfo); > > + if (ret) { > > + /* \todo Turn to hard failure once sensors info is mandatory. */ > > + LOG(IPU3, Warning) << "Camera sensor information not available"; > > + sensorInfo = {}; > > + ret = 0; > > + } > > + > > + streamConfig[0] = { > > + .pixelFormat = data->outStream_.configuration().pixelFormat, > > + .size = data->outStream_.configuration().size, > > + }; > > + streamConfig[0] = { > > Overwriting streamConfig[0] ? > > It's anyway so far ignored in the IPA > > > + .pixelFormat = data->vfStream_.configuration().pixelFormat, > > + .size = data->vfStream_.configuration().size, > > + }; > > + > > + entityControls.emplace(0, data->cio2_.sensor()->controls()); > > + > > + data->ipa_->configure(sensorInfo, streamConfig, entityControls, > > + ipaConfig, nullptr); > > + > > return 0; > > > > error: > > + data->ipa_->stop(); > > freeBuffers(camera); > > LOG(IPU3, Error) << "Failed to start camera " << camera->id(); > > > > @@ -631,6 +674,8 @@ void PipelineHandlerIPU3::stop(Camera *camera) > > if (ret) > > LOG(IPU3, Warning) << "Failed to stop camera " << camera->id(); > > > > + data->ipa_->stop(); > > + > > freeBuffers(camera); > > } > > > > @@ -762,12 +807,20 @@ int PipelineHandlerIPU3::registerCameras() > > if (ret) > > continue; > > > > + ret = data->loadIPA(); > > + if (ret) > > + continue; > > + > > /* Initialize the camera properties. */ > > data->properties_ = cio2->sensor()->properties(); > > > > /* Initialze the camera controls. */ > > data->controlInfo_ = IPU3Controls; > > > > + data->delayedCtrls_ = cio2->sensor()->delayedContols(); > > + data->cio2_.frameStart().connect(data->delayedCtrls_, > > + &DelayedControls::frameStart); > > + > > /** > > * \todo Dynamically assign ImgU and output devices to each > > * stream and camera; as of now, limit support to two cameras > > @@ -811,6 +864,34 @@ int PipelineHandlerIPU3::registerCameras() > > return numCameras ? 0 : -ENODEV; > > } > > > > +int IPU3CameraData::loadIPA() > > +{ > > + ipa_ = IPAManager::createIPA(pipe_, 1, 1); > > + if (!ipa_) > > + return -ENOENT; > > + > > + ipa_->queueFrameAction.connect(this, &IPU3CameraData::actOnIpa); > > + > > + ipa_->init(IPASettings{}); > > + > > + return 0; > > +} > > + > > +void IPU3CameraData::actOnIpa([[maybe_unused]] unsigned int id, > > + const IPAOperationData &action) > > +{ > > + switch (action.operation) { > > + case IPU3_IPA_ACTION_SET_SENSOR_CONTROLS: { > > + const ControlList &controls = action.controls[0]; > > + delayedCtrls_->push(controls); > > Do we want to have a CameraSensor::setDelayedControls() ? > This direct handling of delayed controls bypasses the CameraSensor > completely pushing to each pipeline handler the controls translation > burden I think we want to embed an instance of DelayedControls in CameraSensor, yes, and create a nice API to interface with it. I think this is also not a blocker for this series, but it should be done sooner than later. Overall I think this patch is a good step forward. It would be nice to address the crash that Jean-Michel experienced though :-) > > + break; > > + } > > + default: > > + LOG(IPU3, Error) << "Unknown action " << action.operation; > > + break; > > + } > > +} > > + > > /* ----------------------------------------------------------------------------- > > * Buffer Ready slots > > */
diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 62e99a308a6136a7..d161c2e68c0db0f2 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -14,11 +14,13 @@ #include <libcamera/camera.h> #include <libcamera/control_ids.h> #include <libcamera/formats.h> +#include <libcamera/ipa/ipu3.h> #include <libcamera/request.h> #include <libcamera/stream.h> #include "libcamera/internal/camera_sensor.h" #include "libcamera/internal/device_enumerator.h" +#include "libcamera/internal/ipa_manager.h" #include "libcamera/internal/log.h" #include "libcamera/internal/media_device.h" #include "libcamera/internal/pipeline_handler.h" @@ -49,10 +51,12 @@ class IPU3CameraData : public CameraData { public: IPU3CameraData(PipelineHandler *pipe) - : CameraData(pipe) + : CameraData(pipe), delayedCtrls_(nullptr) { } + int loadIPA(); + void imguOutputBufferReady(FrameBuffer *buffer); void cio2BufferReady(FrameBuffer *buffer); @@ -62,6 +66,11 @@ public: Stream outStream_; Stream vfStream_; Stream rawStream_; + + DelayedControls *delayedCtrls_; + +private: + void actOnIpa(unsigned int id, const IPAOperationData &op); }; class IPU3CameraConfiguration : public CameraConfiguration @@ -590,6 +599,12 @@ int PipelineHandlerIPU3::start(Camera *camera) IPU3CameraData *data = cameraData(camera); CIO2Device *cio2 = &data->cio2_; ImgUDevice *imgu = data->imgu_; + + CameraSensorInfo sensorInfo = {}; + std::map<unsigned int, IPAStream> streamConfig; + std::map<unsigned int, const ControlInfoMap &> entityControls; + IPAOperationData ipaConfig; + int ret; /* Allocate buffers for internal pipeline usage. */ @@ -597,6 +612,10 @@ int PipelineHandlerIPU3::start(Camera *camera) if (ret) return ret; + ret = data->ipa_->start(); + if (ret) + goto error; + /* * Start the ImgU video devices, buffers will be queued to the * ImgU output and viewfinder when requests will be queued. @@ -612,9 +631,33 @@ int PipelineHandlerIPU3::start(Camera *camera) goto error; } + /* Inform IPA of stream configuration and sensor controls. */ + ret = data->cio2_.sensor()->sensorInfo(&sensorInfo); + if (ret) { + /* \todo Turn to hard failure once sensors info is mandatory. */ + LOG(IPU3, Warning) << "Camera sensor information not available"; + sensorInfo = {}; + ret = 0; + } + + streamConfig[0] = { + .pixelFormat = data->outStream_.configuration().pixelFormat, + .size = data->outStream_.configuration().size, + }; + streamConfig[0] = { + .pixelFormat = data->vfStream_.configuration().pixelFormat, + .size = data->vfStream_.configuration().size, + }; + + entityControls.emplace(0, data->cio2_.sensor()->controls()); + + data->ipa_->configure(sensorInfo, streamConfig, entityControls, + ipaConfig, nullptr); + return 0; error: + data->ipa_->stop(); freeBuffers(camera); LOG(IPU3, Error) << "Failed to start camera " << camera->id(); @@ -631,6 +674,8 @@ void PipelineHandlerIPU3::stop(Camera *camera) if (ret) LOG(IPU3, Warning) << "Failed to stop camera " << camera->id(); + data->ipa_->stop(); + freeBuffers(camera); } @@ -762,12 +807,20 @@ int PipelineHandlerIPU3::registerCameras() if (ret) continue; + ret = data->loadIPA(); + if (ret) + continue; + /* Initialize the camera properties. */ data->properties_ = cio2->sensor()->properties(); /* Initialze the camera controls. */ data->controlInfo_ = IPU3Controls; + data->delayedCtrls_ = cio2->sensor()->delayedContols(); + data->cio2_.frameStart().connect(data->delayedCtrls_, + &DelayedControls::frameStart); + /** * \todo Dynamically assign ImgU and output devices to each * stream and camera; as of now, limit support to two cameras @@ -811,6 +864,34 @@ int PipelineHandlerIPU3::registerCameras() return numCameras ? 0 : -ENODEV; } +int IPU3CameraData::loadIPA() +{ + ipa_ = IPAManager::createIPA(pipe_, 1, 1); + if (!ipa_) + return -ENOENT; + + ipa_->queueFrameAction.connect(this, &IPU3CameraData::actOnIpa); + + ipa_->init(IPASettings{}); + + return 0; +} + +void IPU3CameraData::actOnIpa([[maybe_unused]] unsigned int id, + const IPAOperationData &action) +{ + switch (action.operation) { + case IPU3_IPA_ACTION_SET_SENSOR_CONTROLS: { + const ControlList &controls = action.controls[0]; + delayedCtrls_->push(controls); + break; + } + default: + LOG(IPU3, Error) << "Unknown action " << action.operation; + break; + } +} + /* ----------------------------------------------------------------------------- * Buffer Ready slots */
Attache to an IPA and allow it to push V4L2 controls that applies on the camera sensor. The IPA is not fully integrated in the pipeline as statistics and parameters buffers are not yet allocated, processed by the IPA nor queued to the hardware. Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> --- src/libcamera/pipeline/ipu3/ipu3.cpp | 83 +++++++++++++++++++++++++++- 1 file changed, 82 insertions(+), 1 deletion(-)