[libcamera-devel,v3,1/6] libcamera: Add SensorOutputSize property
diff mbox series

Message ID 20200929164000.15429-2-david.plowman@raspberrypi.com
State Superseded
Headers show
Series
  • Digital zoom
Related show

Commit Message

David Plowman Sept. 29, 2020, 4:39 p.m. UTC 
The SensorOutputSize camera property reports the image size that the
next step in processing after the sensor and CSI-2 receiver - usually
the ISP - will see. It will normally change when a new camera mode is
selected, and can be used to implement digital zoom.

Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
---
 src/libcamera/property_ids.yaml | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

Comments

Kieran Bingham Sept. 29, 2020, 8:07 p.m. UTC | #1 
Hi David,

On 29/09/2020 17:39, David Plowman wrote:
> The SensorOutputSize camera property reports the image size that the
> next step in processing after the sensor and CSI-2 receiver - usually
> the ISP - will see. It will normally change when a new camera mode is
> selected, and can be used to implement digital zoom.
> 
> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  src/libcamera/property_ids.yaml | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml
> index 74ad0195..8011b88c 100644
> --- a/src/libcamera/property_ids.yaml
> +++ b/src/libcamera/property_ids.yaml
> @@ -640,4 +640,23 @@ controls:
>          \todo Rename this property to ActiveAreas once we will have property
>                categories (i.e. Properties::PixelArray::ActiveAreas)
>  
> +  - SensorOutputSize:
> +      type: Size
> +      description: |
> +        The size, in pixels, of the image being used to produce the
> +        desired output streams. The image size might correspond to the
> +        size of the frames produced by the image sensor but would also
> +        take into account additional cropping (or even re-scaling)
> +        performed by the CSI-2 receiver to adjust the sensor frame
> +        size to conform to the output image sizes and aspect ratios.
> +        The property is meaningful only after the Camera has been
> +        successfully configured and its value changes whenever a new
> +        configuration is applied. It can be used to implement digital
> +        zoom.
> +

I assume this is after all binning etc too, but that's part of the mode
selection so I think that's implied.

> +        \sa controls::ISPCrop
> +
> +        \todo Move this property to CameraConfiguration once the
> +        feature is made available

Instead of the Camera Sensor you mean ?

Can't see anything wrong though so...

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> +
>  ...
>
Jacopo Mondi Sept. 30, 2020, 8:33 a.m. UTC | #2 
Hi kieran

On Tue, Sep 29, 2020 at 09:07:08PM +0100, Kieran Bingham wrote:
> Hi David,
>
> On 29/09/2020 17:39, David Plowman wrote:
> > The SensorOutputSize camera property reports the image size that the
> > next step in processing after the sensor and CSI-2 receiver - usually
> > the ISP - will see. It will normally change when a new camera mode is
> > selected, and can be used to implement digital zoom.
> >
> > Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
> > ---
> >  src/libcamera/property_ids.yaml | 19 +++++++++++++++++++
> >  1 file changed, 19 insertions(+)
> >
> > diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml
> > index 74ad0195..8011b88c 100644
> > --- a/src/libcamera/property_ids.yaml
> > +++ b/src/libcamera/property_ids.yaml
> > @@ -640,4 +640,23 @@ controls:
> >          \todo Rename this property to ActiveAreas once we will have property
> >                categories (i.e. Properties::PixelArray::ActiveAreas)
> >
> > +  - SensorOutputSize:
> > +      type: Size
> > +      description: |
> > +        The size, in pixels, of the image being used to produce the
> > +        desired output streams. The image size might correspond to the
> > +        size of the frames produced by the image sensor but would also
> > +        take into account additional cropping (or even re-scaling)
> > +        performed by the CSI-2 receiver to adjust the sensor frame
> > +        size to conform to the output image sizes and aspect ratios.
> > +        The property is meaningful only after the Camera has been
> > +        successfully configured and its value changes whenever a new
> > +        configuration is applied. It can be used to implement digital
> > +        zoom.
> > +
>
> I assume this is after all binning etc too, but that's part of the mode
> selection so I think that's implied.
>
> > +        \sa controls::ISPCrop
> > +
> > +        \todo Move this property to CameraConfiguration once the
> > +        feature is made available
>
> Instead of the Camera Sensor you mean ?

Instead of the Camera properties

>
> Can't see anything wrong though so...
>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
>
> > +
> >  ...
> >
>
> --
> Regards
> --
> Kieran
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Kieran Bingham Sept. 30, 2020, 8:33 a.m. UTC | #3 
Hi Jacopo,

On 30/09/2020 09:33, Jacopo Mondi wrote:
> Hi kieran
> 
> On Tue, Sep 29, 2020 at 09:07:08PM +0100, Kieran Bingham wrote:
>> Hi David,
>>
>> On 29/09/2020 17:39, David Plowman wrote:
>>> The SensorOutputSize camera property reports the image size that the
>>> next step in processing after the sensor and CSI-2 receiver - usually
>>> the ISP - will see. It will normally change when a new camera mode is
>>> selected, and can be used to implement digital zoom.
>>>
>>> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
>>> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
>>> ---
>>>  src/libcamera/property_ids.yaml | 19 +++++++++++++++++++
>>>  1 file changed, 19 insertions(+)
>>>
>>> diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml
>>> index 74ad0195..8011b88c 100644
>>> --- a/src/libcamera/property_ids.yaml
>>> +++ b/src/libcamera/property_ids.yaml
>>> @@ -640,4 +640,23 @@ controls:
>>>          \todo Rename this property to ActiveAreas once we will have property
>>>                categories (i.e. Properties::PixelArray::ActiveAreas)
>>>
>>> +  - SensorOutputSize:
>>> +      type: Size
>>> +      description: |
>>> +        The size, in pixels, of the image being used to produce the
>>> +        desired output streams. The image size might correspond to the
>>> +        size of the frames produced by the image sensor but would also
>>> +        take into account additional cropping (or even re-scaling)
>>> +        performed by the CSI-2 receiver to adjust the sensor frame
>>> +        size to conform to the output image sizes and aspect ratios.
>>> +        The property is meaningful only after the Camera has been
>>> +        successfully configured and its value changes whenever a new
>>> +        configuration is applied. It can be used to implement digital
>>> +        zoom.
>>> +
>>
>> I assume this is after all binning etc too, but that's part of the mode
>> selection so I think that's implied.
>>
>>> +        \sa controls::ISPCrop
>>> +
>>> +        \todo Move this property to CameraConfiguration once the
>>> +        feature is made available
>>
>> Instead of the Camera Sensor you mean ?
> 
> Instead of the Camera properties


Aha ok - yes that's clearer ;-)

Does that mean perhaps the SensorOutputSize property is something that
would be calculated at validate() time then ?

--
Kieran


>>
>> Can't see anything wrong though so...
>>
>> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
>>
>>> +
>>>  ...
>>>
>>
>> --
>> Regards
>> --
>> Kieran
>> _______________________________________________
>> libcamera-devel mailing list
>> libcamera-devel@lists.libcamera.org
>> https://lists.libcamera.org/listinfo/libcamera-devel
Jacopo Mondi Sept. 30, 2020, 8:44 a.m. UTC | #4 
Hi Kieran,

On Wed, Sep 30, 2020 at 09:33:49AM +0100, Kieran Bingham wrote:
> Hi Jacopo,
>
> On 30/09/2020 09:33, Jacopo Mondi wrote:
> > Hi kieran
> >
> > On Tue, Sep 29, 2020 at 09:07:08PM +0100, Kieran Bingham wrote:
> >> Hi David,
> >>
> >> On 29/09/2020 17:39, David Plowman wrote:
> >>> The SensorOutputSize camera property reports the image size that the
> >>> next step in processing after the sensor and CSI-2 receiver - usually
> >>> the ISP - will see. It will normally change when a new camera mode is
> >>> selected, and can be used to implement digital zoom.
> >>>
> >>> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> >>> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
> >>> ---
> >>>  src/libcamera/property_ids.yaml | 19 +++++++++++++++++++
> >>>  1 file changed, 19 insertions(+)
> >>>
> >>> diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml
> >>> index 74ad0195..8011b88c 100644
> >>> --- a/src/libcamera/property_ids.yaml
> >>> +++ b/src/libcamera/property_ids.yaml
> >>> @@ -640,4 +640,23 @@ controls:
> >>>          \todo Rename this property to ActiveAreas once we will have property
> >>>                categories (i.e. Properties::PixelArray::ActiveAreas)
> >>>
> >>> +  - SensorOutputSize:
> >>> +      type: Size
> >>> +      description: |
> >>> +        The size, in pixels, of the image being used to produce the
> >>> +        desired output streams. The image size might correspond to the
> >>> +        size of the frames produced by the image sensor but would also
> >>> +        take into account additional cropping (or even re-scaling)
> >>> +        performed by the CSI-2 receiver to adjust the sensor frame
> >>> +        size to conform to the output image sizes and aspect ratios.
> >>> +        The property is meaningful only after the Camera has been
> >>> +        successfully configured and its value changes whenever a new
> >>> +        configuration is applied. It can be used to implement digital
> >>> +        zoom.
> >>> +
> >>
> >> I assume this is after all binning etc too, but that's part of the mode
> >> selection so I think that's implied.
> >>
> >>> +        \sa controls::ISPCrop
> >>> +
> >>> +        \todo Move this property to CameraConfiguration once the
> >>> +        feature is made available
> >>
> >> Instead of the Camera Sensor you mean ?
> >
> > Instead of the Camera properties
>
>
> Aha ok - yes that's clearer ;-)
>
> Does that mean perhaps the SensorOutputSize property is something that
> would be calculated at validate() time then ?

Once it has been moved to CameraConfiguration, yes. Right now as it is
updated when a CameraConfiguration is applied to a Camera,
Camera::configure() is the 'right' place where to calculate it imho.

Thanks
   j

>
> --
> Kieran
>
>
> >>
> >> Can't see anything wrong though so...
> >>
> >> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> >>
> >>> +
> >>>  ...
> >>>
> >>
> >> --
> >> Regards
> >> --
> >> Kieran
> >> _______________________________________________
> >> libcamera-devel mailing list
> >> libcamera-devel@lists.libcamera.org
> >> https://lists.libcamera.org/listinfo/libcamera-devel
>
> --
> Regards
> --
> Kieran
Kieran Bingham Sept. 30, 2020, 8:50 a.m. UTC | #5 
Hello,

On 30/09/2020 09:44, Jacopo Mondi wrote:
> Hi Kieran,
> 
> On Wed, Sep 30, 2020 at 09:33:49AM +0100, Kieran Bingham wrote:
>> Hi Jacopo,
>>
>> On 30/09/2020 09:33, Jacopo Mondi wrote:
>>> Hi kieran
>>>
>>> On Tue, Sep 29, 2020 at 09:07:08PM +0100, Kieran Bingham wrote:
>>>> Hi David,
>>>>
>>>> On 29/09/2020 17:39, David Plowman wrote:
>>>>> The SensorOutputSize camera property reports the image size that the
>>>>> next step in processing after the sensor and CSI-2 receiver - usually
>>>>> the ISP - will see. It will normally change when a new camera mode is
>>>>> selected, and can be used to implement digital zoom.
>>>>>
>>>>> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
>>>>> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
>>>>> ---
>>>>>  src/libcamera/property_ids.yaml | 19 +++++++++++++++++++
>>>>>  1 file changed, 19 insertions(+)
>>>>>
>>>>> diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml
>>>>> index 74ad0195..8011b88c 100644
>>>>> --- a/src/libcamera/property_ids.yaml
>>>>> +++ b/src/libcamera/property_ids.yaml
>>>>> @@ -640,4 +640,23 @@ controls:
>>>>>          \todo Rename this property to ActiveAreas once we will have property
>>>>>                categories (i.e. Properties::PixelArray::ActiveAreas)
>>>>>
>>>>> +  - SensorOutputSize:
>>>>> +      type: Size
>>>>> +      description: |
>>>>> +        The size, in pixels, of the image being used to produce the
>>>>> +        desired output streams. The image size might correspond to the
>>>>> +        size of the frames produced by the image sensor but would also
>>>>> +        take into account additional cropping (or even re-scaling)
>>>>> +        performed by the CSI-2 receiver to adjust the sensor frame
>>>>> +        size to conform to the output image sizes and aspect ratios.
>>>>> +        The property is meaningful only after the Camera has been
>>>>> +        successfully configured and its value changes whenever a new
>>>>> +        configuration is applied. It can be used to implement digital
>>>>> +        zoom.

I wonder if we can highlight the 'when a property is valid' in some way.
I see it is in the text of course, but I feel like that deserves a
highlight with a big neon sign around it.

Anyway, thanks for the discussions clearing up the usage. I can see it
all better now.

>>>>> +
>>>>
>>>> I assume this is after all binning etc too, but that's part of the mode
>>>> selection so I think that's implied.
>>>>
>>>>> +        \sa controls::ISPCrop
>>>>> +
>>>>> +        \todo Move this property to CameraConfiguration once the
>>>>> +        feature is made available
>>>>
>>>> Instead of the Camera Sensor you mean ?
>>>
>>> Instead of the Camera properties
>>
>>
>> Aha ok - yes that's clearer ;-)
>>
>> Does that mean perhaps the SensorOutputSize property is something that
>> would be calculated at validate() time then ?
> 
> Once it has been moved to CameraConfiguration, yes. Right now as it is
> updated when a CameraConfiguration is applied to a Camera,
> Camera::configure() is the 'right' place where to calculate it imho.
> 
> Thanks
>    j
> 
>>
>> --
>> Kieran
>>
>>
>>>>
>>>> Can't see anything wrong though so...
>>>>
>>>> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
>>>>
>>>>> +
>>>>>  ...
>>>>>
>>>>
>>>> --
>>>> Regards
>>>> --
>>>> Kieran
>>>> _______________________________________________
>>>> libcamera-devel mailing list
>>>> libcamera-devel@lists.libcamera.org
>>>> https://lists.libcamera.org/listinfo/libcamera-devel
>>
>> --
>> Regards
>> --
>> Kieran

Patch
diff mbox series 

diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml
index 74ad0195..8011b88c 100644
--- a/src/libcamera/property_ids.yaml
+++ b/src/libcamera/property_ids.yaml
@@ -640,4 +640,23 @@  controls:
         \todo Rename this property to ActiveAreas once we will have property
               categories (i.e. Properties::PixelArray::ActiveAreas)
 
+  - SensorOutputSize:
+      type: Size
+      description: |
+        The size, in pixels, of the image being used to produce the
+        desired output streams. The image size might correspond to the
+        size of the frames produced by the image sensor but would also
+        take into account additional cropping (or even re-scaling)
+        performed by the CSI-2 receiver to adjust the sensor frame
+        size to conform to the output image sizes and aspect ratios.
+        The property is meaningful only after the Camera has been
+        successfully configured and its value changes whenever a new
+        configuration is applied. It can be used to implement digital
+        zoom.
+
+        \sa controls::ISPCrop
+
+        \todo Move this property to CameraConfiguration once the
+        feature is made available
+
 ...