[PATCH v4 07/11] libcamera: simple: Consider raw output configurations

[Barnabás Pőcze]

Hi

2025. 04. 07. 10:56 keltezéssel, Milan Zamazal írta:
> When generating simple pipeline configuration, raw output configurations
> are generated but later filtered out.  Let's include processed and/or
> raw output configurations depending on the requested stream roles.
> 
> Raw and processed formats are handled separately.  The intention is that
> in case both raw and processed formats are requested then the raw
> formats should be left intact to the extent possible and not be
> influenced by the processed formats (implying that raw parameters
> compatible with the processed output requirements must be requested by
> the application).
> 
> Raw output configurations are identified by colorSpace being set to
> ColorSpace::Raw.
> 
> This is another preparatory patch without making raw outputs working.
> 
> Signed-off-by: Milan Zamazal <mzamazal at redhat.com>
> ---
>   src/libcamera/pipeline/simple/simple.cpp | 73 ++++++++++++++++--------
>   1 file changed, 48 insertions(+), 25 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index bf9d75f4..6b65d79f 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -439,6 +439,8 @@ private:
>   	const MediaPad *acquirePipeline(SimpleCameraData *data);
>   	void releasePipeline(SimpleCameraData *data);
>   
> +	void setUpFormatSizes(std::map<PixelFormat, std::vector<SizeRange>> &formats);
> +
>   	std::map<const MediaEntity *, EntityData> entities_;
>   
>   	MediaDevice *converter_;
> @@ -1321,35 +1323,28 @@ SimplePipelineHandler::generateConfiguration(Camera *camera, Span<const StreamRo
>   			data->processedRequested_ = true;
>   		}
>   
> -	/* Create the formats map. */
> -	std::map<PixelFormat, std::vector<SizeRange>> formats;
> +	/* Create the formats maps. */
> +	std::map<PixelFormat, std::vector<SizeRange>> processedFormats;
> +	std::map<PixelFormat, std::vector<SizeRange>> rawFormats;
>   
>   	for (const SimpleCameraData::Configuration &cfg : data->configs_)
> -		if (!cfg.raw)
> -			for (PixelFormat format : cfg.outputFormats)
> -				formats[format].push_back(cfg.outputSizes);
> +		for (PixelFormat format : cfg.outputFormats)
> +			(cfg.raw ? rawFormats : processedFormats)[format].push_back(cfg.outputSizes);
>   
> -	/* Sort the sizes and merge any consecutive overlapping ranges. */
> -	for (auto &[format, sizes] : formats) {
> -		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());
> +	if (data->processedRequested_ && processedFormats.empty()) {
> +		LOG(SimplePipeline, Error)
> +			<< "Processed stream requsted but no corresponding output configuration found";
> +		return nullptr;
> +	}
> +	if (data->rawRequested_ && rawFormats.empty()) {
> +		LOG(SimplePipeline, Error)
> +			<< "Raw stream requsted but no corresponding output configuration found";
> +		return nullptr;
>   	}
>   
> +	setUpFormatSizes(processedFormats);
> +	setUpFormatSizes(rawFormats);
> +
>   	/*
>   	 * Create the stream configurations. Take the first entry in the formats
>   	 * map as the default, for lack of a better option.
> @@ -1357,11 +1352,13 @@ SimplePipelineHandler::generateConfiguration(Camera *camera, Span<const StreamRo
>   	 * \todo Implement a better way to pick the default format
>   	 */
>   	for (StreamRole role : roles) {
> +		bool raw = (role == StreamRole::Raw);
> +		auto formats = (raw ? rawFormats : processedFormats);
>   		StreamConfiguration cfg{ StreamFormats{ formats } };
>   		cfg.pixelFormat = formats.begin()->first;
>   		cfg.size = formats.begin()->second[0].max;
>   
> -		if (role == StreamRole::Raw)
> +		if (raw)
>   			cfg.colorSpace = ColorSpace::Raw;
>   		else if (data->swIsp_)
>   			cfg.colorSpace = ColorSpace::Srgb;
> @@ -1374,6 +1371,32 @@ SimplePipelineHandler::generateConfiguration(Camera *camera, Span<const StreamRo
>   	return config;
>   }
>   
> +void SimplePipelineHandler::setUpFormatSizes(
> +	std::map<PixelFormat, std::vector<SizeRange>> &formats)

Does this need to be a member? If not, could it be moved into an anonymous
namespace in this file? Or since this is only used in a function, maybe
just save it into a lambda there for now.


> +{
> +	/* Sort the sizes and merge any consecutive overlapping ranges. */

I see that this has been copied unchanged, but I don't understand how this could
be correct. It does not seem to account for `SizeRange::{h,v}Step` at all.
What am I missing something?


Regards,
Barnabás Pőcze

> +
> +	for (auto &[format, sizes] : formats) {
> +		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());
> +	}
> +}
> +
>   int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)
>   {
>   	SimpleCameraConfiguration *config =