[PATCH/RFC 07/32] libcamera: v4l2_subdevice: Add stream support to get/set functions
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Tue Mar 5 10:13:46 CET 2024
On Tue, Mar 05, 2024 at 09:32:07AM +0100, Jacopo Mondi wrote:
> Hi Laurent
>
> On Fri, Mar 01, 2024 at 11:20:56PM +0200, Laurent Pinchart wrote:
> > Extend the V4L2Subdevice API with stream support for the functions that
> > get and set formats and selection rectangles. Add a Stream structure to
> > identify a subdev pad and stream, and use it to extend the V4L2Subdevice
> > functions that get and set formats and selection rectangles with stream
> > support.
> >
> > To preserve the existing pad-based API, implement overloaded functions
> > that wrap the new stream-based API. This allows callers that are not
> > stream-aware to use a simpler pad-based API, instead of having to
> > explicitly set the stream number to 0 in all API calls.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> > Changes since v1:
> >
> > - Drop incorrect comment change
> > - Use operator<<(Stream)
> > - Add comparison operators
> > ---
> > include/libcamera/internal/v4l2_subdevice.h | 58 ++++-
> > src/libcamera/v4l2_subdevice.cpp | 252 +++++++++++++++-----
> > 2 files changed, 240 insertions(+), 70 deletions(-)
> >
> > diff --git a/include/libcamera/internal/v4l2_subdevice.h b/include/libcamera/internal/v4l2_subdevice.h
> > index 1115cfa55594..6cd36730371a 100644
> > --- a/include/libcamera/internal/v4l2_subdevice.h
> > +++ b/include/libcamera/internal/v4l2_subdevice.h
> > @@ -80,6 +80,21 @@ public:
> > ActiveFormat = V4L2_SUBDEV_FORMAT_ACTIVE,
> > };
> >
> > + struct Stream {
> > + Stream()
> > + : pad(0), stream(0)
> > + {
> > + }
> > +
> > + Stream(unsigned int p, unsigned int s)
> > + : pad(p), stream(s)
> > + {
> > + }
> > +
> > + unsigned int pad;
> > + unsigned int stream;
> > + };
> > +
> > class Routing : public std::vector<struct v4l2_subdev_route>
> > {
> > public:
> > @@ -93,17 +108,39 @@ public:
> >
> > const MediaEntity *entity() const { return entity_; }
> >
> > - int getSelection(unsigned int pad, unsigned int target,
> > + int getSelection(const Stream &stream, unsigned int target,
> > Rectangle *rect);
> > - int setSelection(unsigned int pad, unsigned int target,
> > + int getSelection(unsigned int pad, unsigned int target, Rectangle *rect)
> > + {
> > + return getSelection({ pad, 0 }, target, rect);
> > + }
> > + int setSelection(const Stream &stream, unsigned int target,
> > Rectangle *rect);
> > + int setSelection(unsigned int pad, unsigned int target, Rectangle *rect)
> > + {
> > + return setSelection({ pad, 0 }, target, rect);
> > + }
> >
> > - Formats formats(unsigned int pad);
> > + Formats formats(const Stream &stream);
> > + Formats formats(unsigned int pad)
> > + {
> > + return formats({ pad, 0 });
> > + }
> >
> > + int getFormat(const Stream &stream, V4L2SubdeviceFormat *format,
> > + Whence whence = ActiveFormat);
> > int getFormat(unsigned int pad, V4L2SubdeviceFormat *format,
> > + Whence whence = ActiveFormat)
> > + {
> > + return getFormat({ pad, 0 }, format, whence);
> > + }
> > + int setFormat(const Stream &stream, V4L2SubdeviceFormat *format,
> > Whence whence = ActiveFormat);
> > int setFormat(unsigned int pad, V4L2SubdeviceFormat *format,
> > - Whence whence = ActiveFormat);
> > + Whence whence = ActiveFormat)
> > + {
> > + return setFormat({ pad, 0 }, format, whence);
> > + }
> >
> > int getRouting(Routing *routing, Whence whence = ActiveFormat);
> > int setRouting(Routing *routing, Whence whence = ActiveFormat);
> > @@ -123,8 +160,8 @@ private:
> > std::optional<ColorSpace>
> > toColorSpace(const v4l2_mbus_framefmt &format) const;
> >
> > - std::vector<unsigned int> enumPadCodes(unsigned int pad);
> > - std::vector<SizeRange> enumPadSizes(unsigned int pad,
> > + std::vector<unsigned int> enumPadCodes(const Stream &stream);
> > + std::vector<SizeRange> enumPadSizes(const Stream &stream,
> > unsigned int code);
> >
> > const MediaEntity *entity_;
> > @@ -133,4 +170,13 @@ private:
> > struct V4L2SubdeviceCapability caps_;
> > };
> >
> > +bool operator==(const V4L2Subdevice::Stream &lhs, const V4L2Subdevice::Stream &rhs);
> > +static inline bool operator!=(const V4L2Subdevice::Stream &lhs,
> > + const V4L2Subdevice::Stream &rhs)
> > +{
> > + return !(lhs == rhs);
> > +}
> > +
> > +std::ostream &operator<<(std::ostream &out, const V4L2Subdevice::Stream &stream);
> > +
> > } /* namespace libcamera */
> > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
> > index dadc4d1dab92..5a3c6a02f57c 100644
> > --- a/src/libcamera/v4l2_subdevice.cpp
> > +++ b/src/libcamera/v4l2_subdevice.cpp
> > @@ -814,6 +814,62 @@ std::ostream &operator<<(std::ostream &out, const V4L2SubdeviceFormat &f)
> > * \brief The format operation applies to TRY formats
> > */
> >
> > +/**
> > + * \class V4L2Subdevice::Stream
> > + * \brief V4L2 subdevice stream
> > + *
> > + * This class identifies a subdev stream, by bundling the pad number with the
> > + * stream number. It is used in all stream-aware functions of the V4L2Subdevice
> > + * class to identify the stream the functions operate on.
> > + *
> > + * \var V4L2Subdevice::Stream::pad
> > + * \brief The 0-indexed pad number
> > + *
> > + * \var V4L2Subdevice::Stream::stream
> > + * \brief The stream number
> > + */
> > +
> > +/**
> > + * \fn V4L2Subdevice::Stream::Stream()
> > + * \brief Construct a Stream with pad and stream set to 0
> > + */
> > +
> > +/**
> > + * \fn V4L2Subdevice::Stream::Stream(unsigned int pad, unsigned int stream)
> > + * \brief Construct a Stream with a given \a pad and \a stream number
> > + * \param[in] pad The indexed pad number
> > + * \param[in] stream The stream number
> > + */
> > +
> > +/**
> > + * \brief Compare streams for equality
> > + * \return True if the two streams are equal, false otherwise
> > + */
> > +bool operator==(const V4L2Subdevice::Stream &lhs, const V4L2Subdevice::Stream &rhs)
> > +{
> > + return lhs.pad == rhs.pad && lhs.stream == rhs.stream;
> > +}
> > +
> > +/**
> > + * \fn bool operator!=(const V4L2Subdevice::Stream &lhs, const V4L2Subdevice::Stream &rhs)
> > + * \brief Compare streams for inequality
> > + * \return True if the two streams are not equal, false otherwise
> > + */
> > +
> > +/**
> > + * \brief Insert a text representation of a V4L2Subdevice::Stream into an
> > + * output stream
>
> Compared to the other patches where you have aligned this one to the
> left with one space, this one is still tab-aligned
I'll move to using a single space.
> > + * \param[in] out The output stream
> > + * \param[in] stream The V4L2Subdevice::Stream
> > + * \return The output stream \a out
> > + */
> > +std::ostream &operator<<(std::ostream &out, const V4L2Subdevice::Stream &stream)
> > +{
> > + out << stream.pad << "/" << stream.stream;
> > +
> > + return out;
> > +}
> > +
> > /**
> > * \class V4L2Subdevice::Routing
> > * \brief V4L2 subdevice routing table
> > @@ -905,7 +961,7 @@ int V4L2Subdevice::open()
> >
> > /**
> > * \brief Get selection rectangle \a rect for \a target
> > - * \param[in] pad The 0-indexed pad number the rectangle is retrieved from
> > + * \param[in] stream The stream the rectangle is retrieved from
> > * \param[in] target The selection target defined by the V4L2_SEL_TGT_* flags
> > * \param[out] rect The retrieved selection rectangle
> > *
> > @@ -913,13 +969,14 @@ int V4L2Subdevice::open()
> > *
> > * \return 0 on success or a negative error code otherwise
> > */
> > -int V4L2Subdevice::getSelection(unsigned int pad, unsigned int target,
> > +int V4L2Subdevice::getSelection(const Stream &stream, unsigned int target,
> > Rectangle *rect)
> > {
> > struct v4l2_subdev_selection sel = {};
> >
> > sel.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> > - sel.pad = pad;
> > + sel.pad = stream.pad;
> > + sel.stream = stream.stream;
> > sel.target = target;
> > sel.flags = 0;
> >
> > @@ -927,7 +984,7 @@ int V4L2Subdevice::getSelection(unsigned int pad, unsigned int target,
> > if (ret < 0) {
> > LOG(V4L2, Error)
> > << "Unable to get rectangle " << target << " on pad "
> > - << pad << ": " << strerror(-ret);
> > + << stream << ": " << strerror(-ret);
> > return ret;
> > }
> >
> > @@ -939,9 +996,20 @@ int V4L2Subdevice::getSelection(unsigned int pad, unsigned int target,
> > return 0;
> > }
> >
> > +/**
> > + * \fn V4L2Subdevice::getSelection(unsigned int pad, unsigned int target,
> > + * Rectangle *rect)
>
> ditto. Or maybe we didn't get to any conclusion on this alignment
> thing ?
>
> > + * \brief Get selection rectangle \a rect for \a target
> > + * \param[in] pad The 0-indexed pad number the rectangle is retrieved from
> > + * \param[in] target The selection target defined by the V4L2_SEL_TGT_* flags
> > + * \param[out] rect The retrieved selection rectangle
> > + *
> > + * \return 0 on success or a negative error code otherwise
> > + */
> > +
> > /**
> > * \brief Set selection rectangle \a rect for \a target
> > - * \param[in] pad The 0-indexed pad number the rectangle is to be applied to
> > + * \param[in] stream The stream the rectangle is to be applied to
> > * \param[in] target The selection target defined by the V4L2_SEL_TGT_* flags
> > * \param[inout] rect The selection rectangle to be applied
> > *
> > @@ -949,13 +1017,14 @@ int V4L2Subdevice::getSelection(unsigned int pad, unsigned int target,
> > *
> > * \return 0 on success or a negative error code otherwise
> > */
> > -int V4L2Subdevice::setSelection(unsigned int pad, unsigned int target,
> > +int V4L2Subdevice::setSelection(const Stream &stream, unsigned int target,
> > Rectangle *rect)
> > {
> > struct v4l2_subdev_selection sel = {};
> >
> > sel.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> > - sel.pad = pad;
> > + sel.pad = stream.pad;
> > + sel.stream = stream.stream;
> > sel.target = target;
> > sel.flags = 0;
> >
> > @@ -968,7 +1037,7 @@ int V4L2Subdevice::setSelection(unsigned int pad, unsigned int target,
> > if (ret < 0) {
> > LOG(V4L2, Error)
> > << "Unable to set rectangle " << target << " on pad "
> > - << pad << ": " << strerror(-ret);
> > + << stream << ": " << strerror(-ret);
> > return ret;
> > }
> >
> > @@ -979,26 +1048,40 @@ int V4L2Subdevice::setSelection(unsigned int pad, unsigned int target,
> >
> > return 0;
> > }
> > +
> > /**
> > - * \brief Enumerate all media bus codes and frame sizes on a \a pad
> > - * \param[in] pad The 0-indexed pad number to enumerate formats on
> > + * \fn V4L2Subdevice::setSelection(unsigned int pad, unsigned int target,
> > + * Rectangle *rect)
>
> here too :)
>
> > + * \brief Set selection rectangle \a rect for \a target
> > + * \param[in] pad The 0-indexed pad number the rectangle is to be applied to
> > + * \param[in] target The selection target defined by the V4L2_SEL_TGT_* flags
> > + * \param[inout] rect The selection rectangle to be applied
> > + *
> > + * \todo Define a V4L2SelectionTarget enum for the selection target
> > + *
> > + * \return 0 on success or a negative error code otherwise
> > + */
> > +
> > +/**
> > + * \brief Enumerate all media bus codes and frame sizes on a \a stream
> > + * \param[in] stream The stream to enumerate formats for
> > *
> > * Enumerate all media bus codes and frame sizes supported by the subdevice on
> > - * a \a pad.
> > + * a \a stream.
> > *
> > * \return A list of the supported device formats
> > */
> > -V4L2Subdevice::Formats V4L2Subdevice::formats(unsigned int pad)
> > +V4L2Subdevice::Formats V4L2Subdevice::formats(const Stream &stream)
> > {
> > Formats formats;
> >
> > - if (pad >= entity_->pads().size()) {
> > - LOG(V4L2, Error) << "Invalid pad: " << pad;
> > + if (stream.pad >= entity_->pads().size()) {
> > + LOG(V4L2, Error) << "Invalid pad: " << stream.pad;
> > return {};
> > }
> >
> > - for (unsigned int code : enumPadCodes(pad)) {
> > - std::vector<SizeRange> sizes = enumPadSizes(pad, code);
> > + for (unsigned int code : enumPadCodes(stream)) {
> > + std::vector<SizeRange> sizes = enumPadSizes(stream, code);
> > if (sizes.empty())
> > return {};
> >
> > @@ -1006,7 +1089,7 @@ V4L2Subdevice::Formats V4L2Subdevice::formats(unsigned int pad)
> > if (!inserted.second) {
> > LOG(V4L2, Error)
> > << "Could not add sizes for media bus code "
> > - << code << " on pad " << pad;
> > + << code << " on pad " << stream.pad;
> > return {};
> > }
> > }
> > @@ -1014,6 +1097,17 @@ V4L2Subdevice::Formats V4L2Subdevice::formats(unsigned int pad)
> > return formats;
> > }
> >
> > +/**
> > + * \fn V4L2Subdevice::formats(unsigned int pad)
> > + * \brief Enumerate all media bus codes and frame sizes on a \a pad
> > + * \param[in] pad The 0-indexed pad number to enumerate formats on
> > + *
> > + * Enumerate all media bus codes and frame sizes supported by the subdevice on
> > + * a \a pad
> > + *
> > + * \return A list of the supported device formats
> > + */
> > +
> > std::optional<ColorSpace> V4L2Subdevice::toColorSpace(const v4l2_mbus_framefmt &format) const
> > {
> > /*
> > @@ -1045,25 +1139,26 @@ std::optional<ColorSpace> V4L2Subdevice::toColorSpace(const v4l2_mbus_framefmt &
> > }
> >
> > /**
> > - * \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
> > + * \brief Retrieve the image format set on one of the V4L2 subdevice streams
> > + * \param[in] stream The stream the format is to be retrieved from
> > * \param[out] format The image bus format
> > * \param[in] whence The format to get, \ref V4L2Subdevice::ActiveFormat
> > * "ActiveFormat" or \ref V4L2Subdevice::TryFormat "TryFormat"
> > * \return 0 on success or a negative error code otherwise
> > */
> > -int V4L2Subdevice::getFormat(unsigned int pad, V4L2SubdeviceFormat *format,
> > +int V4L2Subdevice::getFormat(const Stream &stream, V4L2SubdeviceFormat *format,
> > Whence whence)
> > {
> > struct v4l2_subdev_format subdevFmt = {};
> > subdevFmt.which = whence;
> > - subdevFmt.pad = pad;
> > + subdevFmt.pad = stream.pad;
> > + subdevFmt.stream = stream.stream;
> >
> > int ret = ioctl(VIDIOC_SUBDEV_G_FMT, &subdevFmt);
> > if (ret) {
> > LOG(V4L2, Error)
> > - << "Unable to get format on pad " << pad
> > - << ": " << strerror(-ret);
> > + << "Unable to get format on pad " << stream.pad << "/"
> > + << stream.stream << ": " << strerror(-ret);
> > return ret;
> > }
> >
> > @@ -1076,6 +1171,66 @@ int V4L2Subdevice::getFormat(unsigned int pad, V4L2SubdeviceFormat *format,
> > }
> >
> > /**
> > + * \fn V4L2Subdevice::getFormat(unsigned int pad, V4L2SubdeviceFormat *format,
> > + * Whence whence)
> > + * \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
> > + * \param[out] format The image bus format
> > + * \param[in] whence The format to get, \ref V4L2Subdevice::ActiveFormat
> > + * "ActiveFormat" or \ref V4L2Subdevice::TryFormat "TryFormat"
> > + * \return 0 on success or a negative error code otherwise
> > + */
> > +
> > +/**
> > + * \brief Set an image format on one of the V4L2 subdevice pads
> > + * \param[in] stream The stream the format is to be applied to
> > + * \param[inout] format The image bus format to apply to the stream
> > + * \param[in] whence The format to set, \ref V4L2Subdevice::ActiveFormat
> > + * "ActiveFormat" or \ref V4L2Subdevice::TryFormat "TryFormat"
> > + *
> > + * Apply the requested image format to the desired stream and return the
> > + * actually applied format parameters, as getFormat() would do.
> > + *
> > + * \return 0 on success or a negative error code otherwise
> > + */
> > +int V4L2Subdevice::setFormat(const Stream &stream, V4L2SubdeviceFormat *format,
> > + Whence whence)
> > +{
> > + struct v4l2_subdev_format subdevFmt = {};
> > + subdevFmt.which = whence;
> > + subdevFmt.pad = stream.pad;
> > + subdevFmt.stream = stream.stream;
> > + subdevFmt.format.width = format->size.width;
> > + subdevFmt.format.height = format->size.height;
> > + subdevFmt.format.code = format->code;
> > + subdevFmt.format.field = V4L2_FIELD_NONE;
> > + if (format->colorSpace) {
> > + fromColorSpace(format->colorSpace, subdevFmt.format);
> > +
> > + /* The CSC flag is only applicable to source pads. */
> > + if (entity_->pads()[stream.pad]->flags() & MEDIA_PAD_FL_SOURCE)
> > + subdevFmt.format.flags |= V4L2_MBUS_FRAMEFMT_SET_CSC;
> > + }
> > +
> > + int ret = ioctl(VIDIOC_SUBDEV_S_FMT, &subdevFmt);
> > + if (ret) {
> > + LOG(V4L2, Error)
> > + << "Unable to set format on pad " << stream.pad << "/"
> > + << stream.stream << ": " << strerror(-ret);
> > + return ret;
> > + }
> > +
> > + format->size.width = subdevFmt.format.width;
> > + format->size.height = subdevFmt.format.height;
> > + format->code = subdevFmt.format.code;
> > + format->colorSpace = toColorSpace(subdevFmt.format);
> > +
> > + return 0;
> > +}
> > +
> > +/**
> > + * \fn V4L2Subdevice::setFormat(unsigned int pad, V4L2SubdeviceFormat *format,
> > + * Whence whence)
>
> One rogue space ?
>
> > * \brief Set an image format on one of the V4L2 subdevice pads
> > * \param[in] pad The 0-indexed pad number the format is to be applied to
> > * \param[inout] format The image bus format to apply to the subdevice's pad
> > @@ -1087,39 +1242,6 @@ int V4L2Subdevice::getFormat(unsigned int pad, V4L2SubdeviceFormat *format,
> > *
> > * \return 0 on success or a negative error code otherwise
> > */
> > -int V4L2Subdevice::setFormat(unsigned int pad, V4L2SubdeviceFormat *format,
> > - Whence whence)
> > -{
> > - struct v4l2_subdev_format subdevFmt = {};
> > - subdevFmt.which = whence;
> > - subdevFmt.pad = pad;
> > - subdevFmt.format.width = format->size.width;
> > - subdevFmt.format.height = format->size.height;
> > - subdevFmt.format.code = format->code;
> > - subdevFmt.format.field = V4L2_FIELD_NONE;
> > - if (format->colorSpace) {
> > - fromColorSpace(format->colorSpace, subdevFmt.format);
> > -
> > - /* The CSC flag is only applicable to source pads. */
> > - if (entity_->pads()[pad]->flags() & MEDIA_PAD_FL_SOURCE)
> > - subdevFmt.format.flags |= V4L2_MBUS_FRAMEFMT_SET_CSC;
> > - }
> > -
> > - int ret = ioctl(VIDIOC_SUBDEV_S_FMT, &subdevFmt);
> > - if (ret) {
> > - LOG(V4L2, Error)
> > - << "Unable to set format on pad " << pad
> > - << ": " << strerror(-ret);
> > - return ret;
> > - }
> > -
> > - format->size.width = subdevFmt.format.width;
> > - format->size.height = subdevFmt.format.height;
> > - format->code = subdevFmt.format.code;
> > - format->colorSpace = toColorSpace(subdevFmt.format);
> > -
> > - return 0;
> > -}
> >
> > /**
> > * \brief Retrieve the subdevice's internal routing table
> > @@ -1282,14 +1404,15 @@ std::string V4L2Subdevice::logPrefix() const
> > return "'" + entity_->name() + "'";
> > }
> >
> > -std::vector<unsigned int> V4L2Subdevice::enumPadCodes(unsigned int pad)
> > +std::vector<unsigned int> V4L2Subdevice::enumPadCodes(const Stream &stream)
> > {
> > std::vector<unsigned int> codes;
> > int ret;
> >
> > for (unsigned int index = 0; ; index++) {
> > struct v4l2_subdev_mbus_code_enum mbusEnum = {};
> > - mbusEnum.pad = pad;
> > + mbusEnum.pad = stream.pad;
> > + mbusEnum.stream = stream.stream;
> > mbusEnum.index = index;
> > mbusEnum.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> >
> > @@ -1302,15 +1425,15 @@ std::vector<unsigned int> V4L2Subdevice::enumPadCodes(unsigned int pad)
> >
> > if (ret < 0 && ret != -EINVAL) {
> > LOG(V4L2, Error)
> > - << "Unable to enumerate formats on pad " << pad
> > - << ": " << strerror(-ret);
> > + << "Unable to enumerate formats on pad " << stream.pad
> > + << "/" << stream.stream << ": " << strerror(-ret);
>
> This could simply be " << stream << "
Indeed, will fix.
> > return {};
> > }
> >
> > return codes;
> > }
> >
> > -std::vector<SizeRange> V4L2Subdevice::enumPadSizes(unsigned int pad,
> > +std::vector<SizeRange> V4L2Subdevice::enumPadSizes(const Stream &stream,
> > unsigned int code)
> > {
> > std::vector<SizeRange> sizes;
> > @@ -1319,7 +1442,8 @@ std::vector<SizeRange> V4L2Subdevice::enumPadSizes(unsigned int pad,
> > for (unsigned int index = 0;; index++) {
> > struct v4l2_subdev_frame_size_enum sizeEnum = {};
> > sizeEnum.index = index;
> > - sizeEnum.pad = pad;
> > + sizeEnum.pad = stream.pad;
> > + sizeEnum.stream = stream.stream;
> > sizeEnum.code = code;
> > sizeEnum.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> >
> > @@ -1333,8 +1457,8 @@ std::vector<SizeRange> V4L2Subdevice::enumPadSizes(unsigned int pad,
> >
> > if (ret < 0 && ret != -EINVAL && ret != -ENOTTY) {
> > LOG(V4L2, Error)
> > - << "Unable to enumerate sizes on pad " << pad
> > - << ": " << strerror(-ret);
> > + << "Unable to enumerate sizes on pad " << stream.pad
> > + << "/" << stream.stream << ": " << strerror(-ret);
>
> Here as well
>
> All minors really,
> Reviewed-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>
>
> Feel free to fast-track!
>
> > return {};
> > }
> >
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list