[libcamera-devel,14/14] libcamera: pipeline: rkisp1: Add converter support
diff mbox series

Message ID 20220908184850.1874303-15-xavier.roumegue@oss.nxp.com
State Changes Requested
Headers show
Series
  • Add dw100 dewarper support to simple/rkisp1 pipeline
Related show

Commit Message

Xavier Roumegue Sept. 8, 2022, 6:48 p.m. UTC
This adds a converter, if any present in the system, on each streams
(self and main paths). In case a configuration file is successfully
loaded, the converter use is getting compulsory, such as a dewarping map
is unconditionally applied. Otherwise, the converter is only used if the
stream configuration requires it.

Signed-off-by: Xavier Roumegue <xavier.roumegue@oss.nxp.com>
---
 src/libcamera/pipeline/rkisp1/rkisp1.cpp      | 126 +++---
 src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 374 +++++++++++++++++-
 src/libcamera/pipeline/rkisp1/rkisp1_path.h   |  54 ++-
 3 files changed, 485 insertions(+), 69 deletions(-)

Comments

Laurent Pinchart Oct. 4, 2022, 2:36 a.m. UTC | #1
Hi Xavier,

Thank you for the patch.

On Thu, Sep 08, 2022 at 08:48:50PM +0200, Xavier Roumegue via libcamera-devel wrote:
> This adds a converter, if any present in the system, on each streams
> (self and main paths). In case a configuration file is successfully
> loaded, the converter use is getting compulsory, such as a dewarping map
> is unconditionally applied. Otherwise, the converter is only used if the
> stream configuration requires it.

Before doing a full review of this, I'm wondering what use case(s) you
envision for scaling with the dewarper. The ISP has a scaler at the
output, are there cases where you think scaling in the dewarper would
have an advantage ?

> Signed-off-by: Xavier Roumegue <xavier.roumegue@oss.nxp.com>
> ---
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp      | 126 +++---
>  src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 374 +++++++++++++++++-
>  src/libcamera/pipeline/rkisp1/rkisp1_path.h   |  54 ++-
>  3 files changed, 485 insertions(+), 69 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index c1522ca6..6bdf5a3a 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -1,6 +1,7 @@
>  /* SPDX-License-Identifier: LGPL-2.1-or-later */
>  /*
>   * Copyright (C) 2019, Google Inc.
> + * Copyright 2022 NXP
>   *
>   * rkisp1.cpp - Pipeline handler for Rockchip ISP1
>   */
> @@ -194,6 +195,7 @@ private:
>  	Camera *activeCamera_;
>  
>  	const MediaPad *ispSink_;
> +	MediaDevice *converter_;
>  };
>  
>  RkISP1Frames::RkISP1Frames(PipelineHandler *pipe)
> @@ -449,57 +451,44 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()
>  
>  	bool mainPathAvailable = true;
>  	bool selfPathAvailable = data_->selfPath_;
> +	const std::array<Status, 2> cameraStatus = { Valid, Adjusted };
> +
>  	for (unsigned int index : order) {
>  		StreamConfiguration &cfg = config_[index];
>  
> -		/* Try to match stream without adjusting configuration. */
> -		if (mainPathAvailable) {
> -			StreamConfiguration tryCfg = cfg;
> -			if (data_->mainPath_->validate(&tryCfg) == Valid) {
> -				mainPathAvailable = false;
> -				cfg = tryCfg;
> -				cfg.setStream(const_cast<Stream *>(&data_->mainPathStream_));
> -				continue;
> -			}
> -		}
> +		for (auto &_status : cameraStatus) {
> +			Status pipeStatus;
>  
> -		if (selfPathAvailable) {
> -			StreamConfiguration tryCfg = cfg;
> -			if (data_->selfPath_->validate(&tryCfg) == Valid) {
> -				selfPathAvailable = false;
> -				cfg = tryCfg;
> -				cfg.setStream(const_cast<Stream *>(&data_->selfPathStream_));
> -				continue;
> -			}
> -		}
> +			if (mainPathAvailable) {
> +				StreamConfiguration tryCfg = cfg;
> +				pipeStatus = data_->mainPath_->validate(&tryCfg);
>  
> -		/* Try to match stream allowing adjusting configuration. */
> -		if (mainPathAvailable) {
> -			StreamConfiguration tryCfg = cfg;
> -			if (data_->mainPath_->validate(&tryCfg) == Adjusted) {
> -				mainPathAvailable = false;
> -				cfg = tryCfg;
> -				cfg.setStream(const_cast<Stream *>(&data_->mainPathStream_));
> -				status = Adjusted;
> -				continue;
> +				if (pipeStatus == _status) {
> +					mainPathAvailable = false;
> +					cfg = tryCfg;
> +					cfg.setStream(const_cast<Stream *>(&data_->mainPathStream_));
> +					status = _status;
> +					break;
> +				}
>  			}
> -		}
>  
> -		if (selfPathAvailable) {
> -			StreamConfiguration tryCfg = cfg;
> -			if (data_->selfPath_->validate(&tryCfg) == Adjusted) {
> -				selfPathAvailable = false;
> -				cfg = tryCfg;
> -				cfg.setStream(const_cast<Stream *>(&data_->selfPathStream_));
> -				status = Adjusted;
> -				continue;
> +			if (selfPathAvailable) {
> +				StreamConfiguration tryCfg = cfg;
> +				pipeStatus = data_->selfPath_->validate(&tryCfg);
> +
> +				if (pipeStatus == _status) {
> +					selfPathAvailable = false;
> +					cfg = tryCfg;
> +					cfg.setStream(const_cast<Stream *>(&data_->selfPathStream_));
> +					status = _status;
> +					break;
> +				}
>  			}
> +			/* All paths rejected configuration. */
> +			LOG(RkISP1, Debug) << "Camera configuration not supported "
> +					   << cfg.toString();
> +			return Invalid;
>  		}
> -
> -		/* All paths rejected configuraiton. */
> -		LOG(RkISP1, Debug) << "Camera configuration not supported "
> -				   << cfg.toString();
> -		return Invalid;
>  	}
>  
>  	/* Select the sensor format. */
> @@ -680,15 +669,21 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
>  
>  	std::map<unsigned int, IPAStream> streamConfig;
>  
> -	for (const StreamConfiguration &cfg : *config) {
> +	for (StreamConfiguration &cfg : *config) {
> +		size_t idx = 0;
> +		StreamConfiguration internalCfg;
>  		if (cfg.stream() == &data->mainPathStream_) {
> +			idx = 0;
>  			ret = mainPath_.configure(cfg, format);
> -			streamConfig[0] = IPAStream(cfg.pixelFormat,
> -						    cfg.size);
> +			internalCfg = mainPath_.internalStream();
> +			streamConfig[idx] = IPAStream(internalCfg.pixelFormat,
> +						      internalCfg.size);
>  		} else if (hasSelfPath_) {
> +			idx = 1;
>  			ret = selfPath_.configure(cfg, format);
> -			streamConfig[1] = IPAStream(cfg.pixelFormat,
> -						    cfg.size);
> +			internalCfg = selfPath_.internalStream();
> +			streamConfig[idx] = IPAStream(internalCfg.pixelFormat,
> +						      internalCfg.size);
>  		} else {
>  			return -ENODEV;
>  		}
> @@ -754,6 +749,20 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)
>  		data->selfPathStream_.configuration().bufferCount,
>  	});
>  
> +	if (data->mainPath_->isEnabled()) {
> +		ret = data->mainPath_->allocateBuffers(
> +			data->mainPathStream_.configuration().bufferCount);
> +		if (ret < 0)
> +			goto error;
> +	}
> +
> +	if (hasSelfPath_ && data->selfPath_->isEnabled()) {
> +		ret = data->selfPath_->allocateBuffers(
> +			data->selfPathStream_.configuration().bufferCount);
> +		if (ret < 0)
> +			goto error;
> +	}
> +
>  	ret = param_->allocateBuffers(maxCount, &paramBuffers_);
>  	if (ret < 0)
>  		goto error;
> @@ -813,6 +822,12 @@ int PipelineHandlerRkISP1::freeBuffers(Camera *camera)
>  	if (stat_->releaseBuffers())
>  		LOG(RkISP1, Error) << "Failed to release stat buffers";
>  
> +	if (hasSelfPath_ && data->selfPath_->isEnabled())
> +		data->selfPath_->releaseBuffers();
> +
> +	if (data->mainPath_->isEnabled())
> +		data->mainPath_->releaseBuffers();
> +
>  	return 0;
>  }
>  
> @@ -1055,6 +1070,19 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)
>  
>  	hasSelfPath_ = !!media_->getEntityByName("rkisp1_selfpath");
>  
> +	/* Seek for a converter */
> +	for (auto converterName : ConverterFactory::names()) {
> +		LOG(RkISP1, Debug)
> +			<< "Trying " << converterName << " converter";
> +		DeviceMatch converterMatch(converterName);
> +		converter_ = acquireMediaDevice(enumerator, converterMatch);
> +		if (converter_) {
> +			LOG(RkISP1, Debug)
> +				<< "Get support for " << converterName << " converter";
> +			break;
> +		}
> +	}
> +
>  	/* Create the V4L2 subdevices we will need. */
>  	isp_ = V4L2Subdevice::fromEntityName(media_, "rkisp1_isp");
>  	if (isp_->open() < 0)
> @@ -1086,10 +1114,10 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)
>  		return false;
>  
>  	/* Locate and open the ISP main and self paths. */
> -	if (!mainPath_.init(media_))
> +	if (!mainPath_.init(media_, converter_))
>  		return false;
>  
> -	if (hasSelfPath_ && !selfPath_.init(media_))
> +	if (hasSelfPath_ && !selfPath_.init(media_, converter_))
>  		return false;
>  
>  	mainPath_.bufferReady().connect(this, &PipelineHandlerRkISP1::bufferReady);
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> index 2d38f0fb..c19a1e69 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
> @@ -1,6 +1,7 @@
>  /* SPDX-License-Identifier: LGPL-2.1-or-later */
>  /*
>   * Copyright (C) 2020, Google Inc.
> + * Copyright 2022 NXP
>   *
>   * rkisp1path.cpp - Rockchip ISP1 path helper
>   */
> @@ -28,7 +29,46 @@ RkISP1Path::RkISP1Path(const char *name, const Span<const PixelFormat> &formats,
>  {
>  }
>  
> -bool RkISP1Path::init(MediaDevice *media)
> +void RkISP1Path::initConverter(MediaDevice *mediaConverter)
> +{
> +	hasConverter_ = false;
> +	useConverter_ = false;
> +
> +	if (!mediaConverter)
> +		return;
> +
> +	converter_ = ConverterFactory::create(mediaConverter);
> +
> +	if (!converter_->isValid()) {
> +		LOG(RkISP1, Warning)
> +			<< "Failed to create converter, disabling format conversion";
> +		converter_.reset();
> +	} else {
> +		char const *configFromEnv = utils::secure_getenv("LIBCAMERA_RKISP1_CONVERTER_FILE");
> +
> +		if (configFromEnv && *configFromEnv != '\0') {
> +			int nrMappings;
> +			LOG(RkISP1, Debug)
> +				<< "Getting pipeline converter filename as " << std::string(configFromEnv);
> +			nrMappings = converter_->loadConfiguration(std::string(configFromEnv));
> +			if (nrMappings < 0) {
> +				LOG(RkISP1, Error)
> +					<< "Error while reading converter configuration file";
> +			} else {
> +				LOG(RkISP1, Debug)
> +					<< nrMappings << " mapping(s) loaded";
> +				if (nrMappings > 0)
> +					/* We want to force the converter use to apply the mapping */
> +					useConverter_ = true;
> +			}
> +		}
> +		converter_->inputBufferReady.connect(this, &RkISP1Path::pathConverterInputDone);
> +		converter_->outputBufferReady.connect(this, &RkISP1Path::pathConverterOutputDone);
> +		hasConverter_ = true;
> +	}
> +}
> +
> +bool RkISP1Path::init(MediaDevice *media, MediaDevice *mediaConverter)
>  {
>  	std::string resizer = std::string("rkisp1_resizer_") + name_ + "path";
>  	std::string video = std::string("rkisp1_") + name_ + "path";
> @@ -45,10 +85,13 @@ bool RkISP1Path::init(MediaDevice *media)
>  	if (!link_)
>  		return false;
>  
> +	initConverter(mediaConverter);
> +
> +	video_->bufferReady.connect(this, &RkISP1Path::pathBufferReady);
> +
>  	return true;
>  }
> -
> -StreamConfiguration RkISP1Path::generateConfiguration(const Size &resolution)
> +StreamConfiguration RkISP1Path::generateNativeConfiguration(const Size &resolution)
>  {
>  	Size maxResolution = maxResolution_.boundedToAspectRatio(resolution)
>  					   .boundedTo(resolution);
> @@ -67,7 +110,165 @@ StreamConfiguration RkISP1Path::generateConfiguration(const Size &resolution)
>  	return cfg;
>  }
>  
> -CameraConfiguration::Status RkISP1Path::validate(StreamConfiguration *cfg)
> +StreamConfiguration RkISP1Path::generateConfiguration(const Size &resolution)
> +{
> +	StreamConfiguration cfg = generateNativeConfiguration(resolution);
> +	StreamFormats formats = cfg.formats();
> +	std::map<PixelFormat, std::vector<SizeRange>> fullFormats;
> +
> +	for (const PixelFormat &fmt : formats.pixelformats()) {
> +		Configuration config;
> +		SizeRange szRange;
> +
> +		config.inputFormat = fmt;
> +		config.inputSizes = formats.range(fmt);
> +
> +		if (!useConverter_) {
> +			config.outputFormats.push_back(fmt);
> +			config.outputSizes = config.inputSizes;
> +			config.withConverter = false;
> +			configs_.push_back(config);
> +			LOG(RkISP1, Debug)
> +				<< "Pushing native format " << fmt
> +				<< " resolution range " << config.inputSizes;
> +			fullFormats[fmt].push_back(config.inputSizes);
> +		}
> +
> +		if (hasConverter_) {
> +			config.outputFormats = converter_->formats(config.inputFormat);
> +			if (config.outputFormats.empty())
> +				continue;
> +			szRange.max = converter_->sizes(config.inputSizes.max).max;
> +			szRange.min = converter_->sizes(config.inputSizes.min).min;
> +			config.outputSizes = szRange;
> +			config.withConverter = true;
> +			configs_.push_back(config);
> +			for (auto &_fmt : config.outputFormats) {
> +				fullFormats[_fmt].push_back(config.outputSizes);
> +				LOG(RkISP1, Debug)
> +					<< "Pushing converted format " << _fmt
> +					<< " resolution range " << config.outputSizes;
> +			}
> +		}
> +	}
> +	/*
> +	 * Sort the sizes and merge any consecutive overlapping ranges.
> +	 * Imported from src/libcamera/pipeline/simple.cpp
> +	 *
> +	 * TODO: Apply policy of converter use or not.. We likely want to force
> +	 * the converter use in case there is a mapping defined in the
> +	 * configuration file if any... if so, shall we filter out resolution
> +	 * not defined in the configuration file.. or should this policy be let
> +	 * to the application
> +	 * */
> +
> +	for (auto &[format, sizes] : fullFormats) {
> +		std::sort(sizes.begin(), sizes.end(),
> +			  [](SizeRange &a, SizeRange &b) {
> +				  return a.min < b.min;
> +			  });
> +
> +		auto cur = sizes.begin();
> +		auto next = cur;
> +
> +		while (++next != sizes.end()) {
> +			if (cur->max.width >= next->min.width &&
> +			    cur->max.height >= next->min.height)
> +				cur->max = next->max;
> +			else if (++cur != next)
> +				*cur = *next;
> +		}
> +
> +		sizes.erase(++cur, sizes.end());
> +	}
> +
> +	StreamConfiguration fullCfg{ StreamFormats{ fullFormats } };
> +	fullCfg.pixelFormat = fullFormats.begin()->first;
> +	fullCfg.size = fullFormats.begin()->second[0].max;
> +	fullCfg.bufferCount = cfg.bufferCount;
> +
> +	return fullCfg;
> +}
> +
> +CameraConfiguration::Status RkISP1Path::getPipeConfiguration(
> +	StreamConfiguration &streamCfg,
> +	const RkISP1Path::Configuration **cfg) const
> +{
> +	CameraConfiguration::Status _status = CameraConfiguration::Adjusted;
> +
> +	LOG(RkISP1, Debug)
> +		<< "Looking for "
> +		<< streamCfg.pixelFormat << "/" << streamCfg.size
> +		<< " output configuration";
> +	/*
> +	 * Select the fallback configuration
> +	 */
> +	for (auto &_cfg : configs_) {
> +		if (_cfg.withConverter == useConverter_) {
> +			*cfg = &_cfg;
> +			break;
> +		}
> +	}
> +
> +	PixelFormat pixelFormat = (*cfg)->outputFormats.front();
> +	Size size = streamCfg.size;
> +	size.boundTo((*cfg)->outputSizes.max);
> +	size.expandTo((*cfg)->outputSizes.min);
> +
> +	/* Unless the converter use is enforced through a loaded configuration,
> +	 * prefer a configuration without the converter if possible
> +	 */
> +	std::vector<bool> converterUse;
> +	if (!useConverter_)
> +		converterUse.push_back(false);
> +	if (hasConverter_)
> +		converterUse.push_back(true);
> +
> +	for (auto withConverter : converterUse) {
> +		for (auto &_cfg : configs_) {
> +			auto &outFmts = _cfg.outputFormats;
> +			if (withConverter != _cfg.withConverter)
> +				continue;
> +
> +			if ((std::find(outFmts.begin(),
> +				       outFmts.end(),
> +				       streamCfg.pixelFormat)) == outFmts.end())
> +				continue;
> +
> +			*cfg = &_cfg;
> +			pixelFormat = streamCfg.pixelFormat;
> +
> +			if (_cfg.outputSizes.contains(streamCfg.size)) {
> +				_status = CameraConfiguration::Valid;
> +				goto done;
> +			}
> +
> +			size.boundTo((*cfg)->outputSizes.max);
> +			size.expandTo((*cfg)->outputSizes.min);
> +		}
> +	}
> +
> +	streamCfg.pixelFormat = pixelFormat;
> +	streamCfg.size = size;
> +
> +done:
> +	if ((*cfg)->withConverter) {
> +		std::tie(streamCfg.stride, streamCfg.frameSize) =
> +			converter_->strideAndFrameSize(streamCfg.pixelFormat,
> +						       streamCfg.size);
> +		if (streamCfg.stride == 0)
> +			return CameraConfiguration::Invalid;
> +	}
> +
> +	LOG(RkISP1, Debug)
> +		<< "Chosen configuration: "
> +		<< (*cfg)->inputFormat << "/" << (*cfg)->inputSizes
> +		<< " --> " << streamCfg.pixelFormat << "/" << streamCfg.size
> +		<< ((*cfg)->withConverter ? " with" : " without") << " converter";
> +	return _status;
> +}
> +
> +CameraConfiguration::Status RkISP1Path::nativeValidate(StreamConfiguration *cfg)
>  {
>  	const StreamConfiguration reqCfg = *cfg;
>  	CameraConfiguration::Status status = CameraConfiguration::Valid;
> @@ -101,7 +302,39 @@ CameraConfiguration::Status RkISP1Path::validate(StreamConfiguration *cfg)
>  	return status;
>  }
>  
> -int RkISP1Path::configure(const StreamConfiguration &config,
> +CameraConfiguration::Status RkISP1Path::validate(StreamConfiguration *cfg)
> +{
> +	StreamConfiguration tryCfg = *cfg;
> +	StreamConfiguration internalCfg;
> +	const Configuration *pipeCfg;
> +	CameraConfiguration::Status pipeStatus;
> +
> +	pipeStatus = getPipeConfiguration(tryCfg, &pipeCfg);
> +	if (pipeStatus == CameraConfiguration::Invalid)
> +		return CameraConfiguration::Invalid;
> +
> +	if (!pipeCfg->withConverter) {
> +		tryCfg = *cfg;
> +		pipeStatus = nativeValidate(&tryCfg);
> +		if (pipeStatus == CameraConfiguration::Invalid)
> +			return CameraConfiguration::Invalid;
> +		internalCfg = tryCfg;
> +	} else {
> +		internalCfg = tryCfg;
> +		internalCfg.pixelFormat = pipeCfg->inputFormat;
> +		auto _status = nativeValidate(&internalCfg);
> +		if (_status == CameraConfiguration::Invalid)
> +			return CameraConfiguration::Invalid;
> +	}
> +
> +	*cfg = tryCfg;
> +	internalStream_ = internalCfg;
> +	pipeConfig_ = pipeCfg;
> +
> +	return pipeStatus;
> +}
> +
> +int RkISP1Path::nativeConfigure(const StreamConfiguration &config,
>  			  const V4L2SubdeviceFormat &inputFormat)
>  {
>  	int ret;
> @@ -165,6 +398,117 @@ int RkISP1Path::configure(const StreamConfiguration &config,
>  	return 0;
>  }
>  
> +int RkISP1Path::configure(const StreamConfiguration &config,
> +			  const V4L2SubdeviceFormat &inputFormat)
> +{
> +	int ret;
> +	LOG(RkISP1, Debug)
> +		<< "Configuring " << name_ << " path "
> +		<< internalStream_.pixelFormat << "/" << internalStream_.size
> +		<< " --> " << config.pixelFormat << "/" << config.size
> +		<< (pipeConfig_->withConverter ? " with" : " without") << " converter";
> +
> +	ret = nativeConfigure(internalStream_, inputFormat);
> +	if (ret)
> +		return ret;
> +
> +	if (pipeConfig_->withConverter) {
> +		StreamConfiguration cfg = config;
> +		std::vector<std::reference_wrapper<StreamConfiguration>> outputCfgs;
> +		outputCfgs.push_back(cfg);
> +		ret = converter_->configure(internalStream_, outputCfgs);
> +	}
> +
> +	return ret;
> +}
> +
> +int RkISP1Path::exportBuffers(unsigned int bufferCount,
> +			      std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> +{
> +	if (pipeConfig_->withConverter)
> +		return converter_->exportBuffers(0, bufferCount, buffers);
> +	else
> +		return video_->exportBuffers(bufferCount, buffers);
> +}
> +
> +int RkISP1Path::allocateBuffers(unsigned int bufferCount)
> +{
> +	if (pipeConfig_->withConverter) {
> +		int ret = video_->allocateBuffers(bufferCount, &converterBuffers_);
> +		if (ret < 0)
> +			return ret;
> +		if ((unsigned int)ret != bufferCount)
> +			LOG(RkISP1, Warning)
> +				<< "Requested " << bufferCount << " but got " << ret << " buffers";
> +
> +		for (std::unique_ptr<FrameBuffer> &buffer : converterBuffers_)
> +			availableConverterBuffers_.push(buffer.get());
> +	} else {
> +		return video_->importBuffers(bufferCount);
> +	}
> +
> +	return 0;
> +}
> +
> +void RkISP1Path::releaseBuffers()
> +{
> +	while (!availableConverterBuffers_.empty())
> +		availableConverterBuffers_.pop();
> +
> +	converterBuffers_.clear();
> +
> +	video_->releaseBuffers();
> +}
> +
> +void RkISP1Path::pathBufferReady(FrameBuffer *buffer)
> +{
> +	Request *request = buffer->request();
> +
> +	if (request) {
> +		internalBufferReady_.emit(buffer);
> +	} else {
> +		auto iter = converterBuffersMapping_.find(buffer);
> +		if (iter != converterBuffersMapping_.end()) {
> +			converter_->queueBuffer(buffer, iter->second);
> +			converterBuffersMapping_.erase(iter);
> +		} else {
> +			LOG(RkISP1, Error)
> +				<< "Final buffer associated to converted buffer not found on "
> +				<< name_ << " path";
> +		}
> +	}
> +}
> +
> +void RkISP1Path::pathConverterInputDone(FrameBuffer *buffer)
> +{
> +	availableConverterBuffers_.push(buffer);
> +}
> +
> +void RkISP1Path::pathConverterOutputDone(FrameBuffer *buffer)
> +{
> +	internalBufferReady_.emit(buffer);
> +}
> +
> +int RkISP1Path::queueBuffer(FrameBuffer *buffer)
> +{
> +	FrameBuffer *_buffer;
> +
> +	if (pipeConfig_->withConverter) {
> +		if (availableConverterBuffers_.empty()) {
> +			LOG(RkISP1, Error)
> +				<< "converter buffer underrun on " << name_ << " path";
> +			return -ENOMEM;
> +		}
> +		_buffer = availableConverterBuffers_.front();
> +		availableConverterBuffers_.pop();
> +		converterBuffersMapping_[_buffer] = buffer;
> +	} else {
> +		_buffer = buffer;
> +	}
> +
> +	return video_->queueBuffer(_buffer);
> +}
> +
>  int RkISP1Path::start()
>  {
>  	int ret;
> @@ -172,20 +516,23 @@ int RkISP1Path::start()
>  	if (running_)
>  		return -EBUSY;
>  
> -	/* \todo Make buffer count user configurable. */
> -	ret = video_->importBuffers(RKISP1_BUFFER_COUNT);
> -	if (ret)
> -		return ret;
> -
>  	ret = video_->streamOn();
>  	if (ret) {
>  		LOG(RkISP1, Error)
>  			<< "Failed to start " << name_ << " path";
> -
> -		video_->releaseBuffers();
>  		return ret;
>  	}
>  
> +	if (pipeConfig_->withConverter) {
> +		ret = converter_->start();
> +		if (ret) {
> +			LOG(RkISP1, Error)
> +				<< "Failed to start converter on " << name_ << " path";
> +			stop();
> +			return ret;
> +		}
> +	}
> +
>  	running_ = true;
>  
>  	return 0;
> @@ -199,7 +546,8 @@ void RkISP1Path::stop()
>  	if (video_->streamOff())
>  		LOG(RkISP1, Warning) << "Failed to stop " << name_ << " path";
>  
> -	video_->releaseBuffers();
> +	if (pipeConfig_->withConverter)
> +		converter_->stop();
>  
>  	running_ = false;
>  }
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.h b/src/libcamera/pipeline/rkisp1/rkisp1_path.h
> index f3f1ae39..0da3594f 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.h
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.h
> @@ -1,6 +1,7 @@
>  /* SPDX-License-Identifier: LGPL-2.1-or-later */
>  /*
>   * Copyright (C) 2020, Google Inc.
> + * Copyright 2022 NXP
>   *
>   * rkisp1path.h - Rockchip ISP1 path helper
>   */
> @@ -8,6 +9,7 @@
>  #pragma once
>  
>  #include <memory>
> +#include <queue>
>  #include <vector>
>  
>  #include <libcamera/base/signal.h>
> @@ -17,9 +19,11 @@
>  #include <libcamera/geometry.h>
>  #include <libcamera/pixel_format.h>
>  
> +#include "libcamera/internal/converter.h"
>  #include "libcamera/internal/media_object.h"
>  #include "libcamera/internal/v4l2_videodevice.h"
>  
> +
>  namespace libcamera {
>  
>  class MediaDevice;
> @@ -33,7 +37,7 @@ public:
>  	RkISP1Path(const char *name, const Span<const PixelFormat> &formats,
>  		   const Size &minResolution, const Size &maxResolution);
>  
> -	bool init(MediaDevice *media);
> +	bool init(MediaDevice *media, MediaDevice *mediaConverter = nullptr);
>  
>  	int setEnabled(bool enable) { return link_->setEnabled(enable); }
>  	bool isEnabled() const { return link_->flags() & MEDIA_LNK_FL_ENABLED; }
> @@ -45,18 +49,31 @@ public:
>  		      const V4L2SubdeviceFormat &inputFormat);
>  
>  	int exportBuffers(unsigned int bufferCount,
> -			  std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> -	{
> -		return video_->exportBuffers(bufferCount, buffers);
> -	}
> +			  std::vector<std::unique_ptr<FrameBuffer>> *buffers);
> +
> +	int allocateBuffers(unsigned int bufferCount);
> +	void releaseBuffers();
>  
>  	int start();
>  	void stop();
>  
> -	int queueBuffer(FrameBuffer *buffer) { return video_->queueBuffer(buffer); }
> -	Signal<FrameBuffer *> &bufferReady() { return video_->bufferReady; }
> +	int queueBuffer(FrameBuffer *buffer);
> +	Signal<FrameBuffer *> &bufferReady() { return internalBufferReady_; }
> +
> +	StreamConfiguration internalStream() const
> +	{
> +		return internalStream_;
> +	}
>  
>  private:
> +	struct Configuration {
> +		SizeRange inputSizes;
> +		PixelFormat inputFormat;
> +		std::vector<PixelFormat> outputFormats;
> +		SizeRange outputSizes;
> +		bool withConverter;
> +	};
> +
>  	static constexpr unsigned int RKISP1_BUFFER_COUNT = 4;
>  
>  	const char *name_;
> @@ -65,10 +82,33 @@ private:
>  	const Span<const PixelFormat> formats_;
>  	const Size minResolution_;
>  	const Size maxResolution_;
> +	CameraConfiguration::Status getPipeConfiguration(
> +		StreamConfiguration &streamCfg,
> +		const RkISP1Path::Configuration **cfg) const;
> +
> +	StreamConfiguration generateNativeConfiguration(const Size &resolution);
> +	CameraConfiguration::Status nativeValidate(StreamConfiguration *cfg);
> +	int nativeConfigure(const StreamConfiguration &config,
> +			    const V4L2SubdeviceFormat &inputFormat);
>  
>  	std::unique_ptr<V4L2Subdevice> resizer_;
>  	std::unique_ptr<V4L2VideoDevice> video_;
>  	MediaLink *link_;
> +
> +	void initConverter(MediaDevice *mediaConverter);
> +	std::unique_ptr<Converter> converter_;
> +	std::vector<Configuration> configs_;
> +	StreamConfiguration internalStream_;
> +	const Configuration *pipeConfig_;
> +	bool hasConverter_;
> +	bool useConverter_;
> +	Signal<FrameBuffer *> internalBufferReady_;
> +	std::vector<std::unique_ptr<FrameBuffer>> converterBuffers_;
> +	std::queue<FrameBuffer *> availableConverterBuffers_;
> +	std::map<FrameBuffer *, FrameBuffer *> converterBuffersMapping_;
> +	void pathBufferReady(FrameBuffer *buffer);
> +	void pathConverterInputDone(FrameBuffer *buffer);
> +	void pathConverterOutputDone(FrameBuffer *buffer);
>  };
>  
>  class RkISP1MainPath : public RkISP1Path
Xavier Roumegue Oct. 7, 2022, 2:27 p.m. UTC | #2
Hi Laurent,

On 10/4/22 04:36, Laurent Pinchart wrote:
> Hi Xavier,
> 
> Thank you for the patch.
> 
> On Thu, Sep 08, 2022 at 08:48:50PM +0200, Xavier Roumegue via libcamera-devel wrote:
>> This adds a converter, if any present in the system, on each streams
>> (self and main paths). In case a configuration file is successfully
>> loaded, the converter use is getting compulsory, such as a dewarping map
>> is unconditionally applied. Otherwise, the converter is only used if the
>> stream configuration requires it.
> 
> Before doing a full review of this, I'm wondering what use case(s) you
> envision for scaling with the dewarper. The ISP has a scaler at the
> output, are there cases where you think scaling in the dewarper would
> have an advantage ?
Assuming that rkisp1 and converter scalers have the same capabilities, 
there is likely no reason to scale in the converter. But generaly 
speaking, scaling in the converter might make sense if you can benefit 
from a converter scaler features missing in rkisp1 (hflip, vflip, 
rotation, etc...).

An interesting future feature would be to use the multiple output stream 
converter capabilities to generate pyramid used on computer vision 
algorithm.

Xavier

> 
>> Signed-off-by: Xavier Roumegue <xavier.roumegue@oss.nxp.com>
>> ---
>>   src/libcamera/pipeline/rkisp1/rkisp1.cpp      | 126 +++---
>>   src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 374 +++++++++++++++++-
>>   src/libcamera/pipeline/rkisp1/rkisp1_path.h   |  54 ++-
>>   3 files changed, 485 insertions(+), 69 deletions(-)
>>
>> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
>> index c1522ca6..6bdf5a3a 100644
>> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
>> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
>> @@ -1,6 +1,7 @@
>>   /* SPDX-License-Identifier: LGPL-2.1-or-later */
>>   /*
>>    * Copyright (C) 2019, Google Inc.
>> + * Copyright 2022 NXP
>>    *
>>    * rkisp1.cpp - Pipeline handler for Rockchip ISP1
>>    */
>> @@ -194,6 +195,7 @@ private:
>>   	Camera *activeCamera_;
>>   
>>   	const MediaPad *ispSink_;
>> +	MediaDevice *converter_;
>>   };
>>   
>>   RkISP1Frames::RkISP1Frames(PipelineHandler *pipe)
>> @@ -449,57 +451,44 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()
>>   
>>   	bool mainPathAvailable = true;
>>   	bool selfPathAvailable = data_->selfPath_;
>> +	const std::array<Status, 2> cameraStatus = { Valid, Adjusted };
>> +
>>   	for (unsigned int index : order) {
>>   		StreamConfiguration &cfg = config_[index];
>>   
>> -		/* Try to match stream without adjusting configuration. */
>> -		if (mainPathAvailable) {
>> -			StreamConfiguration tryCfg = cfg;
>> -			if (data_->mainPath_->validate(&tryCfg) == Valid) {
>> -				mainPathAvailable = false;
>> -				cfg = tryCfg;
>> -				cfg.setStream(const_cast<Stream *>(&data_->mainPathStream_));
>> -				continue;
>> -			}
>> -		}
>> +		for (auto &_status : cameraStatus) {
>> +			Status pipeStatus;
>>   
>> -		if (selfPathAvailable) {
>> -			StreamConfiguration tryCfg = cfg;
>> -			if (data_->selfPath_->validate(&tryCfg) == Valid) {
>> -				selfPathAvailable = false;
>> -				cfg = tryCfg;
>> -				cfg.setStream(const_cast<Stream *>(&data_->selfPathStream_));
>> -				continue;
>> -			}
>> -		}
>> +			if (mainPathAvailable) {
>> +				StreamConfiguration tryCfg = cfg;
>> +				pipeStatus = data_->mainPath_->validate(&tryCfg);
>>   
>> -		/* Try to match stream allowing adjusting configuration. */
>> -		if (mainPathAvailable) {
>> -			StreamConfiguration tryCfg = cfg;
>> -			if (data_->mainPath_->validate(&tryCfg) == Adjusted) {
>> -				mainPathAvailable = false;
>> -				cfg = tryCfg;
>> -				cfg.setStream(const_cast<Stream *>(&data_->mainPathStream_));
>> -				status = Adjusted;
>> -				continue;
>> +				if (pipeStatus == _status) {
>> +					mainPathAvailable = false;
>> +					cfg = tryCfg;
>> +					cfg.setStream(const_cast<Stream *>(&data_->mainPathStream_));
>> +					status = _status;
>> +					break;
>> +				}
>>   			}
>> -		}
>>   
>> -		if (selfPathAvailable) {
>> -			StreamConfiguration tryCfg = cfg;
>> -			if (data_->selfPath_->validate(&tryCfg) == Adjusted) {
>> -				selfPathAvailable = false;
>> -				cfg = tryCfg;
>> -				cfg.setStream(const_cast<Stream *>(&data_->selfPathStream_));
>> -				status = Adjusted;
>> -				continue;
>> +			if (selfPathAvailable) {
>> +				StreamConfiguration tryCfg = cfg;
>> +				pipeStatus = data_->selfPath_->validate(&tryCfg);
>> +
>> +				if (pipeStatus == _status) {
>> +					selfPathAvailable = false;
>> +					cfg = tryCfg;
>> +					cfg.setStream(const_cast<Stream *>(&data_->selfPathStream_));
>> +					status = _status;
>> +					break;
>> +				}
>>   			}
>> +			/* All paths rejected configuration. */
>> +			LOG(RkISP1, Debug) << "Camera configuration not supported "
>> +					   << cfg.toString();
>> +			return Invalid;
>>   		}
>> -
>> -		/* All paths rejected configuraiton. */
>> -		LOG(RkISP1, Debug) << "Camera configuration not supported "
>> -				   << cfg.toString();
>> -		return Invalid;
>>   	}
>>   
>>   	/* Select the sensor format. */
>> @@ -680,15 +669,21 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
>>   
>>   	std::map<unsigned int, IPAStream> streamConfig;
>>   
>> -	for (const StreamConfiguration &cfg : *config) {
>> +	for (StreamConfiguration &cfg : *config) {
>> +		size_t idx = 0;
>> +		StreamConfiguration internalCfg;
>>   		if (cfg.stream() == &data->mainPathStream_) {
>> +			idx = 0;
>>   			ret = mainPath_.configure(cfg, format);
>> -			streamConfig[0] = IPAStream(cfg.pixelFormat,
>> -						    cfg.size);
>> +			internalCfg = mainPath_.internalStream();
>> +			streamConfig[idx] = IPAStream(internalCfg.pixelFormat,
>> +						      internalCfg.size);
>>   		} else if (hasSelfPath_) {
>> +			idx = 1;
>>   			ret = selfPath_.configure(cfg, format);
>> -			streamConfig[1] = IPAStream(cfg.pixelFormat,
>> -						    cfg.size);
>> +			internalCfg = selfPath_.internalStream();
>> +			streamConfig[idx] = IPAStream(internalCfg.pixelFormat,
>> +						      internalCfg.size);
>>   		} else {
>>   			return -ENODEV;
>>   		}
>> @@ -754,6 +749,20 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)
>>   		data->selfPathStream_.configuration().bufferCount,
>>   	});
>>   
>> +	if (data->mainPath_->isEnabled()) {
>> +		ret = data->mainPath_->allocateBuffers(
>> +			data->mainPathStream_.configuration().bufferCount);
>> +		if (ret < 0)
>> +			goto error;
>> +	}
>> +
>> +	if (hasSelfPath_ && data->selfPath_->isEnabled()) {
>> +		ret = data->selfPath_->allocateBuffers(
>> +			data->selfPathStream_.configuration().bufferCount);
>> +		if (ret < 0)
>> +			goto error;
>> +	}
>> +
>>   	ret = param_->allocateBuffers(maxCount, &paramBuffers_);
>>   	if (ret < 0)
>>   		goto error;
>> @@ -813,6 +822,12 @@ int PipelineHandlerRkISP1::freeBuffers(Camera *camera)
>>   	if (stat_->releaseBuffers())
>>   		LOG(RkISP1, Error) << "Failed to release stat buffers";
>>   
>> +	if (hasSelfPath_ && data->selfPath_->isEnabled())
>> +		data->selfPath_->releaseBuffers();
>> +
>> +	if (data->mainPath_->isEnabled())
>> +		data->mainPath_->releaseBuffers();
>> +
>>   	return 0;
>>   }
>>   
>> @@ -1055,6 +1070,19 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)
>>   
>>   	hasSelfPath_ = !!media_->getEntityByName("rkisp1_selfpath");
>>   
>> +	/* Seek for a converter */
>> +	for (auto converterName : ConverterFactory::names()) {
>> +		LOG(RkISP1, Debug)
>> +			<< "Trying " << converterName << " converter";
>> +		DeviceMatch converterMatch(converterName);
>> +		converter_ = acquireMediaDevice(enumerator, converterMatch);
>> +		if (converter_) {
>> +			LOG(RkISP1, Debug)
>> +				<< "Get support for " << converterName << " converter";
>> +			break;
>> +		}
>> +	}
>> +
>>   	/* Create the V4L2 subdevices we will need. */
>>   	isp_ = V4L2Subdevice::fromEntityName(media_, "rkisp1_isp");
>>   	if (isp_->open() < 0)
>> @@ -1086,10 +1114,10 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)
>>   		return false;
>>   
>>   	/* Locate and open the ISP main and self paths. */
>> -	if (!mainPath_.init(media_))
>> +	if (!mainPath_.init(media_, converter_))
>>   		return false;
>>   
>> -	if (hasSelfPath_ && !selfPath_.init(media_))
>> +	if (hasSelfPath_ && !selfPath_.init(media_, converter_))
>>   		return false;
>>   
>>   	mainPath_.bufferReady().connect(this, &PipelineHandlerRkISP1::bufferReady);
>> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
>> index 2d38f0fb..c19a1e69 100644
>> --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
>> +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
>> @@ -1,6 +1,7 @@
>>   /* SPDX-License-Identifier: LGPL-2.1-or-later */
>>   /*
>>    * Copyright (C) 2020, Google Inc.
>> + * Copyright 2022 NXP
>>    *
>>    * rkisp1path.cpp - Rockchip ISP1 path helper
>>    */
>> @@ -28,7 +29,46 @@ RkISP1Path::RkISP1Path(const char *name, const Span<const PixelFormat> &formats,
>>   {
>>   }
>>   
>> -bool RkISP1Path::init(MediaDevice *media)
>> +void RkISP1Path::initConverter(MediaDevice *mediaConverter)
>> +{
>> +	hasConverter_ = false;
>> +	useConverter_ = false;
>> +
>> +	if (!mediaConverter)
>> +		return;
>> +
>> +	converter_ = ConverterFactory::create(mediaConverter);
>> +
>> +	if (!converter_->isValid()) {
>> +		LOG(RkISP1, Warning)
>> +			<< "Failed to create converter, disabling format conversion";
>> +		converter_.reset();
>> +	} else {
>> +		char const *configFromEnv = utils::secure_getenv("LIBCAMERA_RKISP1_CONVERTER_FILE");
>> +
>> +		if (configFromEnv && *configFromEnv != '\0') {
>> +			int nrMappings;
>> +			LOG(RkISP1, Debug)
>> +				<< "Getting pipeline converter filename as " << std::string(configFromEnv);
>> +			nrMappings = converter_->loadConfiguration(std::string(configFromEnv));
>> +			if (nrMappings < 0) {
>> +				LOG(RkISP1, Error)
>> +					<< "Error while reading converter configuration file";
>> +			} else {
>> +				LOG(RkISP1, Debug)
>> +					<< nrMappings << " mapping(s) loaded";
>> +				if (nrMappings > 0)
>> +					/* We want to force the converter use to apply the mapping */
>> +					useConverter_ = true;
>> +			}
>> +		}
>> +		converter_->inputBufferReady.connect(this, &RkISP1Path::pathConverterInputDone);
>> +		converter_->outputBufferReady.connect(this, &RkISP1Path::pathConverterOutputDone);
>> +		hasConverter_ = true;
>> +	}
>> +}
>> +
>> +bool RkISP1Path::init(MediaDevice *media, MediaDevice *mediaConverter)
>>   {
>>   	std::string resizer = std::string("rkisp1_resizer_") + name_ + "path";
>>   	std::string video = std::string("rkisp1_") + name_ + "path";
>> @@ -45,10 +85,13 @@ bool RkISP1Path::init(MediaDevice *media)
>>   	if (!link_)
>>   		return false;
>>   
>> +	initConverter(mediaConverter);
>> +
>> +	video_->bufferReady.connect(this, &RkISP1Path::pathBufferReady);
>> +
>>   	return true;
>>   }
>> -
>> -StreamConfiguration RkISP1Path::generateConfiguration(const Size &resolution)
>> +StreamConfiguration RkISP1Path::generateNativeConfiguration(const Size &resolution)
>>   {
>>   	Size maxResolution = maxResolution_.boundedToAspectRatio(resolution)
>>   					   .boundedTo(resolution);
>> @@ -67,7 +110,165 @@ StreamConfiguration RkISP1Path::generateConfiguration(const Size &resolution)
>>   	return cfg;
>>   }
>>   
>> -CameraConfiguration::Status RkISP1Path::validate(StreamConfiguration *cfg)
>> +StreamConfiguration RkISP1Path::generateConfiguration(const Size &resolution)
>> +{
>> +	StreamConfiguration cfg = generateNativeConfiguration(resolution);
>> +	StreamFormats formats = cfg.formats();
>> +	std::map<PixelFormat, std::vector<SizeRange>> fullFormats;
>> +
>> +	for (const PixelFormat &fmt : formats.pixelformats()) {
>> +		Configuration config;
>> +		SizeRange szRange;
>> +
>> +		config.inputFormat = fmt;
>> +		config.inputSizes = formats.range(fmt);
>> +
>> +		if (!useConverter_) {
>> +			config.outputFormats.push_back(fmt);
>> +			config.outputSizes = config.inputSizes;
>> +			config.withConverter = false;
>> +			configs_.push_back(config);
>> +			LOG(RkISP1, Debug)
>> +				<< "Pushing native format " << fmt
>> +				<< " resolution range " << config.inputSizes;
>> +			fullFormats[fmt].push_back(config.inputSizes);
>> +		}
>> +
>> +		if (hasConverter_) {
>> +			config.outputFormats = converter_->formats(config.inputFormat);
>> +			if (config.outputFormats.empty())
>> +				continue;
>> +			szRange.max = converter_->sizes(config.inputSizes.max).max;
>> +			szRange.min = converter_->sizes(config.inputSizes.min).min;
>> +			config.outputSizes = szRange;
>> +			config.withConverter = true;
>> +			configs_.push_back(config);
>> +			for (auto &_fmt : config.outputFormats) {
>> +				fullFormats[_fmt].push_back(config.outputSizes);
>> +				LOG(RkISP1, Debug)
>> +					<< "Pushing converted format " << _fmt
>> +					<< " resolution range " << config.outputSizes;
>> +			}
>> +		}
>> +	}
>> +	/*
>> +	 * Sort the sizes and merge any consecutive overlapping ranges.
>> +	 * Imported from src/libcamera/pipeline/simple.cpp
>> +	 *
>> +	 * TODO: Apply policy of converter use or not.. We likely want to force
>> +	 * the converter use in case there is a mapping defined in the
>> +	 * configuration file if any... if so, shall we filter out resolution
>> +	 * not defined in the configuration file.. or should this policy be let
>> +	 * to the application
>> +	 * */
>> +
>> +	for (auto &[format, sizes] : fullFormats) {
>> +		std::sort(sizes.begin(), sizes.end(),
>> +			  [](SizeRange &a, SizeRange &b) {
>> +				  return a.min < b.min;
>> +			  });
>> +
>> +		auto cur = sizes.begin();
>> +		auto next = cur;
>> +
>> +		while (++next != sizes.end()) {
>> +			if (cur->max.width >= next->min.width &&
>> +			    cur->max.height >= next->min.height)
>> +				cur->max = next->max;
>> +			else if (++cur != next)
>> +				*cur = *next;
>> +		}
>> +
>> +		sizes.erase(++cur, sizes.end());
>> +	}
>> +
>> +	StreamConfiguration fullCfg{ StreamFormats{ fullFormats } };
>> +	fullCfg.pixelFormat = fullFormats.begin()->first;
>> +	fullCfg.size = fullFormats.begin()->second[0].max;
>> +	fullCfg.bufferCount = cfg.bufferCount;
>> +
>> +	return fullCfg;
>> +}
>> +
>> +CameraConfiguration::Status RkISP1Path::getPipeConfiguration(
>> +	StreamConfiguration &streamCfg,
>> +	const RkISP1Path::Configuration **cfg) const
>> +{
>> +	CameraConfiguration::Status _status = CameraConfiguration::Adjusted;
>> +
>> +	LOG(RkISP1, Debug)
>> +		<< "Looking for "
>> +		<< streamCfg.pixelFormat << "/" << streamCfg.size
>> +		<< " output configuration";
>> +	/*
>> +	 * Select the fallback configuration
>> +	 */
>> +	for (auto &_cfg : configs_) {
>> +		if (_cfg.withConverter == useConverter_) {
>> +			*cfg = &_cfg;
>> +			break;
>> +		}
>> +	}
>> +
>> +	PixelFormat pixelFormat = (*cfg)->outputFormats.front();
>> +	Size size = streamCfg.size;
>> +	size.boundTo((*cfg)->outputSizes.max);
>> +	size.expandTo((*cfg)->outputSizes.min);
>> +
>> +	/* Unless the converter use is enforced through a loaded configuration,
>> +	 * prefer a configuration without the converter if possible
>> +	 */
>> +	std::vector<bool> converterUse;
>> +	if (!useConverter_)
>> +		converterUse.push_back(false);
>> +	if (hasConverter_)
>> +		converterUse.push_back(true);
>> +
>> +	for (auto withConverter : converterUse) {
>> +		for (auto &_cfg : configs_) {
>> +			auto &outFmts = _cfg.outputFormats;
>> +			if (withConverter != _cfg.withConverter)
>> +				continue;
>> +
>> +			if ((std::find(outFmts.begin(),
>> +				       outFmts.end(),
>> +				       streamCfg.pixelFormat)) == outFmts.end())
>> +				continue;
>> +
>> +			*cfg = &_cfg;
>> +			pixelFormat = streamCfg.pixelFormat;
>> +
>> +			if (_cfg.outputSizes.contains(streamCfg.size)) {
>> +				_status = CameraConfiguration::Valid;
>> +				goto done;
>> +			}
>> +
>> +			size.boundTo((*cfg)->outputSizes.max);
>> +			size.expandTo((*cfg)->outputSizes.min);
>> +		}
>> +	}
>> +
>> +	streamCfg.pixelFormat = pixelFormat;
>> +	streamCfg.size = size;
>> +
>> +done:
>> +	if ((*cfg)->withConverter) {
>> +		std::tie(streamCfg.stride, streamCfg.frameSize) =
>> +			converter_->strideAndFrameSize(streamCfg.pixelFormat,
>> +						       streamCfg.size);
>> +		if (streamCfg.stride == 0)
>> +			return CameraConfiguration::Invalid;
>> +	}
>> +
>> +	LOG(RkISP1, Debug)
>> +		<< "Chosen configuration: "
>> +		<< (*cfg)->inputFormat << "/" << (*cfg)->inputSizes
>> +		<< " --> " << streamCfg.pixelFormat << "/" << streamCfg.size
>> +		<< ((*cfg)->withConverter ? " with" : " without") << " converter";
>> +	return _status;
>> +}
>> +
>> +CameraConfiguration::Status RkISP1Path::nativeValidate(StreamConfiguration *cfg)
>>   {
>>   	const StreamConfiguration reqCfg = *cfg;
>>   	CameraConfiguration::Status status = CameraConfiguration::Valid;
>> @@ -101,7 +302,39 @@ CameraConfiguration::Status RkISP1Path::validate(StreamConfiguration *cfg)
>>   	return status;
>>   }
>>   
>> -int RkISP1Path::configure(const StreamConfiguration &config,
>> +CameraConfiguration::Status RkISP1Path::validate(StreamConfiguration *cfg)
>> +{
>> +	StreamConfiguration tryCfg = *cfg;
>> +	StreamConfiguration internalCfg;
>> +	const Configuration *pipeCfg;
>> +	CameraConfiguration::Status pipeStatus;
>> +
>> +	pipeStatus = getPipeConfiguration(tryCfg, &pipeCfg);
>> +	if (pipeStatus == CameraConfiguration::Invalid)
>> +		return CameraConfiguration::Invalid;
>> +
>> +	if (!pipeCfg->withConverter) {
>> +		tryCfg = *cfg;
>> +		pipeStatus = nativeValidate(&tryCfg);
>> +		if (pipeStatus == CameraConfiguration::Invalid)
>> +			return CameraConfiguration::Invalid;
>> +		internalCfg = tryCfg;
>> +	} else {
>> +		internalCfg = tryCfg;
>> +		internalCfg.pixelFormat = pipeCfg->inputFormat;
>> +		auto _status = nativeValidate(&internalCfg);
>> +		if (_status == CameraConfiguration::Invalid)
>> +			return CameraConfiguration::Invalid;
>> +	}
>> +
>> +	*cfg = tryCfg;
>> +	internalStream_ = internalCfg;
>> +	pipeConfig_ = pipeCfg;
>> +
>> +	return pipeStatus;
>> +}
>> +
>> +int RkISP1Path::nativeConfigure(const StreamConfiguration &config,
>>   			  const V4L2SubdeviceFormat &inputFormat)
>>   {
>>   	int ret;
>> @@ -165,6 +398,117 @@ int RkISP1Path::configure(const StreamConfiguration &config,
>>   	return 0;
>>   }
>>   
>> +int RkISP1Path::configure(const StreamConfiguration &config,
>> +			  const V4L2SubdeviceFormat &inputFormat)
>> +{
>> +	int ret;
>> +	LOG(RkISP1, Debug)
>> +		<< "Configuring " << name_ << " path "
>> +		<< internalStream_.pixelFormat << "/" << internalStream_.size
>> +		<< " --> " << config.pixelFormat << "/" << config.size
>> +		<< (pipeConfig_->withConverter ? " with" : " without") << " converter";
>> +
>> +	ret = nativeConfigure(internalStream_, inputFormat);
>> +	if (ret)
>> +		return ret;
>> +
>> +	if (pipeConfig_->withConverter) {
>> +		StreamConfiguration cfg = config;
>> +		std::vector<std::reference_wrapper<StreamConfiguration>> outputCfgs;
>> +		outputCfgs.push_back(cfg);
>> +		ret = converter_->configure(internalStream_, outputCfgs);
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>> +int RkISP1Path::exportBuffers(unsigned int bufferCount,
>> +			      std::vector<std::unique_ptr<FrameBuffer>> *buffers)
>> +{
>> +	if (pipeConfig_->withConverter)
>> +		return converter_->exportBuffers(0, bufferCount, buffers);
>> +	else
>> +		return video_->exportBuffers(bufferCount, buffers);
>> +}
>> +
>> +int RkISP1Path::allocateBuffers(unsigned int bufferCount)
>> +{
>> +	if (pipeConfig_->withConverter) {
>> +		int ret = video_->allocateBuffers(bufferCount, &converterBuffers_);
>> +		if (ret < 0)
>> +			return ret;
>> +		if ((unsigned int)ret != bufferCount)
>> +			LOG(RkISP1, Warning)
>> +				<< "Requested " << bufferCount << " but got " << ret << " buffers";
>> +
>> +		for (std::unique_ptr<FrameBuffer> &buffer : converterBuffers_)
>> +			availableConverterBuffers_.push(buffer.get());
>> +	} else {
>> +		return video_->importBuffers(bufferCount);
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +void RkISP1Path::releaseBuffers()
>> +{
>> +	while (!availableConverterBuffers_.empty())
>> +		availableConverterBuffers_.pop();
>> +
>> +	converterBuffers_.clear();
>> +
>> +	video_->releaseBuffers();
>> +}
>> +
>> +void RkISP1Path::pathBufferReady(FrameBuffer *buffer)
>> +{
>> +	Request *request = buffer->request();
>> +
>> +	if (request) {
>> +		internalBufferReady_.emit(buffer);
>> +	} else {
>> +		auto iter = converterBuffersMapping_.find(buffer);
>> +		if (iter != converterBuffersMapping_.end()) {
>> +			converter_->queueBuffer(buffer, iter->second);
>> +			converterBuffersMapping_.erase(iter);
>> +		} else {
>> +			LOG(RkISP1, Error)
>> +				<< "Final buffer associated to converted buffer not found on "
>> +				<< name_ << " path";
>> +		}
>> +	}
>> +}
>> +
>> +void RkISP1Path::pathConverterInputDone(FrameBuffer *buffer)
>> +{
>> +	availableConverterBuffers_.push(buffer);
>> +}
>> +
>> +void RkISP1Path::pathConverterOutputDone(FrameBuffer *buffer)
>> +{
>> +	internalBufferReady_.emit(buffer);
>> +}
>> +
>> +int RkISP1Path::queueBuffer(FrameBuffer *buffer)
>> +{
>> +	FrameBuffer *_buffer;
>> +
>> +	if (pipeConfig_->withConverter) {
>> +		if (availableConverterBuffers_.empty()) {
>> +			LOG(RkISP1, Error)
>> +				<< "converter buffer underrun on " << name_ << " path";
>> +			return -ENOMEM;
>> +		}
>> +		_buffer = availableConverterBuffers_.front();
>> +		availableConverterBuffers_.pop();
>> +		converterBuffersMapping_[_buffer] = buffer;
>> +	} else {
>> +		_buffer = buffer;
>> +	}
>> +
>> +	return video_->queueBuffer(_buffer);
>> +}
>> +
>>   int RkISP1Path::start()
>>   {
>>   	int ret;
>> @@ -172,20 +516,23 @@ int RkISP1Path::start()
>>   	if (running_)
>>   		return -EBUSY;
>>   
>> -	/* \todo Make buffer count user configurable. */
>> -	ret = video_->importBuffers(RKISP1_BUFFER_COUNT);
>> -	if (ret)
>> -		return ret;
>> -
>>   	ret = video_->streamOn();
>>   	if (ret) {
>>   		LOG(RkISP1, Error)
>>   			<< "Failed to start " << name_ << " path";
>> -
>> -		video_->releaseBuffers();
>>   		return ret;
>>   	}
>>   
>> +	if (pipeConfig_->withConverter) {
>> +		ret = converter_->start();
>> +		if (ret) {
>> +			LOG(RkISP1, Error)
>> +				<< "Failed to start converter on " << name_ << " path";
>> +			stop();
>> +			return ret;
>> +		}
>> +	}
>> +
>>   	running_ = true;
>>   
>>   	return 0;
>> @@ -199,7 +546,8 @@ void RkISP1Path::stop()
>>   	if (video_->streamOff())
>>   		LOG(RkISP1, Warning) << "Failed to stop " << name_ << " path";
>>   
>> -	video_->releaseBuffers();
>> +	if (pipeConfig_->withConverter)
>> +		converter_->stop();
>>   
>>   	running_ = false;
>>   }
>> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.h b/src/libcamera/pipeline/rkisp1/rkisp1_path.h
>> index f3f1ae39..0da3594f 100644
>> --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.h
>> +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.h
>> @@ -1,6 +1,7 @@
>>   /* SPDX-License-Identifier: LGPL-2.1-or-later */
>>   /*
>>    * Copyright (C) 2020, Google Inc.
>> + * Copyright 2022 NXP
>>    *
>>    * rkisp1path.h - Rockchip ISP1 path helper
>>    */
>> @@ -8,6 +9,7 @@
>>   #pragma once
>>   
>>   #include <memory>
>> +#include <queue>
>>   #include <vector>
>>   
>>   #include <libcamera/base/signal.h>
>> @@ -17,9 +19,11 @@
>>   #include <libcamera/geometry.h>
>>   #include <libcamera/pixel_format.h>
>>   
>> +#include "libcamera/internal/converter.h"
>>   #include "libcamera/internal/media_object.h"
>>   #include "libcamera/internal/v4l2_videodevice.h"
>>   
>> +
>>   namespace libcamera {
>>   
>>   class MediaDevice;
>> @@ -33,7 +37,7 @@ public:
>>   	RkISP1Path(const char *name, const Span<const PixelFormat> &formats,
>>   		   const Size &minResolution, const Size &maxResolution);
>>   
>> -	bool init(MediaDevice *media);
>> +	bool init(MediaDevice *media, MediaDevice *mediaConverter = nullptr);
>>   
>>   	int setEnabled(bool enable) { return link_->setEnabled(enable); }
>>   	bool isEnabled() const { return link_->flags() & MEDIA_LNK_FL_ENABLED; }
>> @@ -45,18 +49,31 @@ public:
>>   		      const V4L2SubdeviceFormat &inputFormat);
>>   
>>   	int exportBuffers(unsigned int bufferCount,
>> -			  std::vector<std::unique_ptr<FrameBuffer>> *buffers)
>> -	{
>> -		return video_->exportBuffers(bufferCount, buffers);
>> -	}
>> +			  std::vector<std::unique_ptr<FrameBuffer>> *buffers);
>> +
>> +	int allocateBuffers(unsigned int bufferCount);
>> +	void releaseBuffers();
>>   
>>   	int start();
>>   	void stop();
>>   
>> -	int queueBuffer(FrameBuffer *buffer) { return video_->queueBuffer(buffer); }
>> -	Signal<FrameBuffer *> &bufferReady() { return video_->bufferReady; }
>> +	int queueBuffer(FrameBuffer *buffer);
>> +	Signal<FrameBuffer *> &bufferReady() { return internalBufferReady_; }
>> +
>> +	StreamConfiguration internalStream() const
>> +	{
>> +		return internalStream_;
>> +	}
>>   
>>   private:
>> +	struct Configuration {
>> +		SizeRange inputSizes;
>> +		PixelFormat inputFormat;
>> +		std::vector<PixelFormat> outputFormats;
>> +		SizeRange outputSizes;
>> +		bool withConverter;
>> +	};
>> +
>>   	static constexpr unsigned int RKISP1_BUFFER_COUNT = 4;
>>   
>>   	const char *name_;
>> @@ -65,10 +82,33 @@ private:
>>   	const Span<const PixelFormat> formats_;
>>   	const Size minResolution_;
>>   	const Size maxResolution_;
>> +	CameraConfiguration::Status getPipeConfiguration(
>> +		StreamConfiguration &streamCfg,
>> +		const RkISP1Path::Configuration **cfg) const;
>> +
>> +	StreamConfiguration generateNativeConfiguration(const Size &resolution);
>> +	CameraConfiguration::Status nativeValidate(StreamConfiguration *cfg);
>> +	int nativeConfigure(const StreamConfiguration &config,
>> +			    const V4L2SubdeviceFormat &inputFormat);
>>   
>>   	std::unique_ptr<V4L2Subdevice> resizer_;
>>   	std::unique_ptr<V4L2VideoDevice> video_;
>>   	MediaLink *link_;
>> +
>> +	void initConverter(MediaDevice *mediaConverter);
>> +	std::unique_ptr<Converter> converter_;
>> +	std::vector<Configuration> configs_;
>> +	StreamConfiguration internalStream_;
>> +	const Configuration *pipeConfig_;
>> +	bool hasConverter_;
>> +	bool useConverter_;
>> +	Signal<FrameBuffer *> internalBufferReady_;
>> +	std::vector<std::unique_ptr<FrameBuffer>> converterBuffers_;
>> +	std::queue<FrameBuffer *> availableConverterBuffers_;
>> +	std::map<FrameBuffer *, FrameBuffer *> converterBuffersMapping_;
>> +	void pathBufferReady(FrameBuffer *buffer);
>> +	void pathConverterInputDone(FrameBuffer *buffer);
>> +	void pathConverterOutputDone(FrameBuffer *buffer);
>>   };
>>   
>>   class RkISP1MainPath : public RkISP1Path
>

Patch
diff mbox series

diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index c1522ca6..6bdf5a3a 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -1,6 +1,7 @@ 
 /* SPDX-License-Identifier: LGPL-2.1-or-later */
 /*
  * Copyright (C) 2019, Google Inc.
+ * Copyright 2022 NXP
  *
  * rkisp1.cpp - Pipeline handler for Rockchip ISP1
  */
@@ -194,6 +195,7 @@  private:
 	Camera *activeCamera_;
 
 	const MediaPad *ispSink_;
+	MediaDevice *converter_;
 };
 
 RkISP1Frames::RkISP1Frames(PipelineHandler *pipe)
@@ -449,57 +451,44 @@  CameraConfiguration::Status RkISP1CameraConfiguration::validate()
 
 	bool mainPathAvailable = true;
 	bool selfPathAvailable = data_->selfPath_;
+	const std::array<Status, 2> cameraStatus = { Valid, Adjusted };
+
 	for (unsigned int index : order) {
 		StreamConfiguration &cfg = config_[index];
 
-		/* Try to match stream without adjusting configuration. */
-		if (mainPathAvailable) {
-			StreamConfiguration tryCfg = cfg;
-			if (data_->mainPath_->validate(&tryCfg) == Valid) {
-				mainPathAvailable = false;
-				cfg = tryCfg;
-				cfg.setStream(const_cast<Stream *>(&data_->mainPathStream_));
-				continue;
-			}
-		}
+		for (auto &_status : cameraStatus) {
+			Status pipeStatus;
 
-		if (selfPathAvailable) {
-			StreamConfiguration tryCfg = cfg;
-			if (data_->selfPath_->validate(&tryCfg) == Valid) {
-				selfPathAvailable = false;
-				cfg = tryCfg;
-				cfg.setStream(const_cast<Stream *>(&data_->selfPathStream_));
-				continue;
-			}
-		}
+			if (mainPathAvailable) {
+				StreamConfiguration tryCfg = cfg;
+				pipeStatus = data_->mainPath_->validate(&tryCfg);
 
-		/* Try to match stream allowing adjusting configuration. */
-		if (mainPathAvailable) {
-			StreamConfiguration tryCfg = cfg;
-			if (data_->mainPath_->validate(&tryCfg) == Adjusted) {
-				mainPathAvailable = false;
-				cfg = tryCfg;
-				cfg.setStream(const_cast<Stream *>(&data_->mainPathStream_));
-				status = Adjusted;
-				continue;
+				if (pipeStatus == _status) {
+					mainPathAvailable = false;
+					cfg = tryCfg;
+					cfg.setStream(const_cast<Stream *>(&data_->mainPathStream_));
+					status = _status;
+					break;
+				}
 			}
-		}
 
-		if (selfPathAvailable) {
-			StreamConfiguration tryCfg = cfg;
-			if (data_->selfPath_->validate(&tryCfg) == Adjusted) {
-				selfPathAvailable = false;
-				cfg = tryCfg;
-				cfg.setStream(const_cast<Stream *>(&data_->selfPathStream_));
-				status = Adjusted;
-				continue;
+			if (selfPathAvailable) {
+				StreamConfiguration tryCfg = cfg;
+				pipeStatus = data_->selfPath_->validate(&tryCfg);
+
+				if (pipeStatus == _status) {
+					selfPathAvailable = false;
+					cfg = tryCfg;
+					cfg.setStream(const_cast<Stream *>(&data_->selfPathStream_));
+					status = _status;
+					break;
+				}
 			}
+			/* All paths rejected configuration. */
+			LOG(RkISP1, Debug) << "Camera configuration not supported "
+					   << cfg.toString();
+			return Invalid;
 		}
-
-		/* All paths rejected configuraiton. */
-		LOG(RkISP1, Debug) << "Camera configuration not supported "
-				   << cfg.toString();
-		return Invalid;
 	}
 
 	/* Select the sensor format. */
@@ -680,15 +669,21 @@  int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
 
 	std::map<unsigned int, IPAStream> streamConfig;
 
-	for (const StreamConfiguration &cfg : *config) {
+	for (StreamConfiguration &cfg : *config) {
+		size_t idx = 0;
+		StreamConfiguration internalCfg;
 		if (cfg.stream() == &data->mainPathStream_) {
+			idx = 0;
 			ret = mainPath_.configure(cfg, format);
-			streamConfig[0] = IPAStream(cfg.pixelFormat,
-						    cfg.size);
+			internalCfg = mainPath_.internalStream();
+			streamConfig[idx] = IPAStream(internalCfg.pixelFormat,
+						      internalCfg.size);
 		} else if (hasSelfPath_) {
+			idx = 1;
 			ret = selfPath_.configure(cfg, format);
-			streamConfig[1] = IPAStream(cfg.pixelFormat,
-						    cfg.size);
+			internalCfg = selfPath_.internalStream();
+			streamConfig[idx] = IPAStream(internalCfg.pixelFormat,
+						      internalCfg.size);
 		} else {
 			return -ENODEV;
 		}
@@ -754,6 +749,20 @@  int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)
 		data->selfPathStream_.configuration().bufferCount,
 	});
 
+	if (data->mainPath_->isEnabled()) {
+		ret = data->mainPath_->allocateBuffers(
+			data->mainPathStream_.configuration().bufferCount);
+		if (ret < 0)
+			goto error;
+	}
+
+	if (hasSelfPath_ && data->selfPath_->isEnabled()) {
+		ret = data->selfPath_->allocateBuffers(
+			data->selfPathStream_.configuration().bufferCount);
+		if (ret < 0)
+			goto error;
+	}
+
 	ret = param_->allocateBuffers(maxCount, &paramBuffers_);
 	if (ret < 0)
 		goto error;
@@ -813,6 +822,12 @@  int PipelineHandlerRkISP1::freeBuffers(Camera *camera)
 	if (stat_->releaseBuffers())
 		LOG(RkISP1, Error) << "Failed to release stat buffers";
 
+	if (hasSelfPath_ && data->selfPath_->isEnabled())
+		data->selfPath_->releaseBuffers();
+
+	if (data->mainPath_->isEnabled())
+		data->mainPath_->releaseBuffers();
+
 	return 0;
 }
 
@@ -1055,6 +1070,19 @@  bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)
 
 	hasSelfPath_ = !!media_->getEntityByName("rkisp1_selfpath");
 
+	/* Seek for a converter */
+	for (auto converterName : ConverterFactory::names()) {
+		LOG(RkISP1, Debug)
+			<< "Trying " << converterName << " converter";
+		DeviceMatch converterMatch(converterName);
+		converter_ = acquireMediaDevice(enumerator, converterMatch);
+		if (converter_) {
+			LOG(RkISP1, Debug)
+				<< "Get support for " << converterName << " converter";
+			break;
+		}
+	}
+
 	/* Create the V4L2 subdevices we will need. */
 	isp_ = V4L2Subdevice::fromEntityName(media_, "rkisp1_isp");
 	if (isp_->open() < 0)
@@ -1086,10 +1114,10 @@  bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)
 		return false;
 
 	/* Locate and open the ISP main and self paths. */
-	if (!mainPath_.init(media_))
+	if (!mainPath_.init(media_, converter_))
 		return false;
 
-	if (hasSelfPath_ && !selfPath_.init(media_))
+	if (hasSelfPath_ && !selfPath_.init(media_, converter_))
 		return false;
 
 	mainPath_.bufferReady().connect(this, &PipelineHandlerRkISP1::bufferReady);
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
index 2d38f0fb..c19a1e69 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp
@@ -1,6 +1,7 @@ 
 /* SPDX-License-Identifier: LGPL-2.1-or-later */
 /*
  * Copyright (C) 2020, Google Inc.
+ * Copyright 2022 NXP
  *
  * rkisp1path.cpp - Rockchip ISP1 path helper
  */
@@ -28,7 +29,46 @@  RkISP1Path::RkISP1Path(const char *name, const Span<const PixelFormat> &formats,
 {
 }
 
-bool RkISP1Path::init(MediaDevice *media)
+void RkISP1Path::initConverter(MediaDevice *mediaConverter)
+{
+	hasConverter_ = false;
+	useConverter_ = false;
+
+	if (!mediaConverter)
+		return;
+
+	converter_ = ConverterFactory::create(mediaConverter);
+
+	if (!converter_->isValid()) {
+		LOG(RkISP1, Warning)
+			<< "Failed to create converter, disabling format conversion";
+		converter_.reset();
+	} else {
+		char const *configFromEnv = utils::secure_getenv("LIBCAMERA_RKISP1_CONVERTER_FILE");
+
+		if (configFromEnv && *configFromEnv != '\0') {
+			int nrMappings;
+			LOG(RkISP1, Debug)
+				<< "Getting pipeline converter filename as " << std::string(configFromEnv);
+			nrMappings = converter_->loadConfiguration(std::string(configFromEnv));
+			if (nrMappings < 0) {
+				LOG(RkISP1, Error)
+					<< "Error while reading converter configuration file";
+			} else {
+				LOG(RkISP1, Debug)
+					<< nrMappings << " mapping(s) loaded";
+				if (nrMappings > 0)
+					/* We want to force the converter use to apply the mapping */
+					useConverter_ = true;
+			}
+		}
+		converter_->inputBufferReady.connect(this, &RkISP1Path::pathConverterInputDone);
+		converter_->outputBufferReady.connect(this, &RkISP1Path::pathConverterOutputDone);
+		hasConverter_ = true;
+	}
+}
+
+bool RkISP1Path::init(MediaDevice *media, MediaDevice *mediaConverter)
 {
 	std::string resizer = std::string("rkisp1_resizer_") + name_ + "path";
 	std::string video = std::string("rkisp1_") + name_ + "path";
@@ -45,10 +85,13 @@  bool RkISP1Path::init(MediaDevice *media)
 	if (!link_)
 		return false;
 
+	initConverter(mediaConverter);
+
+	video_->bufferReady.connect(this, &RkISP1Path::pathBufferReady);
+
 	return true;
 }
-
-StreamConfiguration RkISP1Path::generateConfiguration(const Size &resolution)
+StreamConfiguration RkISP1Path::generateNativeConfiguration(const Size &resolution)
 {
 	Size maxResolution = maxResolution_.boundedToAspectRatio(resolution)
 					   .boundedTo(resolution);
@@ -67,7 +110,165 @@  StreamConfiguration RkISP1Path::generateConfiguration(const Size &resolution)
 	return cfg;
 }
 
-CameraConfiguration::Status RkISP1Path::validate(StreamConfiguration *cfg)
+StreamConfiguration RkISP1Path::generateConfiguration(const Size &resolution)
+{
+	StreamConfiguration cfg = generateNativeConfiguration(resolution);
+	StreamFormats formats = cfg.formats();
+	std::map<PixelFormat, std::vector<SizeRange>> fullFormats;
+
+	for (const PixelFormat &fmt : formats.pixelformats()) {
+		Configuration config;
+		SizeRange szRange;
+
+		config.inputFormat = fmt;
+		config.inputSizes = formats.range(fmt);
+
+		if (!useConverter_) {
+			config.outputFormats.push_back(fmt);
+			config.outputSizes = config.inputSizes;
+			config.withConverter = false;
+			configs_.push_back(config);
+			LOG(RkISP1, Debug)
+				<< "Pushing native format " << fmt
+				<< " resolution range " << config.inputSizes;
+			fullFormats[fmt].push_back(config.inputSizes);
+		}
+
+		if (hasConverter_) {
+			config.outputFormats = converter_->formats(config.inputFormat);
+			if (config.outputFormats.empty())
+				continue;
+			szRange.max = converter_->sizes(config.inputSizes.max).max;
+			szRange.min = converter_->sizes(config.inputSizes.min).min;
+			config.outputSizes = szRange;
+			config.withConverter = true;
+			configs_.push_back(config);
+			for (auto &_fmt : config.outputFormats) {
+				fullFormats[_fmt].push_back(config.outputSizes);
+				LOG(RkISP1, Debug)
+					<< "Pushing converted format " << _fmt
+					<< " resolution range " << config.outputSizes;
+			}
+		}
+	}
+	/*
+	 * Sort the sizes and merge any consecutive overlapping ranges.
+	 * Imported from src/libcamera/pipeline/simple.cpp
+	 *
+	 * TODO: Apply policy of converter use or not.. We likely want to force
+	 * the converter use in case there is a mapping defined in the
+	 * configuration file if any... if so, shall we filter out resolution
+	 * not defined in the configuration file.. or should this policy be let
+	 * to the application
+	 * */
+
+	for (auto &[format, sizes] : fullFormats) {
+		std::sort(sizes.begin(), sizes.end(),
+			  [](SizeRange &a, SizeRange &b) {
+				  return a.min < b.min;
+			  });
+
+		auto cur = sizes.begin();
+		auto next = cur;
+
+		while (++next != sizes.end()) {
+			if (cur->max.width >= next->min.width &&
+			    cur->max.height >= next->min.height)
+				cur->max = next->max;
+			else if (++cur != next)
+				*cur = *next;
+		}
+
+		sizes.erase(++cur, sizes.end());
+	}
+
+	StreamConfiguration fullCfg{ StreamFormats{ fullFormats } };
+	fullCfg.pixelFormat = fullFormats.begin()->first;
+	fullCfg.size = fullFormats.begin()->second[0].max;
+	fullCfg.bufferCount = cfg.bufferCount;
+
+	return fullCfg;
+}
+
+CameraConfiguration::Status RkISP1Path::getPipeConfiguration(
+	StreamConfiguration &streamCfg,
+	const RkISP1Path::Configuration **cfg) const
+{
+	CameraConfiguration::Status _status = CameraConfiguration::Adjusted;
+
+	LOG(RkISP1, Debug)
+		<< "Looking for "
+		<< streamCfg.pixelFormat << "/" << streamCfg.size
+		<< " output configuration";
+	/*
+	 * Select the fallback configuration
+	 */
+	for (auto &_cfg : configs_) {
+		if (_cfg.withConverter == useConverter_) {
+			*cfg = &_cfg;
+			break;
+		}
+	}
+
+	PixelFormat pixelFormat = (*cfg)->outputFormats.front();
+	Size size = streamCfg.size;
+	size.boundTo((*cfg)->outputSizes.max);
+	size.expandTo((*cfg)->outputSizes.min);
+
+	/* Unless the converter use is enforced through a loaded configuration,
+	 * prefer a configuration without the converter if possible
+	 */
+	std::vector<bool> converterUse;
+	if (!useConverter_)
+		converterUse.push_back(false);
+	if (hasConverter_)
+		converterUse.push_back(true);
+
+	for (auto withConverter : converterUse) {
+		for (auto &_cfg : configs_) {
+			auto &outFmts = _cfg.outputFormats;
+			if (withConverter != _cfg.withConverter)
+				continue;
+
+			if ((std::find(outFmts.begin(),
+				       outFmts.end(),
+				       streamCfg.pixelFormat)) == outFmts.end())
+				continue;
+
+			*cfg = &_cfg;
+			pixelFormat = streamCfg.pixelFormat;
+
+			if (_cfg.outputSizes.contains(streamCfg.size)) {
+				_status = CameraConfiguration::Valid;
+				goto done;
+			}
+
+			size.boundTo((*cfg)->outputSizes.max);
+			size.expandTo((*cfg)->outputSizes.min);
+		}
+	}
+
+	streamCfg.pixelFormat = pixelFormat;
+	streamCfg.size = size;
+
+done:
+	if ((*cfg)->withConverter) {
+		std::tie(streamCfg.stride, streamCfg.frameSize) =
+			converter_->strideAndFrameSize(streamCfg.pixelFormat,
+						       streamCfg.size);
+		if (streamCfg.stride == 0)
+			return CameraConfiguration::Invalid;
+	}
+
+	LOG(RkISP1, Debug)
+		<< "Chosen configuration: "
+		<< (*cfg)->inputFormat << "/" << (*cfg)->inputSizes
+		<< " --> " << streamCfg.pixelFormat << "/" << streamCfg.size
+		<< ((*cfg)->withConverter ? " with" : " without") << " converter";
+	return _status;
+}
+
+CameraConfiguration::Status RkISP1Path::nativeValidate(StreamConfiguration *cfg)
 {
 	const StreamConfiguration reqCfg = *cfg;
 	CameraConfiguration::Status status = CameraConfiguration::Valid;
@@ -101,7 +302,39 @@  CameraConfiguration::Status RkISP1Path::validate(StreamConfiguration *cfg)
 	return status;
 }
 
-int RkISP1Path::configure(const StreamConfiguration &config,
+CameraConfiguration::Status RkISP1Path::validate(StreamConfiguration *cfg)
+{
+	StreamConfiguration tryCfg = *cfg;
+	StreamConfiguration internalCfg;
+	const Configuration *pipeCfg;
+	CameraConfiguration::Status pipeStatus;
+
+	pipeStatus = getPipeConfiguration(tryCfg, &pipeCfg);
+	if (pipeStatus == CameraConfiguration::Invalid)
+		return CameraConfiguration::Invalid;
+
+	if (!pipeCfg->withConverter) {
+		tryCfg = *cfg;
+		pipeStatus = nativeValidate(&tryCfg);
+		if (pipeStatus == CameraConfiguration::Invalid)
+			return CameraConfiguration::Invalid;
+		internalCfg = tryCfg;
+	} else {
+		internalCfg = tryCfg;
+		internalCfg.pixelFormat = pipeCfg->inputFormat;
+		auto _status = nativeValidate(&internalCfg);
+		if (_status == CameraConfiguration::Invalid)
+			return CameraConfiguration::Invalid;
+	}
+
+	*cfg = tryCfg;
+	internalStream_ = internalCfg;
+	pipeConfig_ = pipeCfg;
+
+	return pipeStatus;
+}
+
+int RkISP1Path::nativeConfigure(const StreamConfiguration &config,
 			  const V4L2SubdeviceFormat &inputFormat)
 {
 	int ret;
@@ -165,6 +398,117 @@  int RkISP1Path::configure(const StreamConfiguration &config,
 	return 0;
 }
 
+int RkISP1Path::configure(const StreamConfiguration &config,
+			  const V4L2SubdeviceFormat &inputFormat)
+{
+	int ret;
+	LOG(RkISP1, Debug)
+		<< "Configuring " << name_ << " path "
+		<< internalStream_.pixelFormat << "/" << internalStream_.size
+		<< " --> " << config.pixelFormat << "/" << config.size
+		<< (pipeConfig_->withConverter ? " with" : " without") << " converter";
+
+	ret = nativeConfigure(internalStream_, inputFormat);
+	if (ret)
+		return ret;
+
+	if (pipeConfig_->withConverter) {
+		StreamConfiguration cfg = config;
+		std::vector<std::reference_wrapper<StreamConfiguration>> outputCfgs;
+		outputCfgs.push_back(cfg);
+		ret = converter_->configure(internalStream_, outputCfgs);
+	}
+
+	return ret;
+}
+
+int RkISP1Path::exportBuffers(unsigned int bufferCount,
+			      std::vector<std::unique_ptr<FrameBuffer>> *buffers)
+{
+	if (pipeConfig_->withConverter)
+		return converter_->exportBuffers(0, bufferCount, buffers);
+	else
+		return video_->exportBuffers(bufferCount, buffers);
+}
+
+int RkISP1Path::allocateBuffers(unsigned int bufferCount)
+{
+	if (pipeConfig_->withConverter) {
+		int ret = video_->allocateBuffers(bufferCount, &converterBuffers_);
+		if (ret < 0)
+			return ret;
+		if ((unsigned int)ret != bufferCount)
+			LOG(RkISP1, Warning)
+				<< "Requested " << bufferCount << " but got " << ret << " buffers";
+
+		for (std::unique_ptr<FrameBuffer> &buffer : converterBuffers_)
+			availableConverterBuffers_.push(buffer.get());
+	} else {
+		return video_->importBuffers(bufferCount);
+	}
+
+	return 0;
+}
+
+void RkISP1Path::releaseBuffers()
+{
+	while (!availableConverterBuffers_.empty())
+		availableConverterBuffers_.pop();
+
+	converterBuffers_.clear();
+
+	video_->releaseBuffers();
+}
+
+void RkISP1Path::pathBufferReady(FrameBuffer *buffer)
+{
+	Request *request = buffer->request();
+
+	if (request) {
+		internalBufferReady_.emit(buffer);
+	} else {
+		auto iter = converterBuffersMapping_.find(buffer);
+		if (iter != converterBuffersMapping_.end()) {
+			converter_->queueBuffer(buffer, iter->second);
+			converterBuffersMapping_.erase(iter);
+		} else {
+			LOG(RkISP1, Error)
+				<< "Final buffer associated to converted buffer not found on "
+				<< name_ << " path";
+		}
+	}
+}
+
+void RkISP1Path::pathConverterInputDone(FrameBuffer *buffer)
+{
+	availableConverterBuffers_.push(buffer);
+}
+
+void RkISP1Path::pathConverterOutputDone(FrameBuffer *buffer)
+{
+	internalBufferReady_.emit(buffer);
+}
+
+int RkISP1Path::queueBuffer(FrameBuffer *buffer)
+{
+	FrameBuffer *_buffer;
+
+	if (pipeConfig_->withConverter) {
+		if (availableConverterBuffers_.empty()) {
+			LOG(RkISP1, Error)
+				<< "converter buffer underrun on " << name_ << " path";
+			return -ENOMEM;
+		}
+		_buffer = availableConverterBuffers_.front();
+		availableConverterBuffers_.pop();
+		converterBuffersMapping_[_buffer] = buffer;
+	} else {
+		_buffer = buffer;
+	}
+
+	return video_->queueBuffer(_buffer);
+}
+
 int RkISP1Path::start()
 {
 	int ret;
@@ -172,20 +516,23 @@  int RkISP1Path::start()
 	if (running_)
 		return -EBUSY;
 
-	/* \todo Make buffer count user configurable. */
-	ret = video_->importBuffers(RKISP1_BUFFER_COUNT);
-	if (ret)
-		return ret;
-
 	ret = video_->streamOn();
 	if (ret) {
 		LOG(RkISP1, Error)
 			<< "Failed to start " << name_ << " path";
-
-		video_->releaseBuffers();
 		return ret;
 	}
 
+	if (pipeConfig_->withConverter) {
+		ret = converter_->start();
+		if (ret) {
+			LOG(RkISP1, Error)
+				<< "Failed to start converter on " << name_ << " path";
+			stop();
+			return ret;
+		}
+	}
+
 	running_ = true;
 
 	return 0;
@@ -199,7 +546,8 @@  void RkISP1Path::stop()
 	if (video_->streamOff())
 		LOG(RkISP1, Warning) << "Failed to stop " << name_ << " path";
 
-	video_->releaseBuffers();
+	if (pipeConfig_->withConverter)
+		converter_->stop();
 
 	running_ = false;
 }
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.h b/src/libcamera/pipeline/rkisp1/rkisp1_path.h
index f3f1ae39..0da3594f 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1_path.h
+++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.h
@@ -1,6 +1,7 @@ 
 /* SPDX-License-Identifier: LGPL-2.1-or-later */
 /*
  * Copyright (C) 2020, Google Inc.
+ * Copyright 2022 NXP
  *
  * rkisp1path.h - Rockchip ISP1 path helper
  */
@@ -8,6 +9,7 @@ 
 #pragma once
 
 #include <memory>
+#include <queue>
 #include <vector>
 
 #include <libcamera/base/signal.h>
@@ -17,9 +19,11 @@ 
 #include <libcamera/geometry.h>
 #include <libcamera/pixel_format.h>
 
+#include "libcamera/internal/converter.h"
 #include "libcamera/internal/media_object.h"
 #include "libcamera/internal/v4l2_videodevice.h"
 
+
 namespace libcamera {
 
 class MediaDevice;
@@ -33,7 +37,7 @@  public:
 	RkISP1Path(const char *name, const Span<const PixelFormat> &formats,
 		   const Size &minResolution, const Size &maxResolution);
 
-	bool init(MediaDevice *media);
+	bool init(MediaDevice *media, MediaDevice *mediaConverter = nullptr);
 
 	int setEnabled(bool enable) { return link_->setEnabled(enable); }
 	bool isEnabled() const { return link_->flags() & MEDIA_LNK_FL_ENABLED; }
@@ -45,18 +49,31 @@  public:
 		      const V4L2SubdeviceFormat &inputFormat);
 
 	int exportBuffers(unsigned int bufferCount,
-			  std::vector<std::unique_ptr<FrameBuffer>> *buffers)
-	{
-		return video_->exportBuffers(bufferCount, buffers);
-	}
+			  std::vector<std::unique_ptr<FrameBuffer>> *buffers);
+
+	int allocateBuffers(unsigned int bufferCount);
+	void releaseBuffers();
 
 	int start();
 	void stop();
 
-	int queueBuffer(FrameBuffer *buffer) { return video_->queueBuffer(buffer); }
-	Signal<FrameBuffer *> &bufferReady() { return video_->bufferReady; }
+	int queueBuffer(FrameBuffer *buffer);
+	Signal<FrameBuffer *> &bufferReady() { return internalBufferReady_; }
+
+	StreamConfiguration internalStream() const
+	{
+		return internalStream_;
+	}
 
 private:
+	struct Configuration {
+		SizeRange inputSizes;
+		PixelFormat inputFormat;
+		std::vector<PixelFormat> outputFormats;
+		SizeRange outputSizes;
+		bool withConverter;
+	};
+
 	static constexpr unsigned int RKISP1_BUFFER_COUNT = 4;
 
 	const char *name_;
@@ -65,10 +82,33 @@  private:
 	const Span<const PixelFormat> formats_;
 	const Size minResolution_;
 	const Size maxResolution_;
+	CameraConfiguration::Status getPipeConfiguration(
+		StreamConfiguration &streamCfg,
+		const RkISP1Path::Configuration **cfg) const;
+
+	StreamConfiguration generateNativeConfiguration(const Size &resolution);
+	CameraConfiguration::Status nativeValidate(StreamConfiguration *cfg);
+	int nativeConfigure(const StreamConfiguration &config,
+			    const V4L2SubdeviceFormat &inputFormat);
 
 	std::unique_ptr<V4L2Subdevice> resizer_;
 	std::unique_ptr<V4L2VideoDevice> video_;
 	MediaLink *link_;
+
+	void initConverter(MediaDevice *mediaConverter);
+	std::unique_ptr<Converter> converter_;
+	std::vector<Configuration> configs_;
+	StreamConfiguration internalStream_;
+	const Configuration *pipeConfig_;
+	bool hasConverter_;
+	bool useConverter_;
+	Signal<FrameBuffer *> internalBufferReady_;
+	std::vector<std::unique_ptr<FrameBuffer>> converterBuffers_;
+	std::queue<FrameBuffer *> availableConverterBuffers_;
+	std::map<FrameBuffer *, FrameBuffer *> converterBuffersMapping_;
+	void pathBufferReady(FrameBuffer *buffer);
+	void pathConverterInputDone(FrameBuffer *buffer);
+	void pathConverterOutputDone(FrameBuffer *buffer);
 };
 
 class RkISP1MainPath : public RkISP1Path