[libcamera-devel] [PATCH v6 1/5] libcamera: v4l2_subdevice: Implement ENUM_FRAME_SIZES
Jacopo Mondi
jacopo at jmondi.org
Fri Mar 1 13:55:10 CET 2019
Hi Laurent,
On Fri, Mar 01, 2019 at 02:42:40PM +0200, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> On Fri, Mar 01, 2019 at 12:51:35PM +0100, Jacopo Mondi wrote:
> > Implement format and size enumeration methods to list all the available
> > subdevice image resolutions and formats.
> >
> > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > ---
> > src/libcamera/geometry.cpp | 34 +++++++++++
> > src/libcamera/include/geometry.h | 12 ++++
> > src/libcamera/include/v4l2_subdevice.h | 11 +++-
> > src/libcamera/v4l2_subdevice.cpp | 84 ++++++++++++++++++++++++++
> > 4 files changed, 139 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/libcamera/geometry.cpp b/src/libcamera/geometry.cpp
> > index 57f4fc7716d9..b6b6592bdfec 100644
> > --- a/src/libcamera/geometry.cpp
> > +++ b/src/libcamera/geometry.cpp
> > @@ -46,4 +46,38 @@ namespace libcamera {
> > * \brief The distance between the top and bottom sides
> > */
> >
> > +/**
> > + * \struct SizeRange
> > + * \brief Describe a range of image sizes
> > + *
> > + * SizeRange describes a range of image sizes included in the (minWidth,
> > + * minHeight) - (maxWidth, maxHeight) interval. If the minimum and
> > + * maximum sizes are identical it represents a single image resolution.
> > + */
> > +
> > +/**
> > + * \fn SizeRange::SizeRange()
> > + * \brief Construct a size range
> > + */
> > +
> > +/**
> > + * \var SizeRange::minWidth
> > + * \brief The minimum image width
> > + */
> > +
> > +/**
> > + * \var SizeRange::minHeight
> > + * \brief The minimum image height
> > + */
> > +
> > +/**
> > + * \var SizeRange::maxWidth
> > + * \brief The maximum image width
> > + */
> > +
> > +/**
> > + * \var SizeRange::maxHeight
> > + * \brief The maximum image height
> > + */
> > +
> > } /* namespace libcamera */
> > diff --git a/src/libcamera/include/geometry.h b/src/libcamera/include/geometry.h
> > index cc146da7cb0d..eadc4ed4f9cb 100644
> > --- a/src/libcamera/include/geometry.h
> > +++ b/src/libcamera/include/geometry.h
> > @@ -17,6 +17,18 @@ struct Rectangle {
> > unsigned int h;
> > };
> >
> > +struct SizeRange {
> > + SizeRange(unsigned int minW, unsigned int minH,
> > + unsigned int maxW, unsigned int maxH)
> > + : minWidth(minW), minHeight(minH), maxWidth(maxW),
> > + maxHeight(maxH) {}
> > +
> > + unsigned int minWidth;
> > + unsigned int minHeight;
> > + unsigned int maxWidth;
> > + unsigned int maxHeight;
> > +};
> > +
> > } /* namespace libcamera */
> >
> > #endif /* __LIBCAMERA_GEOMETRY_H__ */
> > diff --git a/src/libcamera/include/v4l2_subdevice.h b/src/libcamera/include/v4l2_subdevice.h
> > index 669e79f9a9fd..aa7451e86962 100644
> > --- a/src/libcamera/include/v4l2_subdevice.h
> > +++ b/src/libcamera/include/v4l2_subdevice.h
> > @@ -7,15 +7,16 @@
> > #ifndef __LIBCAMERA_V4L2_SUBDEVICE_H__
> > #define __LIBCAMERA_V4L2_SUBDEVICE_H__
> >
> > +#include <map>
> > #include <string>
> > +#include <vector>
> >
> > +#include "geometry.h"
> > #include "log.h"
> > #include "media_object.h"
> >
> > namespace libcamera {
> >
> > -struct Rectangle;
> > -
> > struct V4L2SubdeviceFormat {
> > uint32_t mbus_code;
> > uint32_t width;
> > @@ -39,6 +40,9 @@ public:
> > int setCrop(unsigned int pad, Rectangle *rect);
> > int setCompose(unsigned int pad, Rectangle *rect);
> >
> > + const std::map<unsigned int, std::vector<SizeRange>>
> > + formats(unsigned int pad);
> > +
> > int getFormat(unsigned int pad, V4L2SubdeviceFormat *format);
> > int setFormat(unsigned int pad, V4L2SubdeviceFormat *format);
> >
> > @@ -46,6 +50,9 @@ protected:
> > std::string logPrefix() const;
> >
> > private:
> > + std::vector<SizeRange> enumPadSizes(unsigned int pad,
> > + unsigned int code);
> > +
> > int setSelection(unsigned int pad, unsigned int target,
> > Rectangle *rect);
> >
> > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
> > index 87377762664d..07035886c766 100644
> > --- a/src/libcamera/v4l2_subdevice.cpp
> > +++ b/src/libcamera/v4l2_subdevice.cpp
> > @@ -183,6 +183,57 @@ int V4L2Subdevice::setCompose(unsigned int pad, Rectangle *rect)
> > return setSelection(pad, V4L2_SEL_TGT_COMPOSE, rect);
> > }
> >
> > +/**
> > + * \brief List the sub-device image resolutions and formats on \a pad
> > + * \param[in] pad The 0-indexed pad number to enumerate formats on
> > + *
> > + * Retrieve a list of image formats and sizes on the \a pad of a video
> > + * subdevice. Subdevices can report either a list of discrete sizes they
> > + * support or a list of intervals expressed as a [min-max] sizes range.
> > + *
> > + * Each image size list is associated with a media bus pixel code for which
> > + * the reported resolutions are supported.
> > + *
> > + * \return A map of image formats associated with a list of image sizes, or
> > + * an empty map on error or if the pad does not exist
> > + */
> > +const std::map<unsigned int, std::vector<SizeRange>>
> > +V4L2Subdevice::formats(unsigned int pad)
> > +{
> > + std::map<unsigned int, std::vector<SizeRange>> formatMap = {};
> > + struct v4l2_subdev_mbus_code_enum mbusEnum = {};
> > + int ret;
> > +
> > + if (pad >= entity_->pads().size()) {
> > + LOG(V4L2Subdev, Error)
> > + << "Invalid pad: " << pad;
>
> This would hold on a single line.
>
> > + return formatMap;
> > + }
> > +
> > + mbusEnum.pad = pad;
> > + mbusEnum.index = 0;
> > + mbusEnum.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> > + while (true) {
> > + ret = ioctl(fd_, VIDIOC_SUBDEV_ENUM_MBUS_CODE, &mbusEnum);
> > + if (ret)
> > + break;
> > +
> > + formatMap[mbusEnum.code] = enumPadSizes(pad, mbusEnum.code);
>
> This won't make it possible to pass an error from
> VIDIOC_SUBDEV_ENUM_FRAME_SIZE to the caller. How about making
> enumPadSizes() return an int and take a pointer to the sizes vector ?
I really struggled here, I wanted to keep the error visible, but also
exploit RVO, which I presume is here used.
On error, we would return an empty size list anyone, is it worth
returning an int?
>
> With those changes,
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>
> > +
> > + mbusEnum.index++;
> > + }
> > +
> > + if (ret && (errno != EINVAL && errno != ENOTTY)) {
> > + ret = -errno;
> > + LOG(V4L2Subdev, Error)
> > + << "Unable to enumerate formats on pad " << pad
> > + << ": " << strerror(-ret);
> > + formatMap.clear();
> > + }
> > +
> > + return formatMap;
> > +}
> > +
> > /**
> > * \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
> > @@ -248,6 +299,39 @@ int V4L2Subdevice::setFormat(unsigned int pad, V4L2SubdeviceFormat *format)
> > return 0;
> > }
> >
> > +std::vector<SizeRange> V4L2Subdevice::enumPadSizes(unsigned int pad,
> > + unsigned int code)
> > +{
> > + struct v4l2_subdev_frame_size_enum sizeEnum = {};
> > + std::vector<SizeRange> sizes = {};
> > + int ret;
> > +
> > + sizeEnum.index = 0;
> > + sizeEnum.pad = pad;
> > + sizeEnum.code = code;
> > + sizeEnum.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> > + while (true) {
> > + ret = ioctl(fd_, VIDIOC_SUBDEV_ENUM_FRAME_SIZE, &sizeEnum);
> > + if (ret)
> > + break;
> > +
> > + sizes.emplace_back(sizeEnum.min_width, sizeEnum.min_height,
> > + sizeEnum.max_width, sizeEnum.max_height);
> > +
> > + sizeEnum.index++;
> > + }
> > +
> > + if (ret && (errno != EINVAL && errno != ENOTTY)) {
> > + ret = -errno;
> > + LOG(V4L2Subdev, Error)
> > + << "Unable to enumerate sizes on pad " << pad
> > + << ": " << strerror(-ret);
> > + sizes.clear();
> > + }
> > +
> > + return sizes;
> > +}
> > +
> > int V4L2Subdevice::setSelection(unsigned int pad, unsigned int target,
> > Rectangle *rect)
> > {
>
> --
> Regards,
>
> Laurent Pinchart
-------------- 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/20190301/371562e2/attachment.sig>
More information about the libcamera-devel
mailing list