[libcamera-devel] [PATCH 1/1] libcamera: controls: Add DigitalGain control

[Kieran Bingham]

Hi Jacopo,

On 23/11/2020 08:58, Jacopo Mondi wrote:
> Hi Kieran,
> 
> On Mon, Nov 16, 2020 at 10:40:25AM +0000, Kieran Bingham wrote:
>> Hi David,
>>
>> On 27/10/2020 14:12, David Plowman wrote:
>>> This control reports the global digital gain applied by the pipeline
>>> as a whole, after capturing a raw image from the sensor.
>>>
>>> Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
>>> ---
>>>  src/libcamera/control_ids.yaml | 11 +++++++++++
>>>  1 file changed, 11 insertions(+)
>>>
>>> diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml
>>> index c8874fa9..e6362c74 100644
>>> --- a/src/libcamera/control_ids.yaml
>>> +++ b/src/libcamera/control_ids.yaml
>>> @@ -530,4 +530,15 @@ controls:
>>>          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.
>>> +
>>> +  - DigitalGain:
>>> +      type: float
>>> +      description: |
>>> +        Global digital gain value applied to the image during all the
>>> +        processing steps after capturing the image from the sensor. Any raw
>>> +        images, therefore, will not include this gain, but the final images
>>> +        output by the imaging pipeline as a whole will include it.
>>> +
>>> +        This control is intended to report the value used by the image
>>> +        processing pipeline.
>>
>>
>> If this is a per-stream thing anyway, I guess it will then be up to
>> pipeline handlers to set this to the appropriate value for each stream
>> when it completes. The fact that this value would not be applicable to a
>> RAW stream makes me think it certainly should be a per-stream metadata
>> style value.
>>
>>
>> I'd hope this could be handled by a common helper in that instance so it
>> doesn't get left out of some pipeline handlers, but included in some,
>> and become inconsistent. Not yet sure how we can handle that, but that
>> will be a core issue anyway.
>>
>>
>> I wonder if we should mark this somehow as read-only, at least until we
>> determine that someone needs to set it.
>>
>> We could introduce a control property between type: and description:
>>   read-only: true
> 
> Isn't a read-only control just a metadata ?
> 
> Wouldn't it be enough for a pipeline that does not support changing
> the control value from applications not reporting it in the list of
> supported Camera's controls, but only report it as part of a completed
> request's metadata ?

Ah, yes of course - because if the control is not listed as supported it
won't be there to set in the first place! I forgot about that.

So - indeed, no requirement to mark anything as read-only. That will be
implicit.

--
Kieran


> 
> Thanks
>   j
> 
>>
>>
>> Otherwise, I see no objections currently. I think we're just waiting on
>> top-level thoughts from Laurent. (And perhaps per-stream controls, but
>> that brings it's own questions )
>>
>> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>>
>>
>>>  ...
>>>
>>
>> --
>> Regards
>> --
>> Kieran
>> _______________________________________________
>> libcamera-devel mailing list
>> libcamera-devel at lists.libcamera.org
>> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards
--
Kieran