| Message ID | 20250124215806.158024-5-mzamazal@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hi Milan, Thank you for the patch. On Fri, Jan 24, 2025 at 10:57:55PM +0100, Milan Zamazal wrote: > When software ISP is active, only software ISP adjusted output > configurations are considered. We want to support raw streams together > with software ISP and for that purpose, we need to track also unmodified > configurations. > > A flag is added to SimpleCameraData::Configuration indicating whether > it's for software ISP. Configurations for unmodified output sizes and > formats are added now whether software ISP is active or not. We later > filter formats and output sizes for particular stream configuration > according to the new configuration flag. This is just preparation and > raw output is still not supported; for this reason, we just look whether > software ISP is active; this will be changed in followup patches. > > Signed-off-by: Milan Zamazal <mzamazal@redhat.com> > --- > src/libcamera/pipeline/simple/simple.cpp | 30 ++++++++++++++---------- > 1 file changed, 18 insertions(+), 12 deletions(-) > > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp > index 06df909b..71853bb2 100644 > --- a/src/libcamera/pipeline/simple/simple.cpp > +++ b/src/libcamera/pipeline/simple/simple.cpp > @@ -273,6 +273,7 @@ public: > Size captureSize; > std::vector<PixelFormat> outputFormats; > SizeRange outputSizes; > + bool swisp; > }; > > std::vector<Stream> streams_; > @@ -654,19 +655,24 @@ void SimpleCameraData::tryPipeline(unsigned int code, const Size &size) > config.sensorSize = size; > config.captureFormat = pixelFormat; > config.captureSize = format.size; > + config.swisp = false; > > if (converter_) { > config.outputFormats = converter_->formats(pixelFormat); > config.outputSizes = converter_->sizes(format.size); > - } else if (swIsp_) { > - config.outputFormats = swIsp_->formats(pixelFormat); > - config.outputSizes = swIsp_->sizes(pixelFormat, format.size); > - if (config.outputFormats.empty()) { > - /* Do not use swIsp for unsupported pixelFormat's. */ > - config.outputFormats = { pixelFormat }; > - config.outputSizes = config.captureSize; > - } > } else { > + if (swIsp_) { > + Configuration swispConfig = config; > + swispConfig.outputFormats = swIsp_->formats(pixelFormat); > + swispConfig.outputSizes = swIsp_->sizes(pixelFormat, format.size); > + if (swispConfig.outputFormats.empty()) { > + /* Do not use swIsp for unsupported pixelFormat's. */ > + swispConfig.outputFormats = { pixelFormat }; > + swispConfig.outputSizes = swispConfig.captureSize; > + } > + swispConfig.swisp = true; > + configs_.push_back(swispConfig); > + } That's really getting too confusing. I've read the rest of the series, and it doesn't get better :-( I think we need to first refactor the existing code to make raw handling identifical for the converter and software ISP cases. It doesn't mean you have to create a common interface for the two elements (it would be nice, but it's more yak shaving than I could reasonably ask for). > config.outputFormats = { pixelFormat }; > config.outputSizes = config.captureSize; > } > @@ -1185,10 +1191,10 @@ SimplePipelineHandler::generateConfiguration(Camera *camera, Span<const StreamRo > /* Create the formats map. */ > std::map<PixelFormat, std::vector<SizeRange>> formats; > > - for (const SimpleCameraData::Configuration &cfg : data->configs_) { > - for (PixelFormat format : cfg.outputFormats) > - formats[format].push_back(cfg.outputSizes); > - } > + for (const SimpleCameraData::Configuration &cfg : data->configs_) > + if (static_cast<bool>(data->swIsp_) == cfg.swisp) > + for (PixelFormat format : cfg.outputFormats) > + formats[format].push_back(cfg.outputSizes); > > /* Sort the sizes and merge any consecutive overlapping ranges. */ > for (auto &[format, sizes] : formats) {
diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp index 06df909b..71853bb2 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -273,6 +273,7 @@ public: Size captureSize; std::vector<PixelFormat> outputFormats; SizeRange outputSizes; + bool swisp; }; std::vector<Stream> streams_; @@ -654,19 +655,24 @@ void SimpleCameraData::tryPipeline(unsigned int code, const Size &size) config.sensorSize = size; config.captureFormat = pixelFormat; config.captureSize = format.size; + config.swisp = false; if (converter_) { config.outputFormats = converter_->formats(pixelFormat); config.outputSizes = converter_->sizes(format.size); - } else if (swIsp_) { - config.outputFormats = swIsp_->formats(pixelFormat); - config.outputSizes = swIsp_->sizes(pixelFormat, format.size); - if (config.outputFormats.empty()) { - /* Do not use swIsp for unsupported pixelFormat's. */ - config.outputFormats = { pixelFormat }; - config.outputSizes = config.captureSize; - } } else { + if (swIsp_) { + Configuration swispConfig = config; + swispConfig.outputFormats = swIsp_->formats(pixelFormat); + swispConfig.outputSizes = swIsp_->sizes(pixelFormat, format.size); + if (swispConfig.outputFormats.empty()) { + /* Do not use swIsp for unsupported pixelFormat's. */ + swispConfig.outputFormats = { pixelFormat }; + swispConfig.outputSizes = swispConfig.captureSize; + } + swispConfig.swisp = true; + configs_.push_back(swispConfig); + } config.outputFormats = { pixelFormat }; config.outputSizes = config.captureSize; } @@ -1185,10 +1191,10 @@ SimplePipelineHandler::generateConfiguration(Camera *camera, Span<const StreamRo /* Create the formats map. */ std::map<PixelFormat, std::vector<SizeRange>> formats; - for (const SimpleCameraData::Configuration &cfg : data->configs_) { - for (PixelFormat format : cfg.outputFormats) - formats[format].push_back(cfg.outputSizes); - } + for (const SimpleCameraData::Configuration &cfg : data->configs_) + if (static_cast<bool>(data->swIsp_) == cfg.swisp) + for (PixelFormat format : cfg.outputFormats) + formats[format].push_back(cfg.outputSizes); /* Sort the sizes and merge any consecutive overlapping ranges. */ for (auto &[format, sizes] : formats) {
When software ISP is active, only software ISP adjusted output configurations are considered. We want to support raw streams together with software ISP and for that purpose, we need to track also unmodified configurations. A flag is added to SimpleCameraData::Configuration indicating whether it's for software ISP. Configurations for unmodified output sizes and formats are added now whether software ISP is active or not. We later filter formats and output sizes for particular stream configuration according to the new configuration flag. This is just preparation and raw output is still not supported; for this reason, we just look whether software ISP is active; this will be changed in followup patches. Signed-off-by: Milan Zamazal <mzamazal@redhat.com> --- src/libcamera/pipeline/simple/simple.cpp | 30 ++++++++++++++---------- 1 file changed, 18 insertions(+), 12 deletions(-)