[v6,11/18] libcamera: pipeline: simple: rename converterBuffers_ and related vars

19769 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.

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

Hi Milan and Andrey,

Thank you for the patch.

On Tue, Mar 19, 2024 at 01:35:58PM +0100, Milan Zamazal wrote:
> 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.
> 
> Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> # sc8280xp Lenovo x13s
> Tested-by: Pavel Machek <pavel@ucw.cz>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> Reviewed-by: Pavel Machek <pavel@ucw.cz>
> Signed-off-by: Andrey Konovalov <andrey.konovalov@linaro.org>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  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 04e77f7e..9c8f7ba3 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -270,17 +270,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
> @@ -504,8 +505,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);
>  		}
>  	}
>  
> @@ -741,7 +742,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);
> @@ -757,16 +758,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);
> @@ -783,9 +784,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)
> @@ -802,14 +803,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;
>  	}
>  
> @@ -818,13 +819,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();
>  
> @@ -1190,14 +1191,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);
>  	}
>  
> @@ -1223,7 +1224,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
> @@ -1244,13 +1245,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];
> @@ -1269,7 +1270,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);
> @@ -1277,7 +1278,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());
>  	}
>  
> @@ -1289,7 +1290,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();
> @@ -1297,7 +1298,7 @@ void SimplePipelineHandler::stopDevice(Camera *camera)
>  
>  	video->bufferReady.disconnect(data, &SimpleCameraData::bufferReady);
>  
> -	data->converterBuffers_.clear();
> +	data->conversionBuffers_.clear();
>  
>  	releasePipeline(data);
>  }
> @@ -1315,7 +1316,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);
> @@ -1324,8 +1325,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;
>  }