[libcamera-devel] [PATCH] libcamera: control_ids: Keep draft controls last
Kieran Bingham
kieran.bingham at ideasonboard.com
Mon Nov 23 10:33:27 CET 2020
On 23/11/2020 09:29, Jacopo Mondi wrote:
> Hi Kieran,
>
> On Mon, Nov 23, 2020 at 09:21:23AM +0000, Kieran Bingham wrote:
>> Hi Jacopo,
>>
>> On 23/11/2020 09:03, Jacopo Mondi wrote:
>>> Let's try not to mix draft controls and regular controls.
>>>
>>> Keep draft controls at the end of the control_ids.yaml file and
>>> add a comment to make clear where the draft controls section begins.
>>>
>>
>> I won't directly object to this if you are concerned about the existing
>> ordering - but it is not required to keep draft controls together.
>>
>> Changing a control from Draft to non-draft will not change it's
>> underlying control id value/enum value - but moving them around the file
>> will.
>
> Well, that's not entirely true. Changing a control from draft to
> non-draft might imply changing its values, we might even decide we
> want to express with two or more controls what is now expressed with a
> single one, or maybe merge two draft controls in a single one. Who
> knows :)
I was referring only to the underlying ID values. ;-)
>> Of course, we're likely to hit re-ordering anyway, as we find ourselves
>> refactoring draft controls ...
>>
>>
>> Perhaps that's a good reason to keep draft controls at the end anyway.
>> That way we won't affect the ID of any non-draft controls...
>
> You know, I sent this out mostly for frivolous reasons ("all draft
> controls shall go at the end of the file!"), but your point of having
> 'stable' controls first, with a non-changing ID has much more value,
> even if we're not looking for ABI stability at this point.
I agree here.
Mixing unstable controls with stable ones, will cause instability to
'stable' ids.
No need for that ;-) And also - no need for any actual sort order in
this file.
We can sort the documentation as a post-processing step in gen-controls
if we want the doxygen to be alphabetical, and keep the ID's as a
continuous - consecutive enum.
So yes, lets keep drafts at the bottom.
--
Kieran
>> Acked-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>>
>
> Thanks
> j
>
>>
>>> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
>>> ---
>>> src/libcamera/control_ids.yaml | 28 ++++++++++++++++------------
>>> 1 file changed, 16 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml
>>> index c8874fa91965..a883e27e22e9 100644
>>> --- a/src/libcamera/control_ids.yaml
>>> +++ b/src/libcamera/control_ids.yaml
>>> @@ -273,6 +273,22 @@ controls:
>>>
>>> size: [3x3]
>>>
>>> + - ScalerCrop:
>>> + type: Rectangle
>>> + description: |
>>> + Sets the image portion that will be scaled to form the whole of
>>> + the final output image. The (x,y) location of this rectangle is
>>> + relative to the PixelArrayActiveAreas that is being used. The units
>>> + remain native sensor pixels, even if the sensor is being used in
>>> + a binning or skipping mode.
>>> +
>>> + This control is only present when the pipeline supports scaling. Its
>>> + maximum valid value is given by the properties::ScalerCropMaximum
>>> + property, and the two can be used to implement digital zoom.
>>> +
>>> + # ----------------------------------------------------------------------------
>>> + # Draft controls section
>>> +
>>> - AePrecaptureTrigger:
>>> type: int32_t
>>> draft: true
>>> @@ -518,16 +534,4 @@ controls:
>>> detection, additional format conversions etc) count as an additional
>>> pipeline stage.
>>>
>>> - - ScalerCrop:
>>> - type: Rectangle
>>> - description: |
>>> - Sets the image portion that will be scaled to form the whole of
>>> - the final output image. The (x,y) location of this rectangle is
>>> - relative to the PixelArrayActiveAreas that is being used. The units
>>> - remain native sensor pixels, even if the sensor is being used in
>>> - a binning or skipping mode.
>>> -
>>> - This control is only present when the pipeline supports scaling. Its
>>> - maximum valid value is given by the properties::ScalerCropMaximum
>>> - property, and the two can be used to implement digital zoom.
>>> ...
>>>
>>
>> --
>> Regards
>> --
>> Kieran
--
Regards
--
Kieran
More information about the libcamera-devel
mailing list