[libcamera-devel] [PATCH 03/13] libcamera: ipu3: Always import buffers for ImgU sinks

[Jacopo Mondi]

Hi Niklas,

On Sat, Jun 27, 2020 at 05:00:33AM +0200, Niklas Söderlund wrote:
> When the IPU3 pipeline was first developed buffers of sinks from the
> ImgU that where not active still needed to have allocated buffers

I think somewhere there's a "buffer" too much, I get what you mean,
but is slightly confused.

> associated with them or streaming was not allowed to start. This is no
> longer true, it's enough that the sinks have imported buffers for
> streaming to start. As we already need to import buffers for streams
> that are active we can align the two cases and always import buffers.

as this changed in the driver since we first developed this, or has
this always been the case?

Also, as asked in 2/13, do we know which output has to have buffers
mandatorily allocated ?

>
> With this there is no longer a reason to store the allocated
> FrameBuffers to keep them alive and the vector tracking them can be
> removed.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
>  src/libcamera/pipeline/ipu3/ipu3.cpp | 25 ++++---------------------
>  1 file changed, 4 insertions(+), 21 deletions(-)
>
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 405550b1302fb370..5a473e18c082cee8 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -49,7 +49,6 @@ public:
>  		V4L2VideoDevice *dev;
>  		unsigned int pad;
>  		std::string name;
> -		std::vector<std::unique_ptr<FrameBuffer>> buffers;
>  	};
>
>  	ImgUDevice()
> @@ -1124,9 +1123,6 @@ int ImgUDevice::configureOutput(ImgUOutput *output,
>   */
>  int ImgUDevice::allocateBuffers(IPU3CameraData *data, unsigned int bufferCount)
>  {
> -	IPU3Stream *outStream = &data->outStream_;
> -	IPU3Stream *vfStream = &data->vfStream_;
> -
>  	/* Share buffers between CIO2 output and ImgU input. */
>  	int ret = input_->importBuffers(bufferCount);
>  	if (ret) {
> @@ -1147,28 +1143,15 @@ int ImgUDevice::allocateBuffers(IPU3CameraData *data, unsigned int bufferCount)
>  		goto error;
>  	}
>
> -	/*
> -	 * Allocate buffers for both outputs. If an output is active, prepare
> -	 * for buffer import, otherwise allocate internal buffers. Use the same
> -	 * number of buffers in either case.
> -	 */
> -	if (outStream->active_)
> -		ret = output_.dev->importBuffers(bufferCount);
> -	else
> -		ret = output_.dev->allocateBuffers(bufferCount,
> -						   &output_.buffers);
> +	ret = output_.dev->importBuffers(bufferCount);
>  	if (ret < 0) {
> -		LOG(IPU3, Error) << "Failed to allocate ImgU output buffers";
> +		LOG(IPU3, Error) << "Failed to import ImgU output buffers";

Nit: I would leave the same error message, as the function that fails
is called ImgUDevice::allocateBuffers(). The video device will
complain anyway for the import error.

>  		goto error;
>  	}
>
> -	if (vfStream->active_)
> -		ret = viewfinder_.dev->importBuffers(bufferCount);
> -	else
> -		ret = viewfinder_.dev->allocateBuffers(bufferCount,
> -						       &viewfinder_.buffers);
> +	ret = viewfinder_.dev->importBuffers(bufferCount);
>  	if (ret < 0) {
> -		LOG(IPU3, Error) << "Failed to allocate ImgU viewfinder buffers";
> +		LOG(IPU3, Error) << "Failed to import ImgU viewfinder buffers";
>  		goto error;
>  	}
>
> --
> 2.27.0
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel