[{"id":28851,"web_url":"https://patchwork.libcamera.org/comment/28851/","msgid":"","date":"2024-03-05T08:32:07","subject":"Re: [PATCH/RFC 07/32] libcamera: v4l2_subdevice: Add stream support\n\tto get/set functions","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Laurent\n\nOn Fri, Mar 01, 2024 at 11:20:56PM +0200, Laurent Pinchart wrote:\n> Extend the V4L2Subdevice API with stream support for the functions that\n> get and set formats and selection rectangles. Add a Stream structure to\n> identify a subdev pad and stream, and use it to extend the V4L2Subdevice\n> functions that get and set formats and selection rectangles with stream\n> support.\n>\n> To preserve the existing pad-based API, implement overloaded functions\n> that wrap the new stream-based API. This allows callers that are not\n> stream-aware to use a simpler pad-based API, instead of having to\n> explicitly set the stream number to 0 in all API calls.\n>\n> Signed-off-by: Laurent Pinchart \n> ---\n> Changes since v1:\n>\n> - Drop incorrect comment change\n> - Use operator<<(Stream)\n> - Add comparison operators\n> ---\n> include/libcamera/internal/v4l2_subdevice.h | 58 ++++-\n> src/libcamera/v4l2_subdevice.cpp | 252 +++++++++++++++-----\n> 2 files changed, 240 insertions(+), 70 deletions(-)\n>\n> diff --git a/include/libcamera/internal/v4l2_subdevice.h b/include/libcamera/internal/v4l2_subdevice.h\n> index 1115cfa55594..6cd36730371a 100644\n> --- a/include/libcamera/internal/v4l2_subdevice.h\n> +++ b/include/libcamera/internal/v4l2_subdevice.h\n> @@ -80,6 +80,21 @@ public:\n> \t\tActiveFormat = V4L2_SUBDEV_FORMAT_ACTIVE,\n> \t};\n>\n> +\tstruct Stream {\n> +\t\tStream()\n> +\t\t\t: pad(0), stream(0)\n> +\t\t{\n> +\t\t}\n> +\n> +\t\tStream(unsigned int p, unsigned int s)\n> +\t\t\t: pad(p), stream(s)\n> +\t\t{\n> +\t\t}\n> +\n> +\t\tunsigned int pad;\n> +\t\tunsigned int stream;\n> +\t};\n> +\n> \tclass Routing : public std::vector\n> \t{\n> \tpublic:\n> @@ -93,17 +108,39 @@ public:\n>\n> \tconst MediaEntity *entity() const { return entity_; }\n>\n> -\tint getSelection(unsigned int pad, unsigned int target,\n> +\tint getSelection(const Stream &stream, unsigned int target,\n> \t\t\t Rectangle *rect);\n> -\tint setSelection(unsigned int pad, unsigned int target,\n> +\tint getSelection(unsigned int pad, unsigned int target, Rectangle *rect)\n> +\t{\n> +\t\treturn getSelection({ pad, 0 }, target, rect);\n> +\t}\n> +\tint setSelection(const Stream &stream, unsigned int target,\n> \t\t\t Rectangle *rect);\n> +\tint setSelection(unsigned int pad, unsigned int target, Rectangle *rect)\n> +\t{\n> +\t\treturn setSelection({ pad, 0 }, target, rect);\n> +\t}\n>\n> -\tFormats formats(unsigned int pad);\n> +\tFormats formats(const Stream &stream);\n> +\tFormats formats(unsigned int pad)\n> +\t{\n> +\t\treturn formats({ pad, 0 });\n> +\t}\n>\n> +\tint getFormat(const Stream &stream, V4L2SubdeviceFormat *format,\n> +\t\t Whence whence = ActiveFormat);\n> \tint getFormat(unsigned int pad, V4L2SubdeviceFormat *format,\n> +\t\t Whence whence = ActiveFormat)\n> +\t{\n> +\t\treturn getFormat({ pad, 0 }, format, whence);\n> +\t}\n> +\tint setFormat(const Stream &stream, V4L2SubdeviceFormat *format,\n> \t\t Whence whence = ActiveFormat);\n> \tint setFormat(unsigned int pad, V4L2SubdeviceFormat *format,\n> -\t\t Whence whence = ActiveFormat);\n> +\t\t Whence whence = ActiveFormat)\n> +\t{\n> +\t\treturn setFormat({ pad, 0 }, format, whence);\n> +\t}\n>\n> \tint getRouting(Routing *routing, Whence whence = ActiveFormat);\n> \tint setRouting(Routing *routing, Whence whence = ActiveFormat);\n> @@ -123,8 +160,8 @@ private:\n> \tstd::optional\n> \ttoColorSpace(const v4l2_mbus_framefmt &format) const;\n>\n> -\tstd::vector enumPadCodes(unsigned int pad);\n> -\tstd::vector enumPadSizes(unsigned int pad,\n> +\tstd::vector enumPadCodes(const Stream &stream);\n> +\tstd::vector enumPadSizes(const Stream &stream,\n> \t\t\t\t\t unsigned int code);\n>\n> \tconst MediaEntity *entity_;\n> @@ -133,4 +170,13 @@ private:\n> \tstruct V4L2SubdeviceCapability caps_;\n> };\n>\n> +bool operator==(const V4L2Subdevice::Stream &lhs, const V4L2Subdevice::Stream &rhs);\n> +static inline bool operator!=(const V4L2Subdevice::Stream &lhs,\n> +\t\t\t const V4L2Subdevice::Stream &rhs)\n> +{\n> +\treturn !(lhs == rhs);\n> +}\n> +\n> +std::ostream &operator<<(std::ostream &out, const V4L2Subdevice::Stream &stream);\n> +\n> } /* namespace libcamera */\n> diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp\n> index dadc4d1dab92..5a3c6a02f57c 100644\n> --- a/src/libcamera/v4l2_subdevice.cpp\n> +++ b/src/libcamera/v4l2_subdevice.cpp\n> @@ -814,6 +814,62 @@ std::ostream &operator<<(std::ostream &out, const V4L2SubdeviceFormat &f)\n> * \\brief The format operation applies to TRY formats\n> */\n>\n> +/**\n> + * \\class V4L2Subdevice::Stream\n> + * \\brief V4L2 subdevice stream\n> + *\n> + * This class identifies a subdev stream, by bundling the pad number with the\n> + * stream number. It is used in all stream-aware functions of the V4L2Subdevice\n> + * class to identify the stream the functions operate on.\n> + *\n> + * \\var V4L2Subdevice::Stream::pad\n> + * \\brief The 0-indexed pad number\n> + *\n> + * \\var V4L2Subdevice::Stream::stream\n> + * \\brief The stream number\n> + */\n> +\n> +/**\n> + * \\fn V4L2Subdevice::Stream::Stream()\n> + * \\brief Construct a Stream with pad and stream set to 0\n> + */\n> +\n> +/**\n> + * \\fn V4L2Subdevice::Stream::Stream(unsigned int pad, unsigned int stream)\n> + * \\brief Construct a Stream with a given \\a pad and \\a stream number\n> + * \\param[in] pad The indexed pad number\n> + * \\param[in] stream The stream number\n> + */\n> +\n> +/**\n> + * \\brief Compare streams for equality\n> + * \\return True if the two streams are equal, false otherwise\n> + */\n> +bool operator==(const V4L2Subdevice::Stream &lhs, const V4L2Subdevice::Stream &rhs)\n> +{\n> +\treturn lhs.pad == rhs.pad && lhs.stream == rhs.stream;\n> +}\n> +\n> +/**\n> + * \\fn bool operator!=(const V4L2Subdevice::Stream &lhs, const V4L2Subdevice::Stream &rhs)\n> + * \\brief Compare streams for inequality\n> + * \\return True if the two streams are not equal, false otherwise\n> + */\n> +\n> +/**\n> + * \\brief Insert a text representation of a V4L2Subdevice::Stream into an\n> + *\toutput stream\n\nCompared to the other patches where you have aligned this one to the\nleft with one space, this one is still tab-aligned\n\n> + * \\param[in] out The output stream\n> + * \\param[in] stream The V4L2Subdevice::Stream\n> + * \\return The output stream \\a out\n> + */\n> +std::ostream &operator<<(std::ostream &out, const V4L2Subdevice::Stream &stream)\n> +{\n> +\tout << stream.pad << \"/\" << stream.stream;\n> +\n> +\treturn out;\n> +}\n> +\n> /**\n> * \\class V4L2Subdevice::Routing\n> * \\brief V4L2 subdevice routing table\n> @@ -905,7 +961,7 @@ int V4L2Subdevice::open()\n>\n> /**\n> * \\brief Get selection rectangle \\a rect for \\a target\n> - * \\param[in] pad The 0-indexed pad number the rectangle is retrieved from\n> + * \\param[in] stream The stream the rectangle is retrieved from\n> * \\param[in] target The selection target defined by the V4L2_SEL_TGT_* flags\n> * \\param[out] rect The retrieved selection rectangle\n> *\n> @@ -913,13 +969,14 @@ int V4L2Subdevice::open()\n> *\n> * \\return 0 on success or a negative error code otherwise\n> */\n> -int V4L2Subdevice::getSelection(unsigned int pad, unsigned int target,\n> +int V4L2Subdevice::getSelection(const Stream &stream, unsigned int target,\n> \t\t\t\tRectangle *rect)\n> {\n> \tstruct v4l2_subdev_selection sel = {};\n>\n> \tsel.which = V4L2_SUBDEV_FORMAT_ACTIVE;\n> -\tsel.pad = pad;\n> +\tsel.pad = stream.pad;\n> +\tsel.stream = stream.stream;\n> \tsel.target = target;\n> \tsel.flags = 0;\n>\n> @@ -927,7 +984,7 @@ int V4L2Subdevice::getSelection(unsigned int pad, unsigned int target,\n> \tif (ret < 0) {\n> \t\tLOG(V4L2, Error)\n> \t\t\t<< \"Unable to get rectangle \" << target << \" on pad \"\n> -\t\t\t<< pad << \": \" << strerror(-ret);\n> +\t\t\t<< stream << \": \" << strerror(-ret);\n> \t\treturn ret;\n> \t}\n>\n> @@ -939,9 +996,20 @@ int V4L2Subdevice::getSelection(unsigned int pad, unsigned int target,\n> \treturn 0;\n> }\n>\n> +/**\n> + * \\fn V4L2Subdevice::getSelection(unsigned int pad, unsigned int target,\n> + *\tRectangle *rect)\n\nditto. Or maybe we didn't get to any conclusion on this alignment\nthing ?\n\n> + * \\brief Get selection rectangle \\a rect for \\a target\n> + * \\param[in] pad The 0-indexed pad number the rectangle is retrieved from\n> + * \\param[in] target The selection target defined by the V4L2_SEL_TGT_* flags\n> + * \\param[out] rect The retrieved selection rectangle\n> + *\n> + * \\return 0 on success or a negative error code otherwise\n> + */\n> +\n> /**\n> * \\brief Set selection rectangle \\a rect for \\a target\n> - * \\param[in] pad The 0-indexed pad number the rectangle is to be applied to\n> + * \\param[in] stream The stream the rectangle is to be applied to\n> * \\param[in] target The selection target defined by the V4L2_SEL_TGT_* flags\n> * \\param[inout] rect The selection rectangle to be applied\n> *\n> @@ -949,13 +1017,14 @@ int V4L2Subdevice::getSelection(unsigned int pad, unsigned int target,\n> *\n> * \\return 0 on success or a negative error code otherwise\n> */\n> -int V4L2Subdevice::setSelection(unsigned int pad, unsigned int target,\n> +int V4L2Subdevice::setSelection(const Stream &stream, unsigned int target,\n> \t\t\t\tRectangle *rect)\n> {\n> \tstruct v4l2_subdev_selection sel = {};\n>\n> \tsel.which = V4L2_SUBDEV_FORMAT_ACTIVE;\n> -\tsel.pad = pad;\n> +\tsel.pad = stream.pad;\n> +\tsel.stream = stream.stream;\n> \tsel.target = target;\n> \tsel.flags = 0;\n>\n> @@ -968,7 +1037,7 @@ int V4L2Subdevice::setSelection(unsigned int pad, unsigned int target,\n> \tif (ret < 0) {\n> \t\tLOG(V4L2, Error)\n> \t\t\t<< \"Unable to set rectangle \" << target << \" on pad \"\n> -\t\t\t<< pad << \": \" << strerror(-ret);\n> +\t\t\t<< stream << \": \" << strerror(-ret);\n> \t\treturn ret;\n> \t}\n>\n> @@ -979,26 +1048,40 @@ int V4L2Subdevice::setSelection(unsigned int pad, unsigned int target,\n>\n> \treturn 0;\n> }\n> +\n> /**\n> - * \\brief Enumerate all media bus codes and frame sizes on a \\a pad\n> - * \\param[in] pad The 0-indexed pad number to enumerate formats on\n> + * \\fn V4L2Subdevice::setSelection(unsigned int pad, unsigned int target,\n> + *\tRectangle *rect)\n\nhere too :)\n\n> + * \\brief Set selection rectangle \\a rect for \\a target\n> + * \\param[in] pad The 0-indexed pad number the rectangle is to be applied to\n> + * \\param[in] target The selection target defined by the V4L2_SEL_TGT_* flags\n> + * \\param[inout] rect The selection rectangle to be applied\n> + *\n> + * \\todo Define a V4L2SelectionTarget enum for the selection target\n> + *\n> + * \\return 0 on success or a negative error code otherwise\n> + */\n> +\n> +/**\n> + * \\brief Enumerate all media bus codes and frame sizes on a \\a stream\n> + * \\param[in] stream The stream to enumerate formats for\n> *\n> * Enumerate all media bus codes and frame sizes supported by the subdevice on\n> - * a \\a pad.\n> + * a \\a stream.\n> *\n> * \\return A list of the supported device formats\n> */\n> -V4L2Subdevice::Formats V4L2Subdevice::formats(unsigned int pad)\n> +V4L2Subdevice::Formats V4L2Subdevice::formats(const Stream &stream)\n> {\n> \tFormats formats;\n>\n> -\tif (pad >= entity_->pads().size()) {\n> -\t\tLOG(V4L2, Error) << \"Invalid pad: \" << pad;\n> +\tif (stream.pad >= entity_->pads().size()) {\n> +\t\tLOG(V4L2, Error) << \"Invalid pad: \" << stream.pad;\n> \t\treturn {};\n> \t}\n>\n> -\tfor (unsigned int code : enumPadCodes(pad)) {\n> -\t\tstd::vector sizes = enumPadSizes(pad, code);\n> +\tfor (unsigned int code : enumPadCodes(stream)) {\n> +\t\tstd::vector sizes = enumPadSizes(stream, code);\n> \t\tif (sizes.empty())\n> \t\t\treturn {};\n>\n> @@ -1006,7 +1089,7 @@ V4L2Subdevice::Formats V4L2Subdevice::formats(unsigned int pad)\n> \t\tif (!inserted.second) {\n> \t\t\tLOG(V4L2, Error)\n> \t\t\t\t<< \"Could not add sizes for media bus code \"\n> -\t\t\t\t<< code << \" on pad \" << pad;\n> +\t\t\t\t<< code << \" on pad \" << stream.pad;\n> \t\t\treturn {};\n> \t\t}\n> \t}\n> @@ -1014,6 +1097,17 @@ V4L2Subdevice::Formats V4L2Subdevice::formats(unsigned int pad)\n> \treturn formats;\n> }\n>\n> +/**\n> + * \\fn V4L2Subdevice::formats(unsigned int pad)\n> + * \\brief Enumerate all media bus codes and frame sizes on a \\a pad\n> + * \\param[in] pad The 0-indexed pad number to enumerate formats on\n> + *\n> + * Enumerate all media bus codes and frame sizes supported by the subdevice on\n> + * a \\a pad\n> + *\n> + * \\return A list of the supported device formats\n> + */\n> +\n> std::optional V4L2Subdevice::toColorSpace(const v4l2_mbus_framefmt &format) const\n> {\n> \t/*\n> @@ -1045,25 +1139,26 @@ std::optional V4L2Subdevice::toColorSpace(const v4l2_mbus_framefmt &\n> }\n>\n> /**\n> - * \\brief Retrieve the image format set on one of the V4L2 subdevice pads\n> - * \\param[in] pad The 0-indexed pad number the format is to be retrieved from\n> + * \\brief Retrieve the image format set on one of the V4L2 subdevice streams\n> + * \\param[in] stream The stream the format is to be retrieved from\n> * \\param[out] format The image bus format\n> * \\param[in] whence The format to get, \\ref V4L2Subdevice::ActiveFormat\n> * \"ActiveFormat\" or \\ref V4L2Subdevice::TryFormat \"TryFormat\"\n> * \\return 0 on success or a negative error code otherwise\n> */\n> -int V4L2Subdevice::getFormat(unsigned int pad, V4L2SubdeviceFormat *format,\n> +int V4L2Subdevice::getFormat(const Stream &stream, V4L2SubdeviceFormat *format,\n> \t\t\t Whence whence)\n> {\n> \tstruct v4l2_subdev_format subdevFmt = {};\n> \tsubdevFmt.which = whence;\n> -\tsubdevFmt.pad = pad;\n> +\tsubdevFmt.pad = stream.pad;\n> +\tsubdevFmt.stream = stream.stream;\n>\n> \tint ret = ioctl(VIDIOC_SUBDEV_G_FMT, &subdevFmt);\n> \tif (ret) {\n> \t\tLOG(V4L2, Error)\n> -\t\t\t<< \"Unable to get format on pad \" << pad\n> -\t\t\t<< \": \" << strerror(-ret);\n> +\t\t\t<< \"Unable to get format on pad \" << stream.pad << \"/\"\n> +\t\t\t<< stream.stream << \": \" << strerror(-ret);\n> \t\treturn ret;\n> \t}\n>\n> @@ -1076,6 +1171,66 @@ int V4L2Subdevice::getFormat(unsigned int pad, V4L2SubdeviceFormat *format,\n> }\n>\n> /**\n> + * \\fn V4L2Subdevice::getFormat(unsigned int pad, V4L2SubdeviceFormat *format,\n> + *\tWhence whence)\n> + * \\brief Retrieve the image format set on one of the V4L2 subdevice pads\n> + * \\param[in] pad The 0-indexed pad number the format is to be retrieved from\n> + * \\param[out] format The image bus format\n> + * \\param[in] whence The format to get, \\ref V4L2Subdevice::ActiveFormat\n> + * \"ActiveFormat\" or \\ref V4L2Subdevice::TryFormat \"TryFormat\"\n> + * \\return 0 on success or a negative error code otherwise\n> + */\n> +\n> +/**\n> + * \\brief Set an image format on one of the V4L2 subdevice pads\n> + * \\param[in] stream The stream the format is to be applied to\n> + * \\param[inout] format The image bus format to apply to the stream\n> + * \\param[in] whence The format to set, \\ref V4L2Subdevice::ActiveFormat\n> + * \"ActiveFormat\" or \\ref V4L2Subdevice::TryFormat \"TryFormat\"\n> + *\n> + * Apply the requested image format to the desired stream and return the\n> + * actually applied format parameters, as getFormat() would do.\n> + *\n> + * \\return 0 on success or a negative error code otherwise\n> + */\n> +int V4L2Subdevice::setFormat(const Stream &stream, V4L2SubdeviceFormat *format,\n> +\t\t\t Whence whence)\n> +{\n> +\tstruct v4l2_subdev_format subdevFmt = {};\n> +\tsubdevFmt.which = whence;\n> +\tsubdevFmt.pad = stream.pad;\n> +\tsubdevFmt.stream = stream.stream;\n> +\tsubdevFmt.format.width = format->size.width;\n> +\tsubdevFmt.format.height = format->size.height;\n> +\tsubdevFmt.format.code = format->code;\n> +\tsubdevFmt.format.field = V4L2_FIELD_NONE;\n> +\tif (format->colorSpace) {\n> +\t\tfromColorSpace(format->colorSpace, subdevFmt.format);\n> +\n> +\t\t/* The CSC flag is only applicable to source pads. */\n> +\t\tif (entity_->pads()[stream.pad]->flags() & MEDIA_PAD_FL_SOURCE)\n> +\t\t\tsubdevFmt.format.flags |= V4L2_MBUS_FRAMEFMT_SET_CSC;\n> +\t}\n> +\n> +\tint ret = ioctl(VIDIOC_SUBDEV_S_FMT, &subdevFmt);\n> +\tif (ret) {\n> +\t\tLOG(V4L2, Error)\n> +\t\t\t<< \"Unable to set format on pad \" << stream.pad << \"/\"\n> +\t\t\t<< stream.stream << \": \" << strerror(-ret);\n> +\t\treturn ret;\n> +\t}\n> +\n> +\tformat->size.width = subdevFmt.format.width;\n> +\tformat->size.height = subdevFmt.format.height;\n> +\tformat->code = subdevFmt.format.code;\n> +\tformat->colorSpace = toColorSpace(subdevFmt.format);\n> +\n> +\treturn 0;\n> +}\n> +\n> +/**\n> + * \\fn V4L2Subdevice::setFormat(unsigned int pad, V4L2SubdeviceFormat *format,\n> + *\tWhence whence)\n\nOne rogue space ?\n\n> * \\brief Set an image format on one of the V4L2 subdevice pads\n> * \\param[in] pad The 0-indexed pad number the format is to be applied to\n> * \\param[inout] format The image bus format to apply to the subdevice's pad\n> @@ -1087,39 +1242,6 @@ int V4L2Subdevice::getFormat(unsigned int pad, V4L2SubdeviceFormat *format,\n> *\n> * \\return 0 on success or a negative error code otherwise\n> */\n> -int V4L2Subdevice::setFormat(unsigned int pad, V4L2SubdeviceFormat *format,\n> -\t\t\t Whence whence)\n> -{\n> -\tstruct v4l2_subdev_format subdevFmt = {};\n> -\tsubdevFmt.which = whence;\n> -\tsubdevFmt.pad = pad;\n> -\tsubdevFmt.format.width = format->size.width;\n> -\tsubdevFmt.format.height = format->size.height;\n> -\tsubdevFmt.format.code = format->code;\n> -\tsubdevFmt.format.field = V4L2_FIELD_NONE;\n> -\tif (format->colorSpace) {\n> -\t\tfromColorSpace(format->colorSpace, subdevFmt.format);\n> -\n> -\t\t/* The CSC flag is only applicable to source pads. */\n> -\t\tif (entity_->pads()[pad]->flags() & MEDIA_PAD_FL_SOURCE)\n> -\t\t\tsubdevFmt.format.flags |= V4L2_MBUS_FRAMEFMT_SET_CSC;\n> -\t}\n> -\n> -\tint ret = ioctl(VIDIOC_SUBDEV_S_FMT, &subdevFmt);\n> -\tif (ret) {\n> -\t\tLOG(V4L2, Error)\n> -\t\t\t<< \"Unable to set format on pad \" << pad\n> -\t\t\t<< \": \" << strerror(-ret);\n> -\t\treturn ret;\n> -\t}\n> -\n> -\tformat->size.width = subdevFmt.format.width;\n> -\tformat->size.height = subdevFmt.format.height;\n> -\tformat->code = subdevFmt.format.code;\n> -\tformat->colorSpace = toColorSpace(subdevFmt.format);\n> -\n> -\treturn 0;\n> -}\n>\n> /**\n> * \\brief Retrieve the subdevice's internal routing table\n> @@ -1282,14 +1404,15 @@ std::string V4L2Subdevice::logPrefix() const\n> \treturn \"'\" + entity_->name() + \"'\";\n> }\n>\n> -std::vector V4L2Subdevice::enumPadCodes(unsigned int pad)\n> +std::vector V4L2Subdevice::enumPadCodes(const Stream &stream)\n> {\n> \tstd::vector codes;\n> \tint ret;\n>\n> \tfor (unsigned int index = 0; ; index++) {\n> \t\tstruct v4l2_subdev_mbus_code_enum mbusEnum = {};\n> -\t\tmbusEnum.pad = pad;\n> +\t\tmbusEnum.pad = stream.pad;\n> +\t\tmbusEnum.stream = stream.stream;\n> \t\tmbusEnum.index = index;\n> \t\tmbusEnum.which = V4L2_SUBDEV_FORMAT_ACTIVE;\n>\n> @@ -1302,15 +1425,15 @@ std::vector V4L2Subdevice::enumPadCodes(unsigned int pad)\n>\n> \tif (ret < 0 && ret != -EINVAL) {\n> \t\tLOG(V4L2, Error)\n> -\t\t\t<< \"Unable to enumerate formats on pad \" << pad\n> -\t\t\t<< \": \" << strerror(-ret);\n> +\t\t\t<< \"Unable to enumerate formats on pad \" << stream.pad\n> +\t\t\t<< \"/\" << stream.stream << \": \" << strerror(-ret);\n\nThis could simply be \" << stream << \"\n\n> \t\treturn {};\n> \t}\n>\n> \treturn codes;\n> }\n>\n> -std::vector V4L2Subdevice::enumPadSizes(unsigned int pad,\n> +std::vector V4L2Subdevice::enumPadSizes(const Stream &stream,\n> \t\t\t\t\t\t unsigned int code)\n> {\n> \tstd::vector sizes;\n> @@ -1319,7 +1442,8 @@ std::vector V4L2Subdevice::enumPadSizes(unsigned int pad,\n> \tfor (unsigned int index = 0;; index++) {\n> \t\tstruct v4l2_subdev_frame_size_enum sizeEnum = {};\n> \t\tsizeEnum.index = index;\n> -\t\tsizeEnum.pad = pad;\n> +\t\tsizeEnum.pad = stream.pad;\n> +\t\tsizeEnum.stream = stream.stream;\n> \t\tsizeEnum.code = code;\n> \t\tsizeEnum.which = V4L2_SUBDEV_FORMAT_ACTIVE;\n>\n> @@ -1333,8 +1457,8 @@ std::vector V4L2Subdevice::enumPadSizes(unsigned int pad,\n>\n> \tif (ret < 0 && ret != -EINVAL && ret != -ENOTTY) {\n> \t\tLOG(V4L2, Error)\n> -\t\t\t<< \"Unable to enumerate sizes on pad \" << pad\n> -\t\t\t<< \": \" << strerror(-ret);\n> +\t\t\t<< \"Unable to enumerate sizes on pad \" << stream.pad\n> +\t\t\t<< \"/\" << stream.stream << \": \" << strerror(-ret);\n\nHere as well\n\nAll minors really,\nReviewed-by: Jacopo Mondi \n\nFeel free to fast-track!\n\n> \t\treturn {};\n> \t}\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id E9E5CC326B\n\tfor ;\n\tTue, 5 Mar 2024 08:32:13 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5A3EF62868;\n\tTue, 5 Mar 2024 09:32:13 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0037E62865\n\tfor ;\n\tTue, 5 Mar 2024 09:32:10 +0100 (CET)","from ideasonboard.com (unknown\n\t[IPv6:2001:b07:5d2e:52c9:cc1e:e404:491f:e6ea])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id DE7B6CC8;\n\tTue, 5 Mar 2024 09:31:53 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"mV4yv1NQ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1709627514;\n\tbh=i+lJdZ12baps/q1+8SRQcIB4fgSuEpezUk3Z/PZX1Uw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=mV4yv1NQP/g7d0+lnjtHqAX6J0Re9PJiuH2tqOpDKQtlxnWaaDZdYMLRhLYEXCGL3\n\tBbeDqRwypaFFDt3epVxy0TJ3Eq23liAxffIJtdvoiSKV8oaqS+CpN2o/w3Y2uziasF\n\tehYT+senzKrhDR0CzGvreGxHdoMGtXFyuIe+Dfes=","Date":"Tue, 5 Mar 2024 09:32:07 +0100","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Subject":"Re: [PATCH/RFC 07/32] libcamera: v4l2_subdevice: Add stream support\n\tto get/set functions","Message-ID":"","References":"<20240301212121.9072-1-laurent.pinchart@ideasonboard.com>\n\t<20240301212121.9072-8-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240301212121.9072-8-laurent.pinchart@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org, Sakari Ailus ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28856,"web_url":"https://patchwork.libcamera.org/comment/28856/","msgid":"<20240305091346.GH12503@pendragon.ideasonboard.com>","date":"2024-03-05T09:13:46","subject":"Re: [PATCH/RFC 07/32] libcamera: v4l2_subdevice: Add stream support\n\tto get/set functions","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Tue, Mar 05, 2024 at 09:32:07AM +0100, Jacopo Mondi wrote:\n> Hi Laurent\n> \n> On Fri, Mar 01, 2024 at 11:20:56PM +0200, Laurent Pinchart wrote:\n> > Extend the V4L2Subdevice API with stream support for the functions that\n> > get and set formats and selection rectangles. Add a Stream structure to\n> > identify a subdev pad and stream, and use it to extend the V4L2Subdevice\n> > functions that get and set formats and selection rectangles with stream\n> > support.\n> >\n> > To preserve the existing pad-based API, implement overloaded functions\n> > that wrap the new stream-based API. This allows callers that are not\n> > stream-aware to use a simpler pad-based API, instead of having to\n> > explicitly set the stream number to 0 in all API calls.\n> >\n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > Changes since v1:\n> >\n> > - Drop incorrect comment change\n> > - Use operator<<(Stream)\n> > - Add comparison operators\n> > ---\n> > include/libcamera/internal/v4l2_subdevice.h | 58 ++++-\n> > src/libcamera/v4l2_subdevice.cpp | 252 +++++++++++++++-----\n> > 2 files changed, 240 insertions(+), 70 deletions(-)\n> >\n> > diff --git a/include/libcamera/internal/v4l2_subdevice.h b/include/libcamera/internal/v4l2_subdevice.h\n> > index 1115cfa55594..6cd36730371a 100644\n> > --- a/include/libcamera/internal/v4l2_subdevice.h\n> > +++ b/include/libcamera/internal/v4l2_subdevice.h\n> > @@ -80,6 +80,21 @@ public:\n> > \t\tActiveFormat = V4L2_SUBDEV_FORMAT_ACTIVE,\n> > \t};\n> >\n> > +\tstruct Stream {\n> > +\t\tStream()\n> > +\t\t\t: pad(0), stream(0)\n> > +\t\t{\n> > +\t\t}\n> > +\n> > +\t\tStream(unsigned int p, unsigned int s)\n> > +\t\t\t: pad(p), stream(s)\n> > +\t\t{\n> > +\t\t}\n> > +\n> > +\t\tunsigned int pad;\n> > +\t\tunsigned int stream;\n> > +\t};\n> > +\n> > \tclass Routing : public std::vector\n> > \t{\n> > \tpublic:\n> > @@ -93,17 +108,39 @@ public:\n> >\n> > \tconst MediaEntity *entity() const { return entity_; }\n> >\n> > -\tint getSelection(unsigned int pad, unsigned int target,\n> > +\tint getSelection(const Stream &stream, unsigned int target,\n> > \t\t\t Rectangle *rect);\n> > -\tint setSelection(unsigned int pad, unsigned int target,\n> > +\tint getSelection(unsigned int pad, unsigned int target, Rectangle *rect)\n> > +\t{\n> > +\t\treturn getSelection({ pad, 0 }, target, rect);\n> > +\t}\n> > +\tint setSelection(const Stream &stream, unsigned int target,\n> > \t\t\t Rectangle *rect);\n> > +\tint setSelection(unsigned int pad, unsigned int target, Rectangle *rect)\n> > +\t{\n> > +\t\treturn setSelection({ pad, 0 }, target, rect);\n> > +\t}\n> >\n> > -\tFormats formats(unsigned int pad);\n> > +\tFormats formats(const Stream &stream);\n> > +\tFormats formats(unsigned int pad)\n> > +\t{\n> > +\t\treturn formats({ pad, 0 });\n> > +\t}\n> >\n> > +\tint getFormat(const Stream &stream, V4L2SubdeviceFormat *format,\n> > +\t\t Whence whence = ActiveFormat);\n> > \tint getFormat(unsigned int pad, V4L2SubdeviceFormat *format,\n> > +\t\t Whence whence = ActiveFormat)\n> > +\t{\n> > +\t\treturn getFormat({ pad, 0 }, format, whence);\n> > +\t}\n> > +\tint setFormat(const Stream &stream, V4L2SubdeviceFormat *format,\n> > \t\t Whence whence = ActiveFormat);\n> > \tint setFormat(unsigned int pad, V4L2SubdeviceFormat *format,\n> > -\t\t Whence whence = ActiveFormat);\n> > +\t\t Whence whence = ActiveFormat)\n> > +\t{\n> > +\t\treturn setFormat({ pad, 0 }, format, whence);\n> > +\t}\n> >\n> > \tint getRouting(Routing *routing, Whence whence = ActiveFormat);\n> > \tint setRouting(Routing *routing, Whence whence = ActiveFormat);\n> > @@ -123,8 +160,8 @@ private:\n> > \tstd::optional\n> > \ttoColorSpace(const v4l2_mbus_framefmt &format) const;\n> >\n> > -\tstd::vector enumPadCodes(unsigned int pad);\n> > -\tstd::vector enumPadSizes(unsigned int pad,\n> > +\tstd::vector enumPadCodes(const Stream &stream);\n> > +\tstd::vector enumPadSizes(const Stream &stream,\n> > \t\t\t\t\t unsigned int code);\n> >\n> > \tconst MediaEntity *entity_;\n> > @@ -133,4 +170,13 @@ private:\n> > \tstruct V4L2SubdeviceCapability caps_;\n> > };\n> >\n> > +bool operator==(const V4L2Subdevice::Stream &lhs, const V4L2Subdevice::Stream &rhs);\n> > +static inline bool operator!=(const V4L2Subdevice::Stream &lhs,\n> > +\t\t\t const V4L2Subdevice::Stream &rhs)\n> > +{\n> > +\treturn !(lhs == rhs);\n> > +}\n> > +\n> > +std::ostream &operator<<(std::ostream &out, const V4L2Subdevice::Stream &stream);\n> > +\n> > } /* namespace libcamera */\n> > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp\n> > index dadc4d1dab92..5a3c6a02f57c 100644\n> > --- a/src/libcamera/v4l2_subdevice.cpp\n> > +++ b/src/libcamera/v4l2_subdevice.cpp\n> > @@ -814,6 +814,62 @@ std::ostream &operator<<(std::ostream &out, const V4L2SubdeviceFormat &f)\n> > * \\brief The format operation applies to TRY formats\n> > */\n> >\n> > +/**\n> > + * \\class V4L2Subdevice::Stream\n> > + * \\brief V4L2 subdevice stream\n> > + *\n> > + * This class identifies a subdev stream, by bundling the pad number with the\n> > + * stream number. It is used in all stream-aware functions of the V4L2Subdevice\n> > + * class to identify the stream the functions operate on.\n> > + *\n> > + * \\var V4L2Subdevice::Stream::pad\n> > + * \\brief The 0-indexed pad number\n> > + *\n> > + * \\var V4L2Subdevice::Stream::stream\n> > + * \\brief The stream number\n> > + */\n> > +\n> > +/**\n> > + * \\fn V4L2Subdevice::Stream::Stream()\n> > + * \\brief Construct a Stream with pad and stream set to 0\n> > + */\n> > +\n> > +/**\n> > + * \\fn V4L2Subdevice::Stream::Stream(unsigned int pad, unsigned int stream)\n> > + * \\brief Construct a Stream with a given \\a pad and \\a stream number\n> > + * \\param[in] pad The indexed pad number\n> > + * \\param[in] stream The stream number\n> > + */\n> > +\n> > +/**\n> > + * \\brief Compare streams for equality\n> > + * \\return True if the two streams are equal, false otherwise\n> > + */\n> > +bool operator==(const V4L2Subdevice::Stream &lhs, const V4L2Subdevice::Stream &rhs)\n> > +{\n> > +\treturn lhs.pad == rhs.pad && lhs.stream == rhs.stream;\n> > +}\n> > +\n> > +/**\n> > + * \\fn bool operator!=(const V4L2Subdevice::Stream &lhs, const V4L2Subdevice::Stream &rhs)\n> > + * \\brief Compare streams for inequality\n> > + * \\return True if the two streams are not equal, false otherwise\n> > + */\n> > +\n> > +/**\n> > + * \\brief Insert a text representation of a V4L2Subdevice::Stream into an\n> > + *\toutput stream\n> \n> Compared to the other patches where you have aligned this one to the\n> left with one space, this one is still tab-aligned\n\nI'll move to using a single space.\n\n> > + * \\param[in] out The output stream\n> > + * \\param[in] stream The V4L2Subdevice::Stream\n> > + * \\return The output stream \\a out\n> > + */\n> > +std::ostream &operator<<(std::ostream &out, const V4L2Subdevice::Stream &stream)\n> > +{\n> > +\tout << stream.pad << \"/\" << stream.stream;\n> > +\n> > +\treturn out;\n> > +}\n> > +\n> > /**\n> > * \\class V4L2Subdevice::Routing\n> > * \\brief V4L2 subdevice routing table\n> > @@ -905,7 +961,7 @@ int V4L2Subdevice::open()\n> >\n> > /**\n> > * \\brief Get selection rectangle \\a rect for \\a target\n> > - * \\param[in] pad The 0-indexed pad number the rectangle is retrieved from\n> > + * \\param[in] stream The stream the rectangle is retrieved from\n> > * \\param[in] target The selection target defined by the V4L2_SEL_TGT_* flags\n> > * \\param[out] rect The retrieved selection rectangle\n> > *\n> > @@ -913,13 +969,14 @@ int V4L2Subdevice::open()\n> > *\n> > * \\return 0 on success or a negative error code otherwise\n> > */\n> > -int V4L2Subdevice::getSelection(unsigned int pad, unsigned int target,\n> > +int V4L2Subdevice::getSelection(const Stream &stream, unsigned int target,\n> > \t\t\t\tRectangle *rect)\n> > {\n> > \tstruct v4l2_subdev_selection sel = {};\n> >\n> > \tsel.which = V4L2_SUBDEV_FORMAT_ACTIVE;\n> > -\tsel.pad = pad;\n> > +\tsel.pad = stream.pad;\n> > +\tsel.stream = stream.stream;\n> > \tsel.target = target;\n> > \tsel.flags = 0;\n> >\n> > @@ -927,7 +984,7 @@ int V4L2Subdevice::getSelection(unsigned int pad, unsigned int target,\n> > \tif (ret < 0) {\n> > \t\tLOG(V4L2, Error)\n> > \t\t\t<< \"Unable to get rectangle \" << target << \" on pad \"\n> > -\t\t\t<< pad << \": \" << strerror(-ret);\n> > +\t\t\t<< stream << \": \" << strerror(-ret);\n> > \t\treturn ret;\n> > \t}\n> >\n> > @@ -939,9 +996,20 @@ int V4L2Subdevice::getSelection(unsigned int pad, unsigned int target,\n> > \treturn 0;\n> > }\n> >\n> > +/**\n> > + * \\fn V4L2Subdevice::getSelection(unsigned int pad, unsigned int target,\n> > + *\tRectangle *rect)\n> \n> ditto. Or maybe we didn't get to any conclusion on this alignment\n> thing ?\n> \n> > + * \\brief Get selection rectangle \\a rect for \\a target\n> > + * \\param[in] pad The 0-indexed pad number the rectangle is retrieved from\n> > + * \\param[in] target The selection target defined by the V4L2_SEL_TGT_* flags\n> > + * \\param[out] rect The retrieved selection rectangle\n> > + *\n> > + * \\return 0 on success or a negative error code otherwise\n> > + */\n> > +\n> > /**\n> > * \\brief Set selection rectangle \\a rect for \\a target\n> > - * \\param[in] pad The 0-indexed pad number the rectangle is to be applied to\n> > + * \\param[in] stream The stream the rectangle is to be applied to\n> > * \\param[in] target The selection target defined by the V4L2_SEL_TGT_* flags\n> > * \\param[inout] rect The selection rectangle to be applied\n> > *\n> > @@ -949,13 +1017,14 @@ int V4L2Subdevice::getSelection(unsigned int pad, unsigned int target,\n> > *\n> > * \\return 0 on success or a negative error code otherwise\n> > */\n> > -int V4L2Subdevice::setSelection(unsigned int pad, unsigned int target,\n> > +int V4L2Subdevice::setSelection(const Stream &stream, unsigned int target,\n> > \t\t\t\tRectangle *rect)\n> > {\n> > \tstruct v4l2_subdev_selection sel = {};\n> >\n> > \tsel.which = V4L2_SUBDEV_FORMAT_ACTIVE;\n> > -\tsel.pad = pad;\n> > +\tsel.pad = stream.pad;\n> > +\tsel.stream = stream.stream;\n> > \tsel.target = target;\n> > \tsel.flags = 0;\n> >\n> > @@ -968,7 +1037,7 @@ int V4L2Subdevice::setSelection(unsigned int pad, unsigned int target,\n> > \tif (ret < 0) {\n> > \t\tLOG(V4L2, Error)\n> > \t\t\t<< \"Unable to set rectangle \" << target << \" on pad \"\n> > -\t\t\t<< pad << \": \" << strerror(-ret);\n> > +\t\t\t<< stream << \": \" << strerror(-ret);\n> > \t\treturn ret;\n> > \t}\n> >\n> > @@ -979,26 +1048,40 @@ int V4L2Subdevice::setSelection(unsigned int pad, unsigned int target,\n> >\n> > \treturn 0;\n> > }\n> > +\n> > /**\n> > - * \\brief Enumerate all media bus codes and frame sizes on a \\a pad\n> > - * \\param[in] pad The 0-indexed pad number to enumerate formats on\n> > + * \\fn V4L2Subdevice::setSelection(unsigned int pad, unsigned int target,\n> > + *\tRectangle *rect)\n> \n> here too :)\n> \n> > + * \\brief Set selection rectangle \\a rect for \\a target\n> > + * \\param[in] pad The 0-indexed pad number the rectangle is to be applied to\n> > + * \\param[in] target The selection target defined by the V4L2_SEL_TGT_* flags\n> > + * \\param[inout] rect The selection rectangle to be applied\n> > + *\n> > + * \\todo Define a V4L2SelectionTarget enum for the selection target\n> > + *\n> > + * \\return 0 on success or a negative error code otherwise\n> > + */\n> > +\n> > +/**\n> > + * \\brief Enumerate all media bus codes and frame sizes on a \\a stream\n> > + * \\param[in] stream The stream to enumerate formats for\n> > *\n> > * Enumerate all media bus codes and frame sizes supported by the subdevice on\n> > - * a \\a pad.\n> > + * a \\a stream.\n> > *\n> > * \\return A list of the supported device formats\n> > */\n> > -V4L2Subdevice::Formats V4L2Subdevice::formats(unsigned int pad)\n> > +V4L2Subdevice::Formats V4L2Subdevice::formats(const Stream &stream)\n> > {\n> > \tFormats formats;\n> >\n> > -\tif (pad >= entity_->pads().size()) {\n> > -\t\tLOG(V4L2, Error) << \"Invalid pad: \" << pad;\n> > +\tif (stream.pad >= entity_->pads().size()) {\n> > +\t\tLOG(V4L2, Error) << \"Invalid pad: \" << stream.pad;\n> > \t\treturn {};\n> > \t}\n> >\n> > -\tfor (unsigned int code : enumPadCodes(pad)) {\n> > -\t\tstd::vector sizes = enumPadSizes(pad, code);\n> > +\tfor (unsigned int code : enumPadCodes(stream)) {\n> > +\t\tstd::vector sizes = enumPadSizes(stream, code);\n> > \t\tif (sizes.empty())\n> > \t\t\treturn {};\n> >\n> > @@ -1006,7 +1089,7 @@ V4L2Subdevice::Formats V4L2Subdevice::formats(unsigned int pad)\n> > \t\tif (!inserted.second) {\n> > \t\t\tLOG(V4L2, Error)\n> > \t\t\t\t<< \"Could not add sizes for media bus code \"\n> > -\t\t\t\t<< code << \" on pad \" << pad;\n> > +\t\t\t\t<< code << \" on pad \" << stream.pad;\n> > \t\t\treturn {};\n> > \t\t}\n> > \t}\n> > @@ -1014,6 +1097,17 @@ V4L2Subdevice::Formats V4L2Subdevice::formats(unsigned int pad)\n> > \treturn formats;\n> > }\n> >\n> > +/**\n> > + * \\fn V4L2Subdevice::formats(unsigned int pad)\n> > + * \\brief Enumerate all media bus codes and frame sizes on a \\a pad\n> > + * \\param[in] pad The 0-indexed pad number to enumerate formats on\n> > + *\n> > + * Enumerate all media bus codes and frame sizes supported by the subdevice on\n> > + * a \\a pad\n> > + *\n> > + * \\return A list of the supported device formats\n> > + */\n> > +\n> > std::optional V4L2Subdevice::toColorSpace(const v4l2_mbus_framefmt &format) const\n> > {\n> > \t/*\n> > @@ -1045,25 +1139,26 @@ std::optional V4L2Subdevice::toColorSpace(const v4l2_mbus_framefmt &\n> > }\n> >\n> > /**\n> > - * \\brief Retrieve the image format set on one of the V4L2 subdevice pads\n> > - * \\param[in] pad The 0-indexed pad number the format is to be retrieved from\n> > + * \\brief Retrieve the image format set on one of the V4L2 subdevice streams\n> > + * \\param[in] stream The stream the format is to be retrieved from\n> > * \\param[out] format The image bus format\n> > * \\param[in] whence The format to get, \\ref V4L2Subdevice::ActiveFormat\n> > * \"ActiveFormat\" or \\ref V4L2Subdevice::TryFormat \"TryFormat\"\n> > * \\return 0 on success or a negative error code otherwise\n> > */\n> > -int V4L2Subdevice::getFormat(unsigned int pad, V4L2SubdeviceFormat *format,\n> > +int V4L2Subdevice::getFormat(const Stream &stream, V4L2SubdeviceFormat *format,\n> > \t\t\t Whence whence)\n> > {\n> > \tstruct v4l2_subdev_format subdevFmt = {};\n> > \tsubdevFmt.which = whence;\n> > -\tsubdevFmt.pad = pad;\n> > +\tsubdevFmt.pad = stream.pad;\n> > +\tsubdevFmt.stream = stream.stream;\n> >\n> > \tint ret = ioctl(VIDIOC_SUBDEV_G_FMT, &subdevFmt);\n> > \tif (ret) {\n> > \t\tLOG(V4L2, Error)\n> > -\t\t\t<< \"Unable to get format on pad \" << pad\n> > -\t\t\t<< \": \" << strerror(-ret);\n> > +\t\t\t<< \"Unable to get format on pad \" << stream.pad << \"/\"\n> > +\t\t\t<< stream.stream << \": \" << strerror(-ret);\n> > \t\treturn ret;\n> > \t}\n> >\n> > @@ -1076,6 +1171,66 @@ int V4L2Subdevice::getFormat(unsigned int pad, V4L2SubdeviceFormat *format,\n> > }\n> >\n> > /**\n> > + * \\fn V4L2Subdevice::getFormat(unsigned int pad, V4L2SubdeviceFormat *format,\n> > + *\tWhence whence)\n> > + * \\brief Retrieve the image format set on one of the V4L2 subdevice pads\n> > + * \\param[in] pad The 0-indexed pad number the format is to be retrieved from\n> > + * \\param[out] format The image bus format\n> > + * \\param[in] whence The format to get, \\ref V4L2Subdevice::ActiveFormat\n> > + * \"ActiveFormat\" or \\ref V4L2Subdevice::TryFormat \"TryFormat\"\n> > + * \\return 0 on success or a negative error code otherwise\n> > + */\n> > +\n> > +/**\n> > + * \\brief Set an image format on one of the V4L2 subdevice pads\n> > + * \\param[in] stream The stream the format is to be applied to\n> > + * \\param[inout] format The image bus format to apply to the stream\n> > + * \\param[in] whence The format to set, \\ref V4L2Subdevice::ActiveFormat\n> > + * \"ActiveFormat\" or \\ref V4L2Subdevice::TryFormat \"TryFormat\"\n> > + *\n> > + * Apply the requested image format to the desired stream and return the\n> > + * actually applied format parameters, as getFormat() would do.\n> > + *\n> > + * \\return 0 on success or a negative error code otherwise\n> > + */\n> > +int V4L2Subdevice::setFormat(const Stream &stream, V4L2SubdeviceFormat *format,\n> > +\t\t\t Whence whence)\n> > +{\n> > +\tstruct v4l2_subdev_format subdevFmt = {};\n> > +\tsubdevFmt.which = whence;\n> > +\tsubdevFmt.pad = stream.pad;\n> > +\tsubdevFmt.stream = stream.stream;\n> > +\tsubdevFmt.format.width = format->size.width;\n> > +\tsubdevFmt.format.height = format->size.height;\n> > +\tsubdevFmt.format.code = format->code;\n> > +\tsubdevFmt.format.field = V4L2_FIELD_NONE;\n> > +\tif (format->colorSpace) {\n> > +\t\tfromColorSpace(format->colorSpace, subdevFmt.format);\n> > +\n> > +\t\t/* The CSC flag is only applicable to source pads. */\n> > +\t\tif (entity_->pads()[stream.pad]->flags() & MEDIA_PAD_FL_SOURCE)\n> > +\t\t\tsubdevFmt.format.flags |= V4L2_MBUS_FRAMEFMT_SET_CSC;\n> > +\t}\n> > +\n> > +\tint ret = ioctl(VIDIOC_SUBDEV_S_FMT, &subdevFmt);\n> > +\tif (ret) {\n> > +\t\tLOG(V4L2, Error)\n> > +\t\t\t<< \"Unable to set format on pad \" << stream.pad << \"/\"\n> > +\t\t\t<< stream.stream << \": \" << strerror(-ret);\n> > +\t\treturn ret;\n> > +\t}\n> > +\n> > +\tformat->size.width = subdevFmt.format.width;\n> > +\tformat->size.height = subdevFmt.format.height;\n> > +\tformat->code = subdevFmt.format.code;\n> > +\tformat->colorSpace = toColorSpace(subdevFmt.format);\n> > +\n> > +\treturn 0;\n> > +}\n> > +\n> > +/**\n> > + * \\fn V4L2Subdevice::setFormat(unsigned int pad, V4L2SubdeviceFormat *format,\n> > + *\tWhence whence)\n> \n> One rogue space ?\n> \n> > * \\brief Set an image format on one of the V4L2 subdevice pads\n> > * \\param[in] pad The 0-indexed pad number the format is to be applied to\n> > * \\param[inout] format The image bus format to apply to the subdevice's pad\n> > @@ -1087,39 +1242,6 @@ int V4L2Subdevice::getFormat(unsigned int pad, V4L2SubdeviceFormat *format,\n> > *\n> > * \\return 0 on success or a negative error code otherwise\n> > */\n> > -int V4L2Subdevice::setFormat(unsigned int pad, V4L2SubdeviceFormat *format,\n> > -\t\t\t Whence whence)\n> > -{\n> > -\tstruct v4l2_subdev_format subdevFmt = {};\n> > -\tsubdevFmt.which = whence;\n> > -\tsubdevFmt.pad = pad;\n> > -\tsubdevFmt.format.width = format->size.width;\n> > -\tsubdevFmt.format.height = format->size.height;\n> > -\tsubdevFmt.format.code = format->code;\n> > -\tsubdevFmt.format.field = V4L2_FIELD_NONE;\n> > -\tif (format->colorSpace) {\n> > -\t\tfromColorSpace(format->colorSpace, subdevFmt.format);\n> > -\n> > -\t\t/* The CSC flag is only applicable to source pads. */\n> > -\t\tif (entity_->pads()[pad]->flags() & MEDIA_PAD_FL_SOURCE)\n> > -\t\t\tsubdevFmt.format.flags |= V4L2_MBUS_FRAMEFMT_SET_CSC;\n> > -\t}\n> > -\n> > -\tint ret = ioctl(VIDIOC_SUBDEV_S_FMT, &subdevFmt);\n> > -\tif (ret) {\n> > -\t\tLOG(V4L2, Error)\n> > -\t\t\t<< \"Unable to set format on pad \" << pad\n> > -\t\t\t<< \": \" << strerror(-ret);\n> > -\t\treturn ret;\n> > -\t}\n> > -\n> > -\tformat->size.width = subdevFmt.format.width;\n> > -\tformat->size.height = subdevFmt.format.height;\n> > -\tformat->code = subdevFmt.format.code;\n> > -\tformat->colorSpace = toColorSpace(subdevFmt.format);\n> > -\n> > -\treturn 0;\n> > -}\n> >\n> > /**\n> > * \\brief Retrieve the subdevice's internal routing table\n> > @@ -1282,14 +1404,15 @@ std::string V4L2Subdevice::logPrefix() const\n> > \treturn \"'\" + entity_->name() + \"'\";\n> > }\n> >\n> > -std::vector V4L2Subdevice::enumPadCodes(unsigned int pad)\n> > +std::vector V4L2Subdevice::enumPadCodes(const Stream &stream)\n> > {\n> > \tstd::vector codes;\n> > \tint ret;\n> >\n> > \tfor (unsigned int index = 0; ; index++) {\n> > \t\tstruct v4l2_subdev_mbus_code_enum mbusEnum = {};\n> > -\t\tmbusEnum.pad = pad;\n> > +\t\tmbusEnum.pad = stream.pad;\n> > +\t\tmbusEnum.stream = stream.stream;\n> > \t\tmbusEnum.index = index;\n> > \t\tmbusEnum.which = V4L2_SUBDEV_FORMAT_ACTIVE;\n> >\n> > @@ -1302,15 +1425,15 @@ std::vector V4L2Subdevice::enumPadCodes(unsigned int pad)\n> >\n> > \tif (ret < 0 && ret != -EINVAL) {\n> > \t\tLOG(V4L2, Error)\n> > -\t\t\t<< \"Unable to enumerate formats on pad \" << pad\n> > -\t\t\t<< \": \" << strerror(-ret);\n> > +\t\t\t<< \"Unable to enumerate formats on pad \" << stream.pad\n> > +\t\t\t<< \"/\" << stream.stream << \": \" << strerror(-ret);\n> \n> This could simply be \" << stream << \"\n\nIndeed, will fix.\n\n> > \t\treturn {};\n> > \t}\n> >\n> > \treturn codes;\n> > }\n> >\n> > -std::vector V4L2Subdevice::enumPadSizes(unsigned int pad,\n> > +std::vector V4L2Subdevice::enumPadSizes(const Stream &stream,\n> > \t\t\t\t\t\t unsigned int code)\n> > {\n> > \tstd::vector sizes;\n> > @@ -1319,7 +1442,8 @@ std::vector V4L2Subdevice::enumPadSizes(unsigned int pad,\n> > \tfor (unsigned int index = 0;; index++) {\n> > \t\tstruct v4l2_subdev_frame_size_enum sizeEnum = {};\n> > \t\tsizeEnum.index = index;\n> > -\t\tsizeEnum.pad = pad;\n> > +\t\tsizeEnum.pad = stream.pad;\n> > +\t\tsizeEnum.stream = stream.stream;\n> > \t\tsizeEnum.code = code;\n> > \t\tsizeEnum.which = V4L2_SUBDEV_FORMAT_ACTIVE;\n> >\n> > @@ -1333,8 +1457,8 @@ std::vector V4L2Subdevice::enumPadSizes(unsigned int pad,\n> >\n> > \tif (ret < 0 && ret != -EINVAL && ret != -ENOTTY) {\n> > \t\tLOG(V4L2, Error)\n> > -\t\t\t<< \"Unable to enumerate sizes on pad \" << pad\n> > -\t\t\t<< \": \" << strerror(-ret);\n> > +\t\t\t<< \"Unable to enumerate sizes on pad \" << stream.pad\n> > +\t\t\t<< \"/\" << stream.stream << \": \" << strerror(-ret);\n> \n> Here as well\n> \n> All minors really,\n> Reviewed-by: Jacopo Mondi \n> \n> Feel free to fast-track!\n> \n> > \t\treturn {};\n> > \t}\n> >","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id D4391C326B\n\tfor ;\n\tTue, 5 Mar 2024 09:13:46 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 02BE36286F;\n\tTue, 5 Mar 2024 10:13:46 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2297662865\n\tfor ;\n\tTue, 5 Mar 2024 10:13:45 +0100 (CET)","from pendragon.ideasonboard.com (89-27-53-110.bb.dnainternet.fi\n\t[89.27.53.110])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 02F13A27;\n\tTue, 5 Mar 2024 10:13:27 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"tlyQNuFn\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1709630008;\n\tbh=U95o/Em2Ury8Va4S5tHn/mxfA0hmU+ZbW2S+XQPhzYY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=tlyQNuFniy6j7NiogHD0eSYF5zERXDDix8y844QQq3QFgrKRRYGpxMOC43joTUmL5\n\tY4aSkmFBpkbhQxUtKDVfHDnU6v1SoY8bcC9JkYeOCK3NT7kIelkuIz83PAH0BTcNRm\n\tLvqVnDfQF/3q9EuZqbuiEt9eu1gh/fCroQbFEWNU=","Date":"Tue, 5 Mar 2024 11:13:46 +0200","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Subject":"Re: [PATCH/RFC 07/32] libcamera: v4l2_subdevice: Add stream support\n\tto get/set functions","Message-ID":"<20240305091346.GH12503@pendragon.ideasonboard.com>","References":"<20240301212121.9072-1-laurent.pinchart@ideasonboard.com>\n\t<20240301212121.9072-8-laurent.pinchart@ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org, Sakari Ailus ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]