[libcamera-devel] [PATCH 7/7] android: soraka: Add camera HAL configuration

[Jacopo Mondi]

Hi Laurent,

On Fri, Mar 26, 2021 at 05:31:36AM +0200, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> On Wed, Mar 24, 2021 at 12:25:27PM +0100, Jacopo Mondi wrote:
> > Add camera HAL configuration file for IPU3 Soraka.
> >
> > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > ---
> >  src/android/data/soraka/camera_hal.yaml | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> >  create mode 100644 src/android/data/soraka/camera_hal.yaml
> >
> > diff --git a/src/android/data/soraka/camera_hal.yaml b/src/android/data/soraka/camera_hal.yaml
> > new file mode 100644
> > index 000000000000..489e601ac5d0
> > --- /dev/null
> > +++ b/src/android/data/soraka/camera_hal.yaml
> > @@ -0,0 +1,14 @@
> > +- manufacturer: Google
> > +- model: Soraka
> > +
> > +- camera:
> > +  - name: "\\_SB_.PCI0.I2C4.CAM1"
> > +  - model: "ov5670"
> > +  - location: "front"
> > +  - rotation: 0
> > +
> > +- camera:
> > +  - name: "\\_SB_.PCI0.I2C2.CAM0"
> > +  - model: "ov13858"
> > +  - location: "back"
> > +  - rotation: 0
>
> I'm sure we can spend an endless amount of time bikeshedding on the YAML
> schema, so I'll only make one comment :-)
>
> At the top level, you have a list, with each list entry being a
> dictionary with a single key:value pair. There's no guarantee on unicity
> of properties with this type of schema, you could for instance have
> multiple "manufacturer" list entries. Same for the properties within
> each camera.
>
> This would I believe be a more natural usage of YAML (to the extent that
> YAML has anything natural about it):
>
> manufacturer: Google
> model: Soraka
>
> cameras:
>   - name: "\\_SB_.PCI0.I2C4.CAM1"
>     model: "ov5670"
>     location: "front"
>     rotation: 0
>
>   - name: "\\_SB_.PCI0.I2C2.CAM0"
>     model: "ov13858"
>     location: "back"
>     rotation: 0
>
> Or, possibly even better, to guarantee unicity of camera entries,
>
> manufacturer: Google
> model: Soraka
>
> cameras:
>   "\\_SB_.PCI0.I2C4.CAM1":
>     model: "ov5670"
>     location: "front"
>     rotation: 0
>
>   "\\_SB_.PCI0.I2C2.CAM0":
>     model: "ov13858"
>     location: "back"
>     rotation: 0
>

This clearly shows how poor my understanding of yaml is.
I'll go with the last proposed option as it guarantees unicity of the
cameras.

Thanks
  j


> The three options produce the following objects in Python:
>
> >>> pprint.pprint(yaml.safe_load(open('yaml1.yaml', 'rb').read()))
> [{'manufacturer': 'Google'},
>  {'model': 'Soraka'},
>  {'camera': [{'name': '\\_SB_.PCI0.I2C4.CAM1'},
>              {'model': 'ov5670'},
>              {'location': 'front'},
>              {'rotation': 0}]},
>  {'camera': [{'name': '\\_SB_.PCI0.I2C2.CAM0'},
>              {'model': 'ov13858'},
>              {'location': 'back'},
>              {'rotation': 0}]}]
> >>> pprint.pprint(yaml.safe_load(open('yaml2.yaml', 'rb').read()))
> {'cameras': [{'location': 'front',
>               'model': 'ov5670',
>               'name': '\\_SB_.PCI0.I2C4.CAM1',
>               'rotation': 0},
>              {'location': 'back',
>               'model': 'ov13858',
>               'name': '\\_SB_.PCI0.I2C2.CAM0',
>               'rotation': 0}],
>  'manufacturer': 'Google',
>  'model': 'Soraka'}
> >>> pprint.pprint(yaml.safe_load(open('yaml3.yaml', 'rb').read()))
> {'cameras': {'\\_SB_.PCI0.I2C2.CAM0': {'location': 'back',
>                                        'model': 'ov13858',
>                                        'rotation': 0},
>              '\\_SB_.PCI0.I2C4.CAM1': {'location': 'front',
>                                        'model': 'ov5670',
>                                        'rotation': 0}},
>  'manufacturer': 'Google',
>  'model': 'Soraka'}
>
> --
> Regards,
>
> Laurent Pinchart