[libcamera-devel,08/13] libcamera: pipeline: rkisp1: Prefix main path video and resizer
diff mbox series

Message ID 20200813005246.3265807-9-niklas.soderlund@ragnatech.se
State Accepted
Headers show
Series
  • libcamera: pipeline: rkisp1: Extend to support two streams
Related show

Commit Message

Niklas Söderlund Aug. 13, 2020, 12:52 a.m. UTC 
In preparation of supporting both the main and self path prefix the main
path specific variables with mainPath.

Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
---
 src/libcamera/pipeline/rkisp1/rkisp1.cpp | 82 ++++++++++++------------
 1 file changed, 41 insertions(+), 41 deletions(-)

Comments

Jacopo Mondi Aug. 20, 2020, 9:01 a.m. UTC | #1 
Hi Niklas,

On Thu, Aug 13, 2020 at 02:52:41AM +0200, Niklas Söderlund wrote:
> In preparation of supporting both the main and self path prefix the main
> path specific variables with mainPath.

Wouldn't it be better to use 'self' and 'main' and omit 'path' ?

>
> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> ---
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 82 ++++++++++++------------
>  1 file changed, 41 insertions(+), 41 deletions(-)
>
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 59614a9f470b7802..60179a1151f18491 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -66,7 +66,7 @@ struct RkISP1FrameInfo {
>
>  	FrameBuffer *paramBuffer;
>  	FrameBuffer *statBuffer;
> -	FrameBuffer *videoBuffer;
> +	FrameBuffer *mainPathBuffer;
>
>  	bool paramFilled;
>  	bool paramDequeued;
> @@ -131,7 +131,7 @@ class RkISP1CameraData : public CameraData
>  public:
>  	RkISP1CameraData(PipelineHandler *pipe, V4L2VideoDevice *video)
>  		: CameraData(pipe), sensor_(nullptr), frame_(0),
> -		  frameInfo_(pipe), video_(video)
> +		  frameInfo_(pipe), mainPathVideo_(video)
>  	{
>  	}
>
> @@ -142,14 +142,14 @@ public:
>
>  	int loadIPA();
>
> -	Stream stream_;
> +	Stream mainPathStream_;
>  	CameraSensor *sensor_;
>  	unsigned int frame_;
>  	std::vector<IPABuffer> ipaBuffers_;
>  	RkISP1Frames frameInfo_;
>  	RkISP1Timeline timeline_;
>
> -	V4L2VideoDevice *video_;
> +	V4L2VideoDevice *mainPathVideo_;
>
>  private:
>  	void queueFrameAction(unsigned int frame,
> @@ -225,8 +225,8 @@ private:
>
>  	MediaDevice *media_;
>  	V4L2Subdevice *isp_;
> -	V4L2Subdevice *resizer_;
> -	V4L2VideoDevice *video_;
> +	V4L2Subdevice *mainPathResizer_;
> +	V4L2VideoDevice *mainPathVideo_;
>  	V4L2VideoDevice *param_;
>  	V4L2VideoDevice *stat_;
>
> @@ -259,8 +259,8 @@ RkISP1FrameInfo *RkISP1Frames::create(const RkISP1CameraData *data, Request *req
>  	}
>  	FrameBuffer *statBuffer = pipe_->availableStatBuffers_.front();
>
> -	FrameBuffer *videoBuffer = request->findBuffer(&data->stream_);
> -	if (!videoBuffer) {
> +	FrameBuffer *mainPathBuffer = request->findBuffer(&data->mainPathStream_);
> +	if (!mainPathBuffer) {
>  		LOG(RkISP1, Error)
>  			<< "Attempt to queue request with invalid stream";
>  		return nullptr;
> @@ -274,7 +274,7 @@ RkISP1FrameInfo *RkISP1Frames::create(const RkISP1CameraData *data, Request *req
>  	info->frame = frame;
>  	info->request = request;
>  	info->paramBuffer = paramBuffer;
> -	info->videoBuffer = videoBuffer;
> +	info->mainPathBuffer = mainPathBuffer;
>  	info->statBuffer = statBuffer;
>  	info->paramFilled = false;
>  	info->paramDequeued = false;
> @@ -333,7 +333,7 @@ RkISP1FrameInfo *RkISP1Frames::find(FrameBuffer *buffer)
>
>  		if (info->paramBuffer == buffer ||
>  		    info->statBuffer == buffer ||
> -		    info->videoBuffer == buffer)
> +		    info->mainPathBuffer == buffer)
>  			return info;
>  	}
>
> @@ -405,7 +405,7 @@ protected:
>
>  		pipe_->param_->queueBuffer(info->paramBuffer);
>  		pipe_->stat_->queueBuffer(info->statBuffer);
> -		pipe_->video_->queueBuffer(info->videoBuffer);
> +		pipe_->mainPathVideo_->queueBuffer(info->mainPathBuffer);
>  	}
>
>  private:
> @@ -544,10 +544,10 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()
>  	cfg.bufferCount = RKISP1_BUFFER_COUNT;
>
>  	V4L2DeviceFormat format = {};
> -	format.fourcc = data_->video_->toV4L2PixelFormat(cfg.pixelFormat);
> +	format.fourcc = data_->mainPathVideo_->toV4L2PixelFormat(cfg.pixelFormat);
>  	format.size = cfg.size;
>
> -	int ret = data_->video_->tryFormat(&format);
> +	int ret = data_->mainPathVideo_->tryFormat(&format);
>  	if (ret)
>  		return Invalid;
>
> @@ -558,8 +558,8 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()
>  }
>
>  PipelineHandlerRkISP1::PipelineHandlerRkISP1(CameraManager *manager)
> -	: PipelineHandler(manager), isp_(nullptr), resizer_(nullptr),
> -	  video_(nullptr), param_(nullptr), stat_(nullptr)
> +	: PipelineHandler(manager), isp_(nullptr), mainPathResizer_(nullptr),
> +	  mainPathVideo_(nullptr), param_(nullptr), stat_(nullptr)
>  {
>  }
>
> @@ -567,8 +567,8 @@ PipelineHandlerRkISP1::~PipelineHandlerRkISP1()
>  {
>  	delete param_;
>  	delete stat_;
> -	delete video_;
> -	delete resizer_;
> +	delete mainPathVideo_;
> +	delete mainPathResizer_;
>  	delete isp_;
>  }
>
> @@ -649,7 +649,7 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
>
>  	LOG(RkISP1, Debug) << "ISP output pad configured with " << format.toString();
>
> -	ret = resizer_->setFormat(0, &format);
> +	ret = mainPathResizer_->setFormat(0, &format);
>  	if (ret < 0)
>  		return ret;
>
> @@ -659,23 +659,23 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
>
>  	LOG(RkISP1, Debug) << "Configuring resizer output pad with " << format.toString();
>
> -	ret = resizer_->setFormat(1, &format);
> +	ret = mainPathResizer_->setFormat(1, &format);
>  	if (ret < 0)
>  		return ret;
>
>  	LOG(RkISP1, Debug) << "Resizer output pad configured with " << format.toString();
>
>  	V4L2DeviceFormat outputFormat = {};
> -	outputFormat.fourcc = video_->toV4L2PixelFormat(cfg.pixelFormat);
> +	outputFormat.fourcc = mainPathVideo_->toV4L2PixelFormat(cfg.pixelFormat);
>  	outputFormat.size = cfg.size;
>  	outputFormat.planesCount = 2;
>
> -	ret = video_->setFormat(&outputFormat);
> +	ret = mainPathVideo_->setFormat(&outputFormat);
>  	if (ret)
>  		return ret;
>
>  	if (outputFormat.size != cfg.size ||
> -	    outputFormat.fourcc != video_->toV4L2PixelFormat(cfg.pixelFormat)) {
> +	    outputFormat.fourcc != mainPathVideo_->toV4L2PixelFormat(cfg.pixelFormat)) {
>  		LOG(RkISP1, Error)
>  			<< "Unable to configure capture in " << cfg.toString();
>  		return -EINVAL;
> @@ -693,7 +693,7 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
>  	if (ret)
>  		return ret;
>
> -	cfg.setStream(&data->stream_);
> +	cfg.setStream(&data->mainPathStream_);
>
>  	return 0;
>  }
> @@ -702,17 +702,17 @@ int PipelineHandlerRkISP1::exportFrameBuffers(Camera *camera, Stream *stream,
>  					      std::vector<std::unique_ptr<FrameBuffer>> *buffers)
>  {
>  	unsigned int count = stream->configuration().bufferCount;
> -	return video_->exportBuffers(count, buffers);
> +	return mainPathVideo_->exportBuffers(count, buffers);
>  }
>
>  int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)
>  {
>  	RkISP1CameraData *data = cameraData(camera);
> -	unsigned int count = data->stream_.configuration().bufferCount;
> +	unsigned int count = data->mainPathStream_.configuration().bufferCount;
>  	unsigned int ipaBufferId = 1;
>  	int ret;
>
> -	ret = video_->importBuffers(count);
> +	ret = mainPathVideo_->importBuffers(count);
>  	if (ret < 0)
>  		goto error;
>
> @@ -745,7 +745,7 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)
>  error:
>  	paramBuffers_.clear();
>  	statBuffers_.clear();
> -	video_->releaseBuffers();
> +	mainPathVideo_->releaseBuffers();
>
>  	return ret;
>  }
> @@ -776,8 +776,8 @@ int PipelineHandlerRkISP1::freeBuffers(Camera *camera)
>  	if (stat_->releaseBuffers())
>  		LOG(RkISP1, Error) << "Failed to release stat buffers";
>
> -	if (video_->releaseBuffers())
> -		LOG(RkISP1, Error) << "Failed to release video buffers";
> +	if (mainPathVideo_->releaseBuffers())
> +		LOG(RkISP1, Error) << "Failed to release main path buffers";
>
>  	return 0;
>  }
> @@ -821,7 +821,7 @@ int PipelineHandlerRkISP1::start(Camera *camera)
>  		return ret;
>  	}
>
> -	ret = video_->streamOn();
> +	ret = mainPathVideo_->streamOn();
>  	if (ret) {
>  		param_->streamOff();
>  		stat_->streamOff();
> @@ -846,8 +846,8 @@ int PipelineHandlerRkISP1::start(Camera *camera)
>
>  	std::map<unsigned int, IPAStream> streamConfig;
>  	streamConfig[0] = {
> -		.pixelFormat = data->stream_.configuration().pixelFormat,
> -		.size = data->stream_.configuration().size,
> +		.pixelFormat = data->mainPathStream_.configuration().pixelFormat,
> +		.size = data->mainPathStream_.configuration().size,
>  	};
>
>  	std::map<unsigned int, const ControlInfoMap &> entityControls;
> @@ -865,7 +865,7 @@ void PipelineHandlerRkISP1::stop(Camera *camera)
>  	RkISP1CameraData *data = cameraData(camera);
>  	int ret;
>
> -	ret = video_->streamOff();
> +	ret = mainPathVideo_->streamOff();
>  	if (ret)
>  		LOG(RkISP1, Warning)
>  			<< "Failed to stop camera " << camera->id();
> @@ -950,7 +950,7 @@ int PipelineHandlerRkISP1::initLinks(const Camera *camera,
>
>  	for (const StreamConfiguration &cfg : config) {
>  		std::string resizer;
> -		if (cfg.stream() == &data->stream_)
> +		if (cfg.stream() == &data->mainPathStream_)
>  			resizer = "rkisp1_resizer_mainpath";
>  		else
>  			return -EINVAL;
> @@ -972,7 +972,7 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)
>  	int ret;
>
>  	std::unique_ptr<RkISP1CameraData> data =
> -		std::make_unique<RkISP1CameraData>(this, video_);
> +		std::make_unique<RkISP1CameraData>(this, mainPathVideo_);
>
>  	ControlInfoMap::Map ctrls;
>  	ctrls.emplace(std::piecewise_construct,
> @@ -993,7 +993,7 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)
>  	if (ret)
>  		return ret;
>
> -	std::set<Stream *> streams{ &data->stream_ };
> +	std::set<Stream *> streams{ &data->mainPathStream_ };
>  	std::shared_ptr<Camera> camera =
>  		Camera::create(this, data->sensor_->id(), streams);
>  	registerCamera(std::move(camera), std::move(data));
> @@ -1023,13 +1023,13 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)
>  	if (isp_->open() < 0)
>  		return false;
>
> -	resizer_ = V4L2Subdevice::fromEntityName(media_, "rkisp1_resizer_mainpath");
> -	if (resizer_->open() < 0)
> +	mainPathResizer_ = V4L2Subdevice::fromEntityName(media_, "rkisp1_resizer_mainpath");
> +	if (mainPathResizer_->open() < 0)
>  		return false;
>
>  	/* Locate and open the capture video node. */
> -	video_ = V4L2VideoDevice::fromEntityName(media_, "rkisp1_mainpath");
> -	if (video_->open() < 0)
> +	mainPathVideo_ = V4L2VideoDevice::fromEntityName(media_, "rkisp1_mainpath");
> +	if (mainPathVideo_->open() < 0)
>  		return false;
>
>  	stat_ = V4L2VideoDevice::fromEntityName(media_, "rkisp1_stats");
> @@ -1040,7 +1040,7 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)
>  	if (param_->open() < 0)
>  		return false;
>
> -	video_->bufferReady.connect(this, &PipelineHandlerRkISP1::bufferReady);
> +	mainPathVideo_->bufferReady.connect(this, &PipelineHandlerRkISP1::bufferReady);
>  	stat_->bufferReady.connect(this, &PipelineHandlerRkISP1::statReady);
>  	param_->bufferReady.connect(this, &PipelineHandlerRkISP1::paramReady);
>
> --
> 2.28.0
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Laurent Pinchart Aug. 20, 2020, 3:20 p.m. UTC | #2 
Hi Niklas,

Thank you for the patch.

On Thu, Aug 13, 2020 at 02:52:41AM +0200, Niklas Söderlund wrote:
> In preparation of supporting both the main and self path prefix the main
> path specific variables with mainPath.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> ---
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 82 ++++++++++++------------
>  1 file changed, 41 insertions(+), 41 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 59614a9f470b7802..60179a1151f18491 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -66,7 +66,7 @@ struct RkISP1FrameInfo {
>  
>  	FrameBuffer *paramBuffer;
>  	FrameBuffer *statBuffer;
> -	FrameBuffer *videoBuffer;
> +	FrameBuffer *mainPathBuffer;
>  
>  	bool paramFilled;
>  	bool paramDequeued;
> @@ -131,7 +131,7 @@ class RkISP1CameraData : public CameraData
>  public:
>  	RkISP1CameraData(PipelineHandler *pipe, V4L2VideoDevice *video)
>  		: CameraData(pipe), sensor_(nullptr), frame_(0),
> -		  frameInfo_(pipe), video_(video)
> +		  frameInfo_(pipe), mainPathVideo_(video)
>  	{
>  	}
>  
> @@ -142,14 +142,14 @@ public:
>  
>  	int loadIPA();
>  
> -	Stream stream_;
> +	Stream mainPathStream_;
>  	CameraSensor *sensor_;
>  	unsigned int frame_;
>  	std::vector<IPABuffer> ipaBuffers_;
>  	RkISP1Frames frameInfo_;
>  	RkISP1Timeline timeline_;
>  
> -	V4L2VideoDevice *video_;
> +	V4L2VideoDevice *mainPathVideo_;
>  
>  private:
>  	void queueFrameAction(unsigned int frame,
> @@ -225,8 +225,8 @@ private:
>  
>  	MediaDevice *media_;
>  	V4L2Subdevice *isp_;
> -	V4L2Subdevice *resizer_;
> -	V4L2VideoDevice *video_;
> +	V4L2Subdevice *mainPathResizer_;
> +	V4L2VideoDevice *mainPathVideo_;
>  	V4L2VideoDevice *param_;
>  	V4L2VideoDevice *stat_;

Would it make sense to create a RkISP1Path class and move the resizer
anv video node there ? The RkISP1Path instances would be stored in
PipelineHandlerRkISP1. I think we can drop the video_ member of
RkISP1CameraData and access it from the pipe in
RkISP1CameraData::validate().

>  
> @@ -259,8 +259,8 @@ RkISP1FrameInfo *RkISP1Frames::create(const RkISP1CameraData *data, Request *req
>  	}
>  	FrameBuffer *statBuffer = pipe_->availableStatBuffers_.front();
>  
> -	FrameBuffer *videoBuffer = request->findBuffer(&data->stream_);
> -	if (!videoBuffer) {
> +	FrameBuffer *mainPathBuffer = request->findBuffer(&data->mainPathStream_);
> +	if (!mainPathBuffer) {
>  		LOG(RkISP1, Error)
>  			<< "Attempt to queue request with invalid stream";
>  		return nullptr;
> @@ -274,7 +274,7 @@ RkISP1FrameInfo *RkISP1Frames::create(const RkISP1CameraData *data, Request *req
>  	info->frame = frame;
>  	info->request = request;
>  	info->paramBuffer = paramBuffer;
> -	info->videoBuffer = videoBuffer;
> +	info->mainPathBuffer = mainPathBuffer;
>  	info->statBuffer = statBuffer;
>  	info->paramFilled = false;
>  	info->paramDequeued = false;
> @@ -333,7 +333,7 @@ RkISP1FrameInfo *RkISP1Frames::find(FrameBuffer *buffer)
>  
>  		if (info->paramBuffer == buffer ||
>  		    info->statBuffer == buffer ||
> -		    info->videoBuffer == buffer)
> +		    info->mainPathBuffer == buffer)
>  			return info;
>  	}
>  
> @@ -405,7 +405,7 @@ protected:
>  
>  		pipe_->param_->queueBuffer(info->paramBuffer);
>  		pipe_->stat_->queueBuffer(info->statBuffer);
> -		pipe_->video_->queueBuffer(info->videoBuffer);
> +		pipe_->mainPathVideo_->queueBuffer(info->mainPathBuffer);
>  	}
>  
>  private:
> @@ -544,10 +544,10 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()
>  	cfg.bufferCount = RKISP1_BUFFER_COUNT;
>  
>  	V4L2DeviceFormat format = {};
> -	format.fourcc = data_->video_->toV4L2PixelFormat(cfg.pixelFormat);
> +	format.fourcc = data_->mainPathVideo_->toV4L2PixelFormat(cfg.pixelFormat);
>  	format.size = cfg.size;
>  
> -	int ret = data_->video_->tryFormat(&format);
> +	int ret = data_->mainPathVideo_->tryFormat(&format);
>  	if (ret)
>  		return Invalid;
>  
> @@ -558,8 +558,8 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()
>  }
>  
>  PipelineHandlerRkISP1::PipelineHandlerRkISP1(CameraManager *manager)
> -	: PipelineHandler(manager), isp_(nullptr), resizer_(nullptr),
> -	  video_(nullptr), param_(nullptr), stat_(nullptr)
> +	: PipelineHandler(manager), isp_(nullptr), mainPathResizer_(nullptr),
> +	  mainPathVideo_(nullptr), param_(nullptr), stat_(nullptr)
>  {
>  }
>  
> @@ -567,8 +567,8 @@ PipelineHandlerRkISP1::~PipelineHandlerRkISP1()
>  {
>  	delete param_;
>  	delete stat_;
> -	delete video_;
> -	delete resizer_;
> +	delete mainPathVideo_;
> +	delete mainPathResizer_;
>  	delete isp_;
>  }
>  
> @@ -649,7 +649,7 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
>  
>  	LOG(RkISP1, Debug) << "ISP output pad configured with " << format.toString();
>  
> -	ret = resizer_->setFormat(0, &format);
> +	ret = mainPathResizer_->setFormat(0, &format);
>  	if (ret < 0)
>  		return ret;
>  
> @@ -659,23 +659,23 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
>  
>  	LOG(RkISP1, Debug) << "Configuring resizer output pad with " << format.toString();
>  
> -	ret = resizer_->setFormat(1, &format);
> +	ret = mainPathResizer_->setFormat(1, &format);
>  	if (ret < 0)
>  		return ret;
>  
>  	LOG(RkISP1, Debug) << "Resizer output pad configured with " << format.toString();
>  
>  	V4L2DeviceFormat outputFormat = {};
> -	outputFormat.fourcc = video_->toV4L2PixelFormat(cfg.pixelFormat);
> +	outputFormat.fourcc = mainPathVideo_->toV4L2PixelFormat(cfg.pixelFormat);
>  	outputFormat.size = cfg.size;
>  	outputFormat.planesCount = 2;
>  
> -	ret = video_->setFormat(&outputFormat);
> +	ret = mainPathVideo_->setFormat(&outputFormat);
>  	if (ret)
>  		return ret;
>  
>  	if (outputFormat.size != cfg.size ||
> -	    outputFormat.fourcc != video_->toV4L2PixelFormat(cfg.pixelFormat)) {
> +	    outputFormat.fourcc != mainPathVideo_->toV4L2PixelFormat(cfg.pixelFormat)) {
>  		LOG(RkISP1, Error)
>  			<< "Unable to configure capture in " << cfg.toString();
>  		return -EINVAL;
> @@ -693,7 +693,7 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
>  	if (ret)
>  		return ret;
>  
> -	cfg.setStream(&data->stream_);
> +	cfg.setStream(&data->mainPathStream_);
>  
>  	return 0;
>  }
> @@ -702,17 +702,17 @@ int PipelineHandlerRkISP1::exportFrameBuffers(Camera *camera, Stream *stream,
>  					      std::vector<std::unique_ptr<FrameBuffer>> *buffers)
>  {
>  	unsigned int count = stream->configuration().bufferCount;
> -	return video_->exportBuffers(count, buffers);
> +	return mainPathVideo_->exportBuffers(count, buffers);
>  }
>  
>  int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)
>  {
>  	RkISP1CameraData *data = cameraData(camera);
> -	unsigned int count = data->stream_.configuration().bufferCount;
> +	unsigned int count = data->mainPathStream_.configuration().bufferCount;
>  	unsigned int ipaBufferId = 1;
>  	int ret;
>  
> -	ret = video_->importBuffers(count);
> +	ret = mainPathVideo_->importBuffers(count);
>  	if (ret < 0)
>  		goto error;
>  
> @@ -745,7 +745,7 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)
>  error:
>  	paramBuffers_.clear();
>  	statBuffers_.clear();
> -	video_->releaseBuffers();
> +	mainPathVideo_->releaseBuffers();
>  
>  	return ret;
>  }
> @@ -776,8 +776,8 @@ int PipelineHandlerRkISP1::freeBuffers(Camera *camera)
>  	if (stat_->releaseBuffers())
>  		LOG(RkISP1, Error) << "Failed to release stat buffers";
>  
> -	if (video_->releaseBuffers())
> -		LOG(RkISP1, Error) << "Failed to release video buffers";
> +	if (mainPathVideo_->releaseBuffers())
> +		LOG(RkISP1, Error) << "Failed to release main path buffers";
>  
>  	return 0;
>  }
> @@ -821,7 +821,7 @@ int PipelineHandlerRkISP1::start(Camera *camera)
>  		return ret;
>  	}
>  
> -	ret = video_->streamOn();
> +	ret = mainPathVideo_->streamOn();
>  	if (ret) {
>  		param_->streamOff();
>  		stat_->streamOff();
> @@ -846,8 +846,8 @@ int PipelineHandlerRkISP1::start(Camera *camera)
>  
>  	std::map<unsigned int, IPAStream> streamConfig;
>  	streamConfig[0] = {
> -		.pixelFormat = data->stream_.configuration().pixelFormat,
> -		.size = data->stream_.configuration().size,
> +		.pixelFormat = data->mainPathStream_.configuration().pixelFormat,
> +		.size = data->mainPathStream_.configuration().size,
>  	};
>  
>  	std::map<unsigned int, const ControlInfoMap &> entityControls;
> @@ -865,7 +865,7 @@ void PipelineHandlerRkISP1::stop(Camera *camera)
>  	RkISP1CameraData *data = cameraData(camera);
>  	int ret;
>  
> -	ret = video_->streamOff();
> +	ret = mainPathVideo_->streamOff();
>  	if (ret)
>  		LOG(RkISP1, Warning)
>  			<< "Failed to stop camera " << camera->id();
> @@ -950,7 +950,7 @@ int PipelineHandlerRkISP1::initLinks(const Camera *camera,
>  
>  	for (const StreamConfiguration &cfg : config) {
>  		std::string resizer;
> -		if (cfg.stream() == &data->stream_)
> +		if (cfg.stream() == &data->mainPathStream_)
>  			resizer = "rkisp1_resizer_mainpath";
>  		else
>  			return -EINVAL;
> @@ -972,7 +972,7 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)
>  	int ret;
>  
>  	std::unique_ptr<RkISP1CameraData> data =
> -		std::make_unique<RkISP1CameraData>(this, video_);
> +		std::make_unique<RkISP1CameraData>(this, mainPathVideo_);
>  
>  	ControlInfoMap::Map ctrls;
>  	ctrls.emplace(std::piecewise_construct,
> @@ -993,7 +993,7 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)
>  	if (ret)
>  		return ret;
>  
> -	std::set<Stream *> streams{ &data->stream_ };
> +	std::set<Stream *> streams{ &data->mainPathStream_ };
>  	std::shared_ptr<Camera> camera =
>  		Camera::create(this, data->sensor_->id(), streams);
>  	registerCamera(std::move(camera), std::move(data));
> @@ -1023,13 +1023,13 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)
>  	if (isp_->open() < 0)
>  		return false;
>  
> -	resizer_ = V4L2Subdevice::fromEntityName(media_, "rkisp1_resizer_mainpath");
> -	if (resizer_->open() < 0)
> +	mainPathResizer_ = V4L2Subdevice::fromEntityName(media_, "rkisp1_resizer_mainpath");
> +	if (mainPathResizer_->open() < 0)
>  		return false;
>  
>  	/* Locate and open the capture video node. */
> -	video_ = V4L2VideoDevice::fromEntityName(media_, "rkisp1_mainpath");
> -	if (video_->open() < 0)
> +	mainPathVideo_ = V4L2VideoDevice::fromEntityName(media_, "rkisp1_mainpath");
> +	if (mainPathVideo_->open() < 0)
>  		return false;
>  
>  	stat_ = V4L2VideoDevice::fromEntityName(media_, "rkisp1_stats");
> @@ -1040,7 +1040,7 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)
>  	if (param_->open() < 0)
>  		return false;
>  
> -	video_->bufferReady.connect(this, &PipelineHandlerRkISP1::bufferReady);
> +	mainPathVideo_->bufferReady.connect(this, &PipelineHandlerRkISP1::bufferReady);
>  	stat_->bufferReady.connect(this, &PipelineHandlerRkISP1::statReady);
>  	param_->bufferReady.connect(this, &PipelineHandlerRkISP1::paramReady);
>
Niklas Söderlund Sept. 14, 2020, 10:25 a.m. UTC | #3 
Hi Jacopo,

On 2020-08-20 11:01:16 +0200, Jacopo Mondi wrote:
> Hi Niklas,
> 
> On Thu, Aug 13, 2020 at 02:52:41AM +0200, Niklas Söderlund wrote:
> > In preparation of supporting both the main and self path prefix the main
> > path specific variables with mainPath.
> 
> Wouldn't it be better to use 'self' and 'main' and omit 'path' ?

I like to keep the 'path' in the name as it becomes clearer, at lest to 
me. I don't feel strongly about this so I'm open to other ideas. As 
Laurent points out in his review maybe we should add a new class to help 
abstract all this, maybe we could revisit the topic then?

> 
> >
> > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> > ---
> >  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 82 ++++++++++++------------
> >  1 file changed, 41 insertions(+), 41 deletions(-)
> >
> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > index 59614a9f470b7802..60179a1151f18491 100644
> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > @@ -66,7 +66,7 @@ struct RkISP1FrameInfo {
> >
> >  	FrameBuffer *paramBuffer;
> >  	FrameBuffer *statBuffer;
> > -	FrameBuffer *videoBuffer;
> > +	FrameBuffer *mainPathBuffer;
> >
> >  	bool paramFilled;
> >  	bool paramDequeued;
> > @@ -131,7 +131,7 @@ class RkISP1CameraData : public CameraData
> >  public:
> >  	RkISP1CameraData(PipelineHandler *pipe, V4L2VideoDevice *video)
> >  		: CameraData(pipe), sensor_(nullptr), frame_(0),
> > -		  frameInfo_(pipe), video_(video)
> > +		  frameInfo_(pipe), mainPathVideo_(video)
> >  	{
> >  	}
> >
> > @@ -142,14 +142,14 @@ public:
> >
> >  	int loadIPA();
> >
> > -	Stream stream_;
> > +	Stream mainPathStream_;
> >  	CameraSensor *sensor_;
> >  	unsigned int frame_;
> >  	std::vector<IPABuffer> ipaBuffers_;
> >  	RkISP1Frames frameInfo_;
> >  	RkISP1Timeline timeline_;
> >
> > -	V4L2VideoDevice *video_;
> > +	V4L2VideoDevice *mainPathVideo_;
> >
> >  private:
> >  	void queueFrameAction(unsigned int frame,
> > @@ -225,8 +225,8 @@ private:
> >
> >  	MediaDevice *media_;
> >  	V4L2Subdevice *isp_;
> > -	V4L2Subdevice *resizer_;
> > -	V4L2VideoDevice *video_;
> > +	V4L2Subdevice *mainPathResizer_;
> > +	V4L2VideoDevice *mainPathVideo_;
> >  	V4L2VideoDevice *param_;
> >  	V4L2VideoDevice *stat_;
> >
> > @@ -259,8 +259,8 @@ RkISP1FrameInfo *RkISP1Frames::create(const RkISP1CameraData *data, Request *req
> >  	}
> >  	FrameBuffer *statBuffer = pipe_->availableStatBuffers_.front();
> >
> > -	FrameBuffer *videoBuffer = request->findBuffer(&data->stream_);
> > -	if (!videoBuffer) {
> > +	FrameBuffer *mainPathBuffer = request->findBuffer(&data->mainPathStream_);
> > +	if (!mainPathBuffer) {
> >  		LOG(RkISP1, Error)
> >  			<< "Attempt to queue request with invalid stream";
> >  		return nullptr;
> > @@ -274,7 +274,7 @@ RkISP1FrameInfo *RkISP1Frames::create(const RkISP1CameraData *data, Request *req
> >  	info->frame = frame;
> >  	info->request = request;
> >  	info->paramBuffer = paramBuffer;
> > -	info->videoBuffer = videoBuffer;
> > +	info->mainPathBuffer = mainPathBuffer;
> >  	info->statBuffer = statBuffer;
> >  	info->paramFilled = false;
> >  	info->paramDequeued = false;
> > @@ -333,7 +333,7 @@ RkISP1FrameInfo *RkISP1Frames::find(FrameBuffer *buffer)
> >
> >  		if (info->paramBuffer == buffer ||
> >  		    info->statBuffer == buffer ||
> > -		    info->videoBuffer == buffer)
> > +		    info->mainPathBuffer == buffer)
> >  			return info;
> >  	}
> >
> > @@ -405,7 +405,7 @@ protected:
> >
> >  		pipe_->param_->queueBuffer(info->paramBuffer);
> >  		pipe_->stat_->queueBuffer(info->statBuffer);
> > -		pipe_->video_->queueBuffer(info->videoBuffer);
> > +		pipe_->mainPathVideo_->queueBuffer(info->mainPathBuffer);
> >  	}
> >
> >  private:
> > @@ -544,10 +544,10 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()
> >  	cfg.bufferCount = RKISP1_BUFFER_COUNT;
> >
> >  	V4L2DeviceFormat format = {};
> > -	format.fourcc = data_->video_->toV4L2PixelFormat(cfg.pixelFormat);
> > +	format.fourcc = data_->mainPathVideo_->toV4L2PixelFormat(cfg.pixelFormat);
> >  	format.size = cfg.size;
> >
> > -	int ret = data_->video_->tryFormat(&format);
> > +	int ret = data_->mainPathVideo_->tryFormat(&format);
> >  	if (ret)
> >  		return Invalid;
> >
> > @@ -558,8 +558,8 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()
> >  }
> >
> >  PipelineHandlerRkISP1::PipelineHandlerRkISP1(CameraManager *manager)
> > -	: PipelineHandler(manager), isp_(nullptr), resizer_(nullptr),
> > -	  video_(nullptr), param_(nullptr), stat_(nullptr)
> > +	: PipelineHandler(manager), isp_(nullptr), mainPathResizer_(nullptr),
> > +	  mainPathVideo_(nullptr), param_(nullptr), stat_(nullptr)
> >  {
> >  }
> >
> > @@ -567,8 +567,8 @@ PipelineHandlerRkISP1::~PipelineHandlerRkISP1()
> >  {
> >  	delete param_;
> >  	delete stat_;
> > -	delete video_;
> > -	delete resizer_;
> > +	delete mainPathVideo_;
> > +	delete mainPathResizer_;
> >  	delete isp_;
> >  }
> >
> > @@ -649,7 +649,7 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
> >
> >  	LOG(RkISP1, Debug) << "ISP output pad configured with " << format.toString();
> >
> > -	ret = resizer_->setFormat(0, &format);
> > +	ret = mainPathResizer_->setFormat(0, &format);
> >  	if (ret < 0)
> >  		return ret;
> >
> > @@ -659,23 +659,23 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
> >
> >  	LOG(RkISP1, Debug) << "Configuring resizer output pad with " << format.toString();
> >
> > -	ret = resizer_->setFormat(1, &format);
> > +	ret = mainPathResizer_->setFormat(1, &format);
> >  	if (ret < 0)
> >  		return ret;
> >
> >  	LOG(RkISP1, Debug) << "Resizer output pad configured with " << format.toString();
> >
> >  	V4L2DeviceFormat outputFormat = {};
> > -	outputFormat.fourcc = video_->toV4L2PixelFormat(cfg.pixelFormat);
> > +	outputFormat.fourcc = mainPathVideo_->toV4L2PixelFormat(cfg.pixelFormat);
> >  	outputFormat.size = cfg.size;
> >  	outputFormat.planesCount = 2;
> >
> > -	ret = video_->setFormat(&outputFormat);
> > +	ret = mainPathVideo_->setFormat(&outputFormat);
> >  	if (ret)
> >  		return ret;
> >
> >  	if (outputFormat.size != cfg.size ||
> > -	    outputFormat.fourcc != video_->toV4L2PixelFormat(cfg.pixelFormat)) {
> > +	    outputFormat.fourcc != mainPathVideo_->toV4L2PixelFormat(cfg.pixelFormat)) {
> >  		LOG(RkISP1, Error)
> >  			<< "Unable to configure capture in " << cfg.toString();
> >  		return -EINVAL;
> > @@ -693,7 +693,7 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
> >  	if (ret)
> >  		return ret;
> >
> > -	cfg.setStream(&data->stream_);
> > +	cfg.setStream(&data->mainPathStream_);
> >
> >  	return 0;
> >  }
> > @@ -702,17 +702,17 @@ int PipelineHandlerRkISP1::exportFrameBuffers(Camera *camera, Stream *stream,
> >  					      std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> >  {
> >  	unsigned int count = stream->configuration().bufferCount;
> > -	return video_->exportBuffers(count, buffers);
> > +	return mainPathVideo_->exportBuffers(count, buffers);
> >  }
> >
> >  int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)
> >  {
> >  	RkISP1CameraData *data = cameraData(camera);
> > -	unsigned int count = data->stream_.configuration().bufferCount;
> > +	unsigned int count = data->mainPathStream_.configuration().bufferCount;
> >  	unsigned int ipaBufferId = 1;
> >  	int ret;
> >
> > -	ret = video_->importBuffers(count);
> > +	ret = mainPathVideo_->importBuffers(count);
> >  	if (ret < 0)
> >  		goto error;
> >
> > @@ -745,7 +745,7 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)
> >  error:
> >  	paramBuffers_.clear();
> >  	statBuffers_.clear();
> > -	video_->releaseBuffers();
> > +	mainPathVideo_->releaseBuffers();
> >
> >  	return ret;
> >  }
> > @@ -776,8 +776,8 @@ int PipelineHandlerRkISP1::freeBuffers(Camera *camera)
> >  	if (stat_->releaseBuffers())
> >  		LOG(RkISP1, Error) << "Failed to release stat buffers";
> >
> > -	if (video_->releaseBuffers())
> > -		LOG(RkISP1, Error) << "Failed to release video buffers";
> > +	if (mainPathVideo_->releaseBuffers())
> > +		LOG(RkISP1, Error) << "Failed to release main path buffers";
> >
> >  	return 0;
> >  }
> > @@ -821,7 +821,7 @@ int PipelineHandlerRkISP1::start(Camera *camera)
> >  		return ret;
> >  	}
> >
> > -	ret = video_->streamOn();
> > +	ret = mainPathVideo_->streamOn();
> >  	if (ret) {
> >  		param_->streamOff();
> >  		stat_->streamOff();
> > @@ -846,8 +846,8 @@ int PipelineHandlerRkISP1::start(Camera *camera)
> >
> >  	std::map<unsigned int, IPAStream> streamConfig;
> >  	streamConfig[0] = {
> > -		.pixelFormat = data->stream_.configuration().pixelFormat,
> > -		.size = data->stream_.configuration().size,
> > +		.pixelFormat = data->mainPathStream_.configuration().pixelFormat,
> > +		.size = data->mainPathStream_.configuration().size,
> >  	};
> >
> >  	std::map<unsigned int, const ControlInfoMap &> entityControls;
> > @@ -865,7 +865,7 @@ void PipelineHandlerRkISP1::stop(Camera *camera)
> >  	RkISP1CameraData *data = cameraData(camera);
> >  	int ret;
> >
> > -	ret = video_->streamOff();
> > +	ret = mainPathVideo_->streamOff();
> >  	if (ret)
> >  		LOG(RkISP1, Warning)
> >  			<< "Failed to stop camera " << camera->id();
> > @@ -950,7 +950,7 @@ int PipelineHandlerRkISP1::initLinks(const Camera *camera,
> >
> >  	for (const StreamConfiguration &cfg : config) {
> >  		std::string resizer;
> > -		if (cfg.stream() == &data->stream_)
> > +		if (cfg.stream() == &data->mainPathStream_)
> >  			resizer = "rkisp1_resizer_mainpath";
> >  		else
> >  			return -EINVAL;
> > @@ -972,7 +972,7 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)
> >  	int ret;
> >
> >  	std::unique_ptr<RkISP1CameraData> data =
> > -		std::make_unique<RkISP1CameraData>(this, video_);
> > +		std::make_unique<RkISP1CameraData>(this, mainPathVideo_);
> >
> >  	ControlInfoMap::Map ctrls;
> >  	ctrls.emplace(std::piecewise_construct,
> > @@ -993,7 +993,7 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)
> >  	if (ret)
> >  		return ret;
> >
> > -	std::set<Stream *> streams{ &data->stream_ };
> > +	std::set<Stream *> streams{ &data->mainPathStream_ };
> >  	std::shared_ptr<Camera> camera =
> >  		Camera::create(this, data->sensor_->id(), streams);
> >  	registerCamera(std::move(camera), std::move(data));
> > @@ -1023,13 +1023,13 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)
> >  	if (isp_->open() < 0)
> >  		return false;
> >
> > -	resizer_ = V4L2Subdevice::fromEntityName(media_, "rkisp1_resizer_mainpath");
> > -	if (resizer_->open() < 0)
> > +	mainPathResizer_ = V4L2Subdevice::fromEntityName(media_, "rkisp1_resizer_mainpath");
> > +	if (mainPathResizer_->open() < 0)
> >  		return false;
> >
> >  	/* Locate and open the capture video node. */
> > -	video_ = V4L2VideoDevice::fromEntityName(media_, "rkisp1_mainpath");
> > -	if (video_->open() < 0)
> > +	mainPathVideo_ = V4L2VideoDevice::fromEntityName(media_, "rkisp1_mainpath");
> > +	if (mainPathVideo_->open() < 0)
> >  		return false;
> >
> >  	stat_ = V4L2VideoDevice::fromEntityName(media_, "rkisp1_stats");
> > @@ -1040,7 +1040,7 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)
> >  	if (param_->open() < 0)
> >  		return false;
> >
> > -	video_->bufferReady.connect(this, &PipelineHandlerRkISP1::bufferReady);
> > +	mainPathVideo_->bufferReady.connect(this, &PipelineHandlerRkISP1::bufferReady);
> >  	stat_->bufferReady.connect(this, &PipelineHandlerRkISP1::statReady);
> >  	param_->bufferReady.connect(this, &PipelineHandlerRkISP1::paramReady);
> >
> > --
> > 2.28.0
> >
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel@lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel
Niklas Söderlund Sept. 14, 2020, 10:27 a.m. UTC | #4 
Hi Laurent,

On 2020-08-20 18:20:07 +0300, Laurent Pinchart wrote:
> Hi Niklas,
> 
> Thank you for the patch.
> 
> On Thu, Aug 13, 2020 at 02:52:41AM +0200, Niklas Söderlund wrote:
> > In preparation of supporting both the main and self path prefix the main
> > path specific variables with mainPath.
> > 
> > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> > ---
> >  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 82 ++++++++++++------------
> >  1 file changed, 41 insertions(+), 41 deletions(-)
> > 
> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > index 59614a9f470b7802..60179a1151f18491 100644
> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > @@ -66,7 +66,7 @@ struct RkISP1FrameInfo {
> >  
> >  	FrameBuffer *paramBuffer;
> >  	FrameBuffer *statBuffer;
> > -	FrameBuffer *videoBuffer;
> > +	FrameBuffer *mainPathBuffer;
> >  
> >  	bool paramFilled;
> >  	bool paramDequeued;
> > @@ -131,7 +131,7 @@ class RkISP1CameraData : public CameraData
> >  public:
> >  	RkISP1CameraData(PipelineHandler *pipe, V4L2VideoDevice *video)
> >  		: CameraData(pipe), sensor_(nullptr), frame_(0),
> > -		  frameInfo_(pipe), video_(video)
> > +		  frameInfo_(pipe), mainPathVideo_(video)
> >  	{
> >  	}
> >  
> > @@ -142,14 +142,14 @@ public:
> >  
> >  	int loadIPA();
> >  
> > -	Stream stream_;
> > +	Stream mainPathStream_;
> >  	CameraSensor *sensor_;
> >  	unsigned int frame_;
> >  	std::vector<IPABuffer> ipaBuffers_;
> >  	RkISP1Frames frameInfo_;
> >  	RkISP1Timeline timeline_;
> >  
> > -	V4L2VideoDevice *video_;
> > +	V4L2VideoDevice *mainPathVideo_;
> >  
> >  private:
> >  	void queueFrameAction(unsigned int frame,
> > @@ -225,8 +225,8 @@ private:
> >  
> >  	MediaDevice *media_;
> >  	V4L2Subdevice *isp_;
> > -	V4L2Subdevice *resizer_;
> > -	V4L2VideoDevice *video_;
> > +	V4L2Subdevice *mainPathResizer_;
> > +	V4L2VideoDevice *mainPathVideo_;
> >  	V4L2VideoDevice *param_;
> >  	V4L2VideoDevice *stat_;
> 
> Would it make sense to create a RkISP1Path class and move the resizer
> anv video node there ? The RkISP1Path instances would be stored in
> PipelineHandlerRkISP1. I think we can drop the video_ member of
> RkISP1CameraData and access it from the pipe in
> RkISP1CameraData::validate().

I think this is a good idea. I do however think we should do this in a 
separate series, either before this series or on-top. I really hate 
having to post series as large as this as they take forever to get 
merged and extending it with more stuff will only make it harder ;-)

Would you be OK with this work being done on-top?

> 
> >  
> > @@ -259,8 +259,8 @@ RkISP1FrameInfo *RkISP1Frames::create(const RkISP1CameraData *data, Request *req
> >  	}
> >  	FrameBuffer *statBuffer = pipe_->availableStatBuffers_.front();
> >  
> > -	FrameBuffer *videoBuffer = request->findBuffer(&data->stream_);
> > -	if (!videoBuffer) {
> > +	FrameBuffer *mainPathBuffer = request->findBuffer(&data->mainPathStream_);
> > +	if (!mainPathBuffer) {
> >  		LOG(RkISP1, Error)
> >  			<< "Attempt to queue request with invalid stream";
> >  		return nullptr;
> > @@ -274,7 +274,7 @@ RkISP1FrameInfo *RkISP1Frames::create(const RkISP1CameraData *data, Request *req
> >  	info->frame = frame;
> >  	info->request = request;
> >  	info->paramBuffer = paramBuffer;
> > -	info->videoBuffer = videoBuffer;
> > +	info->mainPathBuffer = mainPathBuffer;
> >  	info->statBuffer = statBuffer;
> >  	info->paramFilled = false;
> >  	info->paramDequeued = false;
> > @@ -333,7 +333,7 @@ RkISP1FrameInfo *RkISP1Frames::find(FrameBuffer *buffer)
> >  
> >  		if (info->paramBuffer == buffer ||
> >  		    info->statBuffer == buffer ||
> > -		    info->videoBuffer == buffer)
> > +		    info->mainPathBuffer == buffer)
> >  			return info;
> >  	}
> >  
> > @@ -405,7 +405,7 @@ protected:
> >  
> >  		pipe_->param_->queueBuffer(info->paramBuffer);
> >  		pipe_->stat_->queueBuffer(info->statBuffer);
> > -		pipe_->video_->queueBuffer(info->videoBuffer);
> > +		pipe_->mainPathVideo_->queueBuffer(info->mainPathBuffer);
> >  	}
> >  
> >  private:
> > @@ -544,10 +544,10 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()
> >  	cfg.bufferCount = RKISP1_BUFFER_COUNT;
> >  
> >  	V4L2DeviceFormat format = {};
> > -	format.fourcc = data_->video_->toV4L2PixelFormat(cfg.pixelFormat);
> > +	format.fourcc = data_->mainPathVideo_->toV4L2PixelFormat(cfg.pixelFormat);
> >  	format.size = cfg.size;
> >  
> > -	int ret = data_->video_->tryFormat(&format);
> > +	int ret = data_->mainPathVideo_->tryFormat(&format);
> >  	if (ret)
> >  		return Invalid;
> >  
> > @@ -558,8 +558,8 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()
> >  }
> >  
> >  PipelineHandlerRkISP1::PipelineHandlerRkISP1(CameraManager *manager)
> > -	: PipelineHandler(manager), isp_(nullptr), resizer_(nullptr),
> > -	  video_(nullptr), param_(nullptr), stat_(nullptr)
> > +	: PipelineHandler(manager), isp_(nullptr), mainPathResizer_(nullptr),
> > +	  mainPathVideo_(nullptr), param_(nullptr), stat_(nullptr)
> >  {
> >  }
> >  
> > @@ -567,8 +567,8 @@ PipelineHandlerRkISP1::~PipelineHandlerRkISP1()
> >  {
> >  	delete param_;
> >  	delete stat_;
> > -	delete video_;
> > -	delete resizer_;
> > +	delete mainPathVideo_;
> > +	delete mainPathResizer_;
> >  	delete isp_;
> >  }
> >  
> > @@ -649,7 +649,7 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
> >  
> >  	LOG(RkISP1, Debug) << "ISP output pad configured with " << format.toString();
> >  
> > -	ret = resizer_->setFormat(0, &format);
> > +	ret = mainPathResizer_->setFormat(0, &format);
> >  	if (ret < 0)
> >  		return ret;
> >  
> > @@ -659,23 +659,23 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
> >  
> >  	LOG(RkISP1, Debug) << "Configuring resizer output pad with " << format.toString();
> >  
> > -	ret = resizer_->setFormat(1, &format);
> > +	ret = mainPathResizer_->setFormat(1, &format);
> >  	if (ret < 0)
> >  		return ret;
> >  
> >  	LOG(RkISP1, Debug) << "Resizer output pad configured with " << format.toString();
> >  
> >  	V4L2DeviceFormat outputFormat = {};
> > -	outputFormat.fourcc = video_->toV4L2PixelFormat(cfg.pixelFormat);
> > +	outputFormat.fourcc = mainPathVideo_->toV4L2PixelFormat(cfg.pixelFormat);
> >  	outputFormat.size = cfg.size;
> >  	outputFormat.planesCount = 2;
> >  
> > -	ret = video_->setFormat(&outputFormat);
> > +	ret = mainPathVideo_->setFormat(&outputFormat);
> >  	if (ret)
> >  		return ret;
> >  
> >  	if (outputFormat.size != cfg.size ||
> > -	    outputFormat.fourcc != video_->toV4L2PixelFormat(cfg.pixelFormat)) {
> > +	    outputFormat.fourcc != mainPathVideo_->toV4L2PixelFormat(cfg.pixelFormat)) {
> >  		LOG(RkISP1, Error)
> >  			<< "Unable to configure capture in " << cfg.toString();
> >  		return -EINVAL;
> > @@ -693,7 +693,7 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
> >  	if (ret)
> >  		return ret;
> >  
> > -	cfg.setStream(&data->stream_);
> > +	cfg.setStream(&data->mainPathStream_);
> >  
> >  	return 0;
> >  }
> > @@ -702,17 +702,17 @@ int PipelineHandlerRkISP1::exportFrameBuffers(Camera *camera, Stream *stream,
> >  					      std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> >  {
> >  	unsigned int count = stream->configuration().bufferCount;
> > -	return video_->exportBuffers(count, buffers);
> > +	return mainPathVideo_->exportBuffers(count, buffers);
> >  }
> >  
> >  int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)
> >  {
> >  	RkISP1CameraData *data = cameraData(camera);
> > -	unsigned int count = data->stream_.configuration().bufferCount;
> > +	unsigned int count = data->mainPathStream_.configuration().bufferCount;
> >  	unsigned int ipaBufferId = 1;
> >  	int ret;
> >  
> > -	ret = video_->importBuffers(count);
> > +	ret = mainPathVideo_->importBuffers(count);
> >  	if (ret < 0)
> >  		goto error;
> >  
> > @@ -745,7 +745,7 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)
> >  error:
> >  	paramBuffers_.clear();
> >  	statBuffers_.clear();
> > -	video_->releaseBuffers();
> > +	mainPathVideo_->releaseBuffers();
> >  
> >  	return ret;
> >  }
> > @@ -776,8 +776,8 @@ int PipelineHandlerRkISP1::freeBuffers(Camera *camera)
> >  	if (stat_->releaseBuffers())
> >  		LOG(RkISP1, Error) << "Failed to release stat buffers";
> >  
> > -	if (video_->releaseBuffers())
> > -		LOG(RkISP1, Error) << "Failed to release video buffers";
> > +	if (mainPathVideo_->releaseBuffers())
> > +		LOG(RkISP1, Error) << "Failed to release main path buffers";
> >  
> >  	return 0;
> >  }
> > @@ -821,7 +821,7 @@ int PipelineHandlerRkISP1::start(Camera *camera)
> >  		return ret;
> >  	}
> >  
> > -	ret = video_->streamOn();
> > +	ret = mainPathVideo_->streamOn();
> >  	if (ret) {
> >  		param_->streamOff();
> >  		stat_->streamOff();
> > @@ -846,8 +846,8 @@ int PipelineHandlerRkISP1::start(Camera *camera)
> >  
> >  	std::map<unsigned int, IPAStream> streamConfig;
> >  	streamConfig[0] = {
> > -		.pixelFormat = data->stream_.configuration().pixelFormat,
> > -		.size = data->stream_.configuration().size,
> > +		.pixelFormat = data->mainPathStream_.configuration().pixelFormat,
> > +		.size = data->mainPathStream_.configuration().size,
> >  	};
> >  
> >  	std::map<unsigned int, const ControlInfoMap &> entityControls;
> > @@ -865,7 +865,7 @@ void PipelineHandlerRkISP1::stop(Camera *camera)
> >  	RkISP1CameraData *data = cameraData(camera);
> >  	int ret;
> >  
> > -	ret = video_->streamOff();
> > +	ret = mainPathVideo_->streamOff();
> >  	if (ret)
> >  		LOG(RkISP1, Warning)
> >  			<< "Failed to stop camera " << camera->id();
> > @@ -950,7 +950,7 @@ int PipelineHandlerRkISP1::initLinks(const Camera *camera,
> >  
> >  	for (const StreamConfiguration &cfg : config) {
> >  		std::string resizer;
> > -		if (cfg.stream() == &data->stream_)
> > +		if (cfg.stream() == &data->mainPathStream_)
> >  			resizer = "rkisp1_resizer_mainpath";
> >  		else
> >  			return -EINVAL;
> > @@ -972,7 +972,7 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)
> >  	int ret;
> >  
> >  	std::unique_ptr<RkISP1CameraData> data =
> > -		std::make_unique<RkISP1CameraData>(this, video_);
> > +		std::make_unique<RkISP1CameraData>(this, mainPathVideo_);
> >  
> >  	ControlInfoMap::Map ctrls;
> >  	ctrls.emplace(std::piecewise_construct,
> > @@ -993,7 +993,7 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)
> >  	if (ret)
> >  		return ret;
> >  
> > -	std::set<Stream *> streams{ &data->stream_ };
> > +	std::set<Stream *> streams{ &data->mainPathStream_ };
> >  	std::shared_ptr<Camera> camera =
> >  		Camera::create(this, data->sensor_->id(), streams);
> >  	registerCamera(std::move(camera), std::move(data));
> > @@ -1023,13 +1023,13 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)
> >  	if (isp_->open() < 0)
> >  		return false;
> >  
> > -	resizer_ = V4L2Subdevice::fromEntityName(media_, "rkisp1_resizer_mainpath");
> > -	if (resizer_->open() < 0)
> > +	mainPathResizer_ = V4L2Subdevice::fromEntityName(media_, "rkisp1_resizer_mainpath");
> > +	if (mainPathResizer_->open() < 0)
> >  		return false;
> >  
> >  	/* Locate and open the capture video node. */
> > -	video_ = V4L2VideoDevice::fromEntityName(media_, "rkisp1_mainpath");
> > -	if (video_->open() < 0)
> > +	mainPathVideo_ = V4L2VideoDevice::fromEntityName(media_, "rkisp1_mainpath");
> > +	if (mainPathVideo_->open() < 0)
> >  		return false;
> >  
> >  	stat_ = V4L2VideoDevice::fromEntityName(media_, "rkisp1_stats");
> > @@ -1040,7 +1040,7 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)
> >  	if (param_->open() < 0)
> >  		return false;
> >  
> > -	video_->bufferReady.connect(this, &PipelineHandlerRkISP1::bufferReady);
> > +	mainPathVideo_->bufferReady.connect(this, &PipelineHandlerRkISP1::bufferReady);
> >  	stat_->bufferReady.connect(this, &PipelineHandlerRkISP1::statReady);
> >  	param_->bufferReady.connect(this, &PipelineHandlerRkISP1::paramReady);
> >  
> 
> -- 
> Regards,
> 
> Laurent Pinchart

Patch
diff mbox series 

diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index 59614a9f470b7802..60179a1151f18491 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -66,7 +66,7 @@  struct RkISP1FrameInfo {
 
 	FrameBuffer *paramBuffer;
 	FrameBuffer *statBuffer;
-	FrameBuffer *videoBuffer;
+	FrameBuffer *mainPathBuffer;
 
 	bool paramFilled;
 	bool paramDequeued;
@@ -131,7 +131,7 @@  class RkISP1CameraData : public CameraData
 public:
 	RkISP1CameraData(PipelineHandler *pipe, V4L2VideoDevice *video)
 		: CameraData(pipe), sensor_(nullptr), frame_(0),
-		  frameInfo_(pipe), video_(video)
+		  frameInfo_(pipe), mainPathVideo_(video)
 	{
 	}
 
@@ -142,14 +142,14 @@  public:
 
 	int loadIPA();
 
-	Stream stream_;
+	Stream mainPathStream_;
 	CameraSensor *sensor_;
 	unsigned int frame_;
 	std::vector<IPABuffer> ipaBuffers_;
 	RkISP1Frames frameInfo_;
 	RkISP1Timeline timeline_;
 
-	V4L2VideoDevice *video_;
+	V4L2VideoDevice *mainPathVideo_;
 
 private:
 	void queueFrameAction(unsigned int frame,
@@ -225,8 +225,8 @@  private:
 
 	MediaDevice *media_;
 	V4L2Subdevice *isp_;
-	V4L2Subdevice *resizer_;
-	V4L2VideoDevice *video_;
+	V4L2Subdevice *mainPathResizer_;
+	V4L2VideoDevice *mainPathVideo_;
 	V4L2VideoDevice *param_;
 	V4L2VideoDevice *stat_;
 
@@ -259,8 +259,8 @@  RkISP1FrameInfo *RkISP1Frames::create(const RkISP1CameraData *data, Request *req
 	}
 	FrameBuffer *statBuffer = pipe_->availableStatBuffers_.front();
 
-	FrameBuffer *videoBuffer = request->findBuffer(&data->stream_);
-	if (!videoBuffer) {
+	FrameBuffer *mainPathBuffer = request->findBuffer(&data->mainPathStream_);
+	if (!mainPathBuffer) {
 		LOG(RkISP1, Error)
 			<< "Attempt to queue request with invalid stream";
 		return nullptr;
@@ -274,7 +274,7 @@  RkISP1FrameInfo *RkISP1Frames::create(const RkISP1CameraData *data, Request *req
 	info->frame = frame;
 	info->request = request;
 	info->paramBuffer = paramBuffer;
-	info->videoBuffer = videoBuffer;
+	info->mainPathBuffer = mainPathBuffer;
 	info->statBuffer = statBuffer;
 	info->paramFilled = false;
 	info->paramDequeued = false;
@@ -333,7 +333,7 @@  RkISP1FrameInfo *RkISP1Frames::find(FrameBuffer *buffer)
 
 		if (info->paramBuffer == buffer ||
 		    info->statBuffer == buffer ||
-		    info->videoBuffer == buffer)
+		    info->mainPathBuffer == buffer)
 			return info;
 	}
 
@@ -405,7 +405,7 @@  protected:
 
 		pipe_->param_->queueBuffer(info->paramBuffer);
 		pipe_->stat_->queueBuffer(info->statBuffer);
-		pipe_->video_->queueBuffer(info->videoBuffer);
+		pipe_->mainPathVideo_->queueBuffer(info->mainPathBuffer);
 	}
 
 private:
@@ -544,10 +544,10 @@  CameraConfiguration::Status RkISP1CameraConfiguration::validate()
 	cfg.bufferCount = RKISP1_BUFFER_COUNT;
 
 	V4L2DeviceFormat format = {};
-	format.fourcc = data_->video_->toV4L2PixelFormat(cfg.pixelFormat);
+	format.fourcc = data_->mainPathVideo_->toV4L2PixelFormat(cfg.pixelFormat);
 	format.size = cfg.size;
 
-	int ret = data_->video_->tryFormat(&format);
+	int ret = data_->mainPathVideo_->tryFormat(&format);
 	if (ret)
 		return Invalid;
 
@@ -558,8 +558,8 @@  CameraConfiguration::Status RkISP1CameraConfiguration::validate()
 }
 
 PipelineHandlerRkISP1::PipelineHandlerRkISP1(CameraManager *manager)
-	: PipelineHandler(manager), isp_(nullptr), resizer_(nullptr),
-	  video_(nullptr), param_(nullptr), stat_(nullptr)
+	: PipelineHandler(manager), isp_(nullptr), mainPathResizer_(nullptr),
+	  mainPathVideo_(nullptr), param_(nullptr), stat_(nullptr)
 {
 }
 
@@ -567,8 +567,8 @@  PipelineHandlerRkISP1::~PipelineHandlerRkISP1()
 {
 	delete param_;
 	delete stat_;
-	delete video_;
-	delete resizer_;
+	delete mainPathVideo_;
+	delete mainPathResizer_;
 	delete isp_;
 }
 
@@ -649,7 +649,7 @@  int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
 
 	LOG(RkISP1, Debug) << "ISP output pad configured with " << format.toString();
 
-	ret = resizer_->setFormat(0, &format);
+	ret = mainPathResizer_->setFormat(0, &format);
 	if (ret < 0)
 		return ret;
 
@@ -659,23 +659,23 @@  int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
 
 	LOG(RkISP1, Debug) << "Configuring resizer output pad with " << format.toString();
 
-	ret = resizer_->setFormat(1, &format);
+	ret = mainPathResizer_->setFormat(1, &format);
 	if (ret < 0)
 		return ret;
 
 	LOG(RkISP1, Debug) << "Resizer output pad configured with " << format.toString();
 
 	V4L2DeviceFormat outputFormat = {};
-	outputFormat.fourcc = video_->toV4L2PixelFormat(cfg.pixelFormat);
+	outputFormat.fourcc = mainPathVideo_->toV4L2PixelFormat(cfg.pixelFormat);
 	outputFormat.size = cfg.size;
 	outputFormat.planesCount = 2;
 
-	ret = video_->setFormat(&outputFormat);
+	ret = mainPathVideo_->setFormat(&outputFormat);
 	if (ret)
 		return ret;
 
 	if (outputFormat.size != cfg.size ||
-	    outputFormat.fourcc != video_->toV4L2PixelFormat(cfg.pixelFormat)) {
+	    outputFormat.fourcc != mainPathVideo_->toV4L2PixelFormat(cfg.pixelFormat)) {
 		LOG(RkISP1, Error)
 			<< "Unable to configure capture in " << cfg.toString();
 		return -EINVAL;
@@ -693,7 +693,7 @@  int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
 	if (ret)
 		return ret;
 
-	cfg.setStream(&data->stream_);
+	cfg.setStream(&data->mainPathStream_);
 
 	return 0;
 }
@@ -702,17 +702,17 @@  int PipelineHandlerRkISP1::exportFrameBuffers(Camera *camera, Stream *stream,
 					      std::vector<std::unique_ptr<FrameBuffer>> *buffers)
 {
 	unsigned int count = stream->configuration().bufferCount;
-	return video_->exportBuffers(count, buffers);
+	return mainPathVideo_->exportBuffers(count, buffers);
 }
 
 int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)
 {
 	RkISP1CameraData *data = cameraData(camera);
-	unsigned int count = data->stream_.configuration().bufferCount;
+	unsigned int count = data->mainPathStream_.configuration().bufferCount;
 	unsigned int ipaBufferId = 1;
 	int ret;
 
-	ret = video_->importBuffers(count);
+	ret = mainPathVideo_->importBuffers(count);
 	if (ret < 0)
 		goto error;
 
@@ -745,7 +745,7 @@  int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)
 error:
 	paramBuffers_.clear();
 	statBuffers_.clear();
-	video_->releaseBuffers();
+	mainPathVideo_->releaseBuffers();
 
 	return ret;
 }
@@ -776,8 +776,8 @@  int PipelineHandlerRkISP1::freeBuffers(Camera *camera)
 	if (stat_->releaseBuffers())
 		LOG(RkISP1, Error) << "Failed to release stat buffers";
 
-	if (video_->releaseBuffers())
-		LOG(RkISP1, Error) << "Failed to release video buffers";
+	if (mainPathVideo_->releaseBuffers())
+		LOG(RkISP1, Error) << "Failed to release main path buffers";
 
 	return 0;
 }
@@ -821,7 +821,7 @@  int PipelineHandlerRkISP1::start(Camera *camera)
 		return ret;
 	}
 
-	ret = video_->streamOn();
+	ret = mainPathVideo_->streamOn();
 	if (ret) {
 		param_->streamOff();
 		stat_->streamOff();
@@ -846,8 +846,8 @@  int PipelineHandlerRkISP1::start(Camera *camera)
 
 	std::map<unsigned int, IPAStream> streamConfig;
 	streamConfig[0] = {
-		.pixelFormat = data->stream_.configuration().pixelFormat,
-		.size = data->stream_.configuration().size,
+		.pixelFormat = data->mainPathStream_.configuration().pixelFormat,
+		.size = data->mainPathStream_.configuration().size,
 	};
 
 	std::map<unsigned int, const ControlInfoMap &> entityControls;
@@ -865,7 +865,7 @@  void PipelineHandlerRkISP1::stop(Camera *camera)
 	RkISP1CameraData *data = cameraData(camera);
 	int ret;
 
-	ret = video_->streamOff();
+	ret = mainPathVideo_->streamOff();
 	if (ret)
 		LOG(RkISP1, Warning)
 			<< "Failed to stop camera " << camera->id();
@@ -950,7 +950,7 @@  int PipelineHandlerRkISP1::initLinks(const Camera *camera,
 
 	for (const StreamConfiguration &cfg : config) {
 		std::string resizer;
-		if (cfg.stream() == &data->stream_)
+		if (cfg.stream() == &data->mainPathStream_)
 			resizer = "rkisp1_resizer_mainpath";
 		else
 			return -EINVAL;
@@ -972,7 +972,7 @@  int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)
 	int ret;
 
 	std::unique_ptr<RkISP1CameraData> data =
-		std::make_unique<RkISP1CameraData>(this, video_);
+		std::make_unique<RkISP1CameraData>(this, mainPathVideo_);
 
 	ControlInfoMap::Map ctrls;
 	ctrls.emplace(std::piecewise_construct,
@@ -993,7 +993,7 @@  int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)
 	if (ret)
 		return ret;
 
-	std::set<Stream *> streams{ &data->stream_ };
+	std::set<Stream *> streams{ &data->mainPathStream_ };
 	std::shared_ptr<Camera> camera =
 		Camera::create(this, data->sensor_->id(), streams);
 	registerCamera(std::move(camera), std::move(data));
@@ -1023,13 +1023,13 @@  bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)
 	if (isp_->open() < 0)
 		return false;
 
-	resizer_ = V4L2Subdevice::fromEntityName(media_, "rkisp1_resizer_mainpath");
-	if (resizer_->open() < 0)
+	mainPathResizer_ = V4L2Subdevice::fromEntityName(media_, "rkisp1_resizer_mainpath");
+	if (mainPathResizer_->open() < 0)
 		return false;
 
 	/* Locate and open the capture video node. */
-	video_ = V4L2VideoDevice::fromEntityName(media_, "rkisp1_mainpath");
-	if (video_->open() < 0)
+	mainPathVideo_ = V4L2VideoDevice::fromEntityName(media_, "rkisp1_mainpath");
+	if (mainPathVideo_->open() < 0)
 		return false;
 
 	stat_ = V4L2VideoDevice::fromEntityName(media_, "rkisp1_stats");
@@ -1040,7 +1040,7 @@  bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)
 	if (param_->open() < 0)
 		return false;
 
-	video_->bufferReady.connect(this, &PipelineHandlerRkISP1::bufferReady);
+	mainPathVideo_->bufferReady.connect(this, &PipelineHandlerRkISP1::bufferReady);
 	stat_->bufferReady.connect(this, &PipelineHandlerRkISP1::statReady);
 	param_->bufferReady.connect(this, &PipelineHandlerRkISP1::paramReady);