Skip to content

video: Add VIDEO_CTRL_FLAG_EXECUTE_ON_WRITE and VIDEO_CTRL_TYPE_BUTTON#95968

Closed
ngphibang wants to merge 2 commits intozephyrproject-rtos:mainfrom
nxp-upstream:video_control_modifs
Closed

video: Add VIDEO_CTRL_FLAG_EXECUTE_ON_WRITE and VIDEO_CTRL_TYPE_BUTTON#95968
ngphibang wants to merge 2 commits intozephyrproject-rtos:mainfrom
nxp-upstream:video_control_modifs

Conversation

@ngphibang
Copy link
Contributor

  • Add VIDEO_CTRL_FLAG_EXECUTE_ON_WRITE and VIDEO_CTRL_TYPE_BUTTON
@ngphibang ngphibang changed the title Video control modifs Sep 14, 2025
@zephyrbot zephyrbot added the area: Video Video subsystem label Sep 14, 2025
Add VIDEO_CTRL_FLAG_EXECUTE_ON_WRITE flag.

Signed-off-by: Phi Bang Nguyen <phibang.nguyen@nxp.com>
Add VIDEO_CTRL_TYPE_BUTTON to support command-like controls.

Signed-off-by: Phi Bang Nguyen <phibang.nguyen@nxp.com>
Copy link
Contributor

@josuah josuah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM except some doubt around *type = . The rest is not important.
Thank you!

Comment on lines +114 to +120
case VIDEO_CID_PAN_RELATIVE:
case VIDEO_CID_TILT_RELATIVE:
case VIDEO_CID_FOCUS_RELATIVE:
case VIDEO_CID_IRIS_RELATIVE:
case VIDEO_CID_ZOOM_RELATIVE:
*flags |= VIDEO_CTRL_FLAG_WRITE_ONLY | VIDEO_CTRL_FLAG_EXECUTE_ON_WRITE;
break;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we also need to set *type = VIDEO_CTRL_TYPE_INTEGER? Or otherwise use __fallthrough;.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch ! Thanks

Comment on lines +378 to 381
ctrl->type == VIDEO_CTRL_TYPE_INTEGER64
? ctrl->val64 == control->val64
: ctrl->val == control->val) {
return 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is tempting to decompose it like that:

	bool changed = ctrl->type == VIDEO_CTRL_TYPE_INTEGER64
			       ? ctrl->val64 == control->val64
			       : ctrl->val == control->val;
 	if (changed && !(ctrl->flags & VIDEO_CTRL_FLAG_EXECUTE_ON_WRITE)) {
/* No new value */
if (ctrl->type == VIDEO_CTRL_TYPE_INTEGER64 ? ctrl->val64 == control->val64
: ctrl->val == control->val) {
if (!(ctrl->flags & VIDEO_CTRL_FLAG_EXECUTE_ON_WRITE) &&
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this. It also allows for control types other than VIDEO_CTRL_TYPE_BUTTON to be able to always trigger an update.

@ngphibang
Copy link
Contributor Author

ngphibang commented Sep 22, 2025

I applied the suggested changes and move this to #96393. Closing

@ngphibang ngphibang closed this Sep 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: Video Video subsystem

4 participants