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@xxxxxxxxxxx wrote:
> >>>>> From: Yunke Cao <yunkec@xxxxxxxxxx>
> >>>>>
> >>>>> 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@xxxxxxxxxx>
> >>>>> Signed-off-by: Ming Qian <ming.qian@xxxxxxxxxxx>
> >>>>> ---
> >>>>> 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);
--
Regards,
Laurent Pinchart
