[libcamera-devel] [PATCH v2 3/4] ipa: vimc: Map and unmap buffers
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Sat Aug 14 02:35:35 CEST 2021
Hi Umang,
Thank you for the patch.
On Fri, Aug 13, 2021 at 08:14:36PM +0530, Umang Jain wrote:
> VIMC pipeline-handler have dmabuf-backed mock FrameBuffers which are
s/pipeline-handler/pipeline handler/
> specifically targetted mimicking IPA buffers(parameter and statistics).
s/buffers/buffers /
> Map these mock buffers to the VIMC IPA that would enable exercising IPA
> IPC code paths. This will provide leverage to our test suite to test
> IPA IPC code paths, which are common to various platforms.
>
> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
> ---
> include/libcamera/ipa/vimc.mojom | 3 +++
> src/ipa/vimc/vimc.cpp | 26 ++++++++++++++++++++++++++
> src/libcamera/pipeline/vimc/vimc.cpp | 16 ++++++++++++++++
> 3 files changed, 45 insertions(+)
>
> diff --git a/include/libcamera/ipa/vimc.mojom b/include/libcamera/ipa/vimc.mojom
> index ee66353d..8cb240d3 100644
> --- a/include/libcamera/ipa/vimc.mojom
> +++ b/include/libcamera/ipa/vimc.mojom
> @@ -26,6 +26,9 @@ interface IPAVimcInterface {
>
> start() => (int32 ret);
> stop();
> +
> + mapBuffers(array<libcamera.IPABuffer> buffers);
> + unmapBuffers(array<uint32> ids);
> };
>
> interface IPAVimcEventInterface {
> diff --git a/src/ipa/vimc/vimc.cpp b/src/ipa/vimc/vimc.cpp
> index 0535c740..082b2db7 100644
> --- a/src/ipa/vimc/vimc.cpp
> +++ b/src/ipa/vimc/vimc.cpp
> @@ -19,6 +19,8 @@
> #include <libcamera/ipa/ipa_interface.h>
> #include <libcamera/ipa/ipa_module_info.h>
>
> +#include "libcamera/internal/mapped_framebuffer.h"
> +
> namespace libcamera {
>
> LOG_DEFINE_CATEGORY(IPAVimc)
> @@ -38,11 +40,15 @@ public:
> const std::map<unsigned int, IPAStream> &streamConfig,
> const std::map<unsigned int, ControlInfoMap> &entityControls) override;
>
> + void mapBuffers(const std::vector<IPABuffer> &buffers) override;
> + void unmapBuffers(const std::vector<unsigned int> &ids) override;
> +
> private:
> void initTrace();
> void trace(enum ipa::vimc::IPAOperationCode operation);
>
> int fd_;
> + std::map<unsigned int, MappedFrameBuffer> buffers_;
> };
>
> IPAVimc::IPAVimc()
> @@ -99,6 +105,26 @@ int IPAVimc::configure([[maybe_unused]] const IPACameraSensorInfo &sensorInfo,
> return 0;
> }
>
> +void IPAVimc::mapBuffers(const std::vector<IPABuffer> &buffers)
> +{
> + for (const IPABuffer &buffer : buffers) {
> + const FrameBuffer fb(buffer.planes);
> + buffers_.emplace(buffer.id,
> + MappedFrameBuffer(&fb, MappedFrameBuffer::MapFlag::Read));
The point of emplace compared to insert is to avoid a copy construction.
This should be written as
buffers_.emplace(std::piecewise_construct,
std::forward_as_tuple(buffer.id),
std::forward_as_tuple(&fb, MappedFrameBuffer::MapFlag::Read));
> + }
> +}
> +
> +void IPAVimc::unmapBuffers(const std::vector<unsigned int> &ids)
> +{
> + for (unsigned int id : ids) {
> + auto it = buffers_.find(id);
> + if (it == buffers_.end())
> + continue;
> +
> + buffers_.erase(it);
> + }
> +}
> +
> void IPAVimc::initTrace()
> {
> struct stat fifoStat;
> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
> index edf8c58e..c44a6242 100644
> --- a/src/libcamera/pipeline/vimc/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc/vimc.cpp
> @@ -63,6 +63,7 @@ public:
>
> std::unique_ptr<ipa::vimc::IPAProxyVimc> ipa_;
> std::vector<std::unique_ptr<FrameBuffer>> mockIPABufs_;
> + std::vector<IPABuffer> ipaBuffers_;
> };
>
> class VimcCameraConfiguration : public CameraConfiguration
> @@ -334,6 +335,15 @@ int PipelineHandlerVimc::start(Camera *camera, [[maybe_unused]] const ControlLis
> if (ret < 0)
> return ret;
>
> + /* Map the mock IPA buffers to VIMC IPA to exercise IPC code paths */
> + unsigned int ipaBufferId = 1;
> + for (unsigned int i = 0; i < data->mockIPABufs_.size(); i++) {
> + std::unique_ptr<FrameBuffer> &buffer = data->mockIPABufs_[i];
> + buffer->setCookie(ipaBufferId++);
You can use utils::enumerate() here:
for (auto [i, buffer] : utils::enumerate(data->mackIPABufs_)) {
buffer->setCookie(i++);
> + data->ipaBuffers_.emplace_back(buffer->cookie(), buffer->planes());
> + }
> + data->ipa_->mapBuffers(data->ipaBuffers_);
The return value should be checked.
> +
> ret = data->ipa_->start();
> if (ret) {
> data->video_->releaseBuffers();
> @@ -354,7 +364,13 @@ void PipelineHandlerVimc::stop(Camera *camera)
> {
> VimcCameraData *data = cameraData(camera);
> data->video_->streamOff();
> +
> + std::vector<unsigned int> ids;
> + for (IPABuffer &ipabuf : data->ipaBuffers_)
> + ids.push_back(ipabuf.id);
You could use here
for (const std::unique_ptr<FrameBuffer> &buffer : data->mockIPABufs_)
ids.push_back(buffer->cookie());
and replace VimcCameraData::ipaBuffers_ with a local variable in
PipelineHandlerVimc::start().
> + data->ipa_->unmapBuffers(ids);
> data->ipa_->stop();
> +
> data->video_->releaseBuffers();
> }
>
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list