| Message ID | 20241030024307.1114787-3-ming.qian@oss.nxp.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Support V4L2_CTRL_TYPE_RECT and V4L2_CTRL_WHICH_MIN/MAX_VAL | expand |
On Wed, Oct 30, 2024 at 11:43:06AM +0900, ming.qian@oss.nxp.com wrote: > From: Yunke Cao <yunkec@google.com> > > Tested with VIVID > > ./v4l2-ctl -C rect -d 0 > rect: 300x400@200x100 > > ./v4l2-ctl -c rect=1000x2000@0x0 > ./v4l2-ctl -C rect -d 0 > rect: 1000x2000@0x0 > > Signed-off-by: Yunke Cao <yunkec@google.com> > Signed-off-by: Ming Qian <ming.qian@oss.nxp.com> > --- > utils/v4l2-ctl/v4l2-ctl-common.cpp | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/utils/v4l2-ctl/v4l2-ctl-common.cpp b/utils/v4l2-ctl/v4l2-ctl-common.cpp > index 40667575fcc7..538e1951cf81 100644 > --- a/utils/v4l2-ctl/v4l2-ctl-common.cpp > +++ b/utils/v4l2-ctl/v4l2-ctl-common.cpp > @@ -614,6 +614,10 @@ static void print_value(int fd, const v4l2_query_ext_ctrl &qc, const v4l2_ext_co > case V4L2_CTRL_TYPE_AREA: > printf("%dx%d", ctrl.p_area->width, ctrl.p_area->height); > break; > + case V4L2_CTRL_TYPE_RECT: > + printf("%ux%u@%dx%d", ctrl.p_rect->width, ctrl.p_rect->height, I find this notation ambiguous, it's not immediately clear when reading 10x10@20x20 if we're looking at a 10x10 rectangle positioned at (20,20) or the other way around. media-ctl use (20,20)/10x10 which I think would be a better notation. > + ctrl.p_rect->left, ctrl.p_rect->top); > + break; > default: > printf("unsupported payload type"); > break; > @@ -702,6 +706,9 @@ static void print_qctrl(int fd, const v4l2_query_ext_ctrl &qc, > case V4L2_CTRL_TYPE_AREA: > printf("%31s %#8.8x (area) :", s.c_str(), qc.id); > break; > + case V4L2_CTRL_TYPE_RECT: > + printf("%31s %#8.8x (rect) :", s.c_str(), qc.id); > + break; > case V4L2_CTRL_TYPE_HDR10_CLL_INFO: > printf("%31s %#8.8x (hdr10-cll-info):", s.c_str(), qc.id); > break; > @@ -1279,6 +1286,11 @@ void common_set(cv4l_fd &_fd) > sscanf(set_ctrl.second.c_str(), "%ux%u", > &ctrl.p_area->width, &ctrl.p_area->height); > break; > + case V4L2_CTRL_TYPE_RECT: > + sscanf(set_ctrl.second.c_str(), "%ux%u@%dx%d", > + &ctrl.p_rect->width, &ctrl.p_rect->height, > + &ctrl.p_rect->left, &ctrl.p_rect->top); > + break; > default: > fprintf(stderr, "%s: unsupported payload type\n", > qc.name);
On 30/10/2024 10:03, Laurent Pinchart wrote: > On Wed, Oct 30, 2024 at 11:43:06AM +0900, ming.qian@oss.nxp.com wrote: >> From: Yunke Cao <yunkec@google.com> >> >> Tested with VIVID >> >> ./v4l2-ctl -C rect -d 0 >> rect: 300x400@200x100 >> >> ./v4l2-ctl -c rect=1000x2000@0x0 >> ./v4l2-ctl -C rect -d 0 >> rect: 1000x2000@0x0 >> >> Signed-off-by: Yunke Cao <yunkec@google.com> >> Signed-off-by: Ming Qian <ming.qian@oss.nxp.com> >> --- >> utils/v4l2-ctl/v4l2-ctl-common.cpp | 12 ++++++++++++ >> 1 file changed, 12 insertions(+) >> >> diff --git a/utils/v4l2-ctl/v4l2-ctl-common.cpp b/utils/v4l2-ctl/v4l2-ctl-common.cpp >> index 40667575fcc7..538e1951cf81 100644 >> --- a/utils/v4l2-ctl/v4l2-ctl-common.cpp >> +++ b/utils/v4l2-ctl/v4l2-ctl-common.cpp >> @@ -614,6 +614,10 @@ static void print_value(int fd, const v4l2_query_ext_ctrl &qc, const v4l2_ext_co >> case V4L2_CTRL_TYPE_AREA: >> printf("%dx%d", ctrl.p_area->width, ctrl.p_area->height); >> break; >> + case V4L2_CTRL_TYPE_RECT: >> + printf("%ux%u@%dx%d", ctrl.p_rect->width, ctrl.p_rect->height, > > I find this notation ambiguous, it's not immediately clear when reading > 10x10@20x20 if we're looking at a 10x10 rectangle positioned at (20,20) > or the other way around. media-ctl use (20,20)/10x10 which I think would > be a better notation. Good point, I agree. Ming Qian, can you also update patch 1/4 of the kernel patch series to use the same formatting when logging the V4L2_CTRL_TYPE_RECT value? Regards, Hans > >> + ctrl.p_rect->left, ctrl.p_rect->top); >> + break; >> default: >> printf("unsupported payload type"); >> break; >> @@ -702,6 +706,9 @@ static void print_qctrl(int fd, const v4l2_query_ext_ctrl &qc, >> case V4L2_CTRL_TYPE_AREA: >> printf("%31s %#8.8x (area) :", s.c_str(), qc.id); >> break; >> + case V4L2_CTRL_TYPE_RECT: >> + printf("%31s %#8.8x (rect) :", s.c_str(), qc.id); >> + break; >> case V4L2_CTRL_TYPE_HDR10_CLL_INFO: >> printf("%31s %#8.8x (hdr10-cll-info):", s.c_str(), qc.id); >> break; >> @@ -1279,6 +1286,11 @@ void common_set(cv4l_fd &_fd) >> sscanf(set_ctrl.second.c_str(), "%ux%u", >> &ctrl.p_area->width, &ctrl.p_area->height); >> break; >> + case V4L2_CTRL_TYPE_RECT: >> + sscanf(set_ctrl.second.c_str(), "%ux%u@%dx%d", >> + &ctrl.p_rect->width, &ctrl.p_rect->height, >> + &ctrl.p_rect->left, &ctrl.p_rect->top); >> + break; >> default: >> fprintf(stderr, "%s: unsupported payload type\n", >> qc.name); >
Hi Laurent, On 2024/10/30 17:03, Laurent Pinchart wrote: > On Wed, Oct 30, 2024 at 11:43:06AM +0900, ming.qian@oss.nxp.com wrote: >> From: Yunke Cao <yunkec@google.com> >> >> Tested with VIVID >> >> ./v4l2-ctl -C rect -d 0 >> rect: 300x400@200x100 >> >> ./v4l2-ctl -c rect=1000x2000@0x0 >> ./v4l2-ctl -C rect -d 0 >> rect: 1000x2000@0x0 >> >> Signed-off-by: Yunke Cao <yunkec@google.com> >> Signed-off-by: Ming Qian <ming.qian@oss.nxp.com> >> --- >> utils/v4l2-ctl/v4l2-ctl-common.cpp | 12 ++++++++++++ >> 1 file changed, 12 insertions(+) >> >> diff --git a/utils/v4l2-ctl/v4l2-ctl-common.cpp b/utils/v4l2-ctl/v4l2-ctl-common.cpp >> index 40667575fcc7..538e1951cf81 100644 >> --- a/utils/v4l2-ctl/v4l2-ctl-common.cpp >> +++ b/utils/v4l2-ctl/v4l2-ctl-common.cpp >> @@ -614,6 +614,10 @@ static void print_value(int fd, const v4l2_query_ext_ctrl &qc, const v4l2_ext_co >> case V4L2_CTRL_TYPE_AREA: >> printf("%dx%d", ctrl.p_area->width, ctrl.p_area->height); >> break; >> + case V4L2_CTRL_TYPE_RECT: >> + printf("%ux%u@%dx%d", ctrl.p_rect->width, ctrl.p_rect->height, > > I find this notation ambiguous, it's not immediately clear when reading > 10x10@20x20 if we're looking at a 10x10 rectangle positioned at (20,20) > or the other way around. media-ctl use (20,20)/10x10 which I think would > be a better notation. > Thanks for the suggestions, I'll go ahead with this approach. >> + ctrl.p_rect->left, ctrl.p_rect->top); >> + break; >> default: >> printf("unsupported payload type"); >> break; >> @@ -702,6 +706,9 @@ static void print_qctrl(int fd, const v4l2_query_ext_ctrl &qc, >> case V4L2_CTRL_TYPE_AREA: >> printf("%31s %#8.8x (area) :", s.c_str(), qc.id); >> break; >> + case V4L2_CTRL_TYPE_RECT: >> + printf("%31s %#8.8x (rect) :", s.c_str(), qc.id); >> + break; >> case V4L2_CTRL_TYPE_HDR10_CLL_INFO: >> printf("%31s %#8.8x (hdr10-cll-info):", s.c_str(), qc.id); >> break; >> @@ -1279,6 +1286,11 @@ void common_set(cv4l_fd &_fd) >> sscanf(set_ctrl.second.c_str(), "%ux%u", >> &ctrl.p_area->width, &ctrl.p_area->height); >> break; >> + case V4L2_CTRL_TYPE_RECT: >> + sscanf(set_ctrl.second.c_str(), "%ux%u@%dx%d", >> + &ctrl.p_rect->width, &ctrl.p_rect->height, >> + &ctrl.p_rect->left, &ctrl.p_rect->top); >> + break; >> default: >> fprintf(stderr, "%s: unsupported payload type\n", >> qc.name); >
On 2024/10/30 17:19, Hans Verkuil wrote: > On 30/10/2024 10:03, Laurent Pinchart wrote: >> On Wed, Oct 30, 2024 at 11:43:06AM +0900, ming.qian@oss.nxp.com wrote: >>> From: Yunke Cao <yunkec@google.com> >>> >>> Tested with VIVID >>> >>> ./v4l2-ctl -C rect -d 0 >>> rect: 300x400@200x100 >>> >>> ./v4l2-ctl -c rect=1000x2000@0x0 >>> ./v4l2-ctl -C rect -d 0 >>> rect: 1000x2000@0x0 >>> >>> Signed-off-by: Yunke Cao <yunkec@google.com> >>> Signed-off-by: Ming Qian <ming.qian@oss.nxp.com> >>> --- >>> utils/v4l2-ctl/v4l2-ctl-common.cpp | 12 ++++++++++++ >>> 1 file changed, 12 insertions(+) >>> >>> diff --git a/utils/v4l2-ctl/v4l2-ctl-common.cpp b/utils/v4l2-ctl/v4l2-ctl-common.cpp >>> index 40667575fcc7..538e1951cf81 100644 >>> --- a/utils/v4l2-ctl/v4l2-ctl-common.cpp >>> +++ b/utils/v4l2-ctl/v4l2-ctl-common.cpp >>> @@ -614,6 +614,10 @@ static void print_value(int fd, const v4l2_query_ext_ctrl &qc, const v4l2_ext_co >>> case V4L2_CTRL_TYPE_AREA: >>> printf("%dx%d", ctrl.p_area->width, ctrl.p_area->height); >>> break; >>> + case V4L2_CTRL_TYPE_RECT: >>> + printf("%ux%u@%dx%d", ctrl.p_rect->width, ctrl.p_rect->height, >> >> I find this notation ambiguous, it's not immediately clear when reading >> 10x10@20x20 if we're looking at a 10x10 rectangle positioned at (20,20) >> or the other way around. media-ctl use (20,20)/10x10 which I think would >> be a better notation. > > Good point, I agree. > > Ming Qian, can you also update patch 1/4 of the kernel patch series to > use the same formatting when logging the V4L2_CTRL_TYPE_RECT value? > > Regards, > > Hans Yes, I will > >> >>> + ctrl.p_rect->left, ctrl.p_rect->top); >>> + break; >>> default: >>> printf("unsupported payload type"); >>> break; >>> @@ -702,6 +706,9 @@ static void print_qctrl(int fd, const v4l2_query_ext_ctrl &qc, >>> case V4L2_CTRL_TYPE_AREA: >>> printf("%31s %#8.8x (area) :", s.c_str(), qc.id); >>> break; >>> + case V4L2_CTRL_TYPE_RECT: >>> + printf("%31s %#8.8x (rect) :", s.c_str(), qc.id); >>> + break; >>> case V4L2_CTRL_TYPE_HDR10_CLL_INFO: >>> printf("%31s %#8.8x (hdr10-cll-info):", s.c_str(), qc.id); >>> break; >>> @@ -1279,6 +1286,11 @@ void common_set(cv4l_fd &_fd) >>> sscanf(set_ctrl.second.c_str(), "%ux%u", >>> &ctrl.p_area->width, &ctrl.p_area->height); >>> break; >>> + case V4L2_CTRL_TYPE_RECT: >>> + sscanf(set_ctrl.second.c_str(), "%ux%u@%dx%d", >>> + &ctrl.p_rect->width, &ctrl.p_rect->height, >>> + &ctrl.p_rect->left, &ctrl.p_rect->top); >>> + break; >>> default: >>> fprintf(stderr, "%s: unsupported payload type\n", >>> qc.name); >> >
Hi Hans, On 2024/10/30 17:19, Hans Verkuil wrote: > On 30/10/2024 10:03, Laurent Pinchart wrote: >> On Wed, Oct 30, 2024 at 11:43:06AM +0900, ming.qian@oss.nxp.com wrote: >>> From: Yunke Cao <yunkec@google.com> >>> >>> Tested with VIVID >>> >>> ./v4l2-ctl -C rect -d 0 >>> rect: 300x400@200x100 >>> >>> ./v4l2-ctl -c rect=1000x2000@0x0 >>> ./v4l2-ctl -C rect -d 0 >>> rect: 1000x2000@0x0 >>> >>> Signed-off-by: Yunke Cao <yunkec@google.com> >>> Signed-off-by: Ming Qian <ming.qian@oss.nxp.com> >>> --- >>> utils/v4l2-ctl/v4l2-ctl-common.cpp | 12 ++++++++++++ >>> 1 file changed, 12 insertions(+) >>> >>> diff --git a/utils/v4l2-ctl/v4l2-ctl-common.cpp b/utils/v4l2-ctl/v4l2-ctl-common.cpp >>> index 40667575fcc7..538e1951cf81 100644 >>> --- a/utils/v4l2-ctl/v4l2-ctl-common.cpp >>> +++ b/utils/v4l2-ctl/v4l2-ctl-common.cpp >>> @@ -614,6 +614,10 @@ static void print_value(int fd, const v4l2_query_ext_ctrl &qc, const v4l2_ext_co >>> case V4L2_CTRL_TYPE_AREA: >>> printf("%dx%d", ctrl.p_area->width, ctrl.p_area->height); >>> break; >>> + case V4L2_CTRL_TYPE_RECT: >>> + printf("%ux%u@%dx%d", ctrl.p_rect->width, ctrl.p_rect->height, >> >> I find this notation ambiguous, it's not immediately clear when reading >> 10x10@20x20 if we're looking at a 10x10 rectangle positioned at (20,20) >> or the other way around. media-ctl use (20,20)/10x10 which I think would >> be a better notation. > > Good point, I agree. > > Ming Qian, can you also update patch 1/4 of the kernel patch series to > use the same formatting when logging the V4L2_CTRL_TYPE_RECT value? > > Regards, > > Hans There is a issue in v4l2-utils, that ',' is the ending flag in v4l_getsubopt(), then I can't set the rect control, for example: $v4l2-ctl -d 0 -c rect="(0,0)/1000x2000" control '0)/1000x2000' without '=' Thanks, Ming > >> >>> + ctrl.p_rect->left, ctrl.p_rect->top); >>> + break; >>> default: >>> printf("unsupported payload type"); >>> break; >>> @@ -702,6 +706,9 @@ static void print_qctrl(int fd, const v4l2_query_ext_ctrl &qc, >>> case V4L2_CTRL_TYPE_AREA: >>> printf("%31s %#8.8x (area) :", s.c_str(), qc.id); >>> break; >>> + case V4L2_CTRL_TYPE_RECT: >>> + printf("%31s %#8.8x (rect) :", s.c_str(), qc.id); >>> + break; >>> case V4L2_CTRL_TYPE_HDR10_CLL_INFO: >>> printf("%31s %#8.8x (hdr10-cll-info):", s.c_str(), qc.id); >>> break; >>> @@ -1279,6 +1286,11 @@ void common_set(cv4l_fd &_fd) >>> sscanf(set_ctrl.second.c_str(), "%ux%u", >>> &ctrl.p_area->width, &ctrl.p_area->height); >>> break; >>> + case V4L2_CTRL_TYPE_RECT: >>> + sscanf(set_ctrl.second.c_str(), "%ux%u@%dx%d", >>> + &ctrl.p_rect->width, &ctrl.p_rect->height, >>> + &ctrl.p_rect->left, &ctrl.p_rect->top); >>> + break; >>> default: >>> fprintf(stderr, "%s: unsupported payload type\n", >>> qc.name); >> >
On Thu, Oct 31, 2024 at 05:19:02PM +0800, Ming Qian(OSS) wrote: > On 2024/10/30 17:19, Hans Verkuil wrote: > > On 30/10/2024 10:03, Laurent Pinchart wrote: > >> On Wed, Oct 30, 2024 at 11:43:06AM +0900, ming.qian@oss.nxp.com wrote: > >>> From: Yunke Cao <yunkec@google.com> > >>> > >>> Tested with VIVID > >>> > >>> ./v4l2-ctl -C rect -d 0 > >>> rect: 300x400@200x100 > >>> > >>> ./v4l2-ctl -c rect=1000x2000@0x0 > >>> ./v4l2-ctl -C rect -d 0 > >>> rect: 1000x2000@0x0 > >>> > >>> Signed-off-by: Yunke Cao <yunkec@google.com> > >>> Signed-off-by: Ming Qian <ming.qian@oss.nxp.com> > >>> --- > >>> utils/v4l2-ctl/v4l2-ctl-common.cpp | 12 ++++++++++++ > >>> 1 file changed, 12 insertions(+) > >>> > >>> diff --git a/utils/v4l2-ctl/v4l2-ctl-common.cpp b/utils/v4l2-ctl/v4l2-ctl-common.cpp > >>> index 40667575fcc7..538e1951cf81 100644 > >>> --- a/utils/v4l2-ctl/v4l2-ctl-common.cpp > >>> +++ b/utils/v4l2-ctl/v4l2-ctl-common.cpp > >>> @@ -614,6 +614,10 @@ static void print_value(int fd, const v4l2_query_ext_ctrl &qc, const v4l2_ext_co > >>> case V4L2_CTRL_TYPE_AREA: > >>> printf("%dx%d", ctrl.p_area->width, ctrl.p_area->height); > >>> break; > >>> + case V4L2_CTRL_TYPE_RECT: > >>> + printf("%ux%u@%dx%d", ctrl.p_rect->width, ctrl.p_rect->height, > >> > >> I find this notation ambiguous, it's not immediately clear when reading > >> 10x10@20x20 if we're looking at a 10x10 rectangle positioned at (20,20) > >> or the other way around. media-ctl use (20,20)/10x10 which I think would > >> be a better notation. > > > > Good point, I agree. > > > > Ming Qian, can you also update patch 1/4 of the kernel patch series to > > use the same formatting when logging the V4L2_CTRL_TYPE_RECT value? > > > > Regards, > > > > Hans > > There is a issue in v4l2-utils, that ',' is the ending flag in > v4l_getsubopt(), then I can't set the rect control, > for example: > > $v4l2-ctl -d 0 -c rect="(0,0)/1000x2000" > control '0)/1000x2000' without '=' The should be fixable in v4l_getsubopt(). > >>> + ctrl.p_rect->left, ctrl.p_rect->top); > >>> + break; > >>> default: > >>> printf("unsupported payload type"); > >>> break; > >>> @@ -702,6 +706,9 @@ static void print_qctrl(int fd, const v4l2_query_ext_ctrl &qc, > >>> case V4L2_CTRL_TYPE_AREA: > >>> printf("%31s %#8.8x (area) :", s.c_str(), qc.id); > >>> break; > >>> + case V4L2_CTRL_TYPE_RECT: > >>> + printf("%31s %#8.8x (rect) :", s.c_str(), qc.id); > >>> + break; > >>> case V4L2_CTRL_TYPE_HDR10_CLL_INFO: > >>> printf("%31s %#8.8x (hdr10-cll-info):", s.c_str(), qc.id); > >>> break; > >>> @@ -1279,6 +1286,11 @@ void common_set(cv4l_fd &_fd) > >>> sscanf(set_ctrl.second.c_str(), "%ux%u", > >>> &ctrl.p_area->width, &ctrl.p_area->height); > >>> break; > >>> + case V4L2_CTRL_TYPE_RECT: > >>> + sscanf(set_ctrl.second.c_str(), "%ux%u@%dx%d", > >>> + &ctrl.p_rect->width, &ctrl.p_rect->height, > >>> + &ctrl.p_rect->left, &ctrl.p_rect->top); > >>> + break; > >>> default: > >>> fprintf(stderr, "%s: unsupported payload type\n", > >>> qc.name);
Hi Laurent, On 2024/10/31 17:34, Laurent Pinchart wrote: > On Thu, Oct 31, 2024 at 05:19:02PM +0800, Ming Qian(OSS) wrote: >> On 2024/10/30 17:19, Hans Verkuil wrote: >>> On 30/10/2024 10:03, Laurent Pinchart wrote: >>>> On Wed, Oct 30, 2024 at 11:43:06AM +0900, ming.qian@oss.nxp.com wrote: >>>>> From: Yunke Cao <yunkec@google.com> >>>>> >>>>> Tested with VIVID >>>>> >>>>> ./v4l2-ctl -C rect -d 0 >>>>> rect: 300x400@200x100 >>>>> >>>>> ./v4l2-ctl -c rect=1000x2000@0x0 >>>>> ./v4l2-ctl -C rect -d 0 >>>>> rect: 1000x2000@0x0 >>>>> >>>>> Signed-off-by: Yunke Cao <yunkec@google.com> >>>>> Signed-off-by: Ming Qian <ming.qian@oss.nxp.com> >>>>> --- >>>>> utils/v4l2-ctl/v4l2-ctl-common.cpp | 12 ++++++++++++ >>>>> 1 file changed, 12 insertions(+) >>>>> >>>>> diff --git a/utils/v4l2-ctl/v4l2-ctl-common.cpp b/utils/v4l2-ctl/v4l2-ctl-common.cpp >>>>> index 40667575fcc7..538e1951cf81 100644 >>>>> --- a/utils/v4l2-ctl/v4l2-ctl-common.cpp >>>>> +++ b/utils/v4l2-ctl/v4l2-ctl-common.cpp >>>>> @@ -614,6 +614,10 @@ static void print_value(int fd, const v4l2_query_ext_ctrl &qc, const v4l2_ext_co >>>>> case V4L2_CTRL_TYPE_AREA: >>>>> printf("%dx%d", ctrl.p_area->width, ctrl.p_area->height); >>>>> break; >>>>> + case V4L2_CTRL_TYPE_RECT: >>>>> + printf("%ux%u@%dx%d", ctrl.p_rect->width, ctrl.p_rect->height, >>>> >>>> I find this notation ambiguous, it's not immediately clear when reading >>>> 10x10@20x20 if we're looking at a 10x10 rectangle positioned at (20,20) >>>> or the other way around. media-ctl use (20,20)/10x10 which I think would >>>> be a better notation. >>> >>> Good point, I agree. >>> >>> Ming Qian, can you also update patch 1/4 of the kernel patch series to >>> use the same formatting when logging the V4L2_CTRL_TYPE_RECT value? >>> >>> Regards, >>> >>> Hans >> >> There is a issue in v4l2-utils, that ',' is the ending flag in >> v4l_getsubopt(), then I can't set the rect control, >> for example: >> >> $v4l2-ctl -d 0 -c rect="(0,0)/1000x2000" >> control '0)/1000x2000' without '=' > > The should be fixable in v4l_getsubopt(). > I can see the following comments of v4l_getsubopt(), Parse comma separated suboption from *OPTIONP and match against strings in TOKENS. I am not sure if we can change it. Thanks, Ming >>>>> + ctrl.p_rect->left, ctrl.p_rect->top); >>>>> + break; >>>>> default: >>>>> printf("unsupported payload type"); >>>>> break; >>>>> @@ -702,6 +706,9 @@ static void print_qctrl(int fd, const v4l2_query_ext_ctrl &qc, >>>>> case V4L2_CTRL_TYPE_AREA: >>>>> printf("%31s %#8.8x (area) :", s.c_str(), qc.id); >>>>> break; >>>>> + case V4L2_CTRL_TYPE_RECT: >>>>> + printf("%31s %#8.8x (rect) :", s.c_str(), qc.id); >>>>> + break; >>>>> case V4L2_CTRL_TYPE_HDR10_CLL_INFO: >>>>> printf("%31s %#8.8x (hdr10-cll-info):", s.c_str(), qc.id); >>>>> break; >>>>> @@ -1279,6 +1286,11 @@ void common_set(cv4l_fd &_fd) >>>>> sscanf(set_ctrl.second.c_str(), "%ux%u", >>>>> &ctrl.p_area->width, &ctrl.p_area->height); >>>>> break; >>>>> + case V4L2_CTRL_TYPE_RECT: >>>>> + sscanf(set_ctrl.second.c_str(), "%ux%u@%dx%d", >>>>> + &ctrl.p_rect->width, &ctrl.p_rect->height, >>>>> + &ctrl.p_rect->left, &ctrl.p_rect->top); >>>>> + break; >>>>> default: >>>>> fprintf(stderr, "%s: unsupported payload type\n", >>>>> qc.name); >
On Thu, Oct 31, 2024 at 05:46:49PM +0800, Ming Qian(OSS) wrote: > On 2024/10/31 17:34, Laurent Pinchart wrote: > > On Thu, Oct 31, 2024 at 05:19:02PM +0800, Ming Qian(OSS) wrote: > >> On 2024/10/30 17:19, Hans Verkuil wrote: > >>> On 30/10/2024 10:03, Laurent Pinchart wrote: > >>>> On Wed, Oct 30, 2024 at 11:43:06AM +0900, ming.qian@oss.nxp.com wrote: > >>>>> From: Yunke Cao <yunkec@google.com> > >>>>> > >>>>> Tested with VIVID > >>>>> > >>>>> ./v4l2-ctl -C rect -d 0 > >>>>> rect: 300x400@200x100 > >>>>> > >>>>> ./v4l2-ctl -c rect=1000x2000@0x0 > >>>>> ./v4l2-ctl -C rect -d 0 > >>>>> rect: 1000x2000@0x0 > >>>>> > >>>>> Signed-off-by: Yunke Cao <yunkec@google.com> > >>>>> Signed-off-by: Ming Qian <ming.qian@oss.nxp.com> > >>>>> --- > >>>>> utils/v4l2-ctl/v4l2-ctl-common.cpp | 12 ++++++++++++ > >>>>> 1 file changed, 12 insertions(+) > >>>>> > >>>>> diff --git a/utils/v4l2-ctl/v4l2-ctl-common.cpp b/utils/v4l2-ctl/v4l2-ctl-common.cpp > >>>>> index 40667575fcc7..538e1951cf81 100644 > >>>>> --- a/utils/v4l2-ctl/v4l2-ctl-common.cpp > >>>>> +++ b/utils/v4l2-ctl/v4l2-ctl-common.cpp > >>>>> @@ -614,6 +614,10 @@ static void print_value(int fd, const v4l2_query_ext_ctrl &qc, const v4l2_ext_co > >>>>> case V4L2_CTRL_TYPE_AREA: > >>>>> printf("%dx%d", ctrl.p_area->width, ctrl.p_area->height); > >>>>> break; > >>>>> + case V4L2_CTRL_TYPE_RECT: > >>>>> + printf("%ux%u@%dx%d", ctrl.p_rect->width, ctrl.p_rect->height, > >>>> > >>>> I find this notation ambiguous, it's not immediately clear when reading > >>>> 10x10@20x20 if we're looking at a 10x10 rectangle positioned at (20,20) > >>>> or the other way around. media-ctl use (20,20)/10x10 which I think would > >>>> be a better notation. > >>> > >>> Good point, I agree. > >>> > >>> Ming Qian, can you also update patch 1/4 of the kernel patch series to > >>> use the same formatting when logging the V4L2_CTRL_TYPE_RECT value? > >>> > >>> Regards, > >>> > >>> Hans > >> > >> There is a issue in v4l2-utils, that ',' is the ending flag in > >> v4l_getsubopt(), then I can't set the rect control, > >> for example: > >> > >> $v4l2-ctl -d 0 -c rect="(0,0)/1000x2000" > >> control '0)/1000x2000' without '=' > > > > The should be fixable in v4l_getsubopt(). > > > > I can see the following comments of v4l_getsubopt(), > > Parse comma separated suboption from *OPTIONP and match against > strings in TOKENS. > > I am not sure if we can change it. I think we can improve quotes handling by considering quoted substrings as a single value, ignoring commas. Hans any opinion ? > >>>>> + ctrl.p_rect->left, ctrl.p_rect->top); > >>>>> + break; > >>>>> default: > >>>>> printf("unsupported payload type"); > >>>>> break; > >>>>> @@ -702,6 +706,9 @@ static void print_qctrl(int fd, const v4l2_query_ext_ctrl &qc, > >>>>> case V4L2_CTRL_TYPE_AREA: > >>>>> printf("%31s %#8.8x (area) :", s.c_str(), qc.id); > >>>>> break; > >>>>> + case V4L2_CTRL_TYPE_RECT: > >>>>> + printf("%31s %#8.8x (rect) :", s.c_str(), qc.id); > >>>>> + break; > >>>>> case V4L2_CTRL_TYPE_HDR10_CLL_INFO: > >>>>> printf("%31s %#8.8x (hdr10-cll-info):", s.c_str(), qc.id); > >>>>> break; > >>>>> @@ -1279,6 +1286,11 @@ void common_set(cv4l_fd &_fd) > >>>>> sscanf(set_ctrl.second.c_str(), "%ux%u", > >>>>> &ctrl.p_area->width, &ctrl.p_area->height); > >>>>> break; > >>>>> + case V4L2_CTRL_TYPE_RECT: > >>>>> + sscanf(set_ctrl.second.c_str(), "%ux%u@%dx%d", > >>>>> + &ctrl.p_rect->width, &ctrl.p_rect->height, > >>>>> + &ctrl.p_rect->left, &ctrl.p_rect->top); > >>>>> + break; > >>>>> default: > >>>>> fprintf(stderr, "%s: unsupported payload type\n", > >>>>> qc.name);
diff --git a/utils/v4l2-ctl/v4l2-ctl-common.cpp b/utils/v4l2-ctl/v4l2-ctl-common.cpp index 40667575fcc7..538e1951cf81 100644 --- a/utils/v4l2-ctl/v4l2-ctl-common.cpp +++ b/utils/v4l2-ctl/v4l2-ctl-common.cpp @@ -614,6 +614,10 @@ static void print_value(int fd, const v4l2_query_ext_ctrl &qc, const v4l2_ext_co case V4L2_CTRL_TYPE_AREA: printf("%dx%d", ctrl.p_area->width, ctrl.p_area->height); break; + case V4L2_CTRL_TYPE_RECT: + printf("%ux%u@%dx%d", ctrl.p_rect->width, ctrl.p_rect->height, + ctrl.p_rect->left, ctrl.p_rect->top); + break; default: printf("unsupported payload type"); break; @@ -702,6 +706,9 @@ static void print_qctrl(int fd, const v4l2_query_ext_ctrl &qc, case V4L2_CTRL_TYPE_AREA: printf("%31s %#8.8x (area) :", s.c_str(), qc.id); break; + case V4L2_CTRL_TYPE_RECT: + printf("%31s %#8.8x (rect) :", s.c_str(), qc.id); + break; case V4L2_CTRL_TYPE_HDR10_CLL_INFO: printf("%31s %#8.8x (hdr10-cll-info):", s.c_str(), qc.id); break; @@ -1279,6 +1286,11 @@ void common_set(cv4l_fd &_fd) sscanf(set_ctrl.second.c_str(), "%ux%u", &ctrl.p_area->width, &ctrl.p_area->height); break; + case V4L2_CTRL_TYPE_RECT: + sscanf(set_ctrl.second.c_str(), "%ux%u@%dx%d", + &ctrl.p_rect->width, &ctrl.p_rect->height, + &ctrl.p_rect->left, &ctrl.p_rect->top); + break; default: fprintf(stderr, "%s: unsupported payload type\n", qc.name);