[libcamera-devel] [PATCH v3 2/8] libcamera: v4l2_subdevice: Implement ENUM_FRAME_SIZES
Jacopo Mondi
jacopo at jmondi.org
Wed Feb 27 09:18:04 CET 2019
Hi Kieran,
On Tue, Feb 26, 2019 at 11:19:40PM +0000, Kieran Bingham wrote:
> Hi Jacopo,
>
> On 26/02/2019 16:26, 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 | 9 ++
> > src/libcamera/v4l2_subdevice.cpp | 118 +++++++++++++++++++++++++
> > 2 files changed, 127 insertions(+)
> >
> > diff --git a/src/libcamera/include/v4l2_subdevice.h b/src/libcamera/include/v4l2_subdevice.h
> > index eac699a06109..6b21308d2087 100644
> > --- a/src/libcamera/include/v4l2_subdevice.h
> > +++ b/src/libcamera/include/v4l2_subdevice.h
> > @@ -7,7 +7,9 @@
> > #ifndef __LIBCAMERA_V4L2_SUBDEVICE_H__
> > #define __LIBCAMERA_V4L2_SUBDEVICE_H__
> >
> > +#include <map>
> > #include <string>
> > +#include <vector>
> >
> > #include "media_object.h"
> >
> > @@ -38,15 +40,22 @@ public:
> > int setCrop(unsigned int pad, Rectangle *rect);
> > int setCompose(unsigned int pad, Rectangle *rect);
> >
> > + std::vector<V4L2SubdeviceFormat> &formats(unsigned int pad);
> > int getFormat(unsigned int pad, V4L2SubdeviceFormat *format);
> > int setFormat(unsigned int pad, V4L2SubdeviceFormat *format);
> >
> > private:
> > + int listPadSizes(unsigned int pad, unsigned int mbus_code,
> > + std::vector<V4L2SubdeviceFormat> *formats);
> > + std::vector<V4L2SubdeviceFormat> listPadFormats(unsigned int pad);
>
>
> The word 'list' seems a bit redundant in 'listPadSizes()' and
> 'listPadFormats()' ... and seems a bit hungarian notation to me?
>
I interpreted "list" as a verb, so the function name reads like "list
all formats on a pad". Is it weird to you?
> Other than that, I trust that the tests that follow make sure the code
> is doing something sane :)
>
>
> I can't see anything jump out at me yet.
>
>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
Thanks
j
>
>
>
> > +
> > int setSelection(unsigned int pad, unsigned int target,
> > Rectangle *rect);
> >
> > const MediaEntity *entity_;
> > int fd_;
> > +
> > + std::map<unsigned int, std::vector<V4L2SubdeviceFormat>> formats_;
> > };
> >
> > } /* namespace libcamera */
> > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
> > index 56ecf3851cb0..f81a521f9e2a 100644
> > --- a/src/libcamera/v4l2_subdevice.cpp
> > +++ b/src/libcamera/v4l2_subdevice.cpp
> > @@ -5,6 +5,10 @@
> > * v4l2_subdevice.cpp - V4L2 Subdevice
> > */
> >
> > +#include <map>
> > +#include <string>
> > +#include <vector>
> > +
> > #include <fcntl.h>
> > #include <string.h>
> > #include <sys/ioctl.h>
> > @@ -116,6 +120,9 @@ int V4L2Subdevice::open()
> > }
> > fd_ = ret;
> >
> > + for (MediaPad *pad : entity_->pads())
> > + formats_[pad->index()] = listPadFormats(pad->index());
> > +
> > return 0;
> > }
> >
> > @@ -178,6 +185,25 @@ 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
> > + *
> > + * \return A vector of image formats, or an empty vector if the pad does not
> > + * exist
> > + */
> > +std::vector<V4L2SubdeviceFormat> &V4L2Subdevice::formats(unsigned int pad)
> > +{
> > + /*
> > + * If pad does not exist, return an empty vector at position
> > + * pads().size()
> > + */
> > + if (pad > entity_->pads().size())
> > + pad = entity_->pads().size();
> > +
> > + return formats_[pad];
> > +}
> > +
> > /**
> > * \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
> > @@ -242,6 +268,98 @@ int V4L2Subdevice::setFormat(unsigned int pad, V4L2SubdeviceFormat *format)
> > return 0;
> > }
> >
> > +int V4L2Subdevice::listPadSizes(unsigned int pad, unsigned int mbus_code,
> > + std::vector<V4L2SubdeviceFormat> *formats)
> > +{
> > + struct v4l2_subdev_frame_size_enum sizeEnum = {};
> > + int ret;
> > +
> > + sizeEnum.index = 0;
> > + sizeEnum.pad = pad;
> > + sizeEnum.code = mbus_code;
> > + sizeEnum.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> > +
> > + while (!(ret = ioctl(fd_, VIDIOC_SUBDEV_ENUM_FRAME_SIZE, &sizeEnum))) {
> > + V4L2SubdeviceFormat minFormat = {
> > + .mbus_code = mbus_code,
> > + .width = sizeEnum.min_width,
> > + .height = sizeEnum.min_height,
> > + };
> > + formats->push_back(minFormat);
> > +
> > + /*
> > + * Most subdevices report discrete frame resolutions, where
> > + * min and max sizes are identical. For continue frame
> > + * resolutions, store the min and max sizes interval.
> > + */
> > + if (sizeEnum.min_width == sizeEnum.max_width &&
> > + sizeEnum.min_height == sizeEnum.max_height) {
> > + sizeEnum.index++;
> > + continue;
> > + }
> > +
> > + V4L2SubdeviceFormat maxFormat = {
> > + .mbus_code = mbus_code,
> > + .width = sizeEnum.max_width,
> > + .height = sizeEnum.max_height,
> > + };
> > + formats->push_back(maxFormat);
> > +
> > + sizeEnum.index++;
> > + }
> > +
> > + if (ret && (errno != EINVAL && errno != ENOTTY)) {
> > + LOG(V4L2Subdev, Error)
> > + << "Unable to enumerate format on pad " << pad
> > + << " of " << deviceName() << ": "
> > + << strerror(errno);
> > + return ret;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +std::vector<V4L2SubdeviceFormat> V4L2Subdevice::listPadFormats(unsigned int pad)
> > +{
> > + struct v4l2_subdev_mbus_code_enum mbusEnum = {};
> > + std::vector<V4L2SubdeviceFormat> formats = {};
> > + int ret;
> > +
> > + mbusEnum.pad = pad;
> > + mbusEnum.index = 0;
> > + mbusEnum.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> > +
> > + while (!(ret = ioctl(fd_, VIDIOC_SUBDEV_ENUM_MBUS_CODE, &mbusEnum))) {
> > + ret = listPadSizes(pad, mbusEnum.code, &formats);
> > + if (ret)
> > + return formats;
> > +
> > + mbusEnum.index++;
> > + }
> > +
> > + if (ret && (errno != EINVAL && errno != ENOTTY)) {
> > + LOG(V4L2Subdev, Error)
> > + << "Unable to enumerate format on pad " << pad
> > + << " of " << deviceName() << ": " << strerror(-ret);
> > + return formats;
> > + }
> > +
> > + if (mbusEnum.index == 0 && ret && errno == EINVAL) {
> > + /*
> > + * The subdevice might not support ENUM_MBUS_CODE but
> > + * might support ENUM_FRAME_SIZES.
> > + */
> > + struct V4L2SubdeviceFormat subdevFormat;
> > + ret = getFormat(pad, &subdevFormat);
> > + if (ret)
> > + return formats;
> > +
> > + listPadSizes(pad, subdevFormat.mbus_code, &formats);
> > + }
> > +
> > + return formats;
> > +}
> > +
> > int V4L2Subdevice::setSelection(unsigned int pad, unsigned int target,
> > Rectangle *rect)
> > {
> >
>
> --
> Regards
> --
> Kieran
-------------- 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/20190227/616d1847/attachment.sig>
More information about the libcamera-devel
mailing list