[libcamera-devel] [RFC v1 3/7] py: Move ControlValue helpers to py_helpers.cpp

[Tomi Valkeinen]

On 24/06/2022 11:22, Kieran Bingham wrote:
> Quoting Tomi Valkeinen (2022-06-23 15:47:32)
>> Clean up the py_main.cpp a bit by moving the ControlValue helpers to a
>> separate file.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen at ideasonboard.com>
>> ---
>>   src/py/libcamera/meson.build    |  1 +
>>   src/py/libcamera/py_helpers.cpp | 98 +++++++++++++++++++++++++++++++++
>>   src/py/libcamera/py_helpers.h   | 13 +++++
>>   src/py/libcamera/py_main.cpp    | 83 +---------------------------
>>   4 files changed, 114 insertions(+), 81 deletions(-)
>>   create mode 100644 src/py/libcamera/py_helpers.cpp
>>   create mode 100644 src/py/libcamera/py_helpers.h
>>
>> diff --git a/src/py/libcamera/meson.build b/src/py/libcamera/meson.build
>> index eb884538..04578bac 100644
>> --- a/src/py/libcamera/meson.build
>> +++ b/src/py/libcamera/meson.build
>> @@ -15,6 +15,7 @@ pybind11_dep = pybind11_proj.get_variable('pybind11_dep')
>>   pycamera_sources = files([
>>       'py_enums.cpp',
>>       'py_geometry.cpp',
>> +    'py_helpers.cpp',
> 
> Should this be called py_controls? Or do you expect other support code
> to go in here too. Or maybe it's more about types, 'py_types' ..

I felt that a dedicated file for just these controlvalue related 
functions is a bit too much, so I thought the file can be a holder of 
misc small things needed for the bindings.

>>       'py_main.cpp',
>>   ])
>>   
>> diff --git a/src/py/libcamera/py_helpers.cpp b/src/py/libcamera/py_helpers.cpp
>> new file mode 100644
>> index 00000000..d0a8b5c4
>> --- /dev/null
>> +++ b/src/py/libcamera/py_helpers.cpp
>> @@ -0,0 +1,98 @@
>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
>> +/*
>> + * Copyright (C) 2022, Tomi Valkeinen <tomi.valkeinen at ideasonboard.com>
>> + */
>> +
>> +#include "py_helpers.h"
>> +
>> +#include <libcamera/libcamera.h>
>> +
>> +#include <pybind11/functional.h>
>> +#include <pybind11/smart_holder.h>
> 
> You have this in the py_helpers header too, so I don't think it needs to
> be duplicated here.

Ok.

> What's it used for? I thought it was just for the Camera destructor or
> something, I don't see Camera used here ?

It's the main pybind11 header file, when using the smart_holder branch. 
Although... Now that I think about it, we're using the "always use smart 
holder" mode, which probably means we don't need to include 
smart_holder.h but pybind11.h would be fine. Well, it doesn't really 
matter here which one we include to get the pybind11::object.

   Tomi