Hi Ricardo,
Thank you for the patch.
On Tue, Dec 10, 2024 at 10:22:23PM +0000, Ricardo Ribalda wrote:
> Only send events for controls that have actually changed.
> E.g.: We are changing entities A, B and C. If we get an error while
> we change B we do not continue setting C. But the current code sends an
> event also for C.
Does it ? If uvc_ctrl_commit_entity() fails the 'goto done' statement
skips over uvc_ctrl_send_events().
> Due to the fact that the controls are grouped by entities, and the user
> might group them in different orders, we cannot send the events at the
> end, but when we change an entity.
>
> Cc: stable@xxxxxxxxxx
> Fixes: b4012002f3a3 ("[media] uvcvideo: Add support for control events")
> Signed-off-by: Ricardo Ribalda <ribalda@xxxxxxxxxxxx>
> ---
> drivers/media/usb/uvc/uvc_ctrl.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> index 7e2fc97c9f43..9496ac970267 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -1669,7 +1669,9 @@ static bool uvc_ctrl_xctrls_has_control(const struct v4l2_ext_control *xctrls,
> }
>
> static void uvc_ctrl_send_events(struct uvc_fh *handle,
> - const struct v4l2_ext_control *xctrls, unsigned int xctrls_count)
> + struct uvc_entity *entity,
> + const struct v4l2_ext_control *xctrls,
> + unsigned int xctrls_count)
> {
> struct uvc_control_mapping *mapping;
> struct uvc_control *ctrl;
> @@ -1680,6 +1682,9 @@ static void uvc_ctrl_send_events(struct uvc_fh *handle,
> for (i = 0; i < xctrls_count; ++i) {
> ctrl = uvc_find_control(handle->chain, xctrls[i].id, &mapping);
>
> + if (ctrl->entity != entity)
> + continue;
> +
> if (ctrl->info.flags & UVC_CTRL_FLAG_ASYNCHRONOUS)
> /* Notification will be sent from an Interrupt event. */
> continue;
> @@ -1911,11 +1916,12 @@ int __uvc_ctrl_commit(struct uvc_fh *handle, int rollback,
> uvc_ctrl_find_ctrl_idx(entity, ctrls,
> err_ctrl);
> goto done;
> + } else if (ret > 0 && !rollback) {
> + uvc_ctrl_send_events(handle, entity,
> + ctrls->controls, ctrls->count);
> }
> }
>
> - if (!rollback)
> - uvc_ctrl_send_events(handle, ctrls->controls, ctrls->count);
> done:
> mutex_unlock(&chain->ctrl_mutex);
> return ret < 0 ? ret : 0;
--
Regards,
Laurent Pinchart
