Re: [REVIEWv3 PATCH 14/35] v4l2-ctrls: prepare for matrix support.
Em Mon, 17 Feb 2014 10:57:29 +0100
Hans Verkuil hverk...@xs4all.nl escreveu:
From: Hans Verkuil hans.verk...@cisco.com
Add core support for matrices.
Again, this patch has negative values for array index.
I'll stop analyzing here, as it is hard to keep the mind in a
sane state seeing those crazy things ;)
Signed-off-by: Hans Verkuil hans.verk...@cisco.com
---
drivers/media/v4l2-core/v4l2-ctrls.c | 54
+++-
include/media/v4l2-ctrls.h | 8 --
2 files changed, 39 insertions(+), 23 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c
b/drivers/media/v4l2-core/v4l2-ctrls.c
index 49ce52e..f76716e 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -1132,7 +1132,7 @@ static void send_event(struct v4l2_fh *fh, struct
v4l2_ctrl *ctrl, u32 changes)
v4l2_event_queue_fh(sev-fh, ev);
}
-static bool std_equal(const struct v4l2_ctrl *ctrl,
+static bool std_equal(const struct v4l2_ctrl *ctrl, u32 idx,
union v4l2_ctrl_ptr ptr1,
union v4l2_ctrl_ptr ptr2)
{
@@ -1151,7 +1151,7 @@ static bool std_equal(const struct v4l2_ctrl *ctrl,
}
}
-static void std_init(const struct v4l2_ctrl *ctrl,
+static void std_init(const struct v4l2_ctrl *ctrl, u32 idx,
union v4l2_ctrl_ptr ptr)
{
switch (ctrl-type) {
@@ -1178,6 +1178,9 @@ static void std_log(const struct v4l2_ctrl *ctrl)
{
union v4l2_ctrl_ptr ptr = ctrl-stores[0];
+ if (ctrl-is_matrix)
+ pr_cont([%u][%u] , ctrl-rows, ctrl-cols);
+
switch (ctrl-type) {
case V4L2_CTRL_TYPE_INTEGER:
pr_cont(%d, *ptr.p_s32);
@@ -1220,7 +1223,7 @@ static void std_log(const struct v4l2_ctrl *ctrl)
})
/* Validate a new control */
-static int std_validate(const struct v4l2_ctrl *ctrl,
+static int std_validate(const struct v4l2_ctrl *ctrl, u32 idx,
union v4l2_ctrl_ptr ptr)
{
size_t len;
@@ -1444,7 +1447,7 @@ static int cluster_changed(struct v4l2_ctrl *master)
if (ctrl == NULL)
continue;
- ctrl-has_changed = !ctrl-type_ops-equal(ctrl,
+ ctrl-has_changed = !ctrl-type_ops-equal(ctrl, 0,
ctrl-stores[0], ctrl-new);
changed |= ctrl-has_changed;
}
@@ -1502,15 +1505,15 @@ static int validate_new(const struct v4l2_ctrl *ctrl,
case V4L2_CTRL_TYPE_BUTTON:
case V4L2_CTRL_TYPE_CTRL_CLASS:
ptr.p_s32 = c-value;
- return ctrl-type_ops-validate(ctrl, ptr);
+ return ctrl-type_ops-validate(ctrl, 0, ptr);
case V4L2_CTRL_TYPE_INTEGER64:
ptr.p_s64 = c-value64;
- return ctrl-type_ops-validate(ctrl, ptr);
+ return ctrl-type_ops-validate(ctrl, 0, ptr);
default:
ptr.p = c-p;
- return ctrl-type_ops-validate(ctrl, ptr);
+ return ctrl-type_ops-validate(ctrl, 0, ptr);
}
}
@@ -1736,7 +1739,8 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct
v4l2_ctrl_handler *hdl,
const s64 *qmenu_int, void *priv)
{
struct v4l2_ctrl *ctrl;
- unsigned sz_extra;
+ bool is_matrix;
+ unsigned sz_extra, tot_ctrl_size;
void *data;
int err;
int s;
@@ -1748,6 +1752,7 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct
v4l2_ctrl_handler *hdl,
cols = 1;
if (rows == 0)
rows = 1;
+ is_matrix = cols 1 || rows 1;
if (type == V4L2_CTRL_TYPE_INTEGER64)
elem_size = sizeof(s64);
@@ -1755,17 +1760,18 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct
v4l2_ctrl_handler *hdl,
elem_size = max + 1;
else if (type V4L2_CTRL_COMPLEX_TYPES)
elem_size = sizeof(s32);
+ tot_ctrl_size = elem_size * cols * rows;
/* Sanity checks */
- if (id == 0 || name == NULL || id = V4L2_CID_PRIVATE_BASE ||
- elem_size == 0 ||
+ if (id == 0 || name == NULL || !elem_size ||
+ id = V4L2_CID_PRIVATE_BASE ||
(type == V4L2_CTRL_TYPE_MENU qmenu == NULL) ||
(type == V4L2_CTRL_TYPE_INTEGER_MENU qmenu_int == NULL)) {
handler_set_err(hdl, -ERANGE);
return NULL;
}
/* Complex controls are always hidden */
- if (type = V4L2_CTRL_COMPLEX_TYPES)
+ if (is_matrix || type = V4L2_CTRL_COMPLEX_TYPES)
flags |= V4L2_CTRL_FLAG_HIDDEN;
err = check_range(type, min, max, step, def);
if (err) {
@@ -1776,14 +1782,21 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct
v4l2_ctrl_handler *hdl,
handler_set_err(hdl, -ERANGE);
return NULL;
}
+ if (is_matrix
+ (type == V4L2_CTRL_TYPE_BUTTON ||
+ type ==
Re: [REVIEWv3 PATCH 14/35] v4l2-ctrls: prepare for matrix support.
On 03/12/14 11:42, Mauro Carvalho Chehab wrote: Em Mon, 17 Feb 2014 10:57:29 +0100 Hans Verkuil hverk...@xs4all.nl escreveu: From: Hans Verkuil hans.verk...@cisco.com Add core support for matrices. Again, this patch has negative values for array index. I'll stop analyzing here, as it is hard to keep the mind in a sane state seeing those crazy things ;) Rather than getting bogged down in these details can you please give your opinion on the public API aspects. There is no point for me to spend time on this and then get it NACKed because you don't like the API itself and want something completely different. Internal things I can change, but I'm not going to spend a second on that unless I know the concept stands. Otherwise it is wasted time. This is something we need to improve on with regards to our processes: when it comes to API enhancements you really need to be involved earlier or it's going to be a huge waste of everyones time it is gets NACked. Not to mention demotivating and frustrating for all concerned. Regards, Hans -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [REVIEWv3 PATCH 14/35] v4l2-ctrls: prepare for matrix support.
Em Wed, 12 Mar 2014 13:21:41 +0100 Hans Verkuil hverk...@xs4all.nl escreveu: On 03/12/14 11:42, Mauro Carvalho Chehab wrote: Em Mon, 17 Feb 2014 10:57:29 +0100 Hans Verkuil hverk...@xs4all.nl escreveu: From: Hans Verkuil hans.verk...@cisco.com Add core support for matrices. Again, this patch has negative values for array index. I'll stop analyzing here, as it is hard to keep the mind in a sane state seeing those crazy things ;) Rather than getting bogged down in these details can you please give your opinion on the public API aspects. There is no point for me to spend time on this and then get it NACKed because you don't like the API itself and want something completely different. Internal things I can change, but I'm not going to spend a second on that unless I know the concept stands. Otherwise it is wasted time. Ok, what patches after 16/35 contains the API bits? The changes I saw so far seem ok, with the adjustments I pointed. This is something we need to improve on with regards to our processes: when it comes to API enhancements you really need to be involved earlier or it's going to be a huge waste of everyones time it is gets NACked. Not to mention demotivating and frustrating for all concerned. As I commented before: those complex API changes should ideally be discussed during our mini-summits, as it allows us to better understand the hole proposal. -- Regards, Mauro -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [REVIEWv3 PATCH 14/35] v4l2-ctrls: prepare for matrix support.
Em Wed, 12 Mar 2014 13:21:41 +0100 Hans Verkuil hverk...@xs4all.nl escreveu: On 03/12/14 11:42, Mauro Carvalho Chehab wrote: Em Mon, 17 Feb 2014 10:57:29 +0100 Hans Verkuil hverk...@xs4all.nl escreveu: From: Hans Verkuil hans.verk...@cisco.com Add core support for matrices. Again, this patch has negative values for array index. I'll stop analyzing here, as it is hard to keep the mind in a sane state seeing those crazy things ;) Rather than getting bogged down in these details can you please give your opinion on the public API aspects. There is no point for me to spend time on this and then get it NACKed because you don't like the API itself and want something completely different. Internal things I can change, but I'm not going to spend a second on that unless I know the concept stands. Otherwise it is wasted time. Ok, what patches after 16/35 contains the API bits? The changes I saw so far seem ok, with the adjustments I pointed. This is something we need to improve on with regards to our processes: when it comes to API enhancements you really need to be involved earlier or it's going to be a huge waste of everyones time it is gets NACked. Not to mention demotivating and frustrating for all concerned. As I commented before: those complex API changes should ideally be discussed during our mini-summits, as it allows us to better understand the hole proposal and the taken approach. -- Regards, Mauro -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [REVIEWv3 PATCH 14/35] v4l2-ctrls: prepare for matrix support.
On 03/12/2014 02:00 PM, Mauro Carvalho Chehab wrote: Em Wed, 12 Mar 2014 13:21:41 +0100 Hans Verkuil hverk...@xs4all.nl escreveu: On 03/12/14 11:42, Mauro Carvalho Chehab wrote: Em Mon, 17 Feb 2014 10:57:29 +0100 Hans Verkuil hverk...@xs4all.nl escreveu: From: Hans Verkuil hans.verk...@cisco.com Add core support for matrices. Again, this patch has negative values for array index. I'll stop analyzing here, as it is hard to keep the mind in a sane state seeing those crazy things ;) Rather than getting bogged down in these details can you please give your opinion on the public API aspects. There is no point for me to spend time on this and then get it NACKed because you don't like the API itself and want something completely different. Internal things I can change, but I'm not going to spend a second on that unless I know the concept stands. Otherwise it is wasted time. Ok, what patches after 16/35 contains the API bits? Core control functionality: 4, 5, 19-22. Adding support for the motion detection matrices: 24, 25, 27, 28. Adding support for the motion detection event: 29, 30. Sorry for sounding so irritated in my email: today is one of those frustrating days where nothing works out the way you want it and where I should have stayed in bed in the morning, and that spilled over in my mail. Regards, Hans The changes I saw so far seem ok, with the adjustments I pointed. This is something we need to improve on with regards to our processes: when it comes to API enhancements you really need to be involved earlier or it's going to be a huge waste of everyones time it is gets NACked. Not to mention demotivating and frustrating for all concerned. As I commented before: those complex API changes should ideally be discussed during our mini-summits, as it allows us to better understand the hole proposal and the taken approach. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [REVIEWv3 PATCH 14/35] v4l2-ctrls: prepare for matrix support.
Hi Mauro, On 12/03/14 14:00, Mauro Carvalho Chehab wrote: [...] As I commented before: those complex API changes should ideally be discussed during our mini-summits, as it allows us to better understand the hole proposal and the taken approach. We discussed this in a dedicated brainstorming meeting in Edinburgh, with Sakari, Laurent, Andrzej. IIRC you didn't take part in that discussions for some reason. After we initially agreed on the API Hans started working on the actual implementation. He put much effort and did a pretty good job IMO. I guess we just need to refine any controversial aspects that might be there and move forward with the API. :) -- Regards, Sylwester -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[REVIEWv3 PATCH 14/35] v4l2-ctrls: prepare for matrix support.
From: Hans Verkuil hans.verk...@cisco.com
Add core support for matrices.
Signed-off-by: Hans Verkuil hans.verk...@cisco.com
---
drivers/media/v4l2-core/v4l2-ctrls.c | 54 +++-
include/media/v4l2-ctrls.h | 8 --
2 files changed, 39 insertions(+), 23 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c
b/drivers/media/v4l2-core/v4l2-ctrls.c
index 49ce52e..f76716e 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -1132,7 +1132,7 @@ static void send_event(struct v4l2_fh *fh, struct
v4l2_ctrl *ctrl, u32 changes)
v4l2_event_queue_fh(sev-fh, ev);
}
-static bool std_equal(const struct v4l2_ctrl *ctrl,
+static bool std_equal(const struct v4l2_ctrl *ctrl, u32 idx,
union v4l2_ctrl_ptr ptr1,
union v4l2_ctrl_ptr ptr2)
{
@@ -1151,7 +1151,7 @@ static bool std_equal(const struct v4l2_ctrl *ctrl,
}
}
-static void std_init(const struct v4l2_ctrl *ctrl,
+static void std_init(const struct v4l2_ctrl *ctrl, u32 idx,
union v4l2_ctrl_ptr ptr)
{
switch (ctrl-type) {
@@ -1178,6 +1178,9 @@ static void std_log(const struct v4l2_ctrl *ctrl)
{
union v4l2_ctrl_ptr ptr = ctrl-stores[0];
+ if (ctrl-is_matrix)
+ pr_cont([%u][%u] , ctrl-rows, ctrl-cols);
+
switch (ctrl-type) {
case V4L2_CTRL_TYPE_INTEGER:
pr_cont(%d, *ptr.p_s32);
@@ -1220,7 +1223,7 @@ static void std_log(const struct v4l2_ctrl *ctrl)
})
/* Validate a new control */
-static int std_validate(const struct v4l2_ctrl *ctrl,
+static int std_validate(const struct v4l2_ctrl *ctrl, u32 idx,
union v4l2_ctrl_ptr ptr)
{
size_t len;
@@ -1444,7 +1447,7 @@ static int cluster_changed(struct v4l2_ctrl *master)
if (ctrl == NULL)
continue;
- ctrl-has_changed = !ctrl-type_ops-equal(ctrl,
+ ctrl-has_changed = !ctrl-type_ops-equal(ctrl, 0,
ctrl-stores[0], ctrl-new);
changed |= ctrl-has_changed;
}
@@ -1502,15 +1505,15 @@ static int validate_new(const struct v4l2_ctrl *ctrl,
case V4L2_CTRL_TYPE_BUTTON:
case V4L2_CTRL_TYPE_CTRL_CLASS:
ptr.p_s32 = c-value;
- return ctrl-type_ops-validate(ctrl, ptr);
+ return ctrl-type_ops-validate(ctrl, 0, ptr);
case V4L2_CTRL_TYPE_INTEGER64:
ptr.p_s64 = c-value64;
- return ctrl-type_ops-validate(ctrl, ptr);
+ return ctrl-type_ops-validate(ctrl, 0, ptr);
default:
ptr.p = c-p;
- return ctrl-type_ops-validate(ctrl, ptr);
+ return ctrl-type_ops-validate(ctrl, 0, ptr);
}
}
@@ -1736,7 +1739,8 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct
v4l2_ctrl_handler *hdl,
const s64 *qmenu_int, void *priv)
{
struct v4l2_ctrl *ctrl;
- unsigned sz_extra;
+ bool is_matrix;
+ unsigned sz_extra, tot_ctrl_size;
void *data;
int err;
int s;
@@ -1748,6 +1752,7 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct
v4l2_ctrl_handler *hdl,
cols = 1;
if (rows == 0)
rows = 1;
+ is_matrix = cols 1 || rows 1;
if (type == V4L2_CTRL_TYPE_INTEGER64)
elem_size = sizeof(s64);
@@ -1755,17 +1760,18 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct
v4l2_ctrl_handler *hdl,
elem_size = max + 1;
else if (type V4L2_CTRL_COMPLEX_TYPES)
elem_size = sizeof(s32);
+ tot_ctrl_size = elem_size * cols * rows;
/* Sanity checks */
- if (id == 0 || name == NULL || id = V4L2_CID_PRIVATE_BASE ||
- elem_size == 0 ||
+ if (id == 0 || name == NULL || !elem_size ||
+ id = V4L2_CID_PRIVATE_BASE ||
(type == V4L2_CTRL_TYPE_MENU qmenu == NULL) ||
(type == V4L2_CTRL_TYPE_INTEGER_MENU qmenu_int == NULL)) {
handler_set_err(hdl, -ERANGE);
return NULL;
}
/* Complex controls are always hidden */
- if (type = V4L2_CTRL_COMPLEX_TYPES)
+ if (is_matrix || type = V4L2_CTRL_COMPLEX_TYPES)
flags |= V4L2_CTRL_FLAG_HIDDEN;
err = check_range(type, min, max, step, def);
if (err) {
@@ -1776,14 +1782,21 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct
v4l2_ctrl_handler *hdl,
handler_set_err(hdl, -ERANGE);
return NULL;
}
+ if (is_matrix
+ (type == V4L2_CTRL_TYPE_BUTTON ||
+type == V4L2_CTRL_TYPE_CTRL_CLASS)) {
+ handler_set_err(hdl, -EINVAL);
+ return NULL;
+ }
- sz_extra = elem_size;
+ sz_extra = tot_ctrl_size;
if (type == V4L2_CTRL_TYPE_BUTTON)
