| Message ID | 20210105190522.682324-4-jacopo@jmondi.org |
|---|---|
| State | Accepted |
| Delegated to: | Jacopo Mondi |
| Headers | show |
| Series |
|
| Related | show |
Hi Jacopo, Thank you for the patch. On Tue, Jan 05, 2021 at 08:05:13PM +0100, Jacopo Mondi wrote: > Calculate the controls::Exposure limits at camera registration time > and register it in the list of camera supported controls. > > Cache the default exposure value to report it in the request metadata. > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > --- > src/libcamera/pipeline/ipu3/ipu3.cpp | 67 ++++++++++++++++++++++++++-- > 1 file changed, 63 insertions(+), 4 deletions(-) > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > index f1151733d9fe..879057dab328 100644 > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > @@ -41,7 +41,7 @@ static constexpr unsigned int IMGU_OUTPUT_HEIGHT_ALIGN = 4; > static constexpr unsigned int IMGU_OUTPUT_WIDTH_MARGIN = 64; > static constexpr unsigned int IMGU_OUTPUT_HEIGHT_MARGIN = 32; > > -static const ControlInfoMap IPU3Controls = { > +static const ControlInfoMap::Map IPU3Controls = { > { &controls::draft::PipelineDepth, ControlInfo(2, 3) }, > }; > > @@ -49,7 +49,7 @@ class IPU3CameraData : public CameraData > { > public: > IPU3CameraData(PipelineHandler *pipe) > - : CameraData(pipe) > + : CameraData(pipe), exposureTime_(0) > { > } > > @@ -62,6 +62,8 @@ public: > Stream outStream_; > Stream vfStream_; > Stream rawStream_; > + > + int32_t exposureTime_; This can't be negative, unsigned int would do. > }; > > class IPU3CameraConfiguration : public CameraConfiguration > @@ -119,6 +121,7 @@ private: > PipelineHandler::cameraData(camera)); > } > > + int initControls(IPU3CameraData *data); > int registerCameras(); > > int allocateBuffers(Camera *camera); > @@ -731,6 +734,60 @@ bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator) > return ret == 0; > } > > +/* /** > + * \brief Initialize the camera controls > + * \param[in] data The camera data > + * > + * Initialize the camera controls by registering the pipeline handler > + * ones along with the controls assembled by inspecting the sensor > + * capabilities. Should this mention IPU3Controls ? Maybe the following ? * Initialize the camera controls as the union of the static pipeline handler * controls (IPU3Controls) and controls created dynamically from the sensor * capabilities. > + * > + * \return 0 on success or a negative error code for error s/for error/otherwise/ > + */ > +int PipelineHandlerIPU3::initControls(IPU3CameraData *data) > +{ > + const CameraSensor *sensor = data->cio2_.sensor(); > + CameraSensorInfo sensorInfo{}; > + > + int ret = sensor->sensorInfo(&sensorInfo); > + if (ret) > + return ret; > + > + ControlInfoMap::Map controls = IPU3Controls; > + > + /* > + * Compute exposure time limits. > + * > + * \todo The exposure limits depend on the sensor configuration. > + * Initialize the control using the line lenght and pixel rate of the s/lenght/length/ > + * current configurtion, as reported by the CameraSensorInfo. Use the s/configurtion/configuration/ > + * V4L2_CID_EXPOSURE control to get exposure min and max and convert it > + * from lines into micro-seconds. s/into/to/ s/micro-seconds/microseconds/ > + */ > + float pixelRate = sensorInfo.pixelRate / 1e6f; > + const ControlInfoMap &sensorControls = sensor->controls(); > + const ControlInfo &v4l2Exposure = sensorControls.find(V4L2_CID_EXPOSURE)->second; > + int32_t minExposure = v4l2Exposure.min().get<int32_t>() > + * sensorInfo.lineLength / pixelRate; > + int32_t maxExposure = v4l2Exposure.max().get<int32_t>() > + * sensorInfo.lineLength / pixelRate; > + int32_t defExposure = v4l2Exposure.def().get<int32_t>() > + * sensorInfo.lineLength / pixelRate; For long exposure time this will overflow as the first two operands are 32-bit integers. How about the following ? double lineDuration = sensorInfo.pixelRate / 1e6f /pixelRate; ... int32_t minExposure = v4l2Exposure.min().get<int32_t>() * lineDuration; int32_t maxExposure = v4l2Exposure.max().get<int32_t>() * lineDuration; int32_t defExposure = v4l2Exposure.def().get<int32_t>() * lineDuration; > + > + /* > + * \todo Report the actual exposure time, use the default for the > + * moment. > + */ > + data->exposureTime_ = defExposure; > + > + controls[&controls::ExposureTime] = ControlInfo(minExposure, maxExposure, > + defExposure); > + > + data->controlInfo_ = std::move(controls); > + > + return 0; > +} > + > /** > * \brief Initialise ImgU and CIO2 devices associated with cameras > * > @@ -776,8 +833,9 @@ int PipelineHandlerIPU3::registerCameras() > /* Initialize the camera properties. */ > data->properties_ = cio2->sensor()->properties(); > > - /* Initialze the camera controls. */ > - data->controlInfo_ = IPU3Controls; > + ret = initControls(data.get()); > + if (ret) > + continue; > > /** > * \todo Dynamically assign ImgU and output devices to each > @@ -842,6 +900,7 @@ void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer) > > /* Mark the request as complete. */ > request->metadata().set(controls::draft::PipelineDepth, 3); /* \todo Move the ExposureTime control to the IPA */ Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + request->metadata().set(controls::ExposureTime, exposureTime_); > pipe_->completeRequest(request); > } >
Hi Laurent, On Mon, Jan 11, 2021 at 12:22:54AM +0200, Laurent Pinchart wrote: > Hi Jacopo, > > Thank you for the patch. > > > + */ > > + float pixelRate = sensorInfo.pixelRate / 1e6f; > > + const ControlInfoMap &sensorControls = sensor->controls(); > > + const ControlInfo &v4l2Exposure = sensorControls.find(V4L2_CID_EXPOSURE)->second; > > + int32_t minExposure = v4l2Exposure.min().get<int32_t>() > > + * sensorInfo.lineLength / pixelRate; > > + int32_t maxExposure = v4l2Exposure.max().get<int32_t>() > > + * sensorInfo.lineLength / pixelRate; > > + int32_t defExposure = v4l2Exposure.def().get<int32_t>() > > + * sensorInfo.lineLength / pixelRate; > > For long exposure time this will overflow as the first two operands are > 32-bit integers. How about the following ? > > double lineDuration = sensorInfo.pixelRate / 1e6f /pixelRate; did you mean: sensorInfo.lineLength / (sensorInfo.pixelRate / 1e6f) or rather sensorInfo.lineLength * 1e6f / sensorInfo.pixelRate but there's an overflow risk as well here. > ... > int32_t minExposure = v4l2Exposure.min().get<int32_t>() * lineDuration; > int32_t maxExposure = v4l2Exposure.max().get<int32_t>() * lineDuration; > int32_t defExposure = v4l2Exposure.def().get<int32_t>() * lineDuration; > > > + > > + /* > > + * \todo Report the actual exposure time, use the default for the > > + * moment. > > + */ > > + data->exposureTime_ = defExposure; > > + > > + controls[&controls::ExposureTime] = ControlInfo(minExposure, maxExposure, > > + defExposure); > > + > > + data->controlInfo_ = std::move(controls); > > + > > + return 0; > > +} > > + > > /** > > * \brief Initialise ImgU and CIO2 devices associated with cameras > > * > > @@ -776,8 +833,9 @@ int PipelineHandlerIPU3::registerCameras() > > /* Initialize the camera properties. */ > > data->properties_ = cio2->sensor()->properties(); > > > > - /* Initialze the camera controls. */ > > - data->controlInfo_ = IPU3Controls; > > + ret = initControls(data.get()); > > + if (ret) > > + continue; > > > > /** > > * \todo Dynamically assign ImgU and output devices to each > > @@ -842,6 +900,7 @@ void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer) > > > > /* Mark the request as complete. */ > > request->metadata().set(controls::draft::PipelineDepth, 3); > > /* \todo Move the ExposureTime control to the IPA */ > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Thanks j > > + request->metadata().set(controls::ExposureTime, exposureTime_); > > pipe_->completeRequest(request); > > } > > > > -- > Regards, > > Laurent Pinchart
Hi Jacopo, Thanks for your work. On 2021-01-05 20:05:13 +0100, Jacopo Mondi wrote: > Calculate the controls::Exposure limits at camera registration time > and register it in the list of camera supported controls. > > Cache the default exposure value to report it in the request metadata. > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > --- > src/libcamera/pipeline/ipu3/ipu3.cpp | 67 ++++++++++++++++++++++++++-- > 1 file changed, 63 insertions(+), 4 deletions(-) > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > index f1151733d9fe..879057dab328 100644 > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > @@ -41,7 +41,7 @@ static constexpr unsigned int IMGU_OUTPUT_HEIGHT_ALIGN = 4; > static constexpr unsigned int IMGU_OUTPUT_WIDTH_MARGIN = 64; > static constexpr unsigned int IMGU_OUTPUT_HEIGHT_MARGIN = 32; > > -static const ControlInfoMap IPU3Controls = { > +static const ControlInfoMap::Map IPU3Controls = { > { &controls::draft::PipelineDepth, ControlInfo(2, 3) }, > }; > > @@ -49,7 +49,7 @@ class IPU3CameraData : public CameraData > { > public: > IPU3CameraData(PipelineHandler *pipe) > - : CameraData(pipe) > + : CameraData(pipe), exposureTime_(0) > { > } > > @@ -62,6 +62,8 @@ public: > Stream outStream_; > Stream vfStream_; > Stream rawStream_; > + > + int32_t exposureTime_; > }; > > class IPU3CameraConfiguration : public CameraConfiguration > @@ -119,6 +121,7 @@ private: > PipelineHandler::cameraData(camera)); > } > > + int initControls(IPU3CameraData *data); > int registerCameras(); > > int allocateBuffers(Camera *camera); > @@ -731,6 +734,60 @@ bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator) > return ret == 0; > } > > +/* > + * \brief Initialize the camera controls > + * \param[in] data The camera data > + * > + * Initialize the camera controls by registering the pipeline handler > + * ones along with the controls assembled by inspecting the sensor > + * capabilities. > + * > + * \return 0 on success or a negative error code for error > + */ > +int PipelineHandlerIPU3::initControls(IPU3CameraData *data) > +{ > + const CameraSensor *sensor = data->cio2_.sensor(); > + CameraSensorInfo sensorInfo{}; > + > + int ret = sensor->sensorInfo(&sensorInfo); > + if (ret) > + return ret; > + > + ControlInfoMap::Map controls = IPU3Controls; > + > + /* > + * Compute exposure time limits. > + * > + * \todo The exposure limits depend on the sensor configuration. > + * Initialize the control using the line lenght and pixel rate of the > + * current configurtion, as reported by the CameraSensorInfo. Use the > + * V4L2_CID_EXPOSURE control to get exposure min and max and convert it > + * from lines into micro-seconds. > + */ > + float pixelRate = sensorInfo.pixelRate / 1e6f; > + const ControlInfoMap &sensorControls = sensor->controls(); > + const ControlInfo &v4l2Exposure = sensorControls.find(V4L2_CID_EXPOSURE)->second; > + int32_t minExposure = v4l2Exposure.min().get<int32_t>() > + * sensorInfo.lineLength / pixelRate; > + int32_t maxExposure = v4l2Exposure.max().get<int32_t>() > + * sensorInfo.lineLength / pixelRate; > + int32_t defExposure = v4l2Exposure.def().get<int32_t>() > + * sensorInfo.lineLength / pixelRate; > + > + /* > + * \todo Report the actual exposure time, use the default for the > + * moment. > + */ > + data->exposureTime_ = defExposure; > + > + controls[&controls::ExposureTime] = ControlInfo(minExposure, maxExposure, > + defExposure); nit: This is the only usage of of the local 'controls' variable, would it make sens to remove it and use 'data->controlInfo_' directly and avoid the std::move() below? While reading the code I thought there more things going on here then an advance optimization ;-) With or without this fix, but with the issues Laurent points out fixed, Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > + > + data->controlInfo_ = std::move(controls); > + > + return 0; > +} > + > /** > * \brief Initialise ImgU and CIO2 devices associated with cameras > * > @@ -776,8 +833,9 @@ int PipelineHandlerIPU3::registerCameras() > /* Initialize the camera properties. */ > data->properties_ = cio2->sensor()->properties(); > > - /* Initialze the camera controls. */ > - data->controlInfo_ = IPU3Controls; > + ret = initControls(data.get()); > + if (ret) > + continue; > > /** > * \todo Dynamically assign ImgU and output devices to each > @@ -842,6 +900,7 @@ void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer) > > /* Mark the request as complete. */ > request->metadata().set(controls::draft::PipelineDepth, 3); > + request->metadata().set(controls::ExposureTime, exposureTime_); > pipe_->completeRequest(request); > } > > -- > 2.29.2 > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Niklas, On Mon, Jan 18, 2021 at 04:16:19PM +0100, Niklas Söderlund wrote: > Hi Jacopo, > > Thanks for your work. > > On 2021-01-05 20:05:13 +0100, Jacopo Mondi wrote: > > Calculate the controls::Exposure limits at camera registration time > > and register it in the list of camera supported controls. > > > > Cache the default exposure value to report it in the request metadata. > > > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > > --- > > src/libcamera/pipeline/ipu3/ipu3.cpp | 67 ++++++++++++++++++++++++++-- > > 1 file changed, 63 insertions(+), 4 deletions(-) > > > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > > index f1151733d9fe..879057dab328 100644 > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > > @@ -41,7 +41,7 @@ static constexpr unsigned int IMGU_OUTPUT_HEIGHT_ALIGN = 4; > > static constexpr unsigned int IMGU_OUTPUT_WIDTH_MARGIN = 64; > > static constexpr unsigned int IMGU_OUTPUT_HEIGHT_MARGIN = 32; > > > > -static const ControlInfoMap IPU3Controls = { > > +static const ControlInfoMap::Map IPU3Controls = { > > { &controls::draft::PipelineDepth, ControlInfo(2, 3) }, > > }; > > > > @@ -49,7 +49,7 @@ class IPU3CameraData : public CameraData > > { > > public: > > IPU3CameraData(PipelineHandler *pipe) > > - : CameraData(pipe) > > + : CameraData(pipe), exposureTime_(0) > > { > > } > > > > @@ -62,6 +62,8 @@ public: > > Stream outStream_; > > Stream vfStream_; > > Stream rawStream_; > > + > > + int32_t exposureTime_; > > }; > > > > class IPU3CameraConfiguration : public CameraConfiguration > > @@ -119,6 +121,7 @@ private: > > PipelineHandler::cameraData(camera)); > > } > > > > + int initControls(IPU3CameraData *data); > > int registerCameras(); > > > > int allocateBuffers(Camera *camera); > > @@ -731,6 +734,60 @@ bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator) > > return ret == 0; > > } > > > > +/* > > + * \brief Initialize the camera controls > > + * \param[in] data The camera data > > + * > > + * Initialize the camera controls by registering the pipeline handler > > + * ones along with the controls assembled by inspecting the sensor > > + * capabilities. > > + * > > + * \return 0 on success or a negative error code for error > > + */ > > +int PipelineHandlerIPU3::initControls(IPU3CameraData *data) > > +{ > > + const CameraSensor *sensor = data->cio2_.sensor(); > > + CameraSensorInfo sensorInfo{}; > > + > > + int ret = sensor->sensorInfo(&sensorInfo); > > + if (ret) > > + return ret; > > + > > + ControlInfoMap::Map controls = IPU3Controls; > > + > > + /* > > + * Compute exposure time limits. > > + * > > + * \todo The exposure limits depend on the sensor configuration. > > + * Initialize the control using the line lenght and pixel rate of the > > + * current configurtion, as reported by the CameraSensorInfo. Use the > > + * V4L2_CID_EXPOSURE control to get exposure min and max and convert it > > + * from lines into micro-seconds. > > + */ > > + float pixelRate = sensorInfo.pixelRate / 1e6f; > > + const ControlInfoMap &sensorControls = sensor->controls(); > > + const ControlInfo &v4l2Exposure = sensorControls.find(V4L2_CID_EXPOSURE)->second; > > + int32_t minExposure = v4l2Exposure.min().get<int32_t>() > > + * sensorInfo.lineLength / pixelRate; > > + int32_t maxExposure = v4l2Exposure.max().get<int32_t>() > > + * sensorInfo.lineLength / pixelRate; > > + int32_t defExposure = v4l2Exposure.def().get<int32_t>() > > + * sensorInfo.lineLength / pixelRate; > > + > > + /* > > + * \todo Report the actual exposure time, use the default for the > > + * moment. > > + */ > > + data->exposureTime_ = defExposure; > > + > > + controls[&controls::ExposureTime] = ControlInfo(minExposure, maxExposure, > > + defExposure); > > nit: This is the only usage of of the local 'controls' variable, would > it make sens to remove it and use 'data->controlInfo_' directly and > avoid the std::move() below? While reading the code I thought there more > things going on here then an advance optimization ;-) If I'm not mistaken that's actually how the ControlInfoMap API requires an instance to be intialized, to trigger the generation of the numerical ids map and support access by numerical index. > > With or without this fix, but with the issues Laurent points out fixed, > > Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > Thanks j > > + > > + data->controlInfo_ = std::move(controls); > > + > > + return 0; > > +} > > + > > /** > > * \brief Initialise ImgU and CIO2 devices associated with cameras > > * > > @@ -776,8 +833,9 @@ int PipelineHandlerIPU3::registerCameras() > > /* Initialize the camera properties. */ > > data->properties_ = cio2->sensor()->properties(); > > > > - /* Initialze the camera controls. */ > > - data->controlInfo_ = IPU3Controls; > > + ret = initControls(data.get()); > > + if (ret) > > + continue; > > > > /** > > * \todo Dynamically assign ImgU and output devices to each > > @@ -842,6 +900,7 @@ void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer) > > > > /* Mark the request as complete. */ > > request->metadata().set(controls::draft::PipelineDepth, 3); > > + request->metadata().set(controls::ExposureTime, exposureTime_); > > pipe_->completeRequest(request); > > } > > > > -- > > 2.29.2 > > > > _______________________________________________ > > libcamera-devel mailing list > > libcamera-devel@lists.libcamera.org > > https://lists.libcamera.org/listinfo/libcamera-devel > > -- > Regards, > Niklas Söderlund
Hi Jacopo, On Mon, Jan 18, 2021 at 01:27:32PM +0100, Jacopo Mondi wrote: > On Mon, Jan 11, 2021 at 12:22:54AM +0200, Laurent Pinchart wrote: > > Hi Jacopo, > > > > Thank you for the patch. > > > > > + */ > > > + float pixelRate = sensorInfo.pixelRate / 1e6f; > > > + const ControlInfoMap &sensorControls = sensor->controls(); > > > + const ControlInfo &v4l2Exposure = sensorControls.find(V4L2_CID_EXPOSURE)->second; > > > + int32_t minExposure = v4l2Exposure.min().get<int32_t>() > > > + * sensorInfo.lineLength / pixelRate; > > > + int32_t maxExposure = v4l2Exposure.max().get<int32_t>() > > > + * sensorInfo.lineLength / pixelRate; > > > + int32_t defExposure = v4l2Exposure.def().get<int32_t>() > > > + * sensorInfo.lineLength / pixelRate; > > > > For long exposure time this will overflow as the first two operands are > > 32-bit integers. How about the following ? > > > > double lineDuration = sensorInfo.pixelRate / 1e6f /pixelRate; > > did you mean: > sensorInfo.lineLength / (sensorInfo.pixelRate / 1e6f) > or rather > sensorInfo.lineLength * 1e6f / sensorInfo.pixelRate Yes, that's what I meant, sorry. > but there's an overflow risk as well here. In your code v4l2Exposure.min().get<int32_t>() is an int32_t, and sensorInfo.lineLength is an uint32_t. Multiplying the two has a high risk of overflow. Computing lineDuration as double lineDuration = sensorInfo.lineLength * 1e6f / sensorInfo.pixelRate; won't overflow as multiplying by 1e6f first will produce a float value. > > ... > > int32_t minExposure = v4l2Exposure.min().get<int32_t>() * lineDuration; > > int32_t maxExposure = v4l2Exposure.max().get<int32_t>() * lineDuration; > > int32_t defExposure = v4l2Exposure.def().get<int32_t>() * lineDuration; Then these multiplications may overflow only if the result doesn't fit in an int32_t, which would happen if the exposure time is larger than ~35 minutes. I think that's unlikely. > > > + > > > + /* > > > + * \todo Report the actual exposure time, use the default for the > > > + * moment. > > > + */ > > > + data->exposureTime_ = defExposure; > > > + > > > + controls[&controls::ExposureTime] = ControlInfo(minExposure, maxExposure, > > > + defExposure); > > > + > > > + data->controlInfo_ = std::move(controls); > > > + > > > + return 0; > > > +} > > > + > > > /** > > > * \brief Initialise ImgU and CIO2 devices associated with cameras > > > * > > > @@ -776,8 +833,9 @@ int PipelineHandlerIPU3::registerCameras() > > > /* Initialize the camera properties. */ > > > data->properties_ = cio2->sensor()->properties(); > > > > > > - /* Initialze the camera controls. */ > > > - data->controlInfo_ = IPU3Controls; > > > + ret = initControls(data.get()); > > > + if (ret) > > > + continue; > > > > > > /** > > > * \todo Dynamically assign ImgU and output devices to each > > > @@ -842,6 +900,7 @@ void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer) > > > > > > /* Mark the request as complete. */ > > > request->metadata().set(controls::draft::PipelineDepth, 3); > > > > /* \todo Move the ExposureTime control to the IPA */ > > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > > > + request->metadata().set(controls::ExposureTime, exposureTime_); > > > pipe_->completeRequest(request); > > > }
diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index f1151733d9fe..879057dab328 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -41,7 +41,7 @@ static constexpr unsigned int IMGU_OUTPUT_HEIGHT_ALIGN = 4; static constexpr unsigned int IMGU_OUTPUT_WIDTH_MARGIN = 64; static constexpr unsigned int IMGU_OUTPUT_HEIGHT_MARGIN = 32; -static const ControlInfoMap IPU3Controls = { +static const ControlInfoMap::Map IPU3Controls = { { &controls::draft::PipelineDepth, ControlInfo(2, 3) }, }; @@ -49,7 +49,7 @@ class IPU3CameraData : public CameraData { public: IPU3CameraData(PipelineHandler *pipe) - : CameraData(pipe) + : CameraData(pipe), exposureTime_(0) { } @@ -62,6 +62,8 @@ public: Stream outStream_; Stream vfStream_; Stream rawStream_; + + int32_t exposureTime_; }; class IPU3CameraConfiguration : public CameraConfiguration @@ -119,6 +121,7 @@ private: PipelineHandler::cameraData(camera)); } + int initControls(IPU3CameraData *data); int registerCameras(); int allocateBuffers(Camera *camera); @@ -731,6 +734,60 @@ bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator) return ret == 0; } +/* + * \brief Initialize the camera controls + * \param[in] data The camera data + * + * Initialize the camera controls by registering the pipeline handler + * ones along with the controls assembled by inspecting the sensor + * capabilities. + * + * \return 0 on success or a negative error code for error + */ +int PipelineHandlerIPU3::initControls(IPU3CameraData *data) +{ + const CameraSensor *sensor = data->cio2_.sensor(); + CameraSensorInfo sensorInfo{}; + + int ret = sensor->sensorInfo(&sensorInfo); + if (ret) + return ret; + + ControlInfoMap::Map controls = IPU3Controls; + + /* + * Compute exposure time limits. + * + * \todo The exposure limits depend on the sensor configuration. + * Initialize the control using the line lenght and pixel rate of the + * current configurtion, as reported by the CameraSensorInfo. Use the + * V4L2_CID_EXPOSURE control to get exposure min and max and convert it + * from lines into micro-seconds. + */ + float pixelRate = sensorInfo.pixelRate / 1e6f; + const ControlInfoMap &sensorControls = sensor->controls(); + const ControlInfo &v4l2Exposure = sensorControls.find(V4L2_CID_EXPOSURE)->second; + int32_t minExposure = v4l2Exposure.min().get<int32_t>() + * sensorInfo.lineLength / pixelRate; + int32_t maxExposure = v4l2Exposure.max().get<int32_t>() + * sensorInfo.lineLength / pixelRate; + int32_t defExposure = v4l2Exposure.def().get<int32_t>() + * sensorInfo.lineLength / pixelRate; + + /* + * \todo Report the actual exposure time, use the default for the + * moment. + */ + data->exposureTime_ = defExposure; + + controls[&controls::ExposureTime] = ControlInfo(minExposure, maxExposure, + defExposure); + + data->controlInfo_ = std::move(controls); + + return 0; +} + /** * \brief Initialise ImgU and CIO2 devices associated with cameras * @@ -776,8 +833,9 @@ int PipelineHandlerIPU3::registerCameras() /* Initialize the camera properties. */ data->properties_ = cio2->sensor()->properties(); - /* Initialze the camera controls. */ - data->controlInfo_ = IPU3Controls; + ret = initControls(data.get()); + if (ret) + continue; /** * \todo Dynamically assign ImgU and output devices to each @@ -842,6 +900,7 @@ void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer) /* Mark the request as complete. */ request->metadata().set(controls::draft::PipelineDepth, 3); + request->metadata().set(controls::ExposureTime, exposureTime_); pipe_->completeRequest(request); }
Calculate the controls::Exposure limits at camera registration time and register it in the list of camera supported controls. Cache the default exposure value to report it in the request metadata. Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> --- src/libcamera/pipeline/ipu3/ipu3.cpp | 67 ++++++++++++++++++++++++++-- 1 file changed, 63 insertions(+), 4 deletions(-)