[libcamera-devel] [PATCH 09/13] libcamera: ipu3: Do not duplicate data in IPU3Stream

[Jacopo Mondi]

On Sat, Jun 27, 2020 at 05:00:39AM +0200, Niklas Söderlund wrote:
> Do not keep the duplicated ImgUDevice::ImgUOutput information in both
> the stream and camera data classes. Remove it from the stream and only
> access it from the camera data class.
>
> Which stream is which can instead be checked by comparing it to the
> known streams in camera data. This match how streams are checked in
> other parts of the code making the driver more coherent.


not a driver :)

>
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
>  src/libcamera/pipeline/ipu3/ipu3.cpp | 31 +++++++++++++++-------------
>  1 file changed, 17 insertions(+), 14 deletions(-)
>
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index ae7a01b81dacf498..1f320dc166767bab 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -35,13 +35,11 @@ class IPU3Stream : public Stream
>  {
>  public:
>  	IPU3Stream()
> -		: active_(false), raw_(false), device_(nullptr)
> +		: active_(false)
>  	{
>  	}
>
>  	bool active_;
> -	bool raw_;
> -	ImgUDevice::ImgUOutput *device_;
>  };
>
>  class IPU3CameraData : public CameraData
> @@ -276,7 +274,7 @@ CameraConfiguration::Status IPU3CameraConfiguration::validate()
>  		const StreamConfiguration oldCfg = cfg;
>  		const IPU3Stream *stream = streams_[i];
>
> -		if (stream->raw_) {
> +		if (stream == &data_->rawStream_) {
>  			cfg = cio2Configuration_;
>  		} else {
>  			bool scale = stream == &data_->vfStream_;
> @@ -572,13 +570,16 @@ int PipelineHandlerIPU3::exportFrameBuffers(Camera *camera, Stream *stream,
>  					    std::vector<std::unique_ptr<FrameBuffer>> *buffers)
>  {
>  	IPU3CameraData *data = cameraData(camera);
> -	IPU3Stream *ipu3stream = static_cast<IPU3Stream *>(stream);
>  	unsigned int count = stream->configuration().bufferCount;
>
> -	if (ipu3stream->raw_)
> +	if (stream == &data->outStream_)
> +		return data->imgu_->output_.dev->exportBuffers(count, buffers);
> +	else if (stream == &data->vfStream_)
> +		return data->imgu_->viewfinder_.dev->exportBuffers(count, buffers);
> +	else if (stream == &data->rawStream_)
>  		return data->cio2_.exportBuffers(count, buffers);
>
> -	return ipu3stream->device_->dev->exportBuffers(count, buffers);
> +	return -EINVAL;
>  }
>
>  /**
> @@ -685,11 +686,16 @@ int PipelineHandlerIPU3::queueRequestDevice(Camera *camera, Request *request)
>  	/* Queue all buffers from the request aimed for the ImgU. */
>  	for (auto it : request->buffers()) {
>  		IPU3Stream *stream = static_cast<IPU3Stream *>(it.first);
> -		if (stream->raw_)
> -			continue;
> -
>  		FrameBuffer *buffer = it.second;
> -		int ret = stream->device_->dev->queueBuffer(buffer);
> +		int ret;
> +
> +		if (stream == &data->outStream_)
> +			ret = data->imgu_->output_.dev->queueBuffer(buffer);
> +		else if (stream == &data->vfStream_)
> +			ret = data->imgu_->viewfinder_.dev->queueBuffer(buffer);
> +		else
> +			continue;
> +

I would keep the previous code flow. Up to you

                if (stream == &data->rawStream_)
                        continue;

                int ret = 0;
		if (stream == &data->outStream_)
			ret = data->imgu_->output_.dev->queueBuffer(buffer);
		else if (stream == &data->vfStream_)
			ret = data->imgu_->viewfinder_.dev->queueBuffer(buffer);

>  		if (ret < 0)
>  			error = ret;
>  	}
> @@ -801,9 +807,6 @@ int PipelineHandlerIPU3::registerCameras()
>  		 * second.
>  		 */
>  		data->imgu_ = numCameras ? &imgu1_ : &imgu0_;
> -		data->outStream_.device_ = &data->imgu_->output_;
> -		data->vfStream_.device_ = &data->imgu_->viewfinder_;
> -		data->rawStream_.raw_ = true;
>
>  		/*
>  		 * Connect video devices' 'bufferReady' signals to their
> --
> 2.27.0
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel