| Message ID | 20260507213721.2137448-4-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
2026. 05. 07. 23:37 keltezéssel, Laurent Pinchart írta: > The implementation of multiple V4L2Params member functions does not > depend on the class template argument. Move them to a base class, to > avoid inlining them. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- Looks ok to me. Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > src/ipa/libipa/v4l2_params.cpp | 151 ++++++++++++++++++++++++--------- > src/ipa/libipa/v4l2_params.h | 81 +++++------------- > src/ipa/rkisp1/params.cpp | 4 +- > 3 files changed, 132 insertions(+), 104 deletions(-) > > diff --git a/src/ipa/libipa/v4l2_params.cpp b/src/ipa/libipa/v4l2_params.cpp > index d44a366a60b8..476dc0281448 100644 > --- a/src/ipa/libipa/v4l2_params.cpp > +++ b/src/ipa/libipa/v4l2_params.cpp > @@ -117,6 +117,115 @@ namespace ipa { > * \brief Memory area reserved for the ISP configuration block > */ > > +/** > + * \class V4L2ParamsBase > + * \brief Base class for V4L2Params > + * > + * The V4L2ParamsBase is an integral part of V4L2Params. It serves as a > + * container for all code that does not depend on the V4L2Params template > + * arguments, to avoid duplicate copies of inline code. > + */ > + > +/** > + * \brief Construct an instance of V4L2ParamsBase > + * \param[in] data Reference to the v4l2-buffer memory mapped area > + * \param[in] version The ISP parameters version the implementation supports > + */ > +V4L2ParamsBase::V4L2ParamsBase(Span<uint8_t> data, unsigned int version) > + : data_(data) > +{ > + struct v4l2_isp_params_buffer *params = > + reinterpret_cast<struct v4l2_isp_params_buffer *>(data_.data()); > + params->data_size = 0; > + params->version = version; > + > + used_ = offsetof(struct v4l2_isp_params_buffer, data); > +} > + > +/** > + * \fn V4L2ParamsBase::bytesused() > + * \brief Retrieve the used size of the parameters buffer (in bytes) > + * > + * The parameters buffer size is mostly used to populate the v4l2_buffer > + * bytesused field before queueing the buffer to the ISP. > + * > + * \return The number of bytes occupied by the ISP configuration parameters > + */ > + > +/** > + * \brief Populate an ISP configuration block a returns a reference to its > + * memory > + * \param[in] type The ISP block identifier enumerated by the IPA module > + * \param[in] blockType The kernel-defined ISP block identifier, used to > + * populate the block header > + * \param[in] blockSize The ISP block size, used to populate the block header > + * > + * Initialize the block header with \a blockType and \a blockSize and > + * returns a reference to the memory used to store an ISP configuration block. > + * > + * IPA modules that derive the V4L2Params class shall use this function to > + * retrieve the memory area that will be used to construct a V4L2ParamsBlock<T> > + * before returning it to the caller. > + */ > +Span<uint8_t> V4L2ParamsBase::block(uint16_t type, unsigned int blockType, > + size_t blockSize) > +{ > + /* > + * Look up the block in the cache first. If an algorithm > + * requests the same block type twice, it should get the same > + * block. > + */ > + auto cacheIt = blocks_.find(type); > + if (cacheIt != blocks_.end()) > + return cacheIt->second; > + > + /* > + * Make sure we don't run out of space. Assert as otherwise > + * we get a segfault as soon as someone tries to access the > + * empty Span<> returned from here. > + */ > + if (blockSize > data_.size() - used_) { > + LOG(Fatal) > + << "Parameters buffer out of space; potential version mismatch between driver and libcamera"; > + return {}; > + } > + > + /* Allocate a new block, clear its memory, and initialize its header. */ > + Span<uint8_t> block = data_.subspan(used_, blockSize); > + memset(block.data(), 0, block.size()); > + > + struct v4l2_isp_params_block_header *header = > + reinterpret_cast<struct v4l2_isp_params_block_header *>(block.data()); > + header->type = blockType; > + header->size = block.size(); > + > + used_ += block.size(); > + > + struct v4l2_isp_params_buffer *buffer = > + reinterpret_cast<struct v4l2_isp_params_buffer *>(data_.data()); > + buffer->data_size += block.size(); > + > + /* Update the cache. */ > + blocks_[type] = block; > + > + return block; > +} > + > +/** > + * \var V4L2ParamsBase::data_ > + * \brief The ISP parameters buffer memory > + */ > + > +/** > + * \var V4L2ParamsBase::used_ > + * \brief The number of bytes used in the parameters buffer > + */ > + > +/** > + * \var V4L2ParamsBase::blocks_ > + * \brief Cache of ISP configuration blocks > + */ > + > /** > * \class V4L2Params > * \brief Helper class that represent an ISP configuration buffer > @@ -200,54 +309,12 @@ namespace ipa { > * \param[in] version The ISP parameters version the implementation supports > */ > > -/** > - * \fn V4L2Params::bytesused() > - * \brief Retrieve the used size of the parameters buffer (in bytes) > - * > - * The parameters buffer size is mostly used to populate the v4l2_buffer > - * bytesused field before queueing the buffer to the ISP. > - * > - * \return The number of bytes occupied by the ISP configuration parameters > - */ > - > /** > * \fn V4L2Params::block() > * \brief Retrieve the location of an ISP configuration block a return it > * \return A V4L2ParamsBlock instance that points to the ISP configuration block > */ > > -/** > - * \fn V4L2Params::block(uint16_t type, unsigned int blockType, size_t blockSize) > - * \brief Populate an ISP configuration block a returns a reference to its > - * memory > - * \param[in] type The ISP block identifier enumerated by the IPA module > - * \param[in] blockType The kernel-defined ISP block identifier, used to > - * populate the block header > - * \param[in] blockSize The ISP block size, used to populate the block header > - * > - * Initialize the block header with \a blockType and \a blockSize and > - * returns a reference to the memory used to store an ISP configuration block. > - * > - * IPA modules that derive the V4L2Params class shall use this function to > - * retrieve the memory area that will be used to construct a V4L2ParamsBlock<T> > - * before returning it to the caller. > - */ > - > -/** > - * \var V4L2Params::data_ > - * \brief The ISP parameters buffer memory > - */ > - > -/** > - * \var V4L2Params::used_ > - * \brief The number of bytes used in the parameters buffer > - */ > - > -/** > - * \var V4L2Params::blocks_ > - * \brief Cache of ISP configuration blocks > - */ > - > } /* namespace ipa */ > > } /* namespace libcamera */ > diff --git a/src/ipa/libipa/v4l2_params.h b/src/ipa/libipa/v4l2_params.h > index 4f84360ee449..5f57167d7646 100644 > --- a/src/ipa/libipa/v4l2_params.h > +++ b/src/ipa/libipa/v4l2_params.h > @@ -69,25 +69,34 @@ protected: > Span<uint8_t> data_; > }; > > +class V4L2ParamsBase > +{ > +public: > + V4L2ParamsBase(Span<uint8_t> data, unsigned int version); > + > + size_t bytesused() const { return used_; } > + > +protected: > + Span<uint8_t> block(uint16_t type, unsigned int blockType, > + size_t blockSize); > + > + Span<uint8_t> data_; > + size_t used_; > + > + std::map<uint16_t, Span<uint8_t>> blocks_; > +}; > + > template<typename Traits> > -class V4L2Params > +class V4L2Params : public V4L2ParamsBase > { > public: > static_assert(std::is_same_v<std::underlying_type_t<typename Traits::id_type>, uint16_t>); > > V4L2Params(Span<uint8_t> data, unsigned int version) > - : data_(data) > + : V4L2ParamsBase(data, version) > { > - struct v4l2_isp_params_buffer *params = > - reinterpret_cast<struct v4l2_isp_params_buffer *>(data_.data()); > - params->data_size = 0; > - params->version = version; > - > - used_ = offsetof(struct v4l2_isp_params_buffer, data); > } > > - size_t bytesused() const { return used_; } > - > template<typename Traits::id_type Id> > auto block() > { > @@ -96,58 +105,10 @@ public: > using Type = typename Details::type; > constexpr auto kernelId = Details::blockType; > > - auto data = block(utils::to_underlying(Id), kernelId, sizeof(Type)); > + auto data = V4L2ParamsBase::block(utils::to_underlying(Id), > + kernelId, sizeof(Type)); > return V4L2ParamsBlock<Type>(data); > } > - > -protected: > - Span<uint8_t> block(uint16_t type, unsigned int blockType, > - size_t blockSize) > - { > - /* > - * Look up the block in the cache first. If an algorithm > - * requests the same block type twice, it should get the same > - * block. > - */ > - auto cacheIt = blocks_.find(type); > - if (cacheIt != blocks_.end()) > - return cacheIt->second; > - > - /* > - * Make sure we don't run out of space. Assert as otherwise > - * we get a segfault as soon as someone tries to access the > - * empty Span<> returned from here. > - */ > - if (blockSize > data_.size() - used_) { > - LOG(Fatal) > - << "Parameters buffer out of space; potential version mismatch between driver and libcamera"; > - return {}; > - } > - > - /* Allocate a new block, clear its memory, and initialize its header. */ > - Span<uint8_t> block = data_.subspan(used_, blockSize); > - memset(block.data(), 0, block.size()); > - > - struct v4l2_isp_params_block_header *header = > - reinterpret_cast<struct v4l2_isp_params_block_header *>(block.data()); > - header->type = blockType; > - header->size = block.size(); > - > - used_ += block.size(); > - > - reinterpret_cast<struct v4l2_isp_params_buffer *> > - (data_.data())->data_size += block.size(); > - > - /* Update the cache. */ > - blocks_[type] = block; > - > - return block; > - } > - > - Span<uint8_t> data_; > - size_t used_; > - > - std::map<uint16_t, Span<uint8_t>> blocks_; > }; > > } /* namespace ipa */ > diff --git a/src/ipa/rkisp1/params.cpp b/src/ipa/rkisp1/params.cpp > index b8abbdf6ec66..71df0a939de2 100644 > --- a/src/ipa/rkisp1/params.cpp > +++ b/src/ipa/rkisp1/params.cpp > @@ -128,8 +128,8 @@ Span<uint8_t> RkISP1Params::block(BlockType type) > return data_.subspan(info.offset, info.size); > } > > - return V4L2Params::block(utils::to_underlying(type), info.type, > - info.size); > + return V4L2ParamsBase::block(utils::to_underlying(type), info.type, > + info.size); > } > > } /* namespace ipa::rkisp1 */ > -- > Regards, > > Laurent Pinchart >
Quoting Barnabás Pőcze (2026-05-08 09:38:21) > 2026. 05. 07. 23:37 keltezéssel, Laurent Pinchart írta: > > The implementation of multiple V4L2Params member functions does not > > depend on the class template argument. Move them to a base class, to > > avoid inlining them. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > Looks ok to me. > > Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > > src/ipa/libipa/v4l2_params.cpp | 151 ++++++++++++++++++++++++--------- > > src/ipa/libipa/v4l2_params.h | 81 +++++------------- > > src/ipa/rkisp1/params.cpp | 4 +- > > 3 files changed, 132 insertions(+), 104 deletions(-) > > > > diff --git a/src/ipa/libipa/v4l2_params.cpp b/src/ipa/libipa/v4l2_params.cpp > > index d44a366a60b8..476dc0281448 100644 > > --- a/src/ipa/libipa/v4l2_params.cpp > > +++ b/src/ipa/libipa/v4l2_params.cpp > > @@ -117,6 +117,115 @@ namespace ipa { > > * \brief Memory area reserved for the ISP configuration block > > */ > > > > +/** > > + * \class V4L2ParamsBase > > + * \brief Base class for V4L2Params > > + * > > + * The V4L2ParamsBase is an integral part of V4L2Params. It serves as a > > + * container for all code that does not depend on the V4L2Params template > > + * arguments, to avoid duplicate copies of inline code. > > + */ > > + > > +/** > > + * \brief Construct an instance of V4L2ParamsBase > > + * \param[in] data Reference to the v4l2-buffer memory mapped area > > + * \param[in] version The ISP parameters version the implementation supports > > + */ > > +V4L2ParamsBase::V4L2ParamsBase(Span<uint8_t> data, unsigned int version) > > + : data_(data) > > +{ > > + struct v4l2_isp_params_buffer *params = > > + reinterpret_cast<struct v4l2_isp_params_buffer *>(data_.data()); > > + params->data_size = 0; > > + params->version = version; > > + > > + used_ = offsetof(struct v4l2_isp_params_buffer, data); > > +} > > + > > +/** > > + * \fn V4L2ParamsBase::bytesused() > > + * \brief Retrieve the used size of the parameters buffer (in bytes) > > + * > > + * The parameters buffer size is mostly used to populate the v4l2_buffer > > + * bytesused field before queueing the buffer to the ISP. > > + * > > + * \return The number of bytes occupied by the ISP configuration parameters > > + */ > > + > > +/** > > + * \brief Populate an ISP configuration block a returns a reference to its > > + * memory > > + * \param[in] type The ISP block identifier enumerated by the IPA module > > + * \param[in] blockType The kernel-defined ISP block identifier, used to > > + * populate the block header > > + * \param[in] blockSize The ISP block size, used to populate the block header > > + * > > + * Initialize the block header with \a blockType and \a blockSize and > > + * returns a reference to the memory used to store an ISP configuration block. > > + * > > + * IPA modules that derive the V4L2Params class shall use this function to > > + * retrieve the memory area that will be used to construct a V4L2ParamsBlock<T> > > + * before returning it to the caller. > > + */ > > +Span<uint8_t> V4L2ParamsBase::block(uint16_t type, unsigned int blockType, > > + size_t blockSize) > > +{ > > + /* > > + * Look up the block in the cache first. If an algorithm > > + * requests the same block type twice, it should get the same > > + * block. > > + */ > > + auto cacheIt = blocks_.find(type); > > + if (cacheIt != blocks_.end()) > > + return cacheIt->second; > > + > > + /* > > + * Make sure we don't run out of space. Assert as otherwise > > + * we get a segfault as soon as someone tries to access the > > + * empty Span<> returned from here. > > + */ > > + if (blockSize > data_.size() - used_) { > > + LOG(Fatal) > > + << "Parameters buffer out of space; potential version mismatch between driver and libcamera"; > > + return {}; > > + } > > + > > + /* Allocate a new block, clear its memory, and initialize its header. */ > > + Span<uint8_t> block = data_.subspan(used_, blockSize); > > + memset(block.data(), 0, block.size()); > > + > > + struct v4l2_isp_params_block_header *header = > > + reinterpret_cast<struct v4l2_isp_params_block_header *>(block.data()); > > + header->type = blockType; > > + header->size = block.size(); > > + > > + used_ += block.size(); > > + > > + struct v4l2_isp_params_buffer *buffer = > > + reinterpret_cast<struct v4l2_isp_params_buffer *>(data_.data()); > > + buffer->data_size += block.size(); > > + > > + /* Update the cache. */ > > + blocks_[type] = block; > > + > > + return block; > > +} > > + > > +/** > > + * \var V4L2ParamsBase::data_ > > + * \brief The ISP parameters buffer memory > > + */ > > + > > +/** > > + * \var V4L2ParamsBase::used_ > > + * \brief The number of bytes used in the parameters buffer > > + */ > > + > > +/** > > + * \var V4L2ParamsBase::blocks_ > > + * \brief Cache of ISP configuration blocks > > + */ > > + > > /** > > * \class V4L2Params > > * \brief Helper class that represent an ISP configuration buffer > > @@ -200,54 +309,12 @@ namespace ipa { > > * \param[in] version The ISP parameters version the implementation supports > > */ > > > > -/** > > - * \fn V4L2Params::bytesused() > > - * \brief Retrieve the used size of the parameters buffer (in bytes) > > - * > > - * The parameters buffer size is mostly used to populate the v4l2_buffer > > - * bytesused field before queueing the buffer to the ISP. > > - * > > - * \return The number of bytes occupied by the ISP configuration parameters > > - */ > > - > > /** > > * \fn V4L2Params::block() > > * \brief Retrieve the location of an ISP configuration block a return it > > * \return A V4L2ParamsBlock instance that points to the ISP configuration block > > */ > > > > -/** > > - * \fn V4L2Params::block(uint16_t type, unsigned int blockType, size_t blockSize) > > - * \brief Populate an ISP configuration block a returns a reference to its > > - * memory > > - * \param[in] type The ISP block identifier enumerated by the IPA module > > - * \param[in] blockType The kernel-defined ISP block identifier, used to > > - * populate the block header > > - * \param[in] blockSize The ISP block size, used to populate the block header > > - * > > - * Initialize the block header with \a blockType and \a blockSize and > > - * returns a reference to the memory used to store an ISP configuration block. > > - * > > - * IPA modules that derive the V4L2Params class shall use this function to > > - * retrieve the memory area that will be used to construct a V4L2ParamsBlock<T> > > - * before returning it to the caller. > > - */ > > - > > -/** > > - * \var V4L2Params::data_ > > - * \brief The ISP parameters buffer memory > > - */ > > - > > -/** > > - * \var V4L2Params::used_ > > - * \brief The number of bytes used in the parameters buffer > > - */ > > - > > -/** > > - * \var V4L2Params::blocks_ > > - * \brief Cache of ISP configuration blocks > > - */ > > - > > } /* namespace ipa */ > > > > } /* namespace libcamera */ > > diff --git a/src/ipa/libipa/v4l2_params.h b/src/ipa/libipa/v4l2_params.h > > index 4f84360ee449..5f57167d7646 100644 > > --- a/src/ipa/libipa/v4l2_params.h > > +++ b/src/ipa/libipa/v4l2_params.h > > @@ -69,25 +69,34 @@ protected: > > Span<uint8_t> data_; > > }; > > > > +class V4L2ParamsBase > > +{ > > +public: > > + V4L2ParamsBase(Span<uint8_t> data, unsigned int version); > > + > > + size_t bytesused() const { return used_; } > > + > > +protected: > > + Span<uint8_t> block(uint16_t type, unsigned int blockType, > > + size_t blockSize); > > + > > + Span<uint8_t> data_; > > + size_t used_; > > + > > + std::map<uint16_t, Span<uint8_t>> blocks_; > > +}; > > + > > template<typename Traits> > > -class V4L2Params > > +class V4L2Params : public V4L2ParamsBase > > { > > public: > > static_assert(std::is_same_v<std::underlying_type_t<typename Traits::id_type>, uint16_t>); > > > > V4L2Params(Span<uint8_t> data, unsigned int version) > > - : data_(data) > > + : V4L2ParamsBase(data, version) > > { > > - struct v4l2_isp_params_buffer *params = > > - reinterpret_cast<struct v4l2_isp_params_buffer *>(data_.data()); > > - params->data_size = 0; > > - params->version = version; > > - > > - used_ = offsetof(struct v4l2_isp_params_buffer, data); > > } > > > > - size_t bytesused() const { return used_; } > > - > > template<typename Traits::id_type Id> > > auto block() > > { > > @@ -96,58 +105,10 @@ public: > > using Type = typename Details::type; > > constexpr auto kernelId = Details::blockType; > > > > - auto data = block(utils::to_underlying(Id), kernelId, sizeof(Type)); > > + auto data = V4L2ParamsBase::block(utils::to_underlying(Id), > > + kernelId, sizeof(Type)); > > return V4L2ParamsBlock<Type>(data); > > } > > - > > -protected: > > - Span<uint8_t> block(uint16_t type, unsigned int blockType, > > - size_t blockSize) > > - { > > - /* > > - * Look up the block in the cache first. If an algorithm > > - * requests the same block type twice, it should get the same > > - * block. > > - */ > > - auto cacheIt = blocks_.find(type); > > - if (cacheIt != blocks_.end()) > > - return cacheIt->second; > > - > > - /* > > - * Make sure we don't run out of space. Assert as otherwise > > - * we get a segfault as soon as someone tries to access the > > - * empty Span<> returned from here. > > - */ > > - if (blockSize > data_.size() - used_) { > > - LOG(Fatal) > > - << "Parameters buffer out of space; potential version mismatch between driver and libcamera"; > > - return {}; > > - } > > - > > - /* Allocate a new block, clear its memory, and initialize its header. */ > > - Span<uint8_t> block = data_.subspan(used_, blockSize); > > - memset(block.data(), 0, block.size()); > > - > > - struct v4l2_isp_params_block_header *header = > > - reinterpret_cast<struct v4l2_isp_params_block_header *>(block.data()); > > - header->type = blockType; > > - header->size = block.size(); > > - > > - used_ += block.size(); > > - > > - reinterpret_cast<struct v4l2_isp_params_buffer *> > > - (data_.data())->data_size += block.size(); > > - > > - /* Update the cache. */ > > - blocks_[type] = block; > > - > > - return block; > > - } > > - > > - Span<uint8_t> data_; > > - size_t used_; > > - > > - std::map<uint16_t, Span<uint8_t>> blocks_; > > }; > > > > } /* namespace ipa */ > > diff --git a/src/ipa/rkisp1/params.cpp b/src/ipa/rkisp1/params.cpp > > index b8abbdf6ec66..71df0a939de2 100644 > > --- a/src/ipa/rkisp1/params.cpp > > +++ b/src/ipa/rkisp1/params.cpp > > @@ -128,8 +128,8 @@ Span<uint8_t> RkISP1Params::block(BlockType type) > > return data_.subspan(info.offset, info.size); > > } > > > > - return V4L2Params::block(utils::to_underlying(type), info.type, > > - info.size); > > + return V4L2ParamsBase::block(utils::to_underlying(type), info.type, > > + info.size); > > } > > > > } /* namespace ipa::rkisp1 */ > > -- > > Regards, > > > > Laurent Pinchart > > >
Hi Laurent On Fri, May 08, 2026 at 12:37:21AM +0300, Laurent Pinchart wrote: > The implementation of multiple V4L2Params member functions does not > depend on the class template argument. Move them to a base class, to > avoid inlining them. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > src/ipa/libipa/v4l2_params.cpp | 151 ++++++++++++++++++++++++--------- > src/ipa/libipa/v4l2_params.h | 81 +++++------------- > src/ipa/rkisp1/params.cpp | 4 +- > 3 files changed, 132 insertions(+), 104 deletions(-) > > diff --git a/src/ipa/libipa/v4l2_params.cpp b/src/ipa/libipa/v4l2_params.cpp > index d44a366a60b8..476dc0281448 100644 > --- a/src/ipa/libipa/v4l2_params.cpp > +++ b/src/ipa/libipa/v4l2_params.cpp > @@ -117,6 +117,115 @@ namespace ipa { > * \brief Memory area reserved for the ISP configuration block > */ > > +/** > + * \class V4L2ParamsBase > + * \brief Base class for V4L2Params > + * > + * The V4L2ParamsBase is an integral part of V4L2Params. It serves as a > + * container for all code that does not depend on the V4L2Params template > + * arguments, to avoid duplicate copies of inline code. > + */ > + > +/** > + * \brief Construct an instance of V4L2ParamsBase > + * \param[in] data Reference to the v4l2-buffer memory mapped area > + * \param[in] version The ISP parameters version the implementation supports > + */ > +V4L2ParamsBase::V4L2ParamsBase(Span<uint8_t> data, unsigned int version) > + : data_(data) > +{ > + struct v4l2_isp_params_buffer *params = > + reinterpret_cast<struct v4l2_isp_params_buffer *>(data_.data()); > + params->data_size = 0; > + params->version = version; > + > + used_ = offsetof(struct v4l2_isp_params_buffer, data); > +} > + > +/** > + * \fn V4L2ParamsBase::bytesused() > + * \brief Retrieve the used size of the parameters buffer (in bytes) > + * > + * The parameters buffer size is mostly used to populate the v4l2_buffer > + * bytesused field before queueing the buffer to the ISP. > + * > + * \return The number of bytes occupied by the ISP configuration parameters > + */ > + > +/** > + * \brief Populate an ISP configuration block a returns a reference to its > + * memory > + * \param[in] type The ISP block identifier enumerated by the IPA module > + * \param[in] blockType The kernel-defined ISP block identifier, used to > + * populate the block header > + * \param[in] blockSize The ISP block size, used to populate the block header > + * > + * Initialize the block header with \a blockType and \a blockSize and > + * returns a reference to the memory used to store an ISP configuration block. > + * > + * IPA modules that derive the V4L2Params class shall use this function to > + * retrieve the memory area that will be used to construct a V4L2ParamsBlock<T> > + * before returning it to the caller. > + */ > +Span<uint8_t> V4L2ParamsBase::block(uint16_t type, unsigned int blockType, > + size_t blockSize) > +{ > + /* > + * Look up the block in the cache first. If an algorithm > + * requests the same block type twice, it should get the same > + * block. > + */ > + auto cacheIt = blocks_.find(type); > + if (cacheIt != blocks_.end()) > + return cacheIt->second; > + > + /* > + * Make sure we don't run out of space. Assert as otherwise > + * we get a segfault as soon as someone tries to access the > + * empty Span<> returned from here. > + */ > + if (blockSize > data_.size() - used_) { > + LOG(Fatal) > + << "Parameters buffer out of space; potential version mismatch between driver and libcamera"; > + return {}; > + } > + > + /* Allocate a new block, clear its memory, and initialize its header. */ > + Span<uint8_t> block = data_.subspan(used_, blockSize); > + memset(block.data(), 0, block.size()); > + > + struct v4l2_isp_params_block_header *header = > + reinterpret_cast<struct v4l2_isp_params_block_header *>(block.data()); > + header->type = blockType; > + header->size = block.size(); > + > + used_ += block.size(); > + > + struct v4l2_isp_params_buffer *buffer = > + reinterpret_cast<struct v4l2_isp_params_buffer *>(data_.data()); > + buffer->data_size += block.size(); > + > + /* Update the cache. */ > + blocks_[type] = block; > + > + return block; > +} > + > +/** > + * \var V4L2ParamsBase::data_ > + * \brief The ISP parameters buffer memory > + */ > + > +/** > + * \var V4L2ParamsBase::used_ > + * \brief The number of bytes used in the parameters buffer > + */ > + > +/** > + * \var V4L2ParamsBase::blocks_ > + * \brief Cache of ISP configuration blocks > + */ > + > /** > * \class V4L2Params > * \brief Helper class that represent an ISP configuration buffer > @@ -200,54 +309,12 @@ namespace ipa { > * \param[in] version The ISP parameters version the implementation supports > */ > > -/** > - * \fn V4L2Params::bytesused() > - * \brief Retrieve the used size of the parameters buffer (in bytes) > - * > - * The parameters buffer size is mostly used to populate the v4l2_buffer > - * bytesused field before queueing the buffer to the ISP. > - * > - * \return The number of bytes occupied by the ISP configuration parameters > - */ > - > /** > * \fn V4L2Params::block() > * \brief Retrieve the location of an ISP configuration block a return it > * \return A V4L2ParamsBlock instance that points to the ISP configuration block > */ > > -/** > - * \fn V4L2Params::block(uint16_t type, unsigned int blockType, size_t blockSize) > - * \brief Populate an ISP configuration block a returns a reference to its > - * memory > - * \param[in] type The ISP block identifier enumerated by the IPA module > - * \param[in] blockType The kernel-defined ISP block identifier, used to > - * populate the block header > - * \param[in] blockSize The ISP block size, used to populate the block header > - * > - * Initialize the block header with \a blockType and \a blockSize and > - * returns a reference to the memory used to store an ISP configuration block. > - * > - * IPA modules that derive the V4L2Params class shall use this function to > - * retrieve the memory area that will be used to construct a V4L2ParamsBlock<T> > - * before returning it to the caller. > - */ > - > -/** > - * \var V4L2Params::data_ > - * \brief The ISP parameters buffer memory > - */ > - > -/** > - * \var V4L2Params::used_ > - * \brief The number of bytes used in the parameters buffer > - */ > - > -/** > - * \var V4L2Params::blocks_ > - * \brief Cache of ISP configuration blocks > - */ > - > } /* namespace ipa */ > > } /* namespace libcamera */ > diff --git a/src/ipa/libipa/v4l2_params.h b/src/ipa/libipa/v4l2_params.h > index 4f84360ee449..5f57167d7646 100644 > --- a/src/ipa/libipa/v4l2_params.h > +++ b/src/ipa/libipa/v4l2_params.h > @@ -69,25 +69,34 @@ protected: > Span<uint8_t> data_; > }; > > +class V4L2ParamsBase > +{ > +public: > + V4L2ParamsBase(Span<uint8_t> data, unsigned int version); Should the constructor be protected ? > + > + size_t bytesused() const { return used_; } > + > +protected: > + Span<uint8_t> block(uint16_t type, unsigned int blockType, > + size_t blockSize); > + > + Span<uint8_t> data_; > + size_t used_; > + > + std::map<uint16_t, Span<uint8_t>> blocks_; > +}; > + > template<typename Traits> > -class V4L2Params > +class V4L2Params : public V4L2ParamsBase > { > public: > static_assert(std::is_same_v<std::underlying_type_t<typename Traits::id_type>, uint16_t>); > > V4L2Params(Span<uint8_t> data, unsigned int version) > - : data_(data) > + : V4L2ParamsBase(data, version) > { > - struct v4l2_isp_params_buffer *params = > - reinterpret_cast<struct v4l2_isp_params_buffer *>(data_.data()); > - params->data_size = 0; > - params->version = version; > - > - used_ = offsetof(struct v4l2_isp_params_buffer, data); > } > > - size_t bytesused() const { return used_; } > - > template<typename Traits::id_type Id> > auto block() > { > @@ -96,58 +105,10 @@ public: > using Type = typename Details::type; > constexpr auto kernelId = Details::blockType; > > - auto data = block(utils::to_underlying(Id), kernelId, sizeof(Type)); > + auto data = V4L2ParamsBase::block(utils::to_underlying(Id), > + kernelId, sizeof(Type)); > return V4L2ParamsBlock<Type>(data); > } > - > -protected: > - Span<uint8_t> block(uint16_t type, unsigned int blockType, > - size_t blockSize) > - { > - /* > - * Look up the block in the cache first. If an algorithm > - * requests the same block type twice, it should get the same > - * block. > - */ > - auto cacheIt = blocks_.find(type); > - if (cacheIt != blocks_.end()) > - return cacheIt->second; > - > - /* > - * Make sure we don't run out of space. Assert as otherwise > - * we get a segfault as soon as someone tries to access the > - * empty Span<> returned from here. > - */ > - if (blockSize > data_.size() - used_) { > - LOG(Fatal) > - << "Parameters buffer out of space; potential version mismatch between driver and libcamera"; > - return {}; > - } > - > - /* Allocate a new block, clear its memory, and initialize its header. */ > - Span<uint8_t> block = data_.subspan(used_, blockSize); > - memset(block.data(), 0, block.size()); > - > - struct v4l2_isp_params_block_header *header = > - reinterpret_cast<struct v4l2_isp_params_block_header *>(block.data()); > - header->type = blockType; > - header->size = block.size(); > - > - used_ += block.size(); > - > - reinterpret_cast<struct v4l2_isp_params_buffer *> > - (data_.data())->data_size += block.size(); > - > - /* Update the cache. */ > - blocks_[type] = block; > - > - return block; > - } > - > - Span<uint8_t> data_; > - size_t used_; > - > - std::map<uint16_t, Span<uint8_t>> blocks_; > }; > > } /* namespace ipa */ > diff --git a/src/ipa/rkisp1/params.cpp b/src/ipa/rkisp1/params.cpp > index b8abbdf6ec66..71df0a939de2 100644 > --- a/src/ipa/rkisp1/params.cpp > +++ b/src/ipa/rkisp1/params.cpp > @@ -128,8 +128,8 @@ Span<uint8_t> RkISP1Params::block(BlockType type) > return data_.subspan(info.offset, info.size); > } > > - return V4L2Params::block(utils::to_underlying(type), info.type, > - info.size); > + return V4L2ParamsBase::block(utils::to_underlying(type), info.type, > + info.size); > } > > } /* namespace ipa::rkisp1 */ > -- > Regards, > > Laurent Pinchart >
On Fri, May 08, 2026 at 11:45:55AM +0200, Jacopo Mondi wrote: > On Fri, May 08, 2026 at 12:37:21AM +0300, Laurent Pinchart wrote: > > The implementation of multiple V4L2Params member functions does not > > depend on the class template argument. Move them to a base class, to > > avoid inlining them. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > src/ipa/libipa/v4l2_params.cpp | 151 ++++++++++++++++++++++++--------- > > src/ipa/libipa/v4l2_params.h | 81 +++++------------- > > src/ipa/rkisp1/params.cpp | 4 +- > > 3 files changed, 132 insertions(+), 104 deletions(-) > > > > diff --git a/src/ipa/libipa/v4l2_params.cpp b/src/ipa/libipa/v4l2_params.cpp > > index d44a366a60b8..476dc0281448 100644 > > --- a/src/ipa/libipa/v4l2_params.cpp > > +++ b/src/ipa/libipa/v4l2_params.cpp > > @@ -117,6 +117,115 @@ namespace ipa { > > * \brief Memory area reserved for the ISP configuration block > > */ > > > > +/** > > + * \class V4L2ParamsBase > > + * \brief Base class for V4L2Params > > + * > > + * The V4L2ParamsBase is an integral part of V4L2Params. It serves as a > > + * container for all code that does not depend on the V4L2Params template > > + * arguments, to avoid duplicate copies of inline code. > > + */ > > + > > +/** > > + * \brief Construct an instance of V4L2ParamsBase > > + * \param[in] data Reference to the v4l2-buffer memory mapped area > > + * \param[in] version The ISP parameters version the implementation supports > > + */ > > +V4L2ParamsBase::V4L2ParamsBase(Span<uint8_t> data, unsigned int version) > > + : data_(data) > > +{ > > + struct v4l2_isp_params_buffer *params = > > + reinterpret_cast<struct v4l2_isp_params_buffer *>(data_.data()); > > + params->data_size = 0; > > + params->version = version; > > + > > + used_ = offsetof(struct v4l2_isp_params_buffer, data); > > +} > > + > > +/** > > + * \fn V4L2ParamsBase::bytesused() > > + * \brief Retrieve the used size of the parameters buffer (in bytes) > > + * > > + * The parameters buffer size is mostly used to populate the v4l2_buffer > > + * bytesused field before queueing the buffer to the ISP. > > + * > > + * \return The number of bytes occupied by the ISP configuration parameters > > + */ > > + > > +/** > > + * \brief Populate an ISP configuration block a returns a reference to its > > + * memory > > + * \param[in] type The ISP block identifier enumerated by the IPA module > > + * \param[in] blockType The kernel-defined ISP block identifier, used to > > + * populate the block header > > + * \param[in] blockSize The ISP block size, used to populate the block header > > + * > > + * Initialize the block header with \a blockType and \a blockSize and > > + * returns a reference to the memory used to store an ISP configuration block. > > + * > > + * IPA modules that derive the V4L2Params class shall use this function to > > + * retrieve the memory area that will be used to construct a V4L2ParamsBlock<T> > > + * before returning it to the caller. > > + */ > > +Span<uint8_t> V4L2ParamsBase::block(uint16_t type, unsigned int blockType, > > + size_t blockSize) > > +{ > > + /* > > + * Look up the block in the cache first. If an algorithm > > + * requests the same block type twice, it should get the same > > + * block. > > + */ > > + auto cacheIt = blocks_.find(type); > > + if (cacheIt != blocks_.end()) > > + return cacheIt->second; > > + > > + /* > > + * Make sure we don't run out of space. Assert as otherwise > > + * we get a segfault as soon as someone tries to access the > > + * empty Span<> returned from here. > > + */ > > + if (blockSize > data_.size() - used_) { > > + LOG(Fatal) > > + << "Parameters buffer out of space; potential version mismatch between driver and libcamera"; > > + return {}; > > + } > > + > > + /* Allocate a new block, clear its memory, and initialize its header. */ > > + Span<uint8_t> block = data_.subspan(used_, blockSize); > > + memset(block.data(), 0, block.size()); > > + > > + struct v4l2_isp_params_block_header *header = > > + reinterpret_cast<struct v4l2_isp_params_block_header *>(block.data()); > > + header->type = blockType; > > + header->size = block.size(); > > + > > + used_ += block.size(); > > + > > + struct v4l2_isp_params_buffer *buffer = > > + reinterpret_cast<struct v4l2_isp_params_buffer *>(data_.data()); > > + buffer->data_size += block.size(); > > + > > + /* Update the cache. */ > > + blocks_[type] = block; > > + > > + return block; > > +} > > + > > +/** > > + * \var V4L2ParamsBase::data_ > > + * \brief The ISP parameters buffer memory > > + */ > > + > > +/** > > + * \var V4L2ParamsBase::used_ > > + * \brief The number of bytes used in the parameters buffer > > + */ > > + > > +/** > > + * \var V4L2ParamsBase::blocks_ > > + * \brief Cache of ISP configuration blocks > > + */ > > + > > /** > > * \class V4L2Params > > * \brief Helper class that represent an ISP configuration buffer > > @@ -200,54 +309,12 @@ namespace ipa { > > * \param[in] version The ISP parameters version the implementation supports > > */ > > > > -/** > > - * \fn V4L2Params::bytesused() > > - * \brief Retrieve the used size of the parameters buffer (in bytes) > > - * > > - * The parameters buffer size is mostly used to populate the v4l2_buffer > > - * bytesused field before queueing the buffer to the ISP. > > - * > > - * \return The number of bytes occupied by the ISP configuration parameters > > - */ > > - > > /** > > * \fn V4L2Params::block() > > * \brief Retrieve the location of an ISP configuration block a return it > > * \return A V4L2ParamsBlock instance that points to the ISP configuration block > > */ > > > > -/** > > - * \fn V4L2Params::block(uint16_t type, unsigned int blockType, size_t blockSize) > > - * \brief Populate an ISP configuration block a returns a reference to its > > - * memory > > - * \param[in] type The ISP block identifier enumerated by the IPA module > > - * \param[in] blockType The kernel-defined ISP block identifier, used to > > - * populate the block header > > - * \param[in] blockSize The ISP block size, used to populate the block header > > - * > > - * Initialize the block header with \a blockType and \a blockSize and > > - * returns a reference to the memory used to store an ISP configuration block. > > - * > > - * IPA modules that derive the V4L2Params class shall use this function to > > - * retrieve the memory area that will be used to construct a V4L2ParamsBlock<T> > > - * before returning it to the caller. > > - */ > > - > > -/** > > - * \var V4L2Params::data_ > > - * \brief The ISP parameters buffer memory > > - */ > > - > > -/** > > - * \var V4L2Params::used_ > > - * \brief The number of bytes used in the parameters buffer > > - */ > > - > > -/** > > - * \var V4L2Params::blocks_ > > - * \brief Cache of ISP configuration blocks > > - */ > > - > > } /* namespace ipa */ > > > > } /* namespace libcamera */ > > diff --git a/src/ipa/libipa/v4l2_params.h b/src/ipa/libipa/v4l2_params.h > > index 4f84360ee449..5f57167d7646 100644 > > --- a/src/ipa/libipa/v4l2_params.h > > +++ b/src/ipa/libipa/v4l2_params.h > > @@ -69,25 +69,34 @@ protected: > > Span<uint8_t> data_; > > }; > > > > +class V4L2ParamsBase > > +{ > > +public: > > + V4L2ParamsBase(Span<uint8_t> data, unsigned int version); > > Should the constructor be protected ? I suppose it won't hurt, so nobody will try to instantiate this base class directly (I'm not sure why anyone would, but who knows ?). > > + > > + size_t bytesused() const { return used_; } > > + > > +protected: > > + Span<uint8_t> block(uint16_t type, unsigned int blockType, > > + size_t blockSize); > > + > > + Span<uint8_t> data_; > > + size_t used_; > > + > > + std::map<uint16_t, Span<uint8_t>> blocks_; > > +}; > > + > > template<typename Traits> > > -class V4L2Params > > +class V4L2Params : public V4L2ParamsBase > > { > > public: > > static_assert(std::is_same_v<std::underlying_type_t<typename Traits::id_type>, uint16_t>); > > > > V4L2Params(Span<uint8_t> data, unsigned int version) > > - : data_(data) > > + : V4L2ParamsBase(data, version) > > { > > - struct v4l2_isp_params_buffer *params = > > - reinterpret_cast<struct v4l2_isp_params_buffer *>(data_.data()); > > - params->data_size = 0; > > - params->version = version; > > - > > - used_ = offsetof(struct v4l2_isp_params_buffer, data); > > } > > > > - size_t bytesused() const { return used_; } > > - > > template<typename Traits::id_type Id> > > auto block() > > { > > @@ -96,58 +105,10 @@ public: > > using Type = typename Details::type; > > constexpr auto kernelId = Details::blockType; > > > > - auto data = block(utils::to_underlying(Id), kernelId, sizeof(Type)); > > + auto data = V4L2ParamsBase::block(utils::to_underlying(Id), > > + kernelId, sizeof(Type)); > > return V4L2ParamsBlock<Type>(data); > > } > > - > > -protected: > > - Span<uint8_t> block(uint16_t type, unsigned int blockType, > > - size_t blockSize) > > - { > > - /* > > - * Look up the block in the cache first. If an algorithm > > - * requests the same block type twice, it should get the same > > - * block. > > - */ > > - auto cacheIt = blocks_.find(type); > > - if (cacheIt != blocks_.end()) > > - return cacheIt->second; > > - > > - /* > > - * Make sure we don't run out of space. Assert as otherwise > > - * we get a segfault as soon as someone tries to access the > > - * empty Span<> returned from here. > > - */ > > - if (blockSize > data_.size() - used_) { > > - LOG(Fatal) > > - << "Parameters buffer out of space; potential version mismatch between driver and libcamera"; > > - return {}; > > - } > > - > > - /* Allocate a new block, clear its memory, and initialize its header. */ > > - Span<uint8_t> block = data_.subspan(used_, blockSize); > > - memset(block.data(), 0, block.size()); > > - > > - struct v4l2_isp_params_block_header *header = > > - reinterpret_cast<struct v4l2_isp_params_block_header *>(block.data()); > > - header->type = blockType; > > - header->size = block.size(); > > - > > - used_ += block.size(); > > - > > - reinterpret_cast<struct v4l2_isp_params_buffer *> > > - (data_.data())->data_size += block.size(); > > - > > - /* Update the cache. */ > > - blocks_[type] = block; > > - > > - return block; > > - } > > - > > - Span<uint8_t> data_; > > - size_t used_; > > - > > - std::map<uint16_t, Span<uint8_t>> blocks_; > > }; > > > > } /* namespace ipa */ > > diff --git a/src/ipa/rkisp1/params.cpp b/src/ipa/rkisp1/params.cpp > > index b8abbdf6ec66..71df0a939de2 100644 > > --- a/src/ipa/rkisp1/params.cpp > > +++ b/src/ipa/rkisp1/params.cpp > > @@ -128,8 +128,8 @@ Span<uint8_t> RkISP1Params::block(BlockType type) > > return data_.subspan(info.offset, info.size); > > } > > > > - return V4L2Params::block(utils::to_underlying(type), info.type, > > - info.size); > > + return V4L2ParamsBase::block(utils::to_underlying(type), info.type, > > + info.size); > > } > > > > } /* namespace ipa::rkisp1 */ > > -- > > Regards, > > > > Laurent Pinchart > >
diff --git a/src/ipa/libipa/v4l2_params.cpp b/src/ipa/libipa/v4l2_params.cpp index d44a366a60b8..476dc0281448 100644 --- a/src/ipa/libipa/v4l2_params.cpp +++ b/src/ipa/libipa/v4l2_params.cpp @@ -117,6 +117,115 @@ namespace ipa { * \brief Memory area reserved for the ISP configuration block */ +/** + * \class V4L2ParamsBase + * \brief Base class for V4L2Params + * + * The V4L2ParamsBase is an integral part of V4L2Params. It serves as a + * container for all code that does not depend on the V4L2Params template + * arguments, to avoid duplicate copies of inline code. + */ + +/** + * \brief Construct an instance of V4L2ParamsBase + * \param[in] data Reference to the v4l2-buffer memory mapped area + * \param[in] version The ISP parameters version the implementation supports + */ +V4L2ParamsBase::V4L2ParamsBase(Span<uint8_t> data, unsigned int version) + : data_(data) +{ + struct v4l2_isp_params_buffer *params = + reinterpret_cast<struct v4l2_isp_params_buffer *>(data_.data()); + params->data_size = 0; + params->version = version; + + used_ = offsetof(struct v4l2_isp_params_buffer, data); +} + +/** + * \fn V4L2ParamsBase::bytesused() + * \brief Retrieve the used size of the parameters buffer (in bytes) + * + * The parameters buffer size is mostly used to populate the v4l2_buffer + * bytesused field before queueing the buffer to the ISP. + * + * \return The number of bytes occupied by the ISP configuration parameters + */ + +/** + * \brief Populate an ISP configuration block a returns a reference to its + * memory + * \param[in] type The ISP block identifier enumerated by the IPA module + * \param[in] blockType The kernel-defined ISP block identifier, used to + * populate the block header + * \param[in] blockSize The ISP block size, used to populate the block header + * + * Initialize the block header with \a blockType and \a blockSize and + * returns a reference to the memory used to store an ISP configuration block. + * + * IPA modules that derive the V4L2Params class shall use this function to + * retrieve the memory area that will be used to construct a V4L2ParamsBlock<T> + * before returning it to the caller. + */ +Span<uint8_t> V4L2ParamsBase::block(uint16_t type, unsigned int blockType, + size_t blockSize) +{ + /* + * Look up the block in the cache first. If an algorithm + * requests the same block type twice, it should get the same + * block. + */ + auto cacheIt = blocks_.find(type); + if (cacheIt != blocks_.end()) + return cacheIt->second; + + /* + * Make sure we don't run out of space. Assert as otherwise + * we get a segfault as soon as someone tries to access the + * empty Span<> returned from here. + */ + if (blockSize > data_.size() - used_) { + LOG(Fatal) + << "Parameters buffer out of space; potential version mismatch between driver and libcamera"; + return {}; + } + + /* Allocate a new block, clear its memory, and initialize its header. */ + Span<uint8_t> block = data_.subspan(used_, blockSize); + memset(block.data(), 0, block.size()); + + struct v4l2_isp_params_block_header *header = + reinterpret_cast<struct v4l2_isp_params_block_header *>(block.data()); + header->type = blockType; + header->size = block.size(); + + used_ += block.size(); + + struct v4l2_isp_params_buffer *buffer = + reinterpret_cast<struct v4l2_isp_params_buffer *>(data_.data()); + buffer->data_size += block.size(); + + /* Update the cache. */ + blocks_[type] = block; + + return block; +} + +/** + * \var V4L2ParamsBase::data_ + * \brief The ISP parameters buffer memory + */ + +/** + * \var V4L2ParamsBase::used_ + * \brief The number of bytes used in the parameters buffer + */ + +/** + * \var V4L2ParamsBase::blocks_ + * \brief Cache of ISP configuration blocks + */ + /** * \class V4L2Params * \brief Helper class that represent an ISP configuration buffer @@ -200,54 +309,12 @@ namespace ipa { * \param[in] version The ISP parameters version the implementation supports */ -/** - * \fn V4L2Params::bytesused() - * \brief Retrieve the used size of the parameters buffer (in bytes) - * - * The parameters buffer size is mostly used to populate the v4l2_buffer - * bytesused field before queueing the buffer to the ISP. - * - * \return The number of bytes occupied by the ISP configuration parameters - */ - /** * \fn V4L2Params::block() * \brief Retrieve the location of an ISP configuration block a return it * \return A V4L2ParamsBlock instance that points to the ISP configuration block */ -/** - * \fn V4L2Params::block(uint16_t type, unsigned int blockType, size_t blockSize) - * \brief Populate an ISP configuration block a returns a reference to its - * memory - * \param[in] type The ISP block identifier enumerated by the IPA module - * \param[in] blockType The kernel-defined ISP block identifier, used to - * populate the block header - * \param[in] blockSize The ISP block size, used to populate the block header - * - * Initialize the block header with \a blockType and \a blockSize and - * returns a reference to the memory used to store an ISP configuration block. - * - * IPA modules that derive the V4L2Params class shall use this function to - * retrieve the memory area that will be used to construct a V4L2ParamsBlock<T> - * before returning it to the caller. - */ - -/** - * \var V4L2Params::data_ - * \brief The ISP parameters buffer memory - */ - -/** - * \var V4L2Params::used_ - * \brief The number of bytes used in the parameters buffer - */ - -/** - * \var V4L2Params::blocks_ - * \brief Cache of ISP configuration blocks - */ - } /* namespace ipa */ } /* namespace libcamera */ diff --git a/src/ipa/libipa/v4l2_params.h b/src/ipa/libipa/v4l2_params.h index 4f84360ee449..5f57167d7646 100644 --- a/src/ipa/libipa/v4l2_params.h +++ b/src/ipa/libipa/v4l2_params.h @@ -69,25 +69,34 @@ protected: Span<uint8_t> data_; }; +class V4L2ParamsBase +{ +public: + V4L2ParamsBase(Span<uint8_t> data, unsigned int version); + + size_t bytesused() const { return used_; } + +protected: + Span<uint8_t> block(uint16_t type, unsigned int blockType, + size_t blockSize); + + Span<uint8_t> data_; + size_t used_; + + std::map<uint16_t, Span<uint8_t>> blocks_; +}; + template<typename Traits> -class V4L2Params +class V4L2Params : public V4L2ParamsBase { public: static_assert(std::is_same_v<std::underlying_type_t<typename Traits::id_type>, uint16_t>); V4L2Params(Span<uint8_t> data, unsigned int version) - : data_(data) + : V4L2ParamsBase(data, version) { - struct v4l2_isp_params_buffer *params = - reinterpret_cast<struct v4l2_isp_params_buffer *>(data_.data()); - params->data_size = 0; - params->version = version; - - used_ = offsetof(struct v4l2_isp_params_buffer, data); } - size_t bytesused() const { return used_; } - template<typename Traits::id_type Id> auto block() { @@ -96,58 +105,10 @@ public: using Type = typename Details::type; constexpr auto kernelId = Details::blockType; - auto data = block(utils::to_underlying(Id), kernelId, sizeof(Type)); + auto data = V4L2ParamsBase::block(utils::to_underlying(Id), + kernelId, sizeof(Type)); return V4L2ParamsBlock<Type>(data); } - -protected: - Span<uint8_t> block(uint16_t type, unsigned int blockType, - size_t blockSize) - { - /* - * Look up the block in the cache first. If an algorithm - * requests the same block type twice, it should get the same - * block. - */ - auto cacheIt = blocks_.find(type); - if (cacheIt != blocks_.end()) - return cacheIt->second; - - /* - * Make sure we don't run out of space. Assert as otherwise - * we get a segfault as soon as someone tries to access the - * empty Span<> returned from here. - */ - if (blockSize > data_.size() - used_) { - LOG(Fatal) - << "Parameters buffer out of space; potential version mismatch between driver and libcamera"; - return {}; - } - - /* Allocate a new block, clear its memory, and initialize its header. */ - Span<uint8_t> block = data_.subspan(used_, blockSize); - memset(block.data(), 0, block.size()); - - struct v4l2_isp_params_block_header *header = - reinterpret_cast<struct v4l2_isp_params_block_header *>(block.data()); - header->type = blockType; - header->size = block.size(); - - used_ += block.size(); - - reinterpret_cast<struct v4l2_isp_params_buffer *> - (data_.data())->data_size += block.size(); - - /* Update the cache. */ - blocks_[type] = block; - - return block; - } - - Span<uint8_t> data_; - size_t used_; - - std::map<uint16_t, Span<uint8_t>> blocks_; }; } /* namespace ipa */ diff --git a/src/ipa/rkisp1/params.cpp b/src/ipa/rkisp1/params.cpp index b8abbdf6ec66..71df0a939de2 100644 --- a/src/ipa/rkisp1/params.cpp +++ b/src/ipa/rkisp1/params.cpp @@ -128,8 +128,8 @@ Span<uint8_t> RkISP1Params::block(BlockType type) return data_.subspan(info.offset, info.size); } - return V4L2Params::block(utils::to_underlying(type), info.type, - info.size); + return V4L2ParamsBase::block(utils::to_underlying(type), info.type, + info.size); } } /* namespace ipa::rkisp1 */
The implementation of multiple V4L2Params member functions does not depend on the class template argument. Move them to a base class, to avoid inlining them. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- src/ipa/libipa/v4l2_params.cpp | 151 ++++++++++++++++++++++++--------- src/ipa/libipa/v4l2_params.h | 81 +++++------------- src/ipa/rkisp1/params.cpp | 4 +- 3 files changed, 132 insertions(+), 104 deletions(-)