On 6/15/22 09:39, Hans Verkuil wrote: > > > On 6/14/22 23:00, Laurent Pinchart wrote: >> Hi Xavier and Hans, >> >> Thank you for the patch. >> >> On Tue, May 03, 2022 at 11:39:19AM +0200, Xavier Roumegue wrote: >>> From: Hans Verkuil <hverkuil-cisco@xxxxxxxxx> >>> >>> Add a dynamic array test control to help test support for this >>> feature. >>> >>> Signed-off-by: Hans Verkuil <hverkuil-cisco@xxxxxxxxx> >>> --- >>> drivers/media/test-drivers/vivid/vivid-ctrls.c | 15 +++++++++++++++ >>> 1 file changed, 15 insertions(+) >>> >>> diff --git a/drivers/media/test-drivers/vivid/vivid-ctrls.c b/drivers/media/test-drivers/vivid/vivid-ctrls.c >>> index e7516dc1227b..7267892dc18a 100644 >>> --- a/drivers/media/test-drivers/vivid/vivid-ctrls.c >>> +++ b/drivers/media/test-drivers/vivid/vivid-ctrls.c >>> @@ -34,6 +34,7 @@ >>> #define VIVID_CID_U8_4D_ARRAY (VIVID_CID_CUSTOM_BASE + 10) >>> #define VIVID_CID_AREA (VIVID_CID_CUSTOM_BASE + 11) >>> #define VIVID_CID_RO_INTEGER (VIVID_CID_CUSTOM_BASE + 12) >>> +#define VIVID_CID_U32_DYN_ARRAY (VIVID_CID_CUSTOM_BASE + 13) >>> >>> #define VIVID_CID_VIVID_BASE (0x00f00000 | 0xf000) >>> #define VIVID_CID_VIVID_CLASS (0x00f00000 | 1) >>> @@ -189,6 +190,19 @@ static const struct v4l2_ctrl_config vivid_ctrl_u32_array = { >>> .dims = { 1 }, >>> }; >>> >>> +static const struct v4l2_ctrl_config vivid_ctrl_u32_dyn_array = { >>> + .ops = &vivid_user_gen_ctrl_ops, >>> + .id = VIVID_CID_U32_DYN_ARRAY, >>> + .name = "U32 Dynamic Array", >>> + .type = V4L2_CTRL_TYPE_U32, >>> + .flags = V4L2_CTRL_FLAG_DYNAMIC_ARRAY, >>> + .def = 50, >>> + .min = 10, >>> + .max = 90, >>> + .step = 1, >>> + .dims = { 100 }, >>> +}; >> >> To meaningfully test this, don't we need the vivid driver to change the >> dimension ? Or is it meant to only test changes made by the application >> ? > > dims[0] is the maximum number of elements allowed, it is not the actual > size of the control. The application sets the control to a size between > 1 and dims[0] (100 in this case) elements. > > When the control is created it will have just 1 element containing the > default control value. There is actually one thing that I would like to improve: allow dynamic arrays to be empty (0 elements). Such a control should probably be empty in the beginning as well. I'll take a stab at this. Regards, Hans > > Regards, > > Hans > >> >>> + >>> static const struct v4l2_ctrl_config vivid_ctrl_u16_matrix = { >>> .ops = &vivid_user_gen_ctrl_ops, >>> .id = VIVID_CID_U16_MATRIX, >>> @@ -1612,6 +1626,7 @@ int vivid_create_controls(struct vivid_dev *dev, bool show_ccs_cap, >>> dev->ro_int32 = v4l2_ctrl_new_custom(hdl_user_gen, &vivid_ctrl_ro_int32, NULL); >>> v4l2_ctrl_new_custom(hdl_user_gen, &vivid_ctrl_area, NULL); >>> v4l2_ctrl_new_custom(hdl_user_gen, &vivid_ctrl_u32_array, NULL); >>> + v4l2_ctrl_new_custom(hdl_user_gen, &vivid_ctrl_u32_dyn_array, NULL); >>> v4l2_ctrl_new_custom(hdl_user_gen, &vivid_ctrl_u16_matrix, NULL); >>> v4l2_ctrl_new_custom(hdl_user_gen, &vivid_ctrl_u8_4d_array, NULL); >>> >>