[libcamera-devel,v2,14/18] libcamera: pipeline: simple: rename converterBuffers_ and related vars

19404 diff mbox series

From: Andrey Konovalov <andrey.konovalov@linaro.org>

The converterBuffers_ and the converterQueue_ are not that specific
to the Converter, and could be used by another entity doing the format
conversion.

Rename converterBuffers_, converterQueue_, and useConverter_ to
conversionBuffers_, conversionQueue_ and useConversion_ to
disassociate them from the Converter.

Signed-off-by: Andrey Konovalov <andrey.konovalov@linaro.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> # sc8280xp Lenovo x13s
Tested-by: Pavel Machek <pavel@ucw.cz>
---
 src/libcamera/pipeline/simple/simple.cpp | 63 ++++++++++++------------
 1 file changed, 32 insertions(+), 31 deletions(-)

Quoting Hans de Goede via libcamera-devel (2024-01-13 14:22:14)
> From: Andrey Konovalov <andrey.konovalov@linaro.org>
> 
> The converterBuffers_ and the converterQueue_ are not that specific
> to the Converter, and could be used by another entity doing the format
> conversion.
> 
> Rename converterBuffers_, converterQueue_, and useConverter_ to
> conversionBuffers_, conversionQueue_ and useConversion_ to
> disassociate them from the Converter.

One day I'd love to see processing blocks that could be constructed and
chained together here. Not a full reimplementation of gstreamer of
course ... but I suspect we could end up with other chained reusable
components through out libcamera that may not always be 'convertors'.

Anyway, that's likely a long way off - and I think in the context of
this series - given the preceeding patches this makes sense so I'll
already put this one here:

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

> 
> Signed-off-by: Andrey Konovalov <andrey.konovalov@linaro.org>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> # sc8280xp Lenovo x13s
> Tested-by: Pavel Machek <pavel@ucw.cz>
> ---
>  src/libcamera/pipeline/simple/simple.cpp | 63 ++++++++++++------------
>  1 file changed, 32 insertions(+), 31 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index 4d0e7255..fa298746 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -269,17 +269,18 @@ public:
>         std::vector<Configuration> configs_;
>         std::map<PixelFormat, std::vector<const Configuration *>> formats_;
>  
> +       std::vector<std::unique_ptr<FrameBuffer>> conversionBuffers_;
> +       std::queue<std::map<unsigned int, FrameBuffer *>> conversionQueue_;
> +       bool useConversion_;
> +
>         std::unique_ptr<Converter> converter_;
> -       std::vector<std::unique_ptr<FrameBuffer>> converterBuffers_;
> -       bool useConverter_;
> -       std::queue<std::map<unsigned int, FrameBuffer *>> converterQueue_;
>  
>  private:
>         void tryPipeline(unsigned int code, const Size &size);
>         static std::vector<const MediaPad *> routedSourcePads(MediaPad *sink);
>  
> -       void converterInputDone(FrameBuffer *buffer);
> -       void converterOutputDone(FrameBuffer *buffer);
> +       void conversionInputDone(FrameBuffer *buffer);
> +       void conversionOutputDone(FrameBuffer *buffer);
>  };
>  
>  class SimpleCameraConfiguration : public CameraConfiguration
> @@ -503,8 +504,8 @@ int SimpleCameraData::init()
>                                 << "Failed to create converter, disabling format conversion";
>                         converter_.reset();
>                 } else {
> -                       converter_->inputBufferReady.connect(this, &SimpleCameraData::converterInputDone);
> -                       converter_->outputBufferReady.connect(this, &SimpleCameraData::converterOutputDone);
> +                       converter_->inputBufferReady.connect(this, &SimpleCameraData::conversionInputDone);
> +                       converter_->outputBufferReady.connect(this, &SimpleCameraData::conversionOutputDone);
>                 }
>         }
>  
> @@ -740,7 +741,7 @@ void SimpleCameraData::bufferReady(FrameBuffer *buffer)
>          * point converting an erroneous buffer.
>          */
>         if (buffer->metadata().status != FrameMetadata::FrameSuccess) {
> -               if (!useConverter_) {
> +               if (!useConversion_) {
>                         /* No conversion, just complete the request. */
>                         Request *request = buffer->request();
>                         pipe->completeBuffer(request, buffer);
> @@ -756,16 +757,16 @@ void SimpleCameraData::bufferReady(FrameBuffer *buffer)
>                 if (buffer->metadata().status != FrameMetadata::FrameCancelled)
>                         video_->queueBuffer(buffer);
>  
> -               if (converterQueue_.empty())
> +               if (conversionQueue_.empty())
>                         return;
>  
>                 Request *request = nullptr;
> -               for (auto &item : converterQueue_.front()) {
> +               for (auto &item : conversionQueue_.front()) {
>                         FrameBuffer *outputBuffer = item.second;
>                         request = outputBuffer->request();
>                         pipe->completeBuffer(request, outputBuffer);
>                 }
> -               converterQueue_.pop();
> +               conversionQueue_.pop();
>  
>                 if (request)
>                         pipe->completeRequest(request);
> @@ -782,9 +783,9 @@ void SimpleCameraData::bufferReady(FrameBuffer *buffer)
>          */
>         Request *request = buffer->request();
>  
> -       if (useConverter_ && !converterQueue_.empty()) {
> +       if (useConversion_ && !conversionQueue_.empty()) {
>                 const std::map<unsigned int, FrameBuffer *> &outputs =
> -                       converterQueue_.front();
> +                       conversionQueue_.front();
>                 if (!outputs.empty()) {
>                         FrameBuffer *outputBuffer = outputs.begin()->second;
>                         if (outputBuffer)
> @@ -801,14 +802,14 @@ void SimpleCameraData::bufferReady(FrameBuffer *buffer)
>          * conversion is needed. If there's no queued request, just requeue the
>          * captured buffer for capture.
>          */
> -       if (useConverter_) {
> -               if (converterQueue_.empty()) {
> +       if (useConversion_) {
> +               if (conversionQueue_.empty()) {
>                         video_->queueBuffer(buffer);
>                         return;
>                 }
>  
> -               converter_->queueBuffers(buffer, converterQueue_.front());
> -               converterQueue_.pop();
> +               converter_->queueBuffers(buffer, conversionQueue_.front());
> +               conversionQueue_.pop();
>                 return;
>         }
>  
> @@ -817,13 +818,13 @@ void SimpleCameraData::bufferReady(FrameBuffer *buffer)
>         pipe->completeRequest(request);
>  }
>  
> -void SimpleCameraData::converterInputDone(FrameBuffer *buffer)
> +void SimpleCameraData::conversionInputDone(FrameBuffer *buffer)
>  {
>         /* Queue the input buffer back for capture. */
>         video_->queueBuffer(buffer);
>  }
>  
> -void SimpleCameraData::converterOutputDone(FrameBuffer *buffer)
> +void SimpleCameraData::conversionOutputDone(FrameBuffer *buffer)
>  {
>         SimplePipelineHandler *pipe = SimpleCameraData::pipe();
>  
> @@ -1159,14 +1160,14 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)
>  
>         /* Configure the converter if needed. */
>         std::vector<std::reference_wrapper<StreamConfiguration>> outputCfgs;
> -       data->useConverter_ = config->needConversion();
> +       data->useConversion_ = config->needConversion();
>  
>         for (unsigned int i = 0; i < config->size(); ++i) {
>                 StreamConfiguration &cfg = config->at(i);
>  
>                 cfg.setStream(&data->streams_[i]);
>  
> -               if (data->useConverter_)
> +               if (data->useConversion_)
>                         outputCfgs.push_back(cfg);
>         }
>  
> @@ -1192,7 +1193,7 @@ int SimplePipelineHandler::exportFrameBuffers(Camera *camera, Stream *stream,
>          * Export buffers on the converter or capture video node, depending on
>          * whether the converter is used or not.
>          */
> -       if (data->useConverter_)
> +       if (data->useConversion_)
>                 return data->converter_->exportBuffers(data->streamIndex(stream),
>                                                        count, buffers);
>         else
> @@ -1213,13 +1214,13 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL
>                 return -EBUSY;
>         }
>  
> -       if (data->useConverter_) {
> +       if (data->useConversion_) {
>                 /*
>                  * When using the converter allocate a fixed number of internal
>                  * buffers.
>                  */
>                 ret = video->allocateBuffers(kNumInternalBuffers,
> -                                            &data->converterBuffers_);
> +                                            &data->conversionBuffers_);
>         } else {
>                 /* Otherwise, prepare for using buffers from the only stream. */
>                 Stream *stream = &data->streams_[0];
> @@ -1238,7 +1239,7 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL
>                 return ret;
>         }
>  
> -       if (data->useConverter_) {
> +       if (data->useConversion_) {
>                 ret = data->converter_->start();
>                 if (ret < 0) {
>                         stop(camera);
> @@ -1246,7 +1247,7 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] const ControlL
>                 }
>  
>                 /* Queue all internal buffers for capture. */
> -               for (std::unique_ptr<FrameBuffer> &buffer : data->converterBuffers_)
> +               for (std::unique_ptr<FrameBuffer> &buffer : data->conversionBuffers_)
>                         video->queueBuffer(buffer.get());
>         }
>  
> @@ -1258,7 +1259,7 @@ void SimplePipelineHandler::stopDevice(Camera *camera)
>         SimpleCameraData *data = cameraData(camera);
>         V4L2VideoDevice *video = data->video_;
>  
> -       if (data->useConverter_)
> +       if (data->useConversion_)
>                 data->converter_->stop();
>  
>         video->streamOff();
> @@ -1266,7 +1267,7 @@ void SimplePipelineHandler::stopDevice(Camera *camera)
>  
>         video->bufferReady.disconnect(data, &SimpleCameraData::bufferReady);
>  
> -       data->converterBuffers_.clear();
> +       data->conversionBuffers_.clear();
>  
>         releasePipeline(data);
>  }
> @@ -1284,7 +1285,7 @@ int SimplePipelineHandler::queueRequestDevice(Camera *camera, Request *request)
>                  * queue, it will be handed to the converter in the capture
>                  * completion handler.
>                  */
> -               if (data->useConverter_) {
> +               if (data->useConversion_) {
>                         buffers.emplace(data->streamIndex(stream), buffer);
>                 } else {
>                         ret = data->video_->queueBuffer(buffer);
> @@ -1293,8 +1294,8 @@ int SimplePipelineHandler::queueRequestDevice(Camera *camera, Request *request)
>                 }
>         }
>  
> -       if (data->useConverter_)
> -               data->converterQueue_.push(std::move(buffers));
> +       if (data->useConversion_)
> +               data->conversionQueue_.push(std::move(buffers));
>  
>         return 0;
>  }
> -- 
> 2.43.0
>