[libcamera-devel] [PATCH v5 1/7] libcamera: properties: Add model property

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Fri, Sep 25, 2020 at 05:07:37PM +0200, Niklas Söderlund wrote:
> A user-friendly camera identification model name. The model name must
> to the extent possible describe the camera sensor model. For most
> devices this is the model name of the sensor. While for some devices the
> sensor model is unavailable as the sensor or the entire camera is part
> of a larger unit and exposed as a black-box to the system. In such cases
> the model name of the smallest component closest to the sensor must be
> used.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
> * Changes since v4
> - Expand description.
> 
> * Changes since v3
> - s/as ASCII/in ASCII/
> ---
>  src/libcamera/property_ids.yaml | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml
> index 74ad0195d6310367..b53e850c8b647c71 100644
> --- a/src/libcamera/property_ids.yaml
> +++ b/src/libcamera/property_ids.yaml
> @@ -387,6 +387,26 @@ controls:
>                                |                    |
>                                +--------------------+
>  
> +  - Model:
> +      type: string
> +      description: |
> +        A user-friendly camera identification model name. The model name must
> +        to the extent possible describe the camera sensor model. For most
> +        devices this is the model name of the sensor. While for some devices the
> +        sensor model is unavailable as the sensor or the entire camera is part
> +        of a larger unit and exposed as a black-box to the system. In such cases
> +        the model name of the smallest component closest to the sensor must be
> +        used.

This is clearer than in v4, but I'm still not very enthousiastic :-( The
camera sensor model isn't very end-user-friendly in general. And what
will happen when we will have multi-sensors cameras ?

I understand that this property isn't mean to be a camera name displayed
to the end-user, but should instead be combined with other camera
information (such as the location for instance) to create a camera name.
Maybe that should be documented here too. There's still something
ill-defined about the property that bothers me.

> +
> +        The model name is not guaranteed to be unique in the system nor does
> +        it guarantee to be stable or have any other properties required to make

s/does it guarantee/is it guaranteed/

> +        it a good candidate to be used as a permanent identifier of a camera.
> +
> +        The model name shall describe the camera in a human readable format and
> +        be shall be encoded in ASCII.

s/be shall be/shall be/

> +
> +        Example model names are 'ov5670', 'imx219' or 'Logitech Webcam C930e'.
> +
>    - UnitCellSize:
>        type: Size
>        description: |

-- 
Regards,

Laurent Pinchart