| Message ID | 20200609121514.9414-1-ezequiel@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | v4l2-core: Print control name in VIDIOC_S/G_(EXT)_CTRL(S) | expand |
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"); > } >
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"); >> } >>
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"); > } >
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
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"); }
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(-)