[v3,2/2] libcamera: pipeline: virtual: Move image generation to separate thread

24471 diff mbox series

Currently the virtual pipeline generates the images synchronously. This is not
ideal because it blocks the camera manager's internal thread, and because its
behaviour is different from other existing pipeline handlers, all of which
complete requests asynchronously.

So move the image generation to a separate thread by deriving `VirtualCameraData`
from `Thread`, as well as `Object` and using the existing asynchronous signal
and method call mechanism.

Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/libcamera/pipeline/virtual/virtual.cpp | 91 +++++++++++++++-------
 src/libcamera/pipeline/virtual/virtual.h   | 11 ++-
 2 files changed, 71 insertions(+), 31 deletions(-)

Quoting Barnabás Pőcze (2025-09-26 12:07:08)
> Currently the virtual pipeline generates the images synchronously. This is not
> ideal because it blocks the camera manager's internal thread, and because its
> behaviour is different from other existing pipeline handlers, all of which
> complete requests asynchronously.
> 
> So move the image generation to a separate thread by deriving `VirtualCameraData`
> from `Thread`, as well as `Object` and using the existing asynchronous signal
> and method call mechanism.
> 
> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> ---
>  src/libcamera/pipeline/virtual/virtual.cpp | 91 +++++++++++++++-------
>  src/libcamera/pipeline/virtual/virtual.h   | 11 ++-
>  2 files changed, 71 insertions(+), 31 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/virtual/virtual.cpp b/src/libcamera/pipeline/virtual/virtual.cpp
> index f9538129c..23eae852f 100644
> --- a/src/libcamera/pipeline/virtual/virtual.cpp
> +++ b/src/libcamera/pipeline/virtual/virtual.cpp
> @@ -106,6 +106,7 @@ private:
>         }
>  
>         bool initFrameGenerator(Camera *camera);
> +       void bufferCompleted(FrameBuffer *buffer);
>  
>         DmaBufAllocator dmaBufAllocator_;
>  
> @@ -129,6 +130,39 @@ VirtualCameraData::VirtualCameraData(PipelineHandler *pipe,
>  
>         /* \todo Support multiple streams and pass multi_stream_test */
>         streamConfigs_.resize(kMaxStream);
> +
> +       moveToThread(this);
> +}
> +
> +void VirtualCameraData::processRequest(Request *request)
> +{
> +       for (auto const &[stream, buffer] : request->buffers()) {
> +               bool found = false;
> +               /* map buffer and fill test patterns */
> +               for (auto &streamConfig : streamConfigs_) {
> +                       if (stream == &streamConfig.stream) {
> +                               FrameMetadata &fmd = buffer->_d()->metadata();
> +
> +                               fmd.status = FrameMetadata::Status::FrameSuccess;
> +                               fmd.sequence = streamConfig.seq++;
> +                               fmd.timestamp = currentTimestamp();
> +
> +                               Span<const FrameBuffer::Plane> planes = buffer->planes();
> +                               for (const auto [i, p] : utils::enumerate(planes))
> +                                       fmd.planes()[i].bytesused = p.length;
> +
> +                               found = true;
> +
> +                               if (streamConfig.frameGenerator->generateFrame(
> +                                           stream->configuration().size, buffer))
> +                                       fmd.status = FrameMetadata::Status::FrameError;
> +
> +                               bufferCompleted.emit(buffer);
> +                               break;
> +                       }
> +               }
> +               ASSERT(found);
> +       }
>  }
>  
>  VirtualCameraConfiguration::VirtualCameraConfiguration(VirtualCameraData *data)
> @@ -291,11 +325,27 @@ int PipelineHandlerVirtual::start([[maybe_unused]] Camera *camera,
>         for (auto &s : data->streamConfigs_)
>                 s.seq = 0;
>  
> +       data->bufferCompleted.connect(this, &PipelineHandlerVirtual::bufferCompleted);
> +       data->start();
> +
>         return 0;
>  }
>  
> -void PipelineHandlerVirtual::stopDevice([[maybe_unused]] Camera *camera)
> +void PipelineHandlerVirtual::stopDevice(Camera *camera)
>  {
> +       VirtualCameraData *data = cameraData(camera);
> +
> +       /* Cancel pending work. */
> +       data->exit();
> +       data->wait();
> +       data->removeMessages(data);
> +
> +       /* Process pending `bufferCompleted` signals. */
> +       thread()->dispatchMessages(Message::Type::InvokeMessage, this);
> +       data->bufferCompleted.disconnect(this);
> +
> +       while (!data->queuedRequests_.empty())
> +               cancelRequest(data->queuedRequests_.front());
>  }
>  
>  int PipelineHandlerVirtual::queueRequestDevice([[maybe_unused]] Camera *camera,
> @@ -304,36 +354,9 @@ int PipelineHandlerVirtual::queueRequestDevice([[maybe_unused]] Camera *camera,
>         VirtualCameraData *data = cameraData(camera);
>         const auto timestamp = currentTimestamp();
>  
> -       for (auto const &[stream, buffer] : request->buffers()) {
> -               bool found = false;
> -               /* map buffer and fill test patterns */
> -               for (auto &streamConfig : data->streamConfigs_) {
> -                       if (stream == &streamConfig.stream) {
> -                               FrameMetadata &fmd = buffer->_d()->metadata();
> -
> -                               fmd.status = FrameMetadata::Status::FrameSuccess;
> -                               fmd.sequence = streamConfig.seq++;
> -                               fmd.timestamp = timestamp;
> -
> -                               Span<const FrameBuffer::Plane> planes = buffer->planes();
> -                               for (const auto [i, p] : utils::enumerate(planes))
> -                                       fmd.planes()[i].bytesused = p.length;
> -
> -                               found = true;
> -
> -                               if (streamConfig.frameGenerator->generateFrame(
> -                                           stream->configuration().size, buffer))
> -                                       fmd.status = FrameMetadata::Status::FrameError;
> -
> -                               completeBuffer(request, buffer);
> -                               break;
> -                       }
> -               }
> -               ASSERT(found);
> -       }
> -
>         request->metadata().set(controls::SensorTimestamp, timestamp);
> -       completeRequest(request);
> +       data->invokeMethod(&VirtualCameraData::processRequest,
> +                          ConnectionTypeQueued, request);
>  
>         return 0;
>  }
> @@ -415,6 +438,14 @@ bool PipelineHandlerVirtual::initFrameGenerator(Camera *camera)
>         return true;
>  }
>  
> +void PipelineHandlerVirtual::bufferCompleted(FrameBuffer *buffer)
> +{
> +       Request *request = buffer->request();
> +
> +       if (completeBuffer(request, buffer))
> +               completeRequest(request);
> +}
> +
>  REGISTER_PIPELINE_HANDLER(PipelineHandlerVirtual, "virtual")
>  
>  } /* namespace libcamera */
> diff --git a/src/libcamera/pipeline/virtual/virtual.h b/src/libcamera/pipeline/virtual/virtual.h
> index 683cb82b4..215e56fa3 100644
> --- a/src/libcamera/pipeline/virtual/virtual.h
> +++ b/src/libcamera/pipeline/virtual/virtual.h
> @@ -11,6 +11,10 @@
>  #include <variant>
>  #include <vector>
>  
> +#include <libcamera/base/object.h>
> +#include <libcamera/base/signal.h>
> +#include <libcamera/base/thread.h>
> +
>  #include <libcamera/geometry.h>
>  #include <libcamera/stream.h>
>  
> @@ -25,7 +29,9 @@ namespace libcamera {
>  
>  using VirtualFrame = std::variant<TestPattern, ImageFrames>;
>  
> -class VirtualCameraData : public Camera::Private
> +class VirtualCameraData : public Camera::Private,
> +                         public Thread,
> +                         public Object
>  {
>  public:
>         const static unsigned int kMaxStream = 3;
> @@ -54,9 +60,12 @@ public:
>  
>         ~VirtualCameraData() = default;
>  
> +       void processRequest(Request *request);
> +
>         Configuration config_;
>  
>         std::vector<StreamConfig> streamConfigs_;
> +       Signal<FrameBuffer *> bufferCompleted;
>  };
>  
>  } /* namespace libcamera */
> -- 
> 2.51.0
>