[libcamera-devel] [PATCH 2/4] v4l2: camera_proxy: Return correct type from VIDIOC_G_FMT

[Kieran Bingham]

Hi Laurent,

On 06/01/2020 17:11, Laurent Pinchart wrote:
> Hi Kieran,
> 
> On Mon, Jan 06, 2020 at 04:58:48PM +0000, Kieran Bingham wrote:
>> On 06/01/2020 16:14, Laurent Pinchart wrote:
>>> The VIDIOC_G_FMT implementation overwrites the v4l2_format type field
>>> with 0. Fix it.
>>
>> Could you clarify this please?
>>
>> You mean the kernel changes the arg->type?
>>
>> The whole function reads as:
>>
>> int V4L2CameraProxy::vidioc_g_fmt(struct v4l2_format *arg)
>> {
>>         LOG(V4L2Compat, Debug) << "Servicing vidioc_g_fmt";
>>
>>         if (!validateBufferType(arg->type))
>>                 return -EINVAL;
>>
>>         memset(&arg->fmt, 0, sizeof(arg->fmt));
>>> +	arg->type = V4L2_BUF_TYPE_VIDEO_CAPTURE; /* LP-Addition */
>>         arg->fmt.pix = curV4L2Format_.fmt.pix;
>>
>>         return 0;
>> }
>>
>>
>> I see usage of arg->type in the validateBufferType before the memset.
>>
>> Does this need to be set correctly before that ? or otherwise what is
>> the purpose of the call to validateBufferType(arg->type)?
> 
> I meant that *our* implementation of VIDIOC_G_FMT unconditionally sets
> arg->type to 0 (due to the memset). It's incorrect, the field should

but ....

> retain its value. And now that I've written this, I realize that we only
> memset arg->fmt, not arg... I'll drop this patch :-)


Aha - there we go then :-D

I thought it was an odd looking patch!

--
Kieran



> 
>>> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>>> ---
>>>  src/v4l2/v4l2_camera_proxy.cpp | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp
>>> index 2eeb12396d90..dd3ee3e6c6ff 100644
>>> --- a/src/v4l2/v4l2_camera_proxy.cpp
>>> +++ b/src/v4l2/v4l2_camera_proxy.cpp
>>> @@ -229,6 +229,7 @@ int V4L2CameraProxy::vidioc_g_fmt(struct v4l2_format *arg)
>>>  		return -EINVAL;
>>>  
>>>  	memset(&arg->fmt, 0, sizeof(arg->fmt));
>>> +	arg->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
>>>  	arg->fmt.pix = curV4L2Format_.fmt.pix;
>>>  
>>>  	return 0;
> 

-- 
Regards
--
Kieran