linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Tomasz Figa <tfiga@chromium.org>
To: Hans Verkuil <hverkuil@xs4all.nl>
Cc: Linux Media Mailing List <linux-media@vger.kernel.org>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	Hirokazu Honda <hiroh@chromium.org>
Subject: Re: [RFP] Which V4L2 ioctls could be replaced by better versions?
Date: Fri, 26 Oct 2018 20:41:26 +0900	[thread overview]
Message-ID: <CAAFQd5COSecRGOSUyQGAe0ob-do0C5=FqhQZoq-d1EULhMiWHg@mail.gmail.com> (raw)
In-Reply-To: <d49940b7-af62-594e-06ad-8ec113589340@xs4all.nl>

On Thu, Sep 20, 2018 at 11:42 PM Hans Verkuil <hverkuil@xs4all.nl> wrote:
>
> Some parts of the V4L2 API are awkward to use and I think it would be
> a good idea to look at possible candidates for that.
>
> Examples are the ioctls that use struct v4l2_buffer: the multiplanar support is
> really horrible, and writing code to support both single and multiplanar is hard.
> We are also running out of fields and the timeval isn't y2038 compliant.
>
> A proof-of-concept is here:
>
> https://git.linuxtv.org/hverkuil/media_tree.git/commit/?h=v4l2-buffer&id=a95549df06d9900f3559afdbb9da06bd4b22d1f3
>
> It's a bit old, but it gives a good impression of what I have in mind.

On a related, but slightly different note, I'm wondering how we should
handle a case where we have an M format (e.g. NV12M with 2 memory
planes), but only 1 DMA-buf with all planes to import. That generally
means that we have to use the same DMA-buf FD with an offset for each
plane. In theory, v4l2_plane::data_offset could be used for this, but
the documentation says that it should be set by the application only
for OUTPUT planes. Moreover, existing drivers tend to just ignore
it...


There is also the opposite problem. Sometimes the application is given
3 different FDs but pointing to the same buffer. If it has to work
with a video device that only supports non-M formats, it can either
fail, making it unusable, or blindly assume that they all point to the
same buffer and just give the first FD to the video device (we do it
in Chromium, since our allocator is guaranteed to keep all planes of
planar formats in one buffer, if to be used with V4L2).

Something that we could do is allowing the QBUF semantics of M formats
for non-M formats, where the application would fill the planes[] array
for all planes with all the FDs it has and the kernel could then
figure out if they point to the same buffer (i.e. resolve to the same
dma_buf struct) or fail if not.

[...]

> Do we have more ioctls that could use a refresh? S/G/TRY_FMT perhaps, again in
> order to improve single vs multiplanar handling.

I'd definitely be more than happy to see the plane handling unified
between non-M and M formats, in general. The list of problems with
current interface:

1) The userspace has to hardcode the computations of bytesperline for
chroma planes of non-M formats (while they are reported for M
formats).

2) Similarly, offsets of the planes in the buffer for non-M formats
must be explicitly calculated in the application,

3) Drivers have to explicitly handle both non-M and M formats or
otherwise they would suffer from issues with application compatibility
or sharing buffers with other devices (one supporting only M and the
other only non-M),

4) Inconsistency in the meaning of planes[0].sizeimage for non-M
formats and M formats, making it impossible to use planes[0].sizeimage
to set the luma plane size in the hardware for non-M formats (since
it's the total size of all planes).

I might have probably forgotten about something, but generally fixing
the 4 above, would be a really big step forward.

Best regards,
Tomasz

  parent reply	other threads:[~2018-10-26 20:18 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-20 14:42 [RFP] Which V4L2 ioctls could be replaced by better versions? Hans Verkuil
2018-09-20 18:14 ` Nicolas Dufresne
2018-10-03  8:24   ` Tomasz Figa
2018-10-16 13:22     ` Hans Verkuil
2018-10-21 16:28     ` Philipp Zabel
2018-10-22  3:37       ` Tomasz Figa
2018-10-22  9:49         ` Dave Stevenson
2018-10-23 13:02         ` Philipp Zabel
2018-10-27  9:38         ` Nicolas Dufresne
2018-11-08  7:45           ` Tomasz Figa
2018-11-09 21:06             ` Nicolas Dufresne
2018-11-11  3:43               ` Tomasz Figa
2018-11-12  9:29                 ` Philipp Zabel
2018-11-12 15:02                   ` Hans Verkuil
2018-10-21 16:17   ` Philipp Zabel
2018-10-17  8:57 ` Laurent Pinchart
2018-10-17  9:16   ` Hans Verkuil
2018-10-17 20:46     ` Laurent Pinchart
2018-10-20 20:28       ` Sakari Ailus
2018-10-26 11:41 ` Tomasz Figa [this message]
2018-10-26 13:42   ` Laurent Pinchart
2019-03-15  4:18     ` Tomasz Figa
2019-03-17 16:10       ` Laurent Pinchart
2019-03-17 17:10         ` Nicolas Dufresne
2019-03-17 17:14         ` Nicolas Dufresne
2019-03-25  9:41         ` Tomasz Figa

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='CAAFQd5COSecRGOSUyQGAe0ob-do0C5=FqhQZoq-d1EULhMiWHg@mail.gmail.com' \
    --to=tfiga@chromium.org \
    --cc=hiroh@chromium.org \
    --cc=hverkuil@xs4all.nl \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-media@vger.kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).