| Message ID | 20190527001543.13593-10-niklas.soderlund@ragnatech.se |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Niklas, On 27/05/2019 01:15, Niklas Söderlund wrote: > Add methods to enumerate pixelformats and frame sizes from a v4l2 > device. The interface is similar to how V4L2Subdevice enumerates mbus > codes and frame sizes. > Could we add a test or two to test/v4l2_device/formats.cpp for this public API addition? > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > src/libcamera/include/v4l2_device.h | 5 ++ > src/libcamera/v4l2_device.cpp | 104 ++++++++++++++++++++++++++++ > 2 files changed, 109 insertions(+) > > diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h > index 2e7bd1e7f4cce276..db2d12757c6f564a 100644 > --- a/src/libcamera/include/v4l2_device.h > +++ b/src/libcamera/include/v4l2_device.h > @@ -16,6 +16,7 @@ > #include <libcamera/geometry.h> > #include <libcamera/signal.h> > > +#include "formats.h" > #include "log.h" > > namespace libcamera { > @@ -126,6 +127,7 @@ public: > > int getFormat(V4L2DeviceFormat *format); > int setFormat(V4L2DeviceFormat *format); > + V4L2DeviceFormats formats(); > > int exportBuffers(BufferPool *pool); > int importBuffers(BufferPool *pool); > @@ -154,6 +156,9 @@ private: > int createPlane(Buffer *buffer, unsigned int plane, > unsigned int length); > > + std::vector<unsigned int> enumPixelformats(); > + std::vector<SizeRange> enumSizes(unsigned int pixelformat); > + > Buffer *dequeueBuffer(); > void bufferAvailable(EventNotifier *notifier); > > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp > index 8366ffc4db559520..a9667092a20505d9 100644 > --- a/src/libcamera/v4l2_device.cpp > +++ b/src/libcamera/v4l2_device.cpp > @@ -564,6 +564,23 @@ int V4L2Device::setFormatMultiplane(V4L2DeviceFormat *format) > return 0; > } > > +/** > + * \brief Enumerate all pixelformats and frame sizes > + * > + * Enumerate all pixelformats and frame sizes reported by the video device. > + * > + * \return All pixelformats and frame sizes How about: \return A list of the supported device formats <except s/list/some-other-type-description/ > > + */ > +V4L2DeviceFormats V4L2Device::formats() Eeep, we have a V4L2DeviceFormat class and a V4L2DeviceFormats class which do (somewhat) different things ... I hope that won't be too confusing... <I've gone back and noted this in reply to the patch that adds the types.> > +{ > + std::map<unsigned int, std::vector<SizeRange>> formatMap = {}; > + > + for (unsigned int pixelformat : enumPixelformats()) > + formatMap[pixelformat] = enumSizes(pixelformat); > + > + return V4L2DeviceFormats(formatMap); > +} > + > int V4L2Device::requestBuffers(unsigned int count) > { > struct v4l2_requestbuffers rb = {}; > @@ -692,6 +709,93 @@ int V4L2Device::createPlane(Buffer *buffer, unsigned int planeIndex, > > return 0; > } newline required here. > +std::vector<unsigned int> V4L2Device::enumPixelformats() > +{ > + std::vector<unsigned int> pixelformats; > + int ret; > + > + for (unsigned int index = 0;; index++) { > + struct v4l2_fmtdesc pixelformatEnum = { > + .index = index, > + .type = bufferType_, > + }; > + > + ret = ioctl(fd_, VIDIOC_ENUM_FMT, &pixelformatEnum); > + if (ret) > + break; > + > + pixelformats.push_back(pixelformatEnum.pixelformat); > + } > + > + if (ret && errno != EINVAL) { > + ret = errno; > + LOG(V4L2, Error) > + << "Unable to enumerate pixelformats: " > + << strerror(ret); > + return {}; > + } > + > + return pixelformats; > +} > + > +std::vector<SizeRange> V4L2Device::enumSizes(unsigned int pixelformat) > +{ > + std::vector<SizeRange> sizes; > + int ret; > + > + for (unsigned int index = 0;; index++) { > + struct v4l2_frmsizeenum frameSize = { > + .index = index, > + .pixel_format = pixelformat, > + }; > + > + ret = ioctl(fd_, VIDIOC_ENUM_FRAMESIZES, &frameSize); > + if (ret) > + break; > + > + if (index != 0 && > + frameSize.type != V4L2_FRMSIZE_TYPE_DISCRETE) { > + LOG(V4L2, Error) > + << "None 0 index for none discrete type"; Non-zero index for non discrete type > + return {}; > + } > + > + switch (frameSize.type) { > + case V4L2_FRMSIZE_TYPE_DISCRETE: > + sizes.emplace_back(frameSize.discrete.width, > + frameSize.discrete.height); > + break; > + case V4L2_FRMSIZE_TYPE_CONTINUOUS: > + sizes.emplace_back(frameSize.stepwise.min_width, > + frameSize.stepwise.min_height, > + frameSize.stepwise.max_width, > + frameSize.stepwise.max_height); > + break; > + case V4L2_FRMSIZE_TYPE_STEPWISE: > + sizes.emplace_back(frameSize.stepwise.min_width, > + frameSize.stepwise.min_height, > + frameSize.stepwise.max_width, > + frameSize.stepwise.max_height, > + frameSize.stepwise.step_width, > + frameSize.stepwise.step_height); > + break; > + default: > + LOG(V4L2, Error) > + << "Unkown VIDIOC_ENUM_FRAMESIZES type " s/Unkown/Unknown/ > + << frameSize.type; > + return {}; > + } > + } > + > + if (ret && errno != EINVAL) { > + ret = errno; > + LOG(V4L2, Error) > + << "Unable to enumerate frame sizes: " << strerror(ret); > + return {}; > + } > + > + return sizes; > +} > > /** > * \brief Import the externally allocated \a pool of buffers >
Hi Niklas, Thank you for the patch. On Wed, May 29, 2019 at 10:53:27PM +0100, Kieran Bingham wrote: > On 27/05/2019 01:15, Niklas Söderlund wrote: > > Add methods to enumerate pixelformats and frame sizes from a v4l2 > > device. The interface is similar to how V4L2Subdevice enumerates mbus > > codes and frame sizes. > > Could we add a test or two to test/v4l2_device/formats.cpp for this > public API addition? > > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > > --- > > src/libcamera/include/v4l2_device.h | 5 ++ > > src/libcamera/v4l2_device.cpp | 104 ++++++++++++++++++++++++++++ > > 2 files changed, 109 insertions(+) > > > > diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h > > index 2e7bd1e7f4cce276..db2d12757c6f564a 100644 > > --- a/src/libcamera/include/v4l2_device.h > > +++ b/src/libcamera/include/v4l2_device.h > > @@ -16,6 +16,7 @@ > > #include <libcamera/geometry.h> > > #include <libcamera/signal.h> > > > > +#include "formats.h" > > #include "log.h" > > > > namespace libcamera { > > @@ -126,6 +127,7 @@ public: > > > > int getFormat(V4L2DeviceFormat *format); > > int setFormat(V4L2DeviceFormat *format); > > + V4L2DeviceFormats formats(); > > > > int exportBuffers(BufferPool *pool); > > int importBuffers(BufferPool *pool); > > @@ -154,6 +156,9 @@ private: > > int createPlane(Buffer *buffer, unsigned int plane, > > unsigned int length); > > > > + std::vector<unsigned int> enumPixelformats(); > > + std::vector<SizeRange> enumSizes(unsigned int pixelformat); > > + > > Buffer *dequeueBuffer(); > > void bufferAvailable(EventNotifier *notifier); > > > > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp > > index 8366ffc4db559520..a9667092a20505d9 100644 > > --- a/src/libcamera/v4l2_device.cpp > > +++ b/src/libcamera/v4l2_device.cpp > > @@ -564,6 +564,23 @@ int V4L2Device::setFormatMultiplane(V4L2DeviceFormat *format) > > return 0; > > } > > > > +/** > > + * \brief Enumerate all pixelformats and frame sizes s/pixelformats/pixel formats/ (same below) > > + * > > + * Enumerate all pixelformats and frame sizes reported by the video device. "reported" or "supported" ? > > + * > > + * \return All pixelformats and frame sizes > > How about: > > \return A list of the supported device formats > > <except s/list/some-other-type-description/ > > > > + */ > > +V4L2DeviceFormats V4L2Device::formats() > > Eeep, we have a V4L2DeviceFormat class and a V4L2DeviceFormats class > which do (somewhat) different things ... > > I hope that won't be too confusing... <I've gone back and noted this in > reply to the patch that adds the types.> It can be a bit confusing indeed, but so far I think this is the best solution I've seen. If anyone can think of a better option, please say so. We can always change it later anyway. > > +{ > > + std::map<unsigned int, std::vector<SizeRange>> formatMap = {}; > > + > > + for (unsigned int pixelformat : enumPixelformats()) > > + formatMap[pixelformat] = enumSizes(pixelformat); > > + > > + return V4L2DeviceFormats(formatMap); > > +} > > + > > int V4L2Device::requestBuffers(unsigned int count) > > { > > struct v4l2_requestbuffers rb = {}; > > @@ -692,6 +709,93 @@ int V4L2Device::createPlane(Buffer *buffer, unsigned int planeIndex, > > > > return 0; > > } > > newline required here. > > > +std::vector<unsigned int> V4L2Device::enumPixelformats() > > +{ > > + std::vector<unsigned int> pixelformats; s/pixelformats/pixelFormats/ (or just formats) ? > > + int ret; > > + > > + for (unsigned int index = 0;; index++) { s/;;/; ;/ > > + struct v4l2_fmtdesc pixelformatEnum = { > > + .index = index, > > + .type = bufferType_, > > + }; > > + > > + ret = ioctl(fd_, VIDIOC_ENUM_FMT, &pixelformatEnum); > > + if (ret) > > + break; > > + > > + pixelformats.push_back(pixelformatEnum.pixelformat); > > + } > > + > > + if (ret && errno != EINVAL) { > > + ret = errno; > > + LOG(V4L2, Error) > > + << "Unable to enumerate pixelformats: " > > + << strerror(ret); > > + return {}; > > + } > > + > > + return pixelformats; > > +} > > + > > +std::vector<SizeRange> V4L2Device::enumSizes(unsigned int pixelformat) s/pixelformat/pixelFormat/ ? > > +{ > > + std::vector<SizeRange> sizes; > > + int ret; > > + > > + for (unsigned int index = 0;; index++) { > > + struct v4l2_frmsizeenum frameSize = { > > + .index = index, > > + .pixel_format = pixelformat, > > + }; > > + > > + ret = ioctl(fd_, VIDIOC_ENUM_FRAMESIZES, &frameSize); > > + if (ret) > > + break; > > + > > + if (index != 0 && > > + frameSize.type != V4L2_FRMSIZE_TYPE_DISCRETE) { > > + LOG(V4L2, Error) > > + << "None 0 index for none discrete type"; > > Non-zero index for non discrete type > > > + return {}; > > + } > > + > > + switch (frameSize.type) { > > + case V4L2_FRMSIZE_TYPE_DISCRETE: > > + sizes.emplace_back(frameSize.discrete.width, > > + frameSize.discrete.height); > > + break; > > + case V4L2_FRMSIZE_TYPE_CONTINUOUS: > > + sizes.emplace_back(frameSize.stepwise.min_width, > > + frameSize.stepwise.min_height, > > + frameSize.stepwise.max_width, > > + frameSize.stepwise.max_height); > > + break; > > + case V4L2_FRMSIZE_TYPE_STEPWISE: > > + sizes.emplace_back(frameSize.stepwise.min_width, > > + frameSize.stepwise.min_height, > > + frameSize.stepwise.max_width, > > + frameSize.stepwise.max_height, > > + frameSize.stepwise.step_width, > > + frameSize.stepwise.step_height); > > + break; > > + default: > > + LOG(V4L2, Error) > > + << "Unkown VIDIOC_ENUM_FRAMESIZES type " > > s/Unkown/Unknown/ > > > + << frameSize.type; > > + return {}; > > + } > > + } > > + > > + if (ret && errno != EINVAL) { > > + ret = errno; > > + LOG(V4L2, Error) > > + << "Unable to enumerate frame sizes: " << strerror(ret); While it makes no big difference as we then don't do anything with ret, we usually assign ret with -errno and use strerror(-ret). It may make sense to use this construct here as well, to avoid possible bugs in case we later change the function to return ret. I won't push much though, it's up to you. > > + return {}; > > + } > > + > > + return sizes; > > +} > > > > /** > > * \brief Import the externally allocated \a pool of buffers
diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h index 2e7bd1e7f4cce276..db2d12757c6f564a 100644 --- a/src/libcamera/include/v4l2_device.h +++ b/src/libcamera/include/v4l2_device.h @@ -16,6 +16,7 @@ #include <libcamera/geometry.h> #include <libcamera/signal.h> +#include "formats.h" #include "log.h" namespace libcamera { @@ -126,6 +127,7 @@ public: int getFormat(V4L2DeviceFormat *format); int setFormat(V4L2DeviceFormat *format); + V4L2DeviceFormats formats(); int exportBuffers(BufferPool *pool); int importBuffers(BufferPool *pool); @@ -154,6 +156,9 @@ private: int createPlane(Buffer *buffer, unsigned int plane, unsigned int length); + std::vector<unsigned int> enumPixelformats(); + std::vector<SizeRange> enumSizes(unsigned int pixelformat); + Buffer *dequeueBuffer(); void bufferAvailable(EventNotifier *notifier); diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp index 8366ffc4db559520..a9667092a20505d9 100644 --- a/src/libcamera/v4l2_device.cpp +++ b/src/libcamera/v4l2_device.cpp @@ -564,6 +564,23 @@ int V4L2Device::setFormatMultiplane(V4L2DeviceFormat *format) return 0; } +/** + * \brief Enumerate all pixelformats and frame sizes + * + * Enumerate all pixelformats and frame sizes reported by the video device. + * + * \return All pixelformats and frame sizes + */ +V4L2DeviceFormats V4L2Device::formats() +{ + std::map<unsigned int, std::vector<SizeRange>> formatMap = {}; + + for (unsigned int pixelformat : enumPixelformats()) + formatMap[pixelformat] = enumSizes(pixelformat); + + return V4L2DeviceFormats(formatMap); +} + int V4L2Device::requestBuffers(unsigned int count) { struct v4l2_requestbuffers rb = {}; @@ -692,6 +709,93 @@ int V4L2Device::createPlane(Buffer *buffer, unsigned int planeIndex, return 0; } +std::vector<unsigned int> V4L2Device::enumPixelformats() +{ + std::vector<unsigned int> pixelformats; + int ret; + + for (unsigned int index = 0;; index++) { + struct v4l2_fmtdesc pixelformatEnum = { + .index = index, + .type = bufferType_, + }; + + ret = ioctl(fd_, VIDIOC_ENUM_FMT, &pixelformatEnum); + if (ret) + break; + + pixelformats.push_back(pixelformatEnum.pixelformat); + } + + if (ret && errno != EINVAL) { + ret = errno; + LOG(V4L2, Error) + << "Unable to enumerate pixelformats: " + << strerror(ret); + return {}; + } + + return pixelformats; +} + +std::vector<SizeRange> V4L2Device::enumSizes(unsigned int pixelformat) +{ + std::vector<SizeRange> sizes; + int ret; + + for (unsigned int index = 0;; index++) { + struct v4l2_frmsizeenum frameSize = { + .index = index, + .pixel_format = pixelformat, + }; + + ret = ioctl(fd_, VIDIOC_ENUM_FRAMESIZES, &frameSize); + if (ret) + break; + + if (index != 0 && + frameSize.type != V4L2_FRMSIZE_TYPE_DISCRETE) { + LOG(V4L2, Error) + << "None 0 index for none discrete type"; + return {}; + } + + switch (frameSize.type) { + case V4L2_FRMSIZE_TYPE_DISCRETE: + sizes.emplace_back(frameSize.discrete.width, + frameSize.discrete.height); + break; + case V4L2_FRMSIZE_TYPE_CONTINUOUS: + sizes.emplace_back(frameSize.stepwise.min_width, + frameSize.stepwise.min_height, + frameSize.stepwise.max_width, + frameSize.stepwise.max_height); + break; + case V4L2_FRMSIZE_TYPE_STEPWISE: + sizes.emplace_back(frameSize.stepwise.min_width, + frameSize.stepwise.min_height, + frameSize.stepwise.max_width, + frameSize.stepwise.max_height, + frameSize.stepwise.step_width, + frameSize.stepwise.step_height); + break; + default: + LOG(V4L2, Error) + << "Unkown VIDIOC_ENUM_FRAMESIZES type " + << frameSize.type; + return {}; + } + } + + if (ret && errno != EINVAL) { + ret = errno; + LOG(V4L2, Error) + << "Unable to enumerate frame sizes: " << strerror(ret); + return {}; + } + + return sizes; +} /** * \brief Import the externally allocated \a pool of buffers
Add methods to enumerate pixelformats and frame sizes from a v4l2 device. The interface is similar to how V4L2Subdevice enumerates mbus codes and frame sizes. Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> --- src/libcamera/include/v4l2_device.h | 5 ++ src/libcamera/v4l2_device.cpp | 104 ++++++++++++++++++++++++++++ 2 files changed, 109 insertions(+)