Re: [PATCH v2 12/17] media: atomisp: Fix VIDIOC_REQBUFS failing when VIDIOC_S_FMT has not been called yet

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On 11/14/22 13:17, Andy Shevchenko wrote:
> On Thu, Oct 20, 2022 at 09:55:28PM +0200, Hans de Goede wrote:
>> camorama calls VIDIOC_REQBUFS(V4L2_MEMORY_MMAP) to test if MMAP support
>> works (this was added specifically because of the previously broken
>> MMAP support in atomisp).
>>
>> Currently this fails because atomisp_get_css_frame_info() fails in this
>> case. Although it is weird to call VIDIOC_REQBUFS before VIDIOC_S_FMT,
>> it is allowed to do this.
>>
>> Fix this not working by doing a S_FMT to V4L2_PIX_FMT_YUV420 + the highest
>> supported resolution.
>>
>> Note this will cause camorama to use mmap mode, which means it will also
>> use libv4l2 to do format conversion. libv4l2 will pick V4L2_PIX_FMT_RGB565
>> as input format and this will lead to a garbled video display. This is
>> a libv4lconvert bug, the RGB565 -> RGB24 path in libv4lconvert assumes
>> that stride == width which is not true on the atomisp.
>>
>> I've already send out a libv4lconvert fix for this. Also this can be worked
> 
> Link: ?
> 

Fixed in my media-atomisp branch.

>> around by passing --dont-use-libv4l2 as argument to camorama.
> 
> ...
> 
>> +		struct v4l2_format f = {
>> +			.fmt.pix.pixelformat = V4L2_PIX_FMT_YUV420,
> 
>> +			.fmt.pix.width = 10000,
>> +			.fmt.pix.height = 10000,
> 
> If it's just an arbitrary maximum, I would suggest to use definitions from
> limits.h, so it will better show the intention.

This is duplicating existing code in another path in atomisp2 where it
does this when it needs to pick a fmt because it needs one and userspace
has not set one yet.

I have added an entry to my atomisp TODO list to writer a helper for
this so that we only have these magic 10000 values only once.

As for if 10000 really is necessary, or if e.g. 65535 might work
as well, that is unclear.

Regards,

Hans



> 
>> +		};
> 




[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux