[libcamera-devel] [PATCH 1/3] libcamera: v4l2_subdevice: Implement ENUM_FRAME_SIZES
Jacopo Mondi
jacopo at jmondi.org
Mon Feb 25 10:44:35 CET 2019
On Mon, Feb 25, 2019 at 10:28:33AM +0100, Jacopo Mondi wrote:
> Hi Niklas,
>
> On Thu, Feb 21, 2019 at 04:31:56PM +0100, Niklas Söderlund wrote:
> > Hi Jacopo,
> >
> > Thanks for your patch.
> >
> > On 2019-02-19 17:56:18 +0100, Jacopo Mondi wrote:
> > > Implement enumFormat() methods to enumerate the available image
> > > resolutions on the subdevice.
> > >
> > > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > > ---
> > > src/libcamera/include/v4l2_subdevice.h | 11 ++++
> > > src/libcamera/v4l2_subdevice.cpp | 90 ++++++++++++++++++++++++++
> > > 2 files changed, 101 insertions(+)
> > >
> > > diff --git a/src/libcamera/include/v4l2_subdevice.h b/src/libcamera/include/v4l2_subdevice.h
> > > index 82fa6685ab52..c7045776555c 100644
> > > --- a/src/libcamera/include/v4l2_subdevice.h
> > > +++ b/src/libcamera/include/v4l2_subdevice.h
> > > @@ -14,6 +14,16 @@ namespace libcamera {
> > > class MediaEntity;
> > > struct Rectangle;
> > >
> > > +struct V4L2SubdeviceFormatEnum {
> > > + unsigned int index;
> > > + uint32_t mbus_code;
> > > +
> > > + uint32_t minWidth;
> > > + uint32_t maxWidth;
> > > + uint32_t minHeight;
> > > + uint32_t maxHeight;
> > > +};
> > > +
> > > struct V4L2SubdeviceFormat {
> > > uint32_t mbus_code;
> > > uint32_t width;
> > > @@ -36,6 +46,7 @@ public:
> > > int setCrop(unsigned int pad, Rectangle *rect);
> > > int setCompose(unsigned int pad, Rectangle *rect);
> > >
> > > + int enumFormat(unsigned int pad, V4L2SubdeviceFormatEnum *formatEnum);
> > > int getFormat(unsigned int pad, V4L2SubdeviceFormat *format);
> > > int setFormat(unsigned int pad, V4L2SubdeviceFormat *format);
> > >
> > > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
> > > index b436f73cc75f..5665154a2762 100644
> > > --- a/src/libcamera/v4l2_subdevice.cpp
> > > +++ b/src/libcamera/v4l2_subdevice.cpp
> > > @@ -26,6 +26,52 @@ namespace libcamera {
> > >
> > > LOG_DEFINE_CATEGORY(V4L2Subdev)
> > >
> > > +/**
> > > + * \struct V4L2SubdeviceFormatEnum
> > > + * \brief The V4L2 sub-device image size enumeration
> > > + *
> > > + * This structure describes an image resolution as enumerated by the V4L2
> > > + * sub-device. The structure is used as format exchange between the caller and
> > > + * the enumFormat() method. The caller is responsible to fill the media bus
> > > + * code to use and the index of the format to be enumerated.
> > > + */
> > > +
> > > +/**
> > > + * \var V4L2SubdeviceFormatEnum::index
> > > + * \brief The index of the format to be enumerated
> > > + */
> > > +
> > > +/**
> > > + * \var V4L2SubdeviceFormatEnum::mbus_code
> > > + * \brief The pixel format identification code for the formats to enumerate
> > > + *
> > > + * \sa V4L2SubdeviceFormat for media bus format pixel code description.
> > > + */
> > > +
> > > +/**
> > > + * \var V4L2SubdeviceFormatEnum::minWidth
> > > + * \brief The minimum width of the enumerated format as reported by the V4L2
> > > + * sub-device
> > > + */
> > > +
> > > +/**
> > > + * \var V4L2SubdeviceFormatEnum::maxWidth
> > > + * \brief The maximum width of the enumerated format as reported by the V4L2
> > > + * sub-device
> > > + */
> > > +
> > > +/**
> > > + * \var V4L2SubdeviceFormatEnum::minHeight
> > > + * \brief The minimum height of the enumerated format as reported by the V4L2
> > > + * sub-device
> > > + */
> > > +
> > > +/**
> > > + * \var V4L2SubdeviceFormatEnum::maxHeight
> > > + * \brief The maximum height of the enumerated format as reported by the V4L2
> > > + * sub-device
> > > + */
> > > +
> > > /**
> > > * \struct V4L2SubdeviceFormat
> > > * \brief The V4L2 sub-device image format and sizes
> > > @@ -171,6 +217,50 @@ int V4L2Subdevice::setCompose(unsigned int pad, Rectangle *rect)
> > > return setSelection(pad, V4L2_SEL_TGT_COMPOSE, rect);
> > > }
> > >
> > > +/**
> > > + * \brief Enumerate the sub-device image resolutions
> > > + * \param[in] pad The 0-indexed pad number to enumerate formats on
> > > + * \param[inout] formatEnum The format enumeration description
> > > + *
> > > + * The method retrieve the image resolution of the image format with index
> > > + * formatEnum.index for the pixel format specified by formatEnum.mbus_code.
> > > + *
> > > + * To enumerate image resolutions, the caller shall start enumerating all
> > > + * formats by setting the formatEnum.index field to 0, and increasing it by
> > > + * one for any successive call, until the -EINVAL return code is returned.
> >
> > Should we not help pipeline handlers by hiding this? And instead return
> > a vector of V4L2SubdeviceFormatEnum with all formats?
> >
>
> Yes, you and Laurent had the same suggestion and is surely better.
> I'll re-implement this.
>
> > > + *
> > > + * \return 0 on success, or a negative error code otherwise
> > > + */
> > > +int V4L2Subdevice::enumFormat(unsigned int pad,
> > > + V4L2SubdeviceFormatEnum *formatEnum)
> > > +{
> > > + struct v4l2_subdev_frame_size_enum sizeEnum = {};
> > > +
> > > + sizeEnum.index = formatEnum->index;
> > > + sizeEnum.code = formatEnum->mbus_code;
> > > + sizeEnum.pad = pad;
> >
> > This is confusing, index and code comes from formatEnum while pad is a
> > separate argument.
> >
>
> All V4L2Subdevice methods accepts a pad as first argument. I would
> keep this consistent.
>
> > > + sizeEnum.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> >
> > I think this should be configurable by the caller or other way
> > customisable. In the not so distant future we will need to add support
> > for V4L2_SUBDEV_FORMAT_TRY in a lot of places better to prepare for it
> > :-)
>
> I'm really not sure about this one, I could add a field to
> V4L2SubdeviceFormatEnum for this...
>
Re-thinking about this, I would leave this out until we don't define a
way to handle FORMAT_TRY globally, as otherwise I would have to expose
that flag outside of V4L2Subdevice which I would prefer not to...
> >
> > > +
> > > + int ret = ioctl(fd_, VIDIOC_SUBDEV_ENUM_FRAME_SIZE, &sizeEnum);
> > > + if (ret) {
> > > + ret = -errno;
> > > + if (ret == -EINVAL)
> > > + return ret;
> > > +
> > > + LOG(V4L2Subdev, Error)
> > > + << "Unable to enumerate format on pad " << pad
> > > + << " of " << deviceNode_ << ": " << strerror(-ret);
> > > + return ret;
> > > + }
> > > +
> > > + formatEnum->maxWidth = sizeEnum.max_width;
> > > + formatEnum->minWidth = sizeEnum.min_width;
> > > + formatEnum->maxHeight = sizeEnum.max_height;
> > > + formatEnum->minHeight = sizeEnum.min_height;
> > > +
> > > + return 0;
> > > +}
> > > +
> > > /**
> > > * \brief Retrieve the image format set on one of the V4L2 subdevice pads
> > > * \param[in] pad The 0-indexed pad number the format is to be retrieved from
> > > --
> > > 2.20.1
> > >
> > > _______________________________________________
> > > libcamera-devel mailing list
> > > libcamera-devel at lists.libcamera.org
> > > https://lists.libcamera.org/listinfo/libcamera-devel
> >
> > --
> > Regards,
> > Niklas Söderlund
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20190225/553a7067/attachment.sig>
More information about the libcamera-devel
mailing list