drivers: video: improve support for custom controls by epc-ake · Pull Request #95807 · zephyrproject-rtos/zephyr

epc-ake · 2025-09-10T16:12:16Z

v4z: Custom control improvements

These are the changes I made to get my custom sensor running with the current API. I opened a draft PR for discussion.

Adding name fields directly to struct video_ctrl to allow custom
control names
Adding video_init_custom_ctrl() function to initialize custom
controls
Make sure to allways set the right type for menu controls
~~- Skip new-value validation when VIDEO_CTRL_FLAG_VOLATILE is set. Consider adding an execute-on-write flag similar to V4L2_CTRL_FLAG_EXECUTE_ON_WRITE.~~
~~- Bug fixes in the video shell.~~

Example usage

I got the inspiration from v4l2:
https://www.kernel.org/doc/html/v4.10/media/kapi/v4l2-controls.html#custom-controls
Custom controls can now be initialized with the ´struct video_ctrl_config´.

static const struct video_ctrl_config example_int_cfg = {
    .id = EXAMPLE_CTRL_ID,
    .name = "Example Integer",
    .type = VIDEO_CTRL_TYPE_INTEGER,
    .flags = 0,
    .range = {.min = 0, .max = 100000, .step = 1, .def  = 100},
};
ret = video_init_custom_ctrl(&ctrls->example_int, dev, &example_int_cfg);

and for custom menu controls

static const char *const example_menu[] = {
    [0] = "Option A",
    [1] = "Option B",
    [2] = "Option C",
    NULL,
};

static const struct video_ctrl_config example_ctrl_cfg = {
    .id = EXAMPLE_CTRL_ID,
    .name = "Example Control",
    .type = VIDEO_CTRL_TYPE_MENU,
    .flags = 0,
    .menu = example_menu,
    .range = {.min = 0, .max = 2, .step = 1, .def  = 0},
};

ret |= video_init_custom_ctrl(&ctrls->example, dev, &example_ctrl_cfg);

ngphibang · 2025-09-10T20:06:35Z

drivers/video/video_ctrls.c

 		cq->int_menu = ctrl->int_menu;
 	}
-	cq->name = video_get_ctrl_name(cq->id);
+	cq->name = ctrl->name;


Suggested:

cq->name = video_get_ctrl_name(cq->id); if (cq->name == NULL) { cq->name = ctrl->name; }

ngphibang · 2025-09-10T20:10:35Z

drivers/video/video_ctrls.c

 	ctrl->type = type;
 	ctrl->flags = flags;
 	ctrl->range = range;
+	ctrl->name = video_get_ctrl_name(id);


You need a new video_init_ctrl_custom_ctrl(..., const char *name) so that drivers can set the name

ngphibang · 2025-09-10T20:13:22Z

drivers/video/video_ctrls.c

 	/* No new value */
-	if (ctrl->type == VIDEO_CTRL_TYPE_INTEGER64 ? ctrl->val64 == control->val64
-						    : ctrl->val == control->val) {
+	if (!(ctrl->flags & VIDEO_CTRL_FLAG_VOLATILE) &&


VIDEO_CTRL_FLAG_EXECUTE_ON_WRITE is the right way to go I think. I can add it and the control button type if you want

Yes please. That would help me a lot. I'll remove this then from this PR.

epc-ake · 2025-09-11T09:19:00Z

@ngphibang I removed the "Skip new-value validation" stuff and tried to properly implement a video_init_custom_ctrl().
I updated the PR example with an example. Can you give some feedback please?

josuah · 2025-09-11T10:20:20Z

drivers/video/video_ctrls.c

+					 cfg->menu_len);
+		break;
+	default:
+		ret = video_init_ctrl(ctrl, dev, cfg->id, cfg->range);


Here, the ->type will be set to the default of "INTEGER" by set_type_flag(), called from video_init_ctrl():

static inline void set_type_flag(uint32_t id, enum video_ctrl_type *type, uint32_t *flags) { *flags = 0; switch (id) { case VIDEO_CID_AUTO_WHITE_BALANCE: case VIDEO_CID_AUTOGAIN: case VIDEO_CID_HFLIP: [...] default: *type = VIDEO_CTRL_TYPE_INTEGER; <<< HERE break; }

drivers/video/video_ctrls.c

- Adding name fields directly to `struct video_ctrl` to allow custom control names - Adding `video_init_custom_ctrl` to initialize custom controls - Make sure to allways set the right type for menu controls Signed-off-by: Armin Kessler <ake@espros.com>

ngphibang · 2025-09-11T11:50:05Z

drivers/video/video_ctrls.h

+struct video_ctrl_config {
+	uint32_t id;
+	const char *name;
+	enum video_ctrl_type type;
+	unsigned long flags;
+	struct video_ctrl_range range;
+	union {
+		const char *const *menu;
+		const int64_t *int_menu;
+	};
+	size_t menu_len;
+};


I wonder if we should refactor struct video_ctrl and struct video_ctrl_config as most of the fields are common, except menu_len

Yea I were also thinking about that.
But I didn't want to start refactoring before showing the actual concept.
If your fine with the footprint of the video_init_custom_ctrl() I can do the refactoring in a second commit if you want.

I was thinking more about this. Actually, the custom control just needs a particular "name" in addition compared to standard control. So, a better solution would be using exisiting APIs for initialization then driver can explicitly set:

ctrl->name = name;

Currently, it is done similarly for flag, driver can explicitely set the flag (e.g. volatile, etc.).
What do you think ?

That's how I started, but I find passing a video_ctrl_config for custom controls cleaner since it results in a clean initialization call without the need for manipulating the struct video_ctrl afterwards. Instead, I’d consider adding a flags parameter to the default control APIs so drivers don’t need to manually set flags like

So instead of.

video_init_int_menu_ctrl(&ctrls->linkfreq, dev, VIDEO_CID_LINK_FREQ, GC2145_640_480_LINK_FREQ_ID, gc2145_link_frequency, ARRAY_SIZE(gc2145_link_frequency)); ctrls->linkfreq.cfg.flags |= VIDEO_CTRL_FLAG_READ_ONLY;

Something like this

video_init_int_menu_ctrl(&ctrls->linkfreq, dev, VIDEO_CID_LINK_FREQ, GC2145_640_480_LINK_FREQ_ID, gc2145_link_frequency, ARRAY_SIZE(gc2145_link_frequency), VIDEO_CTRL_FLAG_READ_ONLY | VIDEO_CTRL_FLAG_VOLATILE);

Ah, sorry for not understand correctly your initial purpose.
For this, I think this will complicate the API parameters. Then one day, if we want to set other things, we will need to extend the API again ... Another disavantage is the increasing stack size ...

Linux also did
https://elixir.bootlin.com/linux/v6.11.1/source/drivers/media/i2c/ov5640.c#L3527-#L3531

I like the approach.
Would you refactor the struct video_ctrl to improve clarity? E.g. with the current struct i see some potential for misinterpretation for driver developers because of fields like has_volatiles, is_auto, cluster etc. that are not intended to be set by the driver.

Yes, I will re-organize the fields and add more documentation

Perfect. So are you taking over this? I will close this PR then.

Yes, thank you for your effort, it helped uncover some weak points in the framework and apologies again for not seeing it clearly the first time.

yes, per-instance is important. This is the current limitation of the video control framework where controls are stored "per-driver". I will try to fix this at the end of the week.

In fact, I was wrong. The current implementation already store controls per-instance. We just need to do some refactor and modifications.

refactoring `struct video_ctrl` to make use of `struct video_ctrl_config` to avoid duplications Signed-off-by: Armin Kessler ake@espros.com

epc-ake · 2025-09-12T08:48:01Z

I was thinking more about this. Actually, the custom control just needs a particular "name" in addition compared to standard control. So, a better solution would be using exisiting APIs for initialization then driver can explicitly set:

ctrl->name = name;

Currently, it is done similarly for flag, driver can explicitely set the flag (e.g. volatile, etc.). What do you think ?

@ngphibang I saw your comment just a view minutes too late and already started refactoring the video_ctrl_config in a second commit.

sonarqubecloud · 2025-09-12T09:09:22Z

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

ngphibang · 2025-09-14T21:49:08Z

@epc-ake : #95968 (added VIDEO_CTRL_FLAG_EXECUTE_ON_WRITE and VIDEO_CTRL_TYPE_BUTTON)

epc-ake · 2025-09-17T07:28:05Z

I close this PR but work will be continued by @ngphibang.

ngphibang · 2025-09-22T20:45:15Z

I continued this on #96393

ngphibang reviewed Sep 10, 2025

View reviewed changes

epc-ake force-pushed the feature/improve_video_ctrl branch from 4fdf5ea to 2d973b3 Compare September 11, 2025 08:53

josuah reviewed Sep 11, 2025

View reviewed changes

drivers/video/video_ctrls.c Outdated Show resolved Hide resolved

epc-ake force-pushed the feature/improve_video_ctrl branch from 2d973b3 to 8300a55 Compare September 11, 2025 11:07

ngphibang reviewed Sep 11, 2025

View reviewed changes

drivers: video: refactor video_ctrl

8039c3e

refactoring `struct video_ctrl` to make use of `struct video_ctrl_config` to avoid duplications Signed-off-by: Armin Kessler ake@espros.com

josuah mentioned this pull request Sep 12, 2025

VIDEO: GC2145/stm32_dcmi support for CROP/Snapshot #93797

Closed

epc-ake closed this Sep 17, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

drivers: video: improve support for custom controls#95807

drivers: video: improve support for custom controls#95807
epc-ake wants to merge 2 commits intozephyrproject-rtos:mainfrom
espros:feature/improve_video_ctrl

epc-ake commented Sep 10, 2025 •

edited

Loading

ngphibang Sep 10, 2025

ngphibang Sep 10, 2025

ngphibang Sep 10, 2025

epc-ake Sep 11, 2025

epc-ake commented Sep 11, 2025

josuah Sep 11, 2025

Uh oh!

ngphibang Sep 11, 2025

epc-ake Sep 11, 2025

ngphibang Sep 12, 2025

epc-ake Sep 12, 2025

ngphibang Sep 12, 2025 •

edited

Loading

epc-ake Sep 17, 2025

ngphibang Sep 17, 2025

epc-ake Sep 17, 2025

ngphibang Sep 17, 2025

ngphibang Sep 22, 2025

epc-ake commented Sep 12, 2025

sonarqubecloud bot commented Sep 12, 2025

ngphibang commented Sep 14, 2025

epc-ake commented Sep 17, 2025

ngphibang commented Sep 22, 2025

Labels

3 participants

Conversation

epc-ake commented Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

v4z: Custom control improvements

Example usage

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

epc-ake commented Sep 11, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ngphibang Sep 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

epc-ake commented Sep 12, 2025

sonarqubecloud bot commented Sep 12, 2025

Quality Gate passed

ngphibang commented Sep 14, 2025

epc-ake commented Sep 17, 2025

ngphibang commented Sep 22, 2025

Labels

3 participants

epc-ake commented Sep 10, 2025 •

edited

Loading

ngphibang Sep 12, 2025 •

edited

Loading