Re: [PATCH v2 2/3] v4l2-ctl: Support V4L2_CTRL_TYPE_RECT

["Ming Qian(OSS)" <ming.qian@oss.nxp.com>]

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);
>>
>