| Message ID | 20190301115139.11060-2-jacopo@jmondi.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
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@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 ? With those changes, Reviewed-by: Laurent Pinchart <laurent.pinchart@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) > {
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@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@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
Hi Jacopo, On Fri, Mar 01, 2019 at 01:55:10PM +0100, Jacopo Mondi wrote: > On Fri, Mar 01, 2019 at 02:42:40PM +0200, Laurent Pinchart wrote: > > 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@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? An empty map on error is fine in my opinion, but a populated map with a few empty vectors would be very confusing. If you pass the vector to enumPadSizes() you will be able to propagate the error and clear the map, while keeping the implementation efficient as you will populate the vector in-place. > > With those changes, > > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@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) > >> {
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; + 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); + + 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) {
Implement format and size enumeration methods to list all the available subdevice image resolutions and formats. Signed-off-by: Jacopo Mondi <jacopo@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(-)