* Re: [PATCH] v4l2-core: Print control name in VIDIOC_S/G_(EXT)_CTRL(S)
2020-06-09 12:15 [PATCH] v4l2-core: Print control name in VIDIOC_S/G_(EXT)_CTRL(S) Ezequiel Garcia
@ 2020-06-11 12:35 ` Helen Koike
2020-06-11 14:29 ` Helen Koike
2020-06-23 11:01 ` Hans Verkuil
2020-07-01 13:17 ` [PATCH v2] " Ezequiel Garcia
2 siblings, 1 reply; 6+ messages in thread
From: Helen Koike @ 2020-06-11 12:35 UTC (permalink / raw)
To: Ezequiel Garcia, linux-media; +Cc: Hans Verkuil, kernel
On 6/9/20 9:15 AM, Ezequiel Garcia wrote:
> While debugging, it's currently really hard to identify controls
> by their ID. Print the control name making the print more helpful.
>
> With this change, the print changes from:
>
> video1: VIDIOC_S_EXT_CTRLS: which=0xf010000, count=5, error_idx=4, request_fd=45, id/size=0x990ce8/1048, id/size=0x990ce9/12, id/size=0x990cea/480, id/size=0x990ceb/896, id/size=0x990cec/400
>
> to:
>
> video1: VIDIOC_S_EXT_CTRLS: which=0xf010000, count=5, error_idx=4, request_fd=42, name=H264 Sequence Parameter Set, id/size=0x990ce8/1048, name=H264 Picture Parameter Set, id/size=0x990ce9/12, name=H264 Scaling Matrix, id/size=0x990cea/480, name=H264 Slice Parameters, id/size=0x990ceb/896, name=H264 Decode Parameters, id/size=0x990cec/400
>
> For instance, this is specially helpful when the ioctl fails. Consider
> the following example:
>
> v4l2-ctrls: prepare_ext_ctrls: video1: pointer control id 0x990cec size too small, 400 bytes but 784 bytes needed
> v4l2-ctrls: try_set_ext_ctrls: video1: video1: try_set_ext_ctrls_common failed (-14)
> video1: VIDIOC_S_EXT_CTRLS: error -14: which=0xf010000, count=5, error_idx=5, request_fd=39, name=H264 Sequence Parameter Set, id/size=0x990ce8/1048, name=H264 Picture Parameter Set, id/size=0x990ce9/12, name=H264 Scaling Matrix, id/size=0x990cea/480, name=H264 Slice Parameters, id/size=0x990ceb/896, name=H264 Decode Parameters, id/size=0x990cec/400
>
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
Thanks Ezequiel,
I'll include this in the next RFC.
Regards,
Helen
> ---
> drivers/media/v4l2-core/v4l2-ioctl.c | 16 +++++++++++-----
> 1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> index 2322f08a98be..4264ac44c48b 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -582,8 +582,10 @@ static void v4l_print_querymenu(const void *arg, bool write_only)
> static void v4l_print_control(const void *arg, bool write_only)
> {
> const struct v4l2_control *p = arg;
> + const char *name = v4l2_ctrl_get_name(p->id);
>
> - pr_cont("id=0x%x, value=%d\n", p->id, p->value);
> + pr_cont("name=%s, id=0x%x, value=%d\n",
> + name ? name : "unknown", p->id, p->value);
> }
>
> static void v4l_print_ext_controls(const void *arg, bool write_only)
> @@ -594,12 +596,16 @@ static void v4l_print_ext_controls(const void *arg, bool write_only)
> pr_cont("which=0x%x, count=%d, error_idx=%d, request_fd=%d",
> p->which, p->count, p->error_idx, p->request_fd);
> for (i = 0; i < p->count; i++) {
> + unsigned int id = p->controls[i].id;
> + const char *name = v4l2_ctrl_get_name(id);
> +
> + name = name ? name : "unknown";
> if (!p->controls[i].size)
> - pr_cont(", id/val=0x%x/0x%x",
> - p->controls[i].id, p->controls[i].value);
> + pr_cont(", name=%s, id/val=0x%x/0x%x",
> + name, id, p->controls[i].value);
> else
> - pr_cont(", id/size=0x%x/%u",
> - p->controls[i].id, p->controls[i].size);
> + pr_cont(", name=%s, id/size=0x%x/%u",
> + name, id, p->controls[i].size);
> }
> pr_cont("\n");
> }
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] v4l2-core: Print control name in VIDIOC_S/G_(EXT)_CTRL(S)
2020-06-11 12:35 ` Helen Koike
@ 2020-06-11 14:29 ` Helen Koike
0 siblings, 0 replies; 6+ messages in thread
From: Helen Koike @ 2020-06-11 14:29 UTC (permalink / raw)
To: Ezequiel Garcia, linux-media; +Cc: Hans Verkuil, kernel
On 6/11/20 9:35 AM, Helen Koike wrote:
>
>
> On 6/9/20 9:15 AM, Ezequiel Garcia wrote:
>> While debugging, it's currently really hard to identify controls
>> by their ID. Print the control name making the print more helpful.
>>
>> With this change, the print changes from:
>>
>> video1: VIDIOC_S_EXT_CTRLS: which=0xf010000, count=5, error_idx=4, request_fd=45, id/size=0x990ce8/1048, id/size=0x990ce9/12, id/size=0x990cea/480, id/size=0x990ceb/896, id/size=0x990cec/400
>>
>> to:
>>
>> video1: VIDIOC_S_EXT_CTRLS: which=0xf010000, count=5, error_idx=4, request_fd=42, name=H264 Sequence Parameter Set, id/size=0x990ce8/1048, name=H264 Picture Parameter Set, id/size=0x990ce9/12, name=H264 Scaling Matrix, id/size=0x990cea/480, name=H264 Slice Parameters, id/size=0x990ceb/896, name=H264 Decode Parameters, id/size=0x990cec/400
>>
>> For instance, this is specially helpful when the ioctl fails. Consider
>> the following example:
>>
>> v4l2-ctrls: prepare_ext_ctrls: video1: pointer control id 0x990cec size too small, 400 bytes but 784 bytes needed
>> v4l2-ctrls: try_set_ext_ctrls: video1: video1: try_set_ext_ctrls_common failed (-14)
>> video1: VIDIOC_S_EXT_CTRLS: error -14: which=0xf010000, count=5, error_idx=5, request_fd=39, name=H264 Sequence Parameter Set, id/size=0x990ce8/1048, name=H264 Picture Parameter Set, id/size=0x990ce9/12, name=H264 Scaling Matrix, id/size=0x990cea/480, name=H264 Slice Parameters, id/size=0x990ceb/896, name=H264 Decode Parameters, id/size=0x990cec/400
>>
>> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
>
> Thanks Ezequiel,
>
> I'll include this in the next RFC.
Please ignore my last email, I made a mistake/confusion.
Regards,
Helen
>
> Regards,
> Helen
>
>> ---
>> drivers/media/v4l2-core/v4l2-ioctl.c | 16 +++++++++++-----
>> 1 file changed, 11 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
>> index 2322f08a98be..4264ac44c48b 100644
>> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
>> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
>> @@ -582,8 +582,10 @@ static void v4l_print_querymenu(const void *arg, bool write_only)
>> static void v4l_print_control(const void *arg, bool write_only)
>> {
>> const struct v4l2_control *p = arg;
>> + const char *name = v4l2_ctrl_get_name(p->id);
>>
>> - pr_cont("id=0x%x, value=%d\n", p->id, p->value);
>> + pr_cont("name=%s, id=0x%x, value=%d\n",
>> + name ? name : "unknown", p->id, p->value);
>> }
>>
>> static void v4l_print_ext_controls(const void *arg, bool write_only)
>> @@ -594,12 +596,16 @@ static void v4l_print_ext_controls(const void *arg, bool write_only)
>> pr_cont("which=0x%x, count=%d, error_idx=%d, request_fd=%d",
>> p->which, p->count, p->error_idx, p->request_fd);
>> for (i = 0; i < p->count; i++) {
>> + unsigned int id = p->controls[i].id;
>> + const char *name = v4l2_ctrl_get_name(id);
>> +
>> + name = name ? name : "unknown";
>> if (!p->controls[i].size)
>> - pr_cont(", id/val=0x%x/0x%x",
>> - p->controls[i].id, p->controls[i].value);
>> + pr_cont(", name=%s, id/val=0x%x/0x%x",
>> + name, id, p->controls[i].value);
>> else
>> - pr_cont(", id/size=0x%x/%u",
>> - p->controls[i].id, p->controls[i].size);
>> + pr_cont(", name=%s, id/size=0x%x/%u",
>> + name, id, p->controls[i].size);
>> }
>> pr_cont("\n");
>> }
>>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] v4l2-core: Print control name in VIDIOC_S/G_(EXT)_CTRL(S)
2020-06-09 12:15 [PATCH] v4l2-core: Print control name in VIDIOC_S/G_(EXT)_CTRL(S) Ezequiel Garcia
2020-06-11 12:35 ` Helen Koike
@ 2020-06-23 11:01 ` Hans Verkuil
2020-06-23 12:32 ` Ezequiel Garcia
2020-07-01 13:17 ` [PATCH v2] " Ezequiel Garcia
2 siblings, 1 reply; 6+ messages in thread
From: Hans Verkuil @ 2020-06-23 11:01 UTC (permalink / raw)
To: Ezequiel Garcia, linux-media; +Cc: kernel
On 09/06/2020 14:15, Ezequiel Garcia wrote:
> While debugging, it's currently really hard to identify controls
> by their ID. Print the control name making the print more helpful.
>
> With this change, the print changes from:
>
> video1: VIDIOC_S_EXT_CTRLS: which=0xf010000, count=5, error_idx=4, request_fd=45, id/size=0x990ce8/1048, id/size=0x990ce9/12, id/size=0x990cea/480, id/size=0x990ceb/896, id/size=0x990cec/400
>
> to:
>
> video1: VIDIOC_S_EXT_CTRLS: which=0xf010000, count=5, error_idx=4, request_fd=42, name=H264 Sequence Parameter Set, id/size=0x990ce8/1048, name=H264 Picture Parameter Set, id/size=0x990ce9/12, name=H264 Scaling Matrix, id/size=0x990cea/480, name=H264 Slice Parameters, id/size=0x990ceb/896, name=H264 Decode Parameters, id/size=0x990cec/400
>
> For instance, this is specially helpful when the ioctl fails. Consider
> the following example:
>
> v4l2-ctrls: prepare_ext_ctrls: video1: pointer control id 0x990cec size too small, 400 bytes but 784 bytes needed
> v4l2-ctrls: try_set_ext_ctrls: video1: video1: try_set_ext_ctrls_common failed (-14)
> video1: VIDIOC_S_EXT_CTRLS: error -14: which=0xf010000, count=5, error_idx=5, request_fd=39, name=H264 Sequence Parameter Set, id/size=0x990ce8/1048, name=H264 Picture Parameter Set, id/size=0x990ce9/12, name=H264 Scaling Matrix, id/size=0x990cea/480, name=H264 Slice Parameters, id/size=0x990ceb/896, name=H264 Decode Parameters, id/size=0x990cec/400
>
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> ---
> drivers/media/v4l2-core/v4l2-ioctl.c | 16 +++++++++++-----
> 1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> index 2322f08a98be..4264ac44c48b 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -582,8 +582,10 @@ static void v4l_print_querymenu(const void *arg, bool write_only)
> static void v4l_print_control(const void *arg, bool write_only)
> {
> const struct v4l2_control *p = arg;
> + const char *name = v4l2_ctrl_get_name(p->id);
>
> - pr_cont("id=0x%x, value=%d\n", p->id, p->value);
> + pr_cont("name=%s, id=0x%x, value=%d\n",
> + name ? name : "unknown", p->id, p->value);
I prefer:
if (name)
pr_cont("name=%s, ", name);
pr_cont("id=0x%x, value=%d\n", p->id, p->value);
> }
>
> static void v4l_print_ext_controls(const void *arg, bool write_only)
> @@ -594,12 +596,16 @@ static void v4l_print_ext_controls(const void *arg, bool write_only)
> pr_cont("which=0x%x, count=%d, error_idx=%d, request_fd=%d",
> p->which, p->count, p->error_idx, p->request_fd);
> for (i = 0; i < p->count; i++) {
> + unsigned int id = p->controls[i].id;
> + const char *name = v4l2_ctrl_get_name(id);
> +
> + name = name ? name : "unknown";
> if (!p->controls[i].size)
> - pr_cont(", id/val=0x%x/0x%x",
> - p->controls[i].id, p->controls[i].value);
> + pr_cont(", name=%s, id/val=0x%x/0x%x",
> + name, id, p->controls[i].value);
> else
> - pr_cont(", id/size=0x%x/%u",
> - p->controls[i].id, p->controls[i].size);
> + pr_cont(", name=%s, id/size=0x%x/%u",
> + name, id, p->controls[i].size);
Same here.
Regards,
Hans
> }
> pr_cont("\n");
> }
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] v4l2-core: Print control name in VIDIOC_S/G_(EXT)_CTRL(S)
2020-06-23 11:01 ` Hans Verkuil
@ 2020-06-23 12:32 ` Ezequiel Garcia
0 siblings, 0 replies; 6+ messages in thread
From: Ezequiel Garcia @ 2020-06-23 12:32 UTC (permalink / raw)
To: Hans Verkuil, linux-media; +Cc: kernel
On Tue, 2020-06-23 at 13:01 +0200, Hans Verkuil wrote:
> On 09/06/2020 14:15, Ezequiel Garcia wrote:
> > While debugging, it's currently really hard to identify controls
> > by their ID. Print the control name making the print more helpful.
> >
> > With this change, the print changes from:
> >
> > video1: VIDIOC_S_EXT_CTRLS: which=0xf010000, count=5, error_idx=4, request_fd=45, id/size=0x990ce8/1048, id/size=0x990ce9/12,
> > id/size=0x990cea/480, id/size=0x990ceb/896, id/size=0x990cec/400
> >
> > to:
> >
> > video1: VIDIOC_S_EXT_CTRLS: which=0xf010000, count=5, error_idx=4, request_fd=42, name=H264 Sequence Parameter Set, id/size=0x990ce8/1048,
> > name=H264 Picture Parameter Set, id/size=0x990ce9/12, name=H264 Scaling Matrix, id/size=0x990cea/480, name=H264 Slice Parameters,
> > id/size=0x990ceb/896, name=H264 Decode Parameters, id/size=0x990cec/400
> >
> > For instance, this is specially helpful when the ioctl fails. Consider
> > the following example:
> >
> > v4l2-ctrls: prepare_ext_ctrls: video1: pointer control id 0x990cec size too small, 400 bytes but 784 bytes needed
> > v4l2-ctrls: try_set_ext_ctrls: video1: video1: try_set_ext_ctrls_common failed (-14)
> > video1: VIDIOC_S_EXT_CTRLS: error -14: which=0xf010000, count=5, error_idx=5, request_fd=39, name=H264 Sequence Parameter Set,
> > id/size=0x990ce8/1048, name=H264 Picture Parameter Set, id/size=0x990ce9/12, name=H264 Scaling Matrix, id/size=0x990cea/480, name=H264 Slice
> > Parameters, id/size=0x990ceb/896, name=H264 Decode Parameters, id/size=0x990cec/400
> >
> > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> > ---
> > drivers/media/v4l2-core/v4l2-ioctl.c | 16 +++++++++++-----
> > 1 file changed, 11 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> > index 2322f08a98be..4264ac44c48b 100644
> > --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> > +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> > @@ -582,8 +582,10 @@ static void v4l_print_querymenu(const void *arg, bool write_only)
> > static void v4l_print_control(const void *arg, bool write_only)
> > {
> > const struct v4l2_control *p = arg;
> > + const char *name = v4l2_ctrl_get_name(p->id);
> >
> > - pr_cont("id=0x%x, value=%d\n", p->id, p->value);
> > + pr_cont("name=%s, id=0x%x, value=%d\n",
> > + name ? name : "unknown", p->id, p->value);
>
> I prefer:
>
> if (name)
> pr_cont("name=%s, ", name);
> pr_cont("id=0x%x, value=%d\n", p->id, p->value);
>
I don't have issues posting a v2 with that if you prefer.
However, seems to me seeing "unknown" if the name wasn't found
is actually a good debugging hint, signalling the control ID
is probably wrong.
Thanks for reviewing,
Ezequiel
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] v4l2-core: Print control name in VIDIOC_S/G_(EXT)_CTRL(S)
2020-06-09 12:15 [PATCH] v4l2-core: Print control name in VIDIOC_S/G_(EXT)_CTRL(S) Ezequiel Garcia
2020-06-11 12:35 ` Helen Koike
2020-06-23 11:01 ` Hans Verkuil
@ 2020-07-01 13:17 ` Ezequiel Garcia
2 siblings, 0 replies; 6+ messages in thread
From: Ezequiel Garcia @ 2020-07-01 13:17 UTC (permalink / raw)
To: linux-media; +Cc: Hans Verkuil, Ezequiel Garcia, kernel
While debugging, it's currently really hard to identify controls
by their ID. Print the control name making the print more helpful.
With this change, the print changes from:
video1: VIDIOC_S_EXT_CTRLS: which=0xf010000, count=5, error_idx=4, request_fd=45, id/size=0x990ce8/1048, id/size=0x990ce9/12, id/size=0x990cea/480, id/size=0x990ceb/896, id/size=0x990cec/400
to:
video1: VIDIOC_S_EXT_CTRLS: which=0xf010000, count=5, error_idx=4, request_fd=42, name=H264 Sequence Parameter Set, id/size=0x990ce8/1048, name=H264 Picture Parameter Set, id/size=0x990ce9/12, name=H264 Scaling Matrix, id/size=0x990cea/480, name=H264 Slice Parameters, id/size=0x990ceb/896, name=H264 Decode Parameters, id/size=0x990cec/400
For instance, this is specially helpful when the ioctl fails. Consider
the following example:
v4l2-ctrls: prepare_ext_ctrls: video1: pointer control id 0x990cec size too small, 400 bytes but 784 bytes needed
v4l2-ctrls: try_set_ext_ctrls: video1: video1: try_set_ext_ctrls_common failed (-14)
video1: VIDIOC_S_EXT_CTRLS: error -14: which=0xf010000, count=5, error_idx=5, request_fd=39, name=H264 Sequence Parameter Set, id/size=0x990ce8/1048, name=H264 Picture Parameter Set, id/size=0x990ce9/12, name=H264 Scaling Matrix, id/size=0x990cea/480, name=H264 Slice Parameters, id/size=0x990ceb/896, name=H264 Decode Parameters, id/size=0x990cec/400
Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
v2:
* As requested by Hans, only print the name if non-null.
---
drivers/media/v4l2-core/v4l2-ioctl.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index 02bfef0da76d..a556880f225a 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -582,7 +582,10 @@ static void v4l_print_querymenu(const void *arg, bool write_only)
static void v4l_print_control(const void *arg, bool write_only)
{
const struct v4l2_control *p = arg;
+ const char *name = v4l2_ctrl_get_name(p->id);
+ if (name)
+ pr_cont("name=%s, ", name);
pr_cont("id=0x%x, value=%d\n", p->id, p->value);
}
@@ -594,12 +597,15 @@ static void v4l_print_ext_controls(const void *arg, bool write_only)
pr_cont("which=0x%x, count=%d, error_idx=%d, request_fd=%d",
p->which, p->count, p->error_idx, p->request_fd);
for (i = 0; i < p->count; i++) {
+ unsigned int id = p->controls[i].id;
+ const char *name = v4l2_ctrl_get_name(id);
+
+ if (name)
+ pr_cont(", name=%s", name);
if (!p->controls[i].size)
- pr_cont(", id/val=0x%x/0x%x",
- p->controls[i].id, p->controls[i].value);
+ pr_cont(", id/val=0x%x/0x%x", id, p->controls[i].value);
else
- pr_cont(", id/size=0x%x/%u",
- p->controls[i].id, p->controls[i].size);
+ pr_cont(", id/size=0x%x/%u", id, p->controls[i].size);
}
pr_cont("\n");
}
--
2.26.0.rc2
^ permalink raw reply related [flat|nested] 6+ messages in thread