All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mauro Carvalho Chehab <mchehab@kernel.org>
To: Hans de Goede <hdegoede@redhat.com>
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>,
	Tsuchiya Yuto <kitakar@gmail.com>,
	Andy Shevchenko <andy@kernel.org>,
	Yury Luneff <yury.lunev@gmail.com>,
	Nable <nable.maininbox@googlemail.com>,
	andrey.i.trufanov@gmail.com, Fabio Aiuto <fabioaiuto83@gmail.com>,
	linux-media@vger.kernel.org, linux-staging@lists.linux.dev
Subject: Re: [PATCH v2 12/17] media: atomisp: Fix VIDIOC_REQBUFS failing when VIDIOC_S_FMT has not been called yet
Date: Sun, 20 Nov 2022 23:29:56 +0000	[thread overview]
Message-ID: <20221120232956.6df4784b@sal.lan> (raw)
In-Reply-To: <f5f0e988-d5a2-12ea-cbb7-caa49f1c00d8@redhat.com>

Em Sun, 20 Nov 2022 23:17:56 +0100
Hans de Goede <hdegoede@redhat.com> escreveu:

> Hi,
> 
> On 11/14/22 21:38, Mauro Carvalho Chehab wrote:
> > Em Thu, 20 Oct 2022 21:55:28 +0200
> > Hans de Goede <hdegoede@redhat.com> escreveu:
> >   
> >> 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.  
> > 
> > Hmm... should we add a patch on Camorama for it to not use libv4l2 if
> > format == V4L2_PIX_FMT_RGB565?  
> 
> This is not controlled by Camorama but by libv4lconvert, AFAICT there are
> no atomisp2 native formats which are supported in Camorama without libv4l  ?

Actually, I added support on Camorama to decode it, plus other formats.
It currently supports:

	V4L2_PIX_FMT_ABGR32
	V4L2_PIX_FMT_ARGB32
	V4L2_PIX_FMT_BGR24
	V4L2_PIX_FMT_BGR32
	V4L2_PIX_FMT_NV12
	V4L2_PIX_FMT_NV21
	V4L2_PIX_FMT_RGB24
	V4L2_PIX_FMT_RGB32
	V4L2_PIX_FMT_RGB565
	V4L2_PIX_FMT_RGB565X
	V4L2_PIX_FMT_UYVY
	V4L2_PIX_FMT_VYUY
	V4L2_PIX_FMT_YUYV
	V4L2_PIX_FMT_YVYU
	V4L2_PIX_FMT_XBGR32
	V4L2_PIX_FMT_XRGB32
	V4L2_PIX_FMT_YUV420
	V4L2_PIX_FMT_YVU420

It is just that it defaults to request RGB24 when not in userptr mode
or when --dont-use-libv4l2 is not used.

I guess it makes sense to make it smarter by using a native format,
using its internal logic if the format is directly supported on it.

It would also make sense to add Bayer and MPEG formats to it
as well.

> >> I've already send out a libv4lconvert fix for this. Also this can be worked
> >> around by passing --dont-use-libv4l2 as argument to camorama.  
> > 
> > Was the patch already applied? The better would be to apply it at
> > libv4l2 upstream, as a fix, porting it to old versions, and mentioning
> > on what versions this got fixed on this changeset.  
> 
> I see that you have already found the patches. I will add a comment
> to the commit msg pointing to the now applied patch.

Ok.

Regards,
Mauro

  reply	other threads:[~2022-11-20 23:30 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-20 19:55 [PATCH v2 00/17] media: atomisp: Convert to videobuf2 Hans de Goede
2022-10-20 19:55 ` [PATCH v2 01/17] media: atomisp: Add hmm_create_from_vmalloc_buf() function Hans de Goede
2022-10-20 19:55 ` [PATCH v2 02/17] media: atomisp: Add ia_css_frame_init_from_info() function Hans de Goede
2022-10-20 19:55 ` [PATCH v2 03/17] media: atomisp: Make atomisp_q_video_buffers_to_css() static Hans de Goede
2022-10-20 19:55 ` [PATCH v2 04/17] media: atomisp: On streamoff wait for buffers owned by the CSS to be given back Hans de Goede
2022-11-14 10:38   ` Andy Shevchenko
2022-11-20 21:55     ` Hans de Goede
2022-10-20 19:55 ` [PATCH v2 05/17] media: atomisp: Remove unused atomisp_buffers_queued[_pipe] functions Hans de Goede
2022-10-20 19:55 ` [PATCH v2 06/17] media: atomisp: Also track buffers in a list when submitted to the ISP Hans de Goede
2022-11-14 11:53   ` Andy Shevchenko
2022-11-20 21:59     ` Hans de Goede
2022-10-20 19:55 ` [PATCH v2 07/17] media: atomisp: Add an index helper variable to atomisp_buf_done() Hans de Goede
2022-10-20 19:55 ` [PATCH v2 08/17] media: atomisp: Use new atomisp_flush_video_pipe() helper in atomisp_streamoff() Hans de Goede
2022-10-20 19:55 ` [PATCH v2 09/17] media: atomisp: Add ia_css_frame_get_info() helper Hans de Goede
2022-11-14 12:00   ` Andy Shevchenko
2022-10-20 19:55 ` [PATCH v2 10/17] media: atomisp: Convert to videobuf2 Hans de Goede
2022-11-14 12:14   ` Andy Shevchenko
2022-11-20 22:11     ` Hans de Goede
2022-10-20 19:55 ` [PATCH v2 11/17] media: atomisp: Make it possible to call atomisp_set_fmt() without a file handle Hans de Goede
2022-10-20 19:55 ` [PATCH v2 12/17] media: atomisp: Fix VIDIOC_REQBUFS failing when VIDIOC_S_FMT has not been called yet Hans de Goede
2022-11-14 12:17   ` Andy Shevchenko
2022-11-20 22:21     ` Hans de Goede
2022-11-14 20:38   ` Mauro Carvalho Chehab
2022-11-20 22:17     ` Hans de Goede
2022-11-20 23:29       ` Mauro Carvalho Chehab [this message]
2022-10-20 19:55 ` [PATCH v2 13/17] media: atomisp: Refactor atomisp_adjust_fmt() Hans de Goede
2022-10-20 19:55 ` [PATCH v2 14/17] media: atomisp: Fix atomisp_try_fmt_cap() always returning YUV420 pixelformat Hans de Goede
2022-10-20 19:55 ` [PATCH v2 15/17] media: atomisp: Make atomisp_g_fmt_cap() default to YUV420 Hans de Goede
2022-11-14 12:18   ` Andy Shevchenko
2022-10-20 19:55 ` [PATCH v2 16/17] media: atomisp: Remove __atomisp_get_pipe() helper Hans de Goede
2022-10-20 19:55 ` [PATCH v2 17/17] media: atomisp: gc0310: Power on sensor from set_fmt() callback Hans de Goede
2022-11-14 12:20   ` Andy Shevchenko
2022-11-20 22:24     ` Hans de Goede
2022-11-14 20:57   ` Mauro Carvalho Chehab
2022-11-14 12:22 ` [PATCH v2 00/17] media: atomisp: Convert to videobuf2 Andy Shevchenko
2022-11-20 22:39   ` Hans de Goede

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221120232956.6df4784b@sal.lan \
    --to=mchehab@kernel.org \
    --cc=andrey.i.trufanov@gmail.com \
    --cc=andy@kernel.org \
    --cc=fabioaiuto83@gmail.com \
    --cc=hdegoede@redhat.com \
    --cc=kitakar@gmail.com \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-staging@lists.linux.dev \
    --cc=nable.maininbox@googlemail.com \
    --cc=sakari.ailus@linux.intel.com \
    --cc=yury.lunev@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.