[libcamera-devel] [PATCH v2 1/2] libcamera: camera_sensor: Fix frame lengths calculated by sensorInfo()
Kieran Bingham
kieran.bingham at ideasonboard.com
Tue Apr 13 23:34:26 CEST 2021
Hi David,
On 07/04/2021 10:33, David Plowman wrote:
> Hi Jacopo
>
> On Tue, 6 Apr 2021 at 13:09, Jacopo Mondi <jacopo at jmondi.org> wrote:
>>
>> Hi David,
>>
>> On Tue, Apr 06, 2021 at 11:40:49AM +0100, David Plowman wrote:
>>> The minimum and maximum vblanking can change when a new format is
>>> applied to the sensor subdevice, so be sure to retrieve up-to-date
>>> values.
>>>
>>> The V4L2Device acquires the new updateControlInfos() method to perform
>>> this function, and which the CameraSensor calls automatically if its
>>> setFormat method is used to update the sensor.
>>>
>>> However, not all pipeline handlers invoke the setFormat method
>>> directly, so the new method must be made publicly available for
>>> pipeline handlers to call if they need to.
>>>
>>> Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
>>> ---
>>> include/libcamera/internal/camera_sensor.h | 2 ++
>>> include/libcamera/internal/v4l2_device.h | 2 ++
>>> src/libcamera/camera_sensor.cpp | 31 +++++++++++++++++++++-
>>> src/libcamera/v4l2_device.cpp | 26 ++++++++++++++++++
>>> 4 files changed, 60 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h
>>> index 3e98f71b..c94744f0 100644
>>> --- a/include/libcamera/internal/camera_sensor.h
>>> +++ b/include/libcamera/internal/camera_sensor.h
>>> @@ -68,6 +68,8 @@ public:
>>> const ControlList &properties() const { return properties_; }
>>> int sensorInfo(CameraSensorInfo *info) const;
>>>
>>> + void updateControlInfos();
>>> +
>>> protected:
>>> std::string logPrefix() const override;
>>>
>>> diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h
>>> index d006bf68..274cbe65 100644
>>> --- a/include/libcamera/internal/v4l2_device.h
>>> +++ b/include/libcamera/internal/v4l2_device.h
>>> @@ -41,6 +41,8 @@ public:
>>> int setFrameStartEnabled(bool enable);
>>> Signal<uint32_t> frameStart;
>>>
>>> + void updateControlInfos();
>>> +
>>> protected:
>>> V4L2Device(const std::string &deviceNode);
>>> ~V4L2Device();
>>> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
>>> index f7ed91d9..5dbe47a7 100644
>>> --- a/src/libcamera/camera_sensor.cpp
>>> +++ b/src/libcamera/camera_sensor.cpp
>>> @@ -656,15 +656,30 @@ V4L2SubdeviceFormat CameraSensor::getFormat(const std::vector<unsigned int> &mbu
>>> /**
>>> * \brief Set the sensor output format
>>> * \param[in] format The desired sensor output format
>>> + *
>>> + * The sensor output format is set. The ranges of any controls associated
>>> + * with the sensor are also updated.
>>> + *
>>> * \return 0 on success or a negative error code otherwise
>>> */
>>> int CameraSensor::setFormat(V4L2SubdeviceFormat *format)
>>> {
>>> - return subdev_->setFormat(pad_, format);
>>> + int ret = subdev_->setFormat(pad_, format);
>>> +
>>> + if (ret == 0)
>>> + updateControlInfos();
>>> +
>>> + return ret;
>>> }
>>>
>>> /**
>>> * \brief Retrieve the supported V4L2 controls and their information
>>> + *
>>> + * Pipeline handlers that do not change the sensor format using the
>>> + * CameraSensor::setFormat method may need to call
>>> + * CameraSensor::updateControlInfos beforehand, to ensure all the control
>>> + * ranges are up to date.
>>> + *
>>> * \return A map of the V4L2 controls supported by the sensor
>>> */
>>> const ControlInfoMap &CameraSensor::controls() const
>>> @@ -750,6 +765,11 @@ int CameraSensor::setControls(ControlList *ctrls)
>>> * Sensor information is only available for raw sensors. When called for a YUV
>>> * sensor, this function returns -EINVAL.
>>> *
>>> + * Pipeline handlers that do not change the sensor format using the
>>> + * CameraSensor::setFormat method may need to call
>>> + * CameraSensor::updateControlInfos beforehand, to ensure all the control
>>> + * ranges are up to date.
>>> + *
>>> * \return 0 on success, a negative error code otherwise
>>> */
>>> int CameraSensor::sensorInfo(CameraSensorInfo *info) const
>>> @@ -821,6 +841,15 @@ int CameraSensor::sensorInfo(CameraSensorInfo *info) const
>>> return 0;
>>> }
>>>
>>> +/**
>>> + * \fn void CameraSensor::updateControlInfos()
>>> + * \brief Update the sensor's ControlInfos in case they have changed
>>> + */
>>> +void CameraSensor::updateControlInfos()
>>> +{
>>> + subdev_->updateControlInfos();
>>> +}
>>> +
>>> std::string CameraSensor::logPrefix() const
>>> {
>>> return "'" + entity_->name() + "'";
>>> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
>>> index decd19ef..9678962c 100644
>>> --- a/src/libcamera/v4l2_device.cpp
>>> +++ b/src/libcamera/v4l2_device.cpp
>>> @@ -514,6 +514,32 @@ void V4L2Device::listControls()
>>> controls_ = std::move(ctrls);
>>> }
>>>
>>> +/*
>>
>> This is now a public method and needs /**
>> Doesn't doxygen give you a warning here ?
>
> Ah yes, the documentation seems to have stopped building some time
> ago... there appear to be a couple more dependencies I have to install
> for it to happen now. No problem!
I've discovered today that it's best to build doxygen from source as you
need a very recent version to have a quiet build.
For me that was as straightforward as:
git clone https://github.com/doxygen/doxygen
cd doxygen
mkdir build
cd build
cmake -DCMAKE_INSTALL_PREFIX=/usr ../ && make -j 8 all install
> Thanks
> David
>
>>
>> The patch looks good otherwise!
>> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
>>
>> Thanks
>> j
>>
>>> + * \brief Update the control information that was previously stored by
>>> + * listControls(). Some parts of this information, such as min and max
>>> + * values of some controls, are liable to change when a new format is set.
>>> + */
>>> +void V4L2Device::updateControlInfos()
>>> +{
>>> + for (auto &controlId : controlIds_) {
>>> + unsigned int id = controlId->id();
>>> +
>>> + struct v4l2_query_ext_ctrl ctrl = {};
>>> + ctrl.id = id;
>>> +
>>> + if (ioctl(VIDIOC_QUERY_EXT_CTRL, &ctrl)) {
>>> + LOG(V4L2, Debug)
>>> + << "Could not refresh control "
>>> + << utils::hex(ctrl.id);
>>> + continue;
>>> + }
I'm curious if 'all' controls need to be refreshed, but I think that's
simpler that trying to filter it with a list that would then bitrot.
So...
Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>>> +
>>> + /* Assume these are present - listControls() made them. */
>>> + controlInfo_[id] = ctrl;
>>> + controls_.at(controlId.get()) = V4L2ControlInfo(ctrl);
>>> + }
>>> +}
>>> +
>>> /*
>>> * \brief Update the value of the first \a count V4L2 controls in \a ctrls using
>>> * values in \a v4l2Ctrls
>>> --
>>> 2.20.1
>>>
>>> _______________________________________________
>>> libcamera-devel mailing list
>>> libcamera-devel at lists.libcamera.org
>>> https://lists.libcamera.org/listinfo/libcamera-devel
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
>
--
Regards
--
Kieran
More information about the libcamera-devel
mailing list