[RFCv2,10/21] v4l2-ctrls: compare values only once.

Commit Message

Hans Verkuil Jan. 20, 2014, 12:46 p.m. UTC

From: Hans Verkuil <[e-mail]>

When setting a control the control's new value is compared to the current
value twice: once by new_to_cur(), once by cluster_changed(). Not a big
deal when dealing with simple values, but it can be a problem when dealing
with compound types or matrices. So fix this: cluster_changed() sets the
has_changed flag, which is used by new_to_cur() instead of having to do
another compare.

Signed-off-by: Hans Verkuil <[e-mail]>
---
 drivers/media/v4l2-core/v4l2-ctrls.c | 17 +++++++++++------
 include/media/v4l2-ctrls.h           |  3 +++
 2 files changed, 14 insertions(+), 6 deletions(-)

Comments

Sylwester Nawrocki/Kernel \(PLT\) /SRPOL/Staff Engineer/Samsung Electronics Jan. 23, 2014, 2:30 p.m. UTC | #1

On 20/01/14 13:46, Hans Verkuil wrote:
> From: Hans Verkuil <[e-mail]>
> 
> When setting a control the control's new value is compared to the current
> value twice: once by new_to_cur(), once by cluster_changed(). Not a big
> deal when dealing with simple values, but it can be a problem when dealing
> with compound types or matrices. So fix this: cluster_changed() sets the
> has_changed flag, which is used by new_to_cur() instead of having to do
> another compare.
> 
> Signed-off-by: Hans Verkuil <[e-mail]>

Reviewed-by: Sylwester Nawrocki <[e-mail]>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
index c0507ed..2eb5b65 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -1386,8 +1386,11 @@  static void new_to_cur(struct v4l2_fh *fh, struct v4l2_ctrl *ctrl, u32 ch_flags)
 
 	if (ctrl == NULL)
 		return;
-	changed = !ctrl->type_ops->equal(ctrl, ctrl->stores[0], ctrl->new);
-	ptr_to_ptr(ctrl, ctrl->new, ctrl->stores[0]);
+
+	/* has_changed is set by cluster_changed */
+	changed = ctrl->has_changed;
+	if (changed)
+		ptr_to_ptr(ctrl, ctrl->new, ctrl->stores[0]);
 
 	if (ch_flags & V4L2_EVENT_CTRL_CH_FLAGS) {
 		/* Note: CH_FLAGS is only set for auto clusters. */
@@ -1424,17 +1427,19 @@  static void cur_to_new(struct v4l2_ctrl *ctrl)
    value that differs from the current value. */
 static int cluster_changed(struct v4l2_ctrl *master)
 {
-	int diff = 0;
+	bool changed = false;
 	int i;
 
-	for (i = 0; !diff && i < master->ncontrols; i++) {
+	for (i = 0; i < master->ncontrols; i++) {
 		struct v4l2_ctrl *ctrl = master->cluster[i];
 
 		if (ctrl == NULL)
 			continue;
-		diff = !ctrl->type_ops->equal(ctrl, ctrl->stores[0], ctrl->new);
+		ctrl->has_changed = !ctrl->type_ops->equal(ctrl,
+						ctrl->stores[0], ctrl->new);
+		changed |= ctrl->has_changed;
 	}
-	return diff;
+	return changed;
 }
 
 /* Control range checking */
diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h
index 8e7fd44..fb3aaf9 100644
--- a/include/media/v4l2-ctrls.h
+++ b/include/media/v4l2-ctrls.h
@@ -96,6 +96,8 @@  typedef void (*v4l2_ctrl_notify_fnc)(struct v4l2_ctrl *ctrl, void *priv);
   * @is_new:	Set when the user specified a new value for this control. It
   *		is also set when called from v4l2_ctrl_handler_setup. Drivers
   *		should never set this flag.
+  * @has_changed: Set when the current value differs from the new value. Drivers
+  *		should never use this flag.
   * @is_private: If set, then this control is private to its handler and it
   *		will not be added to any other handlers. Drivers can set
   *		this flag.
@@ -159,6 +161,7 @@  struct v4l2_ctrl {
 	unsigned int done:1;
 
 	unsigned int is_new:1;
+	unsigned int has_changed:1;
 	unsigned int is_private:1;
 	unsigned int is_auto:1;
 	unsigned int is_int:1;