[libcamera-devel] [PATCH v2 6/6] [HACK] ipu3: Set and get a few sensor controls
Kieran Bingham
kieran.bingham at ideasonboard.com
Wed Jun 12 00:53:19 CEST 2019
Hi Jacopo,
On 11/06/2019 16:05, Jacopo Mondi wrote:
> Hi Kieran,
>
> On Tue, Jun 11, 2019 at 03:09:51PM +0100, Kieran Bingham wrote:
>> Hi Jacopo,
>>
>> On 10/06/2019 17:40, Jacopo Mondi wrote:
>>> Not to merge patch to demonstrate how to set controls on the image
>>> sensor device.
>>>
>>> Not-Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
>>> ---
>>> src/libcamera/pipeline/ipu3/ipu3.cpp | 80 ++++++++++++++++++++++++++++
>>> 1 file changed, 80 insertions(+)
>>>
>>> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
>>> index b3e7fb0e9c64..59c1fe3c56fd 100644
>>> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
>>> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
>>> @@ -688,6 +688,86 @@ int PipelineHandlerIPU3::start(Camera *camera)
>>> ImgUDevice *imgu = data->imgu_;
>>> int ret;
>>>
>>> + /* --- Get control values --- */
>>> + std::vector<unsigned int> controlIds = {
>>> + V4L2_CID_EXPOSURE, V4L2_CID_ANALOGUE_GAIN,
>>> + };
>>> + V4L2Controls controls;
>>> + ret = cio2->sensor_->dev()->getControls(controlIds, &controls);
>>> + if (ret) {
>>> + LOG(Error) << "Failed to get control values";
>>> + return -EINVAL;
>>> + }
>>> +
>>
>> Moving some logic for printing the values (or working out how to use
>> ControlValue class in a V4L2Control) would simplify any debug prints
>> greatly:
>>
>> for (V4L2Control *ctrl : controls) {
>> LOG(Error) << "Control : " << id
>> << " - value: " << ctrl->value();
>> }
>>
>
> Sorry, I might have missed how that would change using ControlValue.
> That class has getInt, getBool etc, right?
The class ControlValue provides toString() and << overrides, so you can
do a direct << on a ControlValue.
It simplifies debug prints at least. Re-reading my statement, it looks
like I hadn't completed my sentence fully. I was trynig to say, "If you
move the logic for printing the values into the V4L2Control class or
V4L2ControlValue or such ..."
>>
>>> + for (V4L2Control *ctrl : controls) {
>>> + unsigned int id = ctrl->id();
>>> +
>>> + switch(ctrl->type()) {
>>> + case V4L2_CTRL_TYPE_INTEGER:
>>> + case V4L2_CTRL_TYPE_BOOLEAN:
>>> + {
>>> + uint32_t val = controls.getInt(id);
>>> + LOG(Error) << "Control : " << id
>>> + << " - value: " << val;
>>> + }
>>> + break;
>>> + case V4L2_CTRL_TYPE_INTEGER64:
>>> + {
>>> + uint64_t val = controls.getInt64(id);
>>> + LOG(Error) << "Control : " << id
>>> + << " - value: " << val;
>>> + }
>>> + break;
>>> + default:
>>> + LOG(Error) << "Unsupported type: " << ctrl->type();
>>> + return -EINVAL;
>>> + }
>>> + }
>>> +
>>
>>
>>
>>> + /* --- Set control values --- */
>>> + V4L2Controls setControls;
>>> + setControls.set(V4L2_CID_EXPOSURE, 2046);
>>> + setControls.set(V4L2_CID_ANALOGUE_GAIN, 1024);
>>> +
>>> + ret = cio2->sensor_->dev()->setControls(setControls);
>>> + if (ret) {
>>> + LOG(IPU3, Error) << "Failed to set controls";
>>> + return ret;
>>> + }
>>> +
>>> + /* --- Get control values back again and verify they have changed --- */
>>> + V4L2Controls newcontrols;
>>> + ret = cio2->sensor_->dev()->getControls(controlIds, &newcontrols);
>>> + if (ret) {
>>> + LOG(Error) << "Failed to get control values";
>>> + return -EINVAL;
>>> + }
>>> +
>>> + for (V4L2Control *ctrl : newcontrols) {
>>> + unsigned int id = ctrl->id();
>>> +
>>> + switch(ctrl->type()) {
>>> + case V4L2_CTRL_TYPE_INTEGER:
>>> + case V4L2_CTRL_TYPE_BOOLEAN:
>>> + {
>>> + uint32_t val = newcontrols.getInt(id);
>>
>> Eeep, Here we are iterating new controls to print them (I get that this
>> is just a print) - but we are then 'getting' the int from the list
>> container instead of the V4L2Control, which is involving 'finding' the
>> control again, and iterating the 'newcontrols' container for each print?
>>
>> Iterating the list twice seems painful to me... but perhaps that's just
>> because of this particular demo use case, and it might not be such an
>> issue in real usage.
>
> Accessing the V4L2Control value would require the user to cast the
> V4L2Control * to the appropriate specialized derived class, something
> we don't want.
>
> In this case yes, using a ControlValue object which has a field for
> all possible value types and provides a getInt() getBool() etc would
> remove the casting requirement and would allow to access a control
> value with a single call (provided the user knows the control value
> type, but we have the same issue here, where user should call the
> opportune get$TYPE() anyhow.
Yes, that will depend on where the type is needed to be stored.
But one thing it does do is allow value exchange without caring about
the type.
One reason why I'd really like to see this class be common (i.e. use
ControlValue) is because then pipeline handlers do not need to care
about the types in the most part. They could do direct copies or stores
of the ControlValue object, or have that object (from the application)
filled in directly.
The application must already know about the Type for dealing with
ControlValue class (which is application facing).
Pipelines could potentially find themselves dealing with lists of
controls to either get or set them, and they won't care about the
data-type; they will just be the bridge between the Application and the
V4L2 layer.
The application however, will know that if it is looking at a Brightness
control it is an integer, and if it's dealing with an Enable control
it's boolean.
Perhaps we might even enable signals and slots on controls so that the
application gets updated directly when a control is updated, allowing it
to update the GUI or such.
>>
>>> + LOG(Error) << "Control : " << id
>>> + << " - value: " << val;
>>> + }
>>> + break;
>>> + case V4L2_CTRL_TYPE_INTEGER64:
>>> + {
>>> + uint64_t val = newcontrols.getInt64(id);
>>> + LOG(Error) << "Control : " << id
>>> + << " - value: " << val;
>>> + }
>>> + break;
>>> + default:
>>> + LOG(Error) << "Unsupported type: " << ctrl->type();
>>> + return -EINVAL;
>>> + }
>>> + }
>>> +
>>> /*
>>> * Start the ImgU video devices, buffers will be queued to the
>>> * ImgU output and viewfinder when requests will be queued.
>>>
>>
>> --
>> Regards
>> --
>> Kieran
--
Regards
--
Kieran
More information about the libcamera-devel
mailing list