[libcamera-devel] [PATCH v3 3/8] libcamera: v4l2_device: Add support for META_CAPTURE devices
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Fri Mar 1 10:53:07 CET 2019
Hi Jacopo,
On Fri, Mar 01, 2019 at 10:40:46AM +0100, Jacopo Mondi wrote:
> On Thu, Feb 28, 2019 at 11:45:37PM +0200, Laurent Pinchart wrote:
> > On Thu, Feb 28, 2019 at 08:15:57PM +0100, Jacopo Mondi wrote:
> >> On Thu, Feb 28, 2019 at 07:25:49PM +0200, Laurent Pinchart wrote:
> >>> On Thu, Feb 28, 2019 at 09:49:53AM +0100, Jacopo Mondi wrote:
> >>>> On Wed, Feb 27, 2019 at 07:54:18PM +0200, Laurent Pinchart wrote:
> >>>>> On Tue, Feb 26, 2019 at 05:26:36PM +0100, Jacopo Mondi wrote:
> >>>>>> Add support for devices that provide video meta-data to v4l2_device.cpp
> >>>>>> and re-arrange bufferType handling in open() method.
> >>>>>>
> >>>>>> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> >>>>>> ---
> >>>>>> src/libcamera/include/v4l2_device.h | 4 +++
> >>>>>> src/libcamera/v4l2_device.cpp | 38 +++++++++++++++++------------
> >>>>>> 2 files changed, 27 insertions(+), 15 deletions(-)
> >>>>>>
> >>>>>> diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h
> >>>>>> index 1d31d1b403bc..52eb6785cc15 100644
> >>>>>> --- a/src/libcamera/include/v4l2_device.h
> >>>>>> +++ b/src/libcamera/include/v4l2_device.h
> >>>>>> @@ -53,6 +53,10 @@ struct V4L2Capability final : v4l2_capability {
> >>>>>> return device_caps() & (V4L2_CAP_VIDEO_CAPTURE |
> >>>>>> V4L2_CAP_VIDEO_CAPTURE_MPLANE);
> >>
> >> [snip]
> >>
> >>>> As done in v4, I introduced an isMetaCapture() method, preparing for
> >>>> an isMetaOutput() one. In this way isCapture() and isOutput() would
> >>>> refer to video streams only and the bufferType intialization looks
> >>>> like:
> >>>
> >>> That's confusing. Let's have
> >>>
> >>> isCapture() -> V4L2_CAP_META_CAPTURE | V4L2_CAP_VIDEO_CAPTURE |
> >>> V4L2_CAP_VIDEO_CAPTURE_MPLANE
> >>> isOutput() -> V4L2_CAP_META_OUTPUT | V4L2_CAP_VIDEO_OUTPUT |
> >>> V4L2_CAP_VIDEO_OUTPUT_MPLANE
> >>> isMeta() -> V4L2_CAP_META_CAPTURE | V4L2_CAP_META_OUTPUT
> >>> isVideo() -> V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_VIDEO_CAPTURE_MPLANE |
> >>> V4L2_CAP_VIDEO_OUTPUT | V4L2_CAP_VIDEO_OUTPUT_MPLANE
> >>>
> >>> and you can then also add, if desired,
> >>>
> >>> isMetaCapture() -> isMeta() && isCapture()
> >>> ...
> >>>
> >>
> >> I think the code below here, which is the only place where those
> >> functions are used would become less nice if this case.
> >>
> >> Those functions are only used to assign bufferType_ and fdEvent_,
> >> let's keep it straightforward as the below series of if/else
> >
> > The code below is fine, my point is that isCapture() should not mean
> > isVideoCapture().
> >
> > bool isCapture() const
> > {
> > return device_caps() & (V4L2_CAP_META_CAPTURE |
> > V4L2_CAP_VIDEO_CAPTURE |
> > V4L2_CAP_VIDEO_CAPTURE_MPLANE);
> > }
> >
> > bool isOutput() const
> > {
> > return device_caps() & (V4L2_CAP_META_OUTPUT |
> > V4L2_CAP_VIDEO_OUTPUT |
> > V4L2_CAP_VIDEO_OUTPUT_MPLANE);
> > }
> >
> > bool isMeta() const
> > {
> > return device_caps() & (V4L2_CAP_META_CAPTURE |
> > V4L2_CAP_META_OUTPUT);
> > }
> >
> > bool isVideo() const
> > {
> > return device_caps() & (V4L2_CAP_VIDEO_CAPTURE |
> > V4L2_CAP_VIDEO_CAPTURE_MPLANE |
> > V4L2_CAP_VIDEO_OUTPUT |
> > V4L2_CAP_VIDEO_OUTPUT_MPLANE);
> > }
> >
> > bool isMetaCapture() const
> > {
> > return isMeta() && isCapture();
> > }
> >
> > bool isMetaOutput() const
> > {
> > return isMeta() && isOutput();
> > }
> >
> > bool isVideoCapture() const
> > {
> > return isVideo() && isCapture();
> > }
> >
> > bool isVideoOutput() const
> > {
> > return isVideo() && isOutput();
> > }
> >
> > and then use those four last functions below.
>
> I see. I don't feel like adding 4 functions and one level of
> indirection is super useful considering this is
> used in a single place, but if that would help moving forward I'll
> change this.
You can get rid of the level of indirection if you prefer (although the
compiler should really optimize this), but what I really want is
s/isCapture/isVideoCapture/ and s/isOutput/isVideoOutput/.
> >>>>
> >>>> * Set buffer type and wait for read notifications on CAPTURE devices
> >>>> * (POLLIN), and write notifications for OUTPUT devices (POLLOUT).
> >>>> */
> >>>> if (caps_.isCapture()) {
> >>>> fdEvent_ = new EventNotifier(fd_, EventNotifier::Read);
> >>>> bufferType_ = caps_.isMultiplanar()
> >>>> ? V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE
> >>>> : V4L2_BUF_TYPE_VIDEO_CAPTURE;
> >>>> } else if (caps_.isOutput()) {
> >>>> fdEvent_ = new EventNotifier(fd_, EventNotifier::Write);
> >>>> bufferType_ = caps_.isMultiplanar()
> >>>> ? V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE
> >>>> : V4L2_BUF_TYPE_VIDEO_OUTPUT;
> >>>> } else if (caps_.isMetaCapture()) {
> >>>> fdEvent_ = new EventNotifier(fd_, EventNotifier::Read);
> >>>> bufferType_ = V4L2_BUF_TYPE_META_CAPTURE;
> >>>> } else if (caps_.isMetaOutput()) {
> >>>> fdEvent_ = new EventNotifier(fd_, EventNotifier::Write);
> >>>> bufferType_ = V4L2_BUF_TYPE_META_OUTPUT;
> >>>> } else {
> >>>> LOG(V4L2, Debug) << "Device is not a supported type";
> >>>> return -EINVAL;
> >>>> }
> >>>>
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list