| Message ID | 20241008101918.10414-2-umang.jain@ideasonboard.com |
|---|---|
| State | Accepted |
| Commit | 047d647452c44b610c58dc4425b95d7933cabdc8 |
| Headers | show |
| Series |
|
| Related | show |
Hi Umang On Tue, Oct 08, 2024 at 03:49:18PM GMT, Umang Jain wrote: > Integrate the RkISP1 pipeline handler to support sensor configuration > provided by applications through CameraConfiguration::sensorConfig. > > The SensorConfiguration must be validated on both RkISP1Path (mainPath > and selfPath), so the parameters of RkISP1Path::validate() have been > updated to include sensorConfig. > > The camera configuration will be marked as invalid when the sensor > configuration is supplied, if: > - Invalid sensor configuration (SensorConfiguration::isValid()) > - Bit depth not supported by RkISP1 pipeline > - RAW stream configuration size doesn't matches sensor configuration > output size This is not the case anymore :) > - Sensor configuration output size is larger than maximum supported > sensor configuration on RkISP1 pipeline s/sensor configuration/sensor input/ > - No matching sensor configuration output size supplied by the sensor > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> > --- > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 42 +++++++++++++--- > src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 48 +++++++++++++++++-- > src/libcamera/pipeline/rkisp1/rkisp1_path.h | 2 + > 3 files changed, 80 insertions(+), 12 deletions(-) > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > index c02c7cf3..42961c12 100644 > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > @@ -447,11 +447,12 @@ bool RkISP1CameraConfiguration::fitsAllPaths(const StreamConfiguration &cfg) > StreamConfiguration config; > > config = cfg; > - if (data_->mainPath_->validate(sensor, &config) != Valid) > + if (data_->mainPath_->validate(sensor, sensorConfig, &config) != Valid) > return false; > > config = cfg; > - if (data_->selfPath_ && data_->selfPath_->validate(sensor, &config) != Valid) > + if (data_->selfPath_ && > + data_->selfPath_->validate(sensor, sensorConfig, &config) != Valid) > return false; > > return true; > @@ -468,6 +469,27 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() > > status = validateColorSpaces(ColorSpaceFlag::StreamsShareColorSpace); > > + /* > + * Make sure that if a sensor configuration has been requested it > + * is valid. > + */ > + if (sensorConfig) { > + if (!sensorConfig->isValid()) { > + LOG(RkISP1, Error) > + << "Invalid sensor configuration request"; > + > + return Invalid; > + } > + Should we add a comment to say RkISP1 only support 8, 10 and 12 bit depths ? The rest now looks good and the above comments can be applied when merging the patch Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> Thanks j > + unsigned int bitDepth = sensorConfig->bitDepth; > + if (bitDepth != 8 && bitDepth != 10 && bitDepth != 12) { > + LOG(RkISP1, Error) > + << "Invalid sensor configuration bit depth"; > + > + return Invalid; > + } > + } > + > /* Cap the number of entries to the available streams. */ > if (config_.size() > pathCount) { > config_.resize(pathCount); > @@ -514,7 +536,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() > /* Try to match stream without adjusting configuration. */ > if (mainPathAvailable) { > StreamConfiguration tryCfg = cfg; > - if (data_->mainPath_->validate(sensor, &tryCfg) == Valid) { > + if (data_->mainPath_->validate(sensor, sensorConfig, &tryCfg) == Valid) { > mainPathAvailable = false; > cfg = tryCfg; > cfg.setStream(const_cast<Stream *>(&data_->mainPathStream_)); > @@ -524,7 +546,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() > > if (selfPathAvailable) { > StreamConfiguration tryCfg = cfg; > - if (data_->selfPath_->validate(sensor, &tryCfg) == Valid) { > + if (data_->selfPath_->validate(sensor, sensorConfig, &tryCfg) == Valid) { > selfPathAvailable = false; > cfg = tryCfg; > cfg.setStream(const_cast<Stream *>(&data_->selfPathStream_)); > @@ -535,7 +557,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() > /* Try to match stream allowing adjusting configuration. */ > if (mainPathAvailable) { > StreamConfiguration tryCfg = cfg; > - if (data_->mainPath_->validate(sensor, &tryCfg) == Adjusted) { > + if (data_->mainPath_->validate(sensor, sensorConfig, &tryCfg) == Adjusted) { > mainPathAvailable = false; > cfg = tryCfg; > cfg.setStream(const_cast<Stream *>(&data_->mainPathStream_)); > @@ -546,7 +568,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() > > if (selfPathAvailable) { > StreamConfiguration tryCfg = cfg; > - if (data_->selfPath_->validate(sensor, &tryCfg) == Adjusted) { > + if (data_->selfPath_->validate(sensor, sensorConfig, &tryCfg) == Adjusted) { > selfPathAvailable = false; > cfg = tryCfg; > cfg.setStream(const_cast<Stream *>(&data_->selfPathStream_)); > @@ -723,7 +745,13 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c) > V4L2SubdeviceFormat format = config->sensorFormat(); > LOG(RkISP1, Debug) << "Configuring sensor with " << format; > > - ret = sensor->setFormat(&format, config->combinedTransform()); > + if (config->sensorConfig) > + ret = sensor->applyConfiguration(*config->sensorConfig, > + config->combinedTransform(), > + &format); > + else > + ret = sensor->setFormat(&format, config->combinedTransform()); > + > if (ret < 0) > return ret; > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp > index da8d25c3..3b5bea96 100644 > --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp > +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp > @@ -251,8 +251,10 @@ RkISP1Path::generateConfiguration(const CameraSensor *sensor, const Size &size, > return cfg; > } > > -CameraConfiguration::Status RkISP1Path::validate(const CameraSensor *sensor, > - StreamConfiguration *cfg) > +CameraConfiguration::Status > +RkISP1Path::validate(const CameraSensor *sensor, > + std::optional<SensorConfiguration> &sensorConfig, > + StreamConfiguration *cfg) > { > const std::vector<unsigned int> &mbusCodes = sensor->mbusCodes(); > Size resolution = filterSensorResolution(sensor); > @@ -282,9 +284,14 @@ CameraConfiguration::Status RkISP1Path::validate(const CameraSensor *sensor, > continue; > > /* > - * Store the raw format with the highest bits per pixel > - * for later usage. > + * If the bits per pixel is supplied from the sensor > + * configuration, choose a raw format that complies with > + * it. Otherwise, store the raw format with the highest > + * bits per pixel for later usage. > */ > + if (sensorConfig && info.bitsPerPixel != sensorConfig->bitDepth) > + continue; > + > if (info.bitsPerPixel > rawBitsPerPixel) { > rawBitsPerPixel = info.bitsPerPixel; > rawFormat = format; > @@ -297,6 +304,9 @@ CameraConfiguration::Status RkISP1Path::validate(const CameraSensor *sensor, > } > } > > + if (sensorConfig && !found) > + return CameraConfiguration::Invalid; > + > bool isRaw = PixelFormatInfo::info(cfg->pixelFormat).colourEncoding == > PixelFormatInfo::ColourEncodingRAW; > > @@ -319,11 +329,39 @@ CameraConfiguration::Status RkISP1Path::validate(const CameraSensor *sensor, > * size. > */ > uint32_t mbusCode = formatToMediaBus.at(cfg->pixelFormat); > + Size rawSize = sensorConfig ? sensorConfig->outputSize > + : cfg->size; > + > V4L2SubdeviceFormat sensorFormat = > - sensor->getFormat({ mbusCode }, cfg->size); > + sensor->getFormat({ mbusCode }, rawSize); > + > + if (sensorConfig && > + sensorConfig->outputSize != sensorFormat.size) > + return CameraConfiguration::Invalid; > > minResolution = sensorFormat.size; > maxResolution = sensorFormat.size; > + } else if (sensorConfig) { > + /* > + * We have already ensured 'rawFormat' has the matching bit > + * depth with sensorConfig.bitDepth hence, only validate the > + * sensorConfig's output size here. > + */ > + Size sensorSize = sensorConfig->outputSize; > + > + if (sensorSize > resolution) > + return CameraConfiguration::Invalid; > + > + uint32_t mbusCode = formatToMediaBus.at(rawFormat); > + V4L2SubdeviceFormat sensorFormat = > + sensor->getFormat({ mbusCode }, sensorSize); > + > + if (sensorFormat.size != sensorSize) > + return CameraConfiguration::Invalid; > + > + minResolution = minResolution_.expandedToAspectRatio(sensorSize); > + maxResolution = maxResolution_.boundedTo(sensorSize) > + .boundedToAspectRatio(sensorSize); > } else { > /* > * Adjust the size based on the sensor resolution and absolute > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.h b/src/libcamera/pipeline/rkisp1/rkisp1_path.h > index 777fcae7..ce9a5666 100644 > --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.h > +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.h > @@ -27,6 +27,7 @@ namespace libcamera { > class CameraSensor; > class MediaDevice; > class V4L2Subdevice; > +class SensorConfiguration; > struct StreamConfiguration; > struct V4L2SubdeviceFormat; > > @@ -44,6 +45,7 @@ public: > const Size &resolution, > StreamRole role); > CameraConfiguration::Status validate(const CameraSensor *sensor, > + std::optional<SensorConfiguration> &sensorConfig, > StreamConfiguration *cfg); > > int configure(const StreamConfiguration &config, > -- > 2.45.0 >
Hi Jacopo On 08/10/24 4:28 pm, Jacopo Mondi wrote: > Hi Umang > > On Tue, Oct 08, 2024 at 03:49:18PM GMT, Umang Jain wrote: >> Integrate the RkISP1 pipeline handler to support sensor configuration >> provided by applications through CameraConfiguration::sensorConfig. >> >> The SensorConfiguration must be validated on both RkISP1Path (mainPath >> and selfPath), so the parameters of RkISP1Path::validate() have been >> updated to include sensorConfig. >> >> The camera configuration will be marked as invalid when the sensor >> configuration is supplied, if: >> - Invalid sensor configuration (SensorConfiguration::isValid()) >> - Bit depth not supported by RkISP1 pipeline >> - RAW stream configuration size doesn't matches sensor configuration >> output size > This is not the case anymore :) Ah yes, indeed > >> - Sensor configuration output size is larger than maximum supported >> sensor configuration on RkISP1 pipeline > s/sensor configuration/sensor input/ > >> - No matching sensor configuration output size supplied by the sensor >> >> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> >> --- >> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 42 +++++++++++++--- >> src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 48 +++++++++++++++++-- >> src/libcamera/pipeline/rkisp1/rkisp1_path.h | 2 + >> 3 files changed, 80 insertions(+), 12 deletions(-) >> >> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp >> index c02c7cf3..42961c12 100644 >> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp >> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp >> @@ -447,11 +447,12 @@ bool RkISP1CameraConfiguration::fitsAllPaths(const StreamConfiguration &cfg) >> StreamConfiguration config; >> >> config = cfg; >> - if (data_->mainPath_->validate(sensor, &config) != Valid) >> + if (data_->mainPath_->validate(sensor, sensorConfig, &config) != Valid) >> return false; >> >> config = cfg; >> - if (data_->selfPath_ && data_->selfPath_->validate(sensor, &config) != Valid) >> + if (data_->selfPath_ && >> + data_->selfPath_->validate(sensor, sensorConfig, &config) != Valid) >> return false; >> >> return true; >> @@ -468,6 +469,27 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() >> >> status = validateColorSpaces(ColorSpaceFlag::StreamsShareColorSpace); >> >> + /* >> + * Make sure that if a sensor configuration has been requested it >> + * is valid. >> + */ >> + if (sensorConfig) { >> + if (!sensorConfig->isValid()) { >> + LOG(RkISP1, Error) >> + << "Invalid sensor configuration request"; >> + >> + return Invalid; >> + } >> + > Should we add a comment to say RkISP1 only support 8, 10 and 12 bit > depths ? We can, but I think it is clear from the code itself ? :D > > The rest now looks good and the above comments can be applied when > merging the patch > > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> Thanks for the reviews! > > Thanks > j > >> + unsigned int bitDepth = sensorConfig->bitDepth; >> + if (bitDepth != 8 && bitDepth != 10 && bitDepth != 12) { >> + LOG(RkISP1, Error) >> + << "Invalid sensor configuration bit depth"; >> + >> + return Invalid; >> + } >> + } >> + >> /* Cap the number of entries to the available streams. */ >> if (config_.size() > pathCount) { >> config_.resize(pathCount); >> @@ -514,7 +536,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() >> /* Try to match stream without adjusting configuration. */ >> if (mainPathAvailable) { >> StreamConfiguration tryCfg = cfg; >> - if (data_->mainPath_->validate(sensor, &tryCfg) == Valid) { >> + if (data_->mainPath_->validate(sensor, sensorConfig, &tryCfg) == Valid) { >> mainPathAvailable = false; >> cfg = tryCfg; >> cfg.setStream(const_cast<Stream *>(&data_->mainPathStream_)); >> @@ -524,7 +546,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() >> >> if (selfPathAvailable) { >> StreamConfiguration tryCfg = cfg; >> - if (data_->selfPath_->validate(sensor, &tryCfg) == Valid) { >> + if (data_->selfPath_->validate(sensor, sensorConfig, &tryCfg) == Valid) { >> selfPathAvailable = false; >> cfg = tryCfg; >> cfg.setStream(const_cast<Stream *>(&data_->selfPathStream_)); >> @@ -535,7 +557,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() >> /* Try to match stream allowing adjusting configuration. */ >> if (mainPathAvailable) { >> StreamConfiguration tryCfg = cfg; >> - if (data_->mainPath_->validate(sensor, &tryCfg) == Adjusted) { >> + if (data_->mainPath_->validate(sensor, sensorConfig, &tryCfg) == Adjusted) { >> mainPathAvailable = false; >> cfg = tryCfg; >> cfg.setStream(const_cast<Stream *>(&data_->mainPathStream_)); >> @@ -546,7 +568,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() >> >> if (selfPathAvailable) { >> StreamConfiguration tryCfg = cfg; >> - if (data_->selfPath_->validate(sensor, &tryCfg) == Adjusted) { >> + if (data_->selfPath_->validate(sensor, sensorConfig, &tryCfg) == Adjusted) { >> selfPathAvailable = false; >> cfg = tryCfg; >> cfg.setStream(const_cast<Stream *>(&data_->selfPathStream_)); >> @@ -723,7 +745,13 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c) >> V4L2SubdeviceFormat format = config->sensorFormat(); >> LOG(RkISP1, Debug) << "Configuring sensor with " << format; >> >> - ret = sensor->setFormat(&format, config->combinedTransform()); >> + if (config->sensorConfig) >> + ret = sensor->applyConfiguration(*config->sensorConfig, >> + config->combinedTransform(), >> + &format); >> + else >> + ret = sensor->setFormat(&format, config->combinedTransform()); >> + >> if (ret < 0) >> return ret; >> >> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp >> index da8d25c3..3b5bea96 100644 >> --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp >> +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp >> @@ -251,8 +251,10 @@ RkISP1Path::generateConfiguration(const CameraSensor *sensor, const Size &size, >> return cfg; >> } >> >> -CameraConfiguration::Status RkISP1Path::validate(const CameraSensor *sensor, >> - StreamConfiguration *cfg) >> +CameraConfiguration::Status >> +RkISP1Path::validate(const CameraSensor *sensor, >> + std::optional<SensorConfiguration> &sensorConfig, >> + StreamConfiguration *cfg) >> { >> const std::vector<unsigned int> &mbusCodes = sensor->mbusCodes(); >> Size resolution = filterSensorResolution(sensor); >> @@ -282,9 +284,14 @@ CameraConfiguration::Status RkISP1Path::validate(const CameraSensor *sensor, >> continue; >> >> /* >> - * Store the raw format with the highest bits per pixel >> - * for later usage. >> + * If the bits per pixel is supplied from the sensor >> + * configuration, choose a raw format that complies with >> + * it. Otherwise, store the raw format with the highest >> + * bits per pixel for later usage. >> */ >> + if (sensorConfig && info.bitsPerPixel != sensorConfig->bitDepth) >> + continue; >> + >> if (info.bitsPerPixel > rawBitsPerPixel) { >> rawBitsPerPixel = info.bitsPerPixel; >> rawFormat = format; >> @@ -297,6 +304,9 @@ CameraConfiguration::Status RkISP1Path::validate(const CameraSensor *sensor, >> } >> } >> >> + if (sensorConfig && !found) >> + return CameraConfiguration::Invalid; >> + >> bool isRaw = PixelFormatInfo::info(cfg->pixelFormat).colourEncoding == >> PixelFormatInfo::ColourEncodingRAW; >> >> @@ -319,11 +329,39 @@ CameraConfiguration::Status RkISP1Path::validate(const CameraSensor *sensor, >> * size. >> */ >> uint32_t mbusCode = formatToMediaBus.at(cfg->pixelFormat); >> + Size rawSize = sensorConfig ? sensorConfig->outputSize >> + : cfg->size; >> + >> V4L2SubdeviceFormat sensorFormat = >> - sensor->getFormat({ mbusCode }, cfg->size); >> + sensor->getFormat({ mbusCode }, rawSize); >> + >> + if (sensorConfig && >> + sensorConfig->outputSize != sensorFormat.size) >> + return CameraConfiguration::Invalid; >> >> minResolution = sensorFormat.size; >> maxResolution = sensorFormat.size; >> + } else if (sensorConfig) { >> + /* >> + * We have already ensured 'rawFormat' has the matching bit >> + * depth with sensorConfig.bitDepth hence, only validate the >> + * sensorConfig's output size here. >> + */ >> + Size sensorSize = sensorConfig->outputSize; >> + >> + if (sensorSize > resolution) >> + return CameraConfiguration::Invalid; >> + >> + uint32_t mbusCode = formatToMediaBus.at(rawFormat); >> + V4L2SubdeviceFormat sensorFormat = >> + sensor->getFormat({ mbusCode }, sensorSize); >> + >> + if (sensorFormat.size != sensorSize) >> + return CameraConfiguration::Invalid; >> + >> + minResolution = minResolution_.expandedToAspectRatio(sensorSize); >> + maxResolution = maxResolution_.boundedTo(sensorSize) >> + .boundedToAspectRatio(sensorSize); >> } else { >> /* >> * Adjust the size based on the sensor resolution and absolute >> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.h b/src/libcamera/pipeline/rkisp1/rkisp1_path.h >> index 777fcae7..ce9a5666 100644 >> --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.h >> +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.h >> @@ -27,6 +27,7 @@ namespace libcamera { >> class CameraSensor; >> class MediaDevice; >> class V4L2Subdevice; >> +class SensorConfiguration; >> struct StreamConfiguration; >> struct V4L2SubdeviceFormat; >> >> @@ -44,6 +45,7 @@ public: >> const Size &resolution, >> StreamRole role); >> CameraConfiguration::Status validate(const CameraSensor *sensor, >> + std::optional<SensorConfiguration> &sensorConfig, >> StreamConfiguration *cfg); >> >> int configure(const StreamConfiguration &config, >> -- >> 2.45.0 >>
Hi Umang, Thank you for the patch. On Tue, Oct 08, 2024 at 03:49:18PM +0530, Umang Jain wrote: > Integrate the RkISP1 pipeline handler to support sensor configuration > provided by applications through CameraConfiguration::sensorConfig. > > The SensorConfiguration must be validated on both RkISP1Path (mainPath > and selfPath), so the parameters of RkISP1Path::validate() have been > updated to include sensorConfig. > > The camera configuration will be marked as invalid when the sensor > configuration is supplied, if: > - Invalid sensor configuration (SensorConfiguration::isValid()) > - Bit depth not supported by RkISP1 pipeline > - RAW stream configuration size doesn't matches sensor configuration > output size > - Sensor configuration output size is larger than maximum supported > sensor configuration on RkISP1 pipeline > - No matching sensor configuration output size supplied by the sensor > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> I tried it out and it works :-). My concern of the stream size beeing limited to the sensorConfig is actually a topic for the dewarper integration and (likely) doesn't apply here. So Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com> Tested-by: Stefan Klug <stefan.klug@ideasonboard.com> Cheers, Stefan > --- > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 42 +++++++++++++--- > src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 48 +++++++++++++++++-- > src/libcamera/pipeline/rkisp1/rkisp1_path.h | 2 + > 3 files changed, 80 insertions(+), 12 deletions(-) > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > index c02c7cf3..42961c12 100644 > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > @@ -447,11 +447,12 @@ bool RkISP1CameraConfiguration::fitsAllPaths(const StreamConfiguration &cfg) > StreamConfiguration config; > > config = cfg; > - if (data_->mainPath_->validate(sensor, &config) != Valid) > + if (data_->mainPath_->validate(sensor, sensorConfig, &config) != Valid) > return false; > > config = cfg; > - if (data_->selfPath_ && data_->selfPath_->validate(sensor, &config) != Valid) > + if (data_->selfPath_ && > + data_->selfPath_->validate(sensor, sensorConfig, &config) != Valid) > return false; > > return true; > @@ -468,6 +469,27 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() > > status = validateColorSpaces(ColorSpaceFlag::StreamsShareColorSpace); > > + /* > + * Make sure that if a sensor configuration has been requested it > + * is valid. > + */ > + if (sensorConfig) { > + if (!sensorConfig->isValid()) { > + LOG(RkISP1, Error) > + << "Invalid sensor configuration request"; > + > + return Invalid; > + } > + > + unsigned int bitDepth = sensorConfig->bitDepth; > + if (bitDepth != 8 && bitDepth != 10 && bitDepth != 12) { > + LOG(RkISP1, Error) > + << "Invalid sensor configuration bit depth"; > + > + return Invalid; > + } > + } > + > /* Cap the number of entries to the available streams. */ > if (config_.size() > pathCount) { > config_.resize(pathCount); > @@ -514,7 +536,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() > /* Try to match stream without adjusting configuration. */ > if (mainPathAvailable) { > StreamConfiguration tryCfg = cfg; > - if (data_->mainPath_->validate(sensor, &tryCfg) == Valid) { > + if (data_->mainPath_->validate(sensor, sensorConfig, &tryCfg) == Valid) { > mainPathAvailable = false; > cfg = tryCfg; > cfg.setStream(const_cast<Stream *>(&data_->mainPathStream_)); > @@ -524,7 +546,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() > > if (selfPathAvailable) { > StreamConfiguration tryCfg = cfg; > - if (data_->selfPath_->validate(sensor, &tryCfg) == Valid) { > + if (data_->selfPath_->validate(sensor, sensorConfig, &tryCfg) == Valid) { > selfPathAvailable = false; > cfg = tryCfg; > cfg.setStream(const_cast<Stream *>(&data_->selfPathStream_)); > @@ -535,7 +557,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() > /* Try to match stream allowing adjusting configuration. */ > if (mainPathAvailable) { > StreamConfiguration tryCfg = cfg; > - if (data_->mainPath_->validate(sensor, &tryCfg) == Adjusted) { > + if (data_->mainPath_->validate(sensor, sensorConfig, &tryCfg) == Adjusted) { > mainPathAvailable = false; > cfg = tryCfg; > cfg.setStream(const_cast<Stream *>(&data_->mainPathStream_)); > @@ -546,7 +568,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() > > if (selfPathAvailable) { > StreamConfiguration tryCfg = cfg; > - if (data_->selfPath_->validate(sensor, &tryCfg) == Adjusted) { > + if (data_->selfPath_->validate(sensor, sensorConfig, &tryCfg) == Adjusted) { > selfPathAvailable = false; > cfg = tryCfg; > cfg.setStream(const_cast<Stream *>(&data_->selfPathStream_)); > @@ -723,7 +745,13 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c) > V4L2SubdeviceFormat format = config->sensorFormat(); > LOG(RkISP1, Debug) << "Configuring sensor with " << format; > > - ret = sensor->setFormat(&format, config->combinedTransform()); > + if (config->sensorConfig) > + ret = sensor->applyConfiguration(*config->sensorConfig, > + config->combinedTransform(), > + &format); > + else > + ret = sensor->setFormat(&format, config->combinedTransform()); > + > if (ret < 0) > return ret; > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp > index da8d25c3..3b5bea96 100644 > --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp > +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp > @@ -251,8 +251,10 @@ RkISP1Path::generateConfiguration(const CameraSensor *sensor, const Size &size, > return cfg; > } > > -CameraConfiguration::Status RkISP1Path::validate(const CameraSensor *sensor, > - StreamConfiguration *cfg) > +CameraConfiguration::Status > +RkISP1Path::validate(const CameraSensor *sensor, > + std::optional<SensorConfiguration> &sensorConfig, > + StreamConfiguration *cfg) > { > const std::vector<unsigned int> &mbusCodes = sensor->mbusCodes(); > Size resolution = filterSensorResolution(sensor); > @@ -282,9 +284,14 @@ CameraConfiguration::Status RkISP1Path::validate(const CameraSensor *sensor, > continue; > > /* > - * Store the raw format with the highest bits per pixel > - * for later usage. > + * If the bits per pixel is supplied from the sensor > + * configuration, choose a raw format that complies with > + * it. Otherwise, store the raw format with the highest > + * bits per pixel for later usage. > */ > + if (sensorConfig && info.bitsPerPixel != sensorConfig->bitDepth) > + continue; > + > if (info.bitsPerPixel > rawBitsPerPixel) { > rawBitsPerPixel = info.bitsPerPixel; > rawFormat = format; > @@ -297,6 +304,9 @@ CameraConfiguration::Status RkISP1Path::validate(const CameraSensor *sensor, > } > } > > + if (sensorConfig && !found) > + return CameraConfiguration::Invalid; > + > bool isRaw = PixelFormatInfo::info(cfg->pixelFormat).colourEncoding == > PixelFormatInfo::ColourEncodingRAW; > > @@ -319,11 +329,39 @@ CameraConfiguration::Status RkISP1Path::validate(const CameraSensor *sensor, > * size. > */ > uint32_t mbusCode = formatToMediaBus.at(cfg->pixelFormat); > + Size rawSize = sensorConfig ? sensorConfig->outputSize > + : cfg->size; > + > V4L2SubdeviceFormat sensorFormat = > - sensor->getFormat({ mbusCode }, cfg->size); > + sensor->getFormat({ mbusCode }, rawSize); > + > + if (sensorConfig && > + sensorConfig->outputSize != sensorFormat.size) > + return CameraConfiguration::Invalid; > > minResolution = sensorFormat.size; > maxResolution = sensorFormat.size; > + } else if (sensorConfig) { > + /* > + * We have already ensured 'rawFormat' has the matching bit > + * depth with sensorConfig.bitDepth hence, only validate the > + * sensorConfig's output size here. > + */ > + Size sensorSize = sensorConfig->outputSize; > + > + if (sensorSize > resolution) > + return CameraConfiguration::Invalid; > + > + uint32_t mbusCode = formatToMediaBus.at(rawFormat); > + V4L2SubdeviceFormat sensorFormat = > + sensor->getFormat({ mbusCode }, sensorSize); > + > + if (sensorFormat.size != sensorSize) > + return CameraConfiguration::Invalid; > + > + minResolution = minResolution_.expandedToAspectRatio(sensorSize); > + maxResolution = maxResolution_.boundedTo(sensorSize) > + .boundedToAspectRatio(sensorSize); > } else { > /* > * Adjust the size based on the sensor resolution and absolute > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.h b/src/libcamera/pipeline/rkisp1/rkisp1_path.h > index 777fcae7..ce9a5666 100644 > --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.h > +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.h > @@ -27,6 +27,7 @@ namespace libcamera { > class CameraSensor; > class MediaDevice; > class V4L2Subdevice; > +class SensorConfiguration; > struct StreamConfiguration; > struct V4L2SubdeviceFormat; > > @@ -44,6 +45,7 @@ public: > const Size &resolution, > StreamRole role); > CameraConfiguration::Status validate(const CameraSensor *sensor, > + std::optional<SensorConfiguration> &sensorConfig, > StreamConfiguration *cfg); > > int configure(const StreamConfiguration &config, > -- > 2.45.0 >
On Tue, Oct 08, 2024 at 03:49:18PM +0530, Umang Jain wrote: > Integrate the RkISP1 pipeline handler to support sensor configuration > provided by applications through CameraConfiguration::sensorConfig. > > The SensorConfiguration must be validated on both RkISP1Path (mainPath > and selfPath), so the parameters of RkISP1Path::validate() have been > updated to include sensorConfig. > > The camera configuration will be marked as invalid when the sensor > configuration is supplied, if: > - Invalid sensor configuration (SensorConfiguration::isValid()) > - Bit depth not supported by RkISP1 pipeline > - RAW stream configuration size doesn't matches sensor configuration > output size > - Sensor configuration output size is larger than maximum supported > sensor configuration on RkISP1 pipeline > - No matching sensor configuration output size supplied by the sensor > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> > --- > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 42 +++++++++++++--- > src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 48 +++++++++++++++++-- > src/libcamera/pipeline/rkisp1/rkisp1_path.h | 2 + > 3 files changed, 80 insertions(+), 12 deletions(-) > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > index c02c7cf3..42961c12 100644 > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > @@ -447,11 +447,12 @@ bool RkISP1CameraConfiguration::fitsAllPaths(const StreamConfiguration &cfg) > StreamConfiguration config; > > config = cfg; > - if (data_->mainPath_->validate(sensor, &config) != Valid) > + if (data_->mainPath_->validate(sensor, sensorConfig, &config) != Valid) > return false; > > config = cfg; > - if (data_->selfPath_ && data_->selfPath_->validate(sensor, &config) != Valid) > + if (data_->selfPath_ && > + data_->selfPath_->validate(sensor, sensorConfig, &config) != Valid) > return false; > > return true; > @@ -468,6 +469,27 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() > > status = validateColorSpaces(ColorSpaceFlag::StreamsShareColorSpace); > > + /* > + * Make sure that if a sensor configuration has been requested it > + * is valid. > + */ > + if (sensorConfig) { > + if (!sensorConfig->isValid()) { > + LOG(RkISP1, Error) > + << "Invalid sensor configuration request"; > + > + return Invalid; > + } > + > + unsigned int bitDepth = sensorConfig->bitDepth; > + if (bitDepth != 8 && bitDepth != 10 && bitDepth != 12) { > + LOG(RkISP1, Error) > + << "Invalid sensor configuration bit depth"; > + > + return Invalid; > + } > + } > + > /* Cap the number of entries to the available streams. */ > if (config_.size() > pathCount) { > config_.resize(pathCount); > @@ -514,7 +536,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() > /* Try to match stream without adjusting configuration. */ > if (mainPathAvailable) { > StreamConfiguration tryCfg = cfg; > - if (data_->mainPath_->validate(sensor, &tryCfg) == Valid) { > + if (data_->mainPath_->validate(sensor, sensorConfig, &tryCfg) == Valid) { > mainPathAvailable = false; > cfg = tryCfg; > cfg.setStream(const_cast<Stream *>(&data_->mainPathStream_)); > @@ -524,7 +546,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() > > if (selfPathAvailable) { > StreamConfiguration tryCfg = cfg; > - if (data_->selfPath_->validate(sensor, &tryCfg) == Valid) { > + if (data_->selfPath_->validate(sensor, sensorConfig, &tryCfg) == Valid) { > selfPathAvailable = false; > cfg = tryCfg; > cfg.setStream(const_cast<Stream *>(&data_->selfPathStream_)); > @@ -535,7 +557,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() > /* Try to match stream allowing adjusting configuration. */ > if (mainPathAvailable) { > StreamConfiguration tryCfg = cfg; > - if (data_->mainPath_->validate(sensor, &tryCfg) == Adjusted) { > + if (data_->mainPath_->validate(sensor, sensorConfig, &tryCfg) == Adjusted) { > mainPathAvailable = false; > cfg = tryCfg; > cfg.setStream(const_cast<Stream *>(&data_->mainPathStream_)); > @@ -546,7 +568,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() > > if (selfPathAvailable) { > StreamConfiguration tryCfg = cfg; > - if (data_->selfPath_->validate(sensor, &tryCfg) == Adjusted) { > + if (data_->selfPath_->validate(sensor, sensorConfig, &tryCfg) == Adjusted) { > selfPathAvailable = false; > cfg = tryCfg; > cfg.setStream(const_cast<Stream *>(&data_->selfPathStream_)); > @@ -723,7 +745,13 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c) > V4L2SubdeviceFormat format = config->sensorFormat(); > LOG(RkISP1, Debug) << "Configuring sensor with " << format; > > - ret = sensor->setFormat(&format, config->combinedTransform()); > + if (config->sensorConfig) > + ret = sensor->applyConfiguration(*config->sensorConfig, > + config->combinedTransform(), > + &format); > + else > + ret = sensor->setFormat(&format, config->combinedTransform()); > + > if (ret < 0) > return ret; > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp > index da8d25c3..3b5bea96 100644 > --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp > +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp > @@ -251,8 +251,10 @@ RkISP1Path::generateConfiguration(const CameraSensor *sensor, const Size &size, > return cfg; > } > > -CameraConfiguration::Status RkISP1Path::validate(const CameraSensor *sensor, > - StreamConfiguration *cfg) > +CameraConfiguration::Status > +RkISP1Path::validate(const CameraSensor *sensor, > + std::optional<SensorConfiguration> &sensorConfig, Shouldn't this be const ? > + StreamConfiguration *cfg) > { > const std::vector<unsigned int> &mbusCodes = sensor->mbusCodes(); > Size resolution = filterSensorResolution(sensor); > @@ -282,9 +284,14 @@ CameraConfiguration::Status RkISP1Path::validate(const CameraSensor *sensor, > continue; > > /* > - * Store the raw format with the highest bits per pixel > - * for later usage. > + * If the bits per pixel is supplied from the sensor > + * configuration, choose a raw format that complies with > + * it. Otherwise, store the raw format with the highest > + * bits per pixel for later usage. > */ > + if (sensorConfig && info.bitsPerPixel != sensorConfig->bitDepth) > + continue; > + > if (info.bitsPerPixel > rawBitsPerPixel) { > rawBitsPerPixel = info.bitsPerPixel; > rawFormat = format; > @@ -297,6 +304,9 @@ CameraConfiguration::Status RkISP1Path::validate(const CameraSensor *sensor, > } > } > > + if (sensorConfig && !found) > + return CameraConfiguration::Invalid; Why is this ? For a non-raw stream, why would we reject an invalid format instead of adjusting it when there's a sensor configuration ? > + > bool isRaw = PixelFormatInfo::info(cfg->pixelFormat).colourEncoding == > PixelFormatInfo::ColourEncodingRAW; > > @@ -319,11 +329,39 @@ CameraConfiguration::Status RkISP1Path::validate(const CameraSensor *sensor, > * size. > */ > uint32_t mbusCode = formatToMediaBus.at(cfg->pixelFormat); > + Size rawSize = sensorConfig ? sensorConfig->outputSize > + : cfg->size; > + > V4L2SubdeviceFormat sensorFormat = > - sensor->getFormat({ mbusCode }, cfg->size); > + sensor->getFormat({ mbusCode }, rawSize); > + > + if (sensorConfig && > + sensorConfig->outputSize != sensorFormat.size) > + return CameraConfiguration::Invalid; > > minResolution = sensorFormat.size; > maxResolution = sensorFormat.size; > + } else if (sensorConfig) { > + /* > + * We have already ensured 'rawFormat' has the matching bit > + * depth with sensorConfig.bitDepth hence, only validate the > + * sensorConfig's output size here. > + */ > + Size sensorSize = sensorConfig->outputSize; > + > + if (sensorSize > resolution) > + return CameraConfiguration::Invalid; Is this check needed ? > + > + uint32_t mbusCode = formatToMediaBus.at(rawFormat); > + V4L2SubdeviceFormat sensorFormat = > + sensor->getFormat({ mbusCode }, sensorSize); > + > + if (sensorFormat.size != sensorSize) > + return CameraConfiguration::Invalid; > + > + minResolution = minResolution_.expandedToAspectRatio(sensorSize); > + maxResolution = maxResolution_.boundedTo(sensorSize) > + .boundedToAspectRatio(sensorSize); Below we have maxResolution = maxResolution_.boundedToAspectRatio(resolution) .boundedTo(resolution); Is there a reason why you swap the calls here ? > } else { > /* > * Adjust the size based on the sensor resolution and absolute > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.h b/src/libcamera/pipeline/rkisp1/rkisp1_path.h > index 777fcae7..ce9a5666 100644 > --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.h > +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.h > @@ -27,6 +27,7 @@ namespace libcamera { > class CameraSensor; > class MediaDevice; > class V4L2Subdevice; > +class SensorConfiguration; Alphabatical order. > struct StreamConfiguration; > struct V4L2SubdeviceFormat; > > @@ -44,6 +45,7 @@ public: > const Size &resolution, > StreamRole role); > CameraConfiguration::Status validate(const CameraSensor *sensor, > + std::optional<SensorConfiguration> &sensorConfig, > StreamConfiguration *cfg); > > int configure(const StreamConfiguration &config,
Hi Laurent On Wed, Oct 09, 2024 at 07:24:20PM GMT, Laurent Pinchart wrote: > On Tue, Oct 08, 2024 at 03:49:18PM +0530, Umang Jain wrote: > > Integrate the RkISP1 pipeline handler to support sensor configuration > > provided by applications through CameraConfiguration::sensorConfig. > > > > The SensorConfiguration must be validated on both RkISP1Path (mainPath > > and selfPath), so the parameters of RkISP1Path::validate() have been > > updated to include sensorConfig. > > > > The camera configuration will be marked as invalid when the sensor > > configuration is supplied, if: > > - Invalid sensor configuration (SensorConfiguration::isValid()) > > - Bit depth not supported by RkISP1 pipeline > > - RAW stream configuration size doesn't matches sensor configuration > > output size > > - Sensor configuration output size is larger than maximum supported > > sensor configuration on RkISP1 pipeline > > - No matching sensor configuration output size supplied by the sensor > > > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> > > --- > > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 42 +++++++++++++--- > > src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 48 +++++++++++++++++-- > > src/libcamera/pipeline/rkisp1/rkisp1_path.h | 2 + > > 3 files changed, 80 insertions(+), 12 deletions(-) > > > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > > index c02c7cf3..42961c12 100644 > > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > > @@ -447,11 +447,12 @@ bool RkISP1CameraConfiguration::fitsAllPaths(const StreamConfiguration &cfg) > > StreamConfiguration config; > > > > config = cfg; > > - if (data_->mainPath_->validate(sensor, &config) != Valid) > > + if (data_->mainPath_->validate(sensor, sensorConfig, &config) != Valid) > > return false; > > > > config = cfg; > > - if (data_->selfPath_ && data_->selfPath_->validate(sensor, &config) != Valid) > > + if (data_->selfPath_ && > > + data_->selfPath_->validate(sensor, sensorConfig, &config) != Valid) > > return false; > > > > return true; > > @@ -468,6 +469,27 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() > > > > status = validateColorSpaces(ColorSpaceFlag::StreamsShareColorSpace); > > > > + /* > > + * Make sure that if a sensor configuration has been requested it > > + * is valid. > > + */ > > + if (sensorConfig) { > > + if (!sensorConfig->isValid()) { > > + LOG(RkISP1, Error) > > + << "Invalid sensor configuration request"; > > + > > + return Invalid; > > + } > > + > > + unsigned int bitDepth = sensorConfig->bitDepth; > > + if (bitDepth != 8 && bitDepth != 10 && bitDepth != 12) { > > + LOG(RkISP1, Error) > > + << "Invalid sensor configuration bit depth"; > > + > > + return Invalid; > > + } > > + } > > + > > /* Cap the number of entries to the available streams. */ > > if (config_.size() > pathCount) { > > config_.resize(pathCount); > > @@ -514,7 +536,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() > > /* Try to match stream without adjusting configuration. */ > > if (mainPathAvailable) { > > StreamConfiguration tryCfg = cfg; > > - if (data_->mainPath_->validate(sensor, &tryCfg) == Valid) { > > + if (data_->mainPath_->validate(sensor, sensorConfig, &tryCfg) == Valid) { > > mainPathAvailable = false; > > cfg = tryCfg; > > cfg.setStream(const_cast<Stream *>(&data_->mainPathStream_)); > > @@ -524,7 +546,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() > > > > if (selfPathAvailable) { > > StreamConfiguration tryCfg = cfg; > > - if (data_->selfPath_->validate(sensor, &tryCfg) == Valid) { > > + if (data_->selfPath_->validate(sensor, sensorConfig, &tryCfg) == Valid) { > > selfPathAvailable = false; > > cfg = tryCfg; > > cfg.setStream(const_cast<Stream *>(&data_->selfPathStream_)); > > @@ -535,7 +557,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() > > /* Try to match stream allowing adjusting configuration. */ > > if (mainPathAvailable) { > > StreamConfiguration tryCfg = cfg; > > - if (data_->mainPath_->validate(sensor, &tryCfg) == Adjusted) { > > + if (data_->mainPath_->validate(sensor, sensorConfig, &tryCfg) == Adjusted) { > > mainPathAvailable = false; > > cfg = tryCfg; > > cfg.setStream(const_cast<Stream *>(&data_->mainPathStream_)); > > @@ -546,7 +568,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() > > > > if (selfPathAvailable) { > > StreamConfiguration tryCfg = cfg; > > - if (data_->selfPath_->validate(sensor, &tryCfg) == Adjusted) { > > + if (data_->selfPath_->validate(sensor, sensorConfig, &tryCfg) == Adjusted) { > > selfPathAvailable = false; > > cfg = tryCfg; > > cfg.setStream(const_cast<Stream *>(&data_->selfPathStream_)); > > @@ -723,7 +745,13 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c) > > V4L2SubdeviceFormat format = config->sensorFormat(); > > LOG(RkISP1, Debug) << "Configuring sensor with " << format; > > > > - ret = sensor->setFormat(&format, config->combinedTransform()); > > + if (config->sensorConfig) > > + ret = sensor->applyConfiguration(*config->sensorConfig, > > + config->combinedTransform(), > > + &format); > > + else > > + ret = sensor->setFormat(&format, config->combinedTransform()); > > + > > if (ret < 0) > > return ret; > > > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp > > index da8d25c3..3b5bea96 100644 > > --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp > > @@ -251,8 +251,10 @@ RkISP1Path::generateConfiguration(const CameraSensor *sensor, const Size &size, > > return cfg; > > } > > > > -CameraConfiguration::Status RkISP1Path::validate(const CameraSensor *sensor, > > - StreamConfiguration *cfg) > > +CameraConfiguration::Status > > +RkISP1Path::validate(const CameraSensor *sensor, > > + std::optional<SensorConfiguration> &sensorConfig, > > Shouldn't this be const ? > > > + StreamConfiguration *cfg) > > { > > const std::vector<unsigned int> &mbusCodes = sensor->mbusCodes(); > > Size resolution = filterSensorResolution(sensor); > > @@ -282,9 +284,14 @@ CameraConfiguration::Status RkISP1Path::validate(const CameraSensor *sensor, > > continue; > > > > /* > > - * Store the raw format with the highest bits per pixel > > - * for later usage. > > + * If the bits per pixel is supplied from the sensor > > + * configuration, choose a raw format that complies with > > + * it. Otherwise, store the raw format with the highest > > + * bits per pixel for later usage. > > */ > > + if (sensorConfig && info.bitsPerPixel != sensorConfig->bitDepth) > > + continue; > > + > > if (info.bitsPerPixel > rawBitsPerPixel) { > > rawBitsPerPixel = info.bitsPerPixel; > > rawFormat = format; > > @@ -297,6 +304,9 @@ CameraConfiguration::Status RkISP1Path::validate(const CameraSensor *sensor, > > } > > } > > > > + if (sensorConfig && !found) > > + return CameraConfiguration::Invalid; > > Why is this ? For a non-raw stream, why would we reject an invalid > format instead of adjusting it when there's a sensor configuration ? > Right, I suggested this but this should have been if (sensorConfig && !rawFormat) return CameraConfiguration::Invalid; instead > > + > > bool isRaw = PixelFormatInfo::info(cfg->pixelFormat).colourEncoding == > > PixelFormatInfo::ColourEncodingRAW; > > > > @@ -319,11 +329,39 @@ CameraConfiguration::Status RkISP1Path::validate(const CameraSensor *sensor, > > * size. > > */ > > uint32_t mbusCode = formatToMediaBus.at(cfg->pixelFormat); > > + Size rawSize = sensorConfig ? sensorConfig->outputSize > > + : cfg->size; > > + > > V4L2SubdeviceFormat sensorFormat = > > - sensor->getFormat({ mbusCode }, cfg->size); > > + sensor->getFormat({ mbusCode }, rawSize); > > + > > + if (sensorConfig && > > + sensorConfig->outputSize != sensorFormat.size) > > + return CameraConfiguration::Invalid; > > > > minResolution = sensorFormat.size; > > maxResolution = sensorFormat.size; > > + } else if (sensorConfig) { > > + /* > > + * We have already ensured 'rawFormat' has the matching bit > > + * depth with sensorConfig.bitDepth hence, only validate the > > + * sensorConfig's output size here. > > + */ > > + Size sensorSize = sensorConfig->outputSize; > > + > > + if (sensorSize > resolution) > > + return CameraConfiguration::Invalid; > > Is this check needed ? > If the sensorConfig requested output size cannot be processed by the ISP, I think we should fail yes > > + > > + uint32_t mbusCode = formatToMediaBus.at(rawFormat); > > + V4L2SubdeviceFormat sensorFormat = > > + sensor->getFormat({ mbusCode }, sensorSize); > > + > > + if (sensorFormat.size != sensorSize) > > + return CameraConfiguration::Invalid; > > + > > + minResolution = minResolution_.expandedToAspectRatio(sensorSize); > > + maxResolution = maxResolution_.boundedTo(sensorSize) > > + .boundedToAspectRatio(sensorSize); > > Below we have > > maxResolution = maxResolution_.boundedToAspectRatio(resolution) > .boundedTo(resolution); > > Is there a reason why you swap the calls here ? I suggested this as well, as my thinking was that it is correct to first bound down to a smaller res and then further reduce down to the desired aspect ratio. However, if we bound down to a smaller res, we already got the desired aspect ratio, so the two ops are interchangeable probably ? > > > } else { > > /* > > * Adjust the size based on the sensor resolution and absolute > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.h b/src/libcamera/pipeline/rkisp1/rkisp1_path.h > > index 777fcae7..ce9a5666 100644 > > --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.h > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.h > > @@ -27,6 +27,7 @@ namespace libcamera { > > class CameraSensor; > > class MediaDevice; > > class V4L2Subdevice; > > +class SensorConfiguration; > > Alphabatical order. > > > struct StreamConfiguration; > > struct V4L2SubdeviceFormat; > > > > @@ -44,6 +45,7 @@ public: > > const Size &resolution, > > StreamRole role); > > CameraConfiguration::Status validate(const CameraSensor *sensor, > > + std::optional<SensorConfiguration> &sensorConfig, > > StreamConfiguration *cfg); > > > > int configure(const StreamConfiguration &config, > > -- > Regards, > > Laurent Pinchart
Hi Laurent, On 09/10/24 9:54 pm, Laurent Pinchart wrote: > On Tue, Oct 08, 2024 at 03:49:18PM +0530, Umang Jain wrote: >> Integrate the RkISP1 pipeline handler to support sensor configuration >> provided by applications through CameraConfiguration::sensorConfig. >> >> The SensorConfiguration must be validated on both RkISP1Path (mainPath >> and selfPath), so the parameters of RkISP1Path::validate() have been >> updated to include sensorConfig. >> >> The camera configuration will be marked as invalid when the sensor >> configuration is supplied, if: >> - Invalid sensor configuration (SensorConfiguration::isValid()) >> - Bit depth not supported by RkISP1 pipeline >> - RAW stream configuration size doesn't matches sensor configuration >> output size >> - Sensor configuration output size is larger than maximum supported >> sensor configuration on RkISP1 pipeline >> - No matching sensor configuration output size supplied by the sensor >> >> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> >> --- >> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 42 +++++++++++++--- >> src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 48 +++++++++++++++++-- >> src/libcamera/pipeline/rkisp1/rkisp1_path.h | 2 + >> 3 files changed, 80 insertions(+), 12 deletions(-) >> >> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp >> index c02c7cf3..42961c12 100644 >> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp >> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp >> @@ -447,11 +447,12 @@ bool RkISP1CameraConfiguration::fitsAllPaths(const StreamConfiguration &cfg) >> StreamConfiguration config; >> >> config = cfg; >> - if (data_->mainPath_->validate(sensor, &config) != Valid) >> + if (data_->mainPath_->validate(sensor, sensorConfig, &config) != Valid) >> return false; >> >> config = cfg; >> - if (data_->selfPath_ && data_->selfPath_->validate(sensor, &config) != Valid) >> + if (data_->selfPath_ && >> + data_->selfPath_->validate(sensor, sensorConfig, &config) != Valid) >> return false; >> >> return true; >> @@ -468,6 +469,27 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() >> >> status = validateColorSpaces(ColorSpaceFlag::StreamsShareColorSpace); >> >> + /* >> + * Make sure that if a sensor configuration has been requested it >> + * is valid. >> + */ >> + if (sensorConfig) { >> + if (!sensorConfig->isValid()) { >> + LOG(RkISP1, Error) >> + << "Invalid sensor configuration request"; >> + >> + return Invalid; >> + } >> + >> + unsigned int bitDepth = sensorConfig->bitDepth; >> + if (bitDepth != 8 && bitDepth != 10 && bitDepth != 12) { >> + LOG(RkISP1, Error) >> + << "Invalid sensor configuration bit depth"; >> + >> + return Invalid; >> + } >> + } >> + >> /* Cap the number of entries to the available streams. */ >> if (config_.size() > pathCount) { >> config_.resize(pathCount); >> @@ -514,7 +536,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() >> /* Try to match stream without adjusting configuration. */ >> if (mainPathAvailable) { >> StreamConfiguration tryCfg = cfg; >> - if (data_->mainPath_->validate(sensor, &tryCfg) == Valid) { >> + if (data_->mainPath_->validate(sensor, sensorConfig, &tryCfg) == Valid) { >> mainPathAvailable = false; >> cfg = tryCfg; >> cfg.setStream(const_cast<Stream *>(&data_->mainPathStream_)); >> @@ -524,7 +546,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() >> >> if (selfPathAvailable) { >> StreamConfiguration tryCfg = cfg; >> - if (data_->selfPath_->validate(sensor, &tryCfg) == Valid) { >> + if (data_->selfPath_->validate(sensor, sensorConfig, &tryCfg) == Valid) { >> selfPathAvailable = false; >> cfg = tryCfg; >> cfg.setStream(const_cast<Stream *>(&data_->selfPathStream_)); >> @@ -535,7 +557,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() >> /* Try to match stream allowing adjusting configuration. */ >> if (mainPathAvailable) { >> StreamConfiguration tryCfg = cfg; >> - if (data_->mainPath_->validate(sensor, &tryCfg) == Adjusted) { >> + if (data_->mainPath_->validate(sensor, sensorConfig, &tryCfg) == Adjusted) { >> mainPathAvailable = false; >> cfg = tryCfg; >> cfg.setStream(const_cast<Stream *>(&data_->mainPathStream_)); >> @@ -546,7 +568,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() >> >> if (selfPathAvailable) { >> StreamConfiguration tryCfg = cfg; >> - if (data_->selfPath_->validate(sensor, &tryCfg) == Adjusted) { >> + if (data_->selfPath_->validate(sensor, sensorConfig, &tryCfg) == Adjusted) { >> selfPathAvailable = false; >> cfg = tryCfg; >> cfg.setStream(const_cast<Stream *>(&data_->selfPathStream_)); >> @@ -723,7 +745,13 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c) >> V4L2SubdeviceFormat format = config->sensorFormat(); >> LOG(RkISP1, Debug) << "Configuring sensor with " << format; >> >> - ret = sensor->setFormat(&format, config->combinedTransform()); >> + if (config->sensorConfig) >> + ret = sensor->applyConfiguration(*config->sensorConfig, >> + config->combinedTransform(), >> + &format); >> + else >> + ret = sensor->setFormat(&format, config->combinedTransform()); >> + >> if (ret < 0) >> return ret; >> >> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp >> index da8d25c3..3b5bea96 100644 >> --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp >> +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp >> @@ -251,8 +251,10 @@ RkISP1Path::generateConfiguration(const CameraSensor *sensor, const Size &size, >> return cfg; >> } >> >> -CameraConfiguration::Status RkISP1Path::validate(const CameraSensor *sensor, >> - StreamConfiguration *cfg) >> +CameraConfiguration::Status >> +RkISP1Path::validate(const CameraSensor *sensor, >> + std::optional<SensorConfiguration> &sensorConfig, > Shouldn't this be const ? sent a fix > >> + StreamConfiguration *cfg) >> { >> const std::vector<unsigned int> &mbusCodes = sensor->mbusCodes(); >> Size resolution = filterSensorResolution(sensor); >> @@ -282,9 +284,14 @@ CameraConfiguration::Status RkISP1Path::validate(const CameraSensor *sensor, >> continue; >> >> /* >> - * Store the raw format with the highest bits per pixel >> - * for later usage. >> + * If the bits per pixel is supplied from the sensor >> + * configuration, choose a raw format that complies with >> + * it. Otherwise, store the raw format with the highest >> + * bits per pixel for later usage. >> */ >> + if (sensorConfig && info.bitsPerPixel != sensorConfig->bitDepth) >> + continue; >> + >> if (info.bitsPerPixel > rawBitsPerPixel) { >> rawBitsPerPixel = info.bitsPerPixel; >> rawFormat = format; >> @@ -297,6 +304,9 @@ CameraConfiguration::Status RkISP1Path::validate(const CameraSensor *sensor, >> } >> } >> >> + if (sensorConfig && !found) >> + return CameraConfiguration::Invalid; > Why is this ? For a non-raw stream, why would we reject an invalid > format instead of adjusting it when there's a sensor configuration ? I misunderstood found, oops. Sent a fix to the list. > >> + >> bool isRaw = PixelFormatInfo::info(cfg->pixelFormat).colourEncoding == >> PixelFormatInfo::ColourEncodingRAW; >> >> @@ -319,11 +329,39 @@ CameraConfiguration::Status RkISP1Path::validate(const CameraSensor *sensor, >> * size. >> */ >> uint32_t mbusCode = formatToMediaBus.at(cfg->pixelFormat); >> + Size rawSize = sensorConfig ? sensorConfig->outputSize >> + : cfg->size; >> + >> V4L2SubdeviceFormat sensorFormat = >> - sensor->getFormat({ mbusCode }, cfg->size); >> + sensor->getFormat({ mbusCode }, rawSize); >> + >> + if (sensorConfig && >> + sensorConfig->outputSize != sensorFormat.size) >> + return CameraConfiguration::Invalid; >> >> minResolution = sensorFormat.size; >> maxResolution = sensorFormat.size; >> + } else if (sensorConfig) { >> + /* >> + * We have already ensured 'rawFormat' has the matching bit >> + * depth with sensorConfig.bitDepth hence, only validate the >> + * sensorConfig's output size here. >> + */ >> + Size sensorSize = sensorConfig->outputSize; >> + >> + if (sensorSize > resolution) >> + return CameraConfiguration::Invalid; > Is this check needed ? My idea is if sensor config size is passed such that it exceeds the limits of the pipeline, it should be invalidated. resolution here is the highest sensor resolution that's supported on the pipeline. >> + >> + uint32_t mbusCode = formatToMediaBus.at(rawFormat); >> + V4L2SubdeviceFormat sensorFormat = >> + sensor->getFormat({ mbusCode }, sensorSize); >> + >> + if (sensorFormat.size != sensorSize) >> + return CameraConfiguration::Invalid; >> + >> + minResolution = minResolution_.expandedToAspectRatio(sensorSize); >> + maxResolution = maxResolution_.boundedTo(sensorSize) >> + .boundedToAspectRatio(sensorSize); > Below we have > > maxResolution = maxResolution_.boundedToAspectRatio(resolution) > .boundedTo(resolution); > > Is there a reason why you swap the calls here ? I think it was Jacopo's review comment from v3: ``` Isn't it better to first "boundedTo" to make sure maxResolutions_ is smaller than sensorSize and then "boundedToAspectRatio" to further shrink it to the aspect ratio of the sensorSize ? ``` which made sense to me. Should we swap the below calls instead ? > >> } else { >> /* >> * Adjust the size based on the sensor resolution and absolute >> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.h b/src/libcamera/pipeline/rkisp1/rkisp1_path.h >> index 777fcae7..ce9a5666 100644 >> --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.h >> +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.h >> @@ -27,6 +27,7 @@ namespace libcamera { >> class CameraSensor; >> class MediaDevice; >> class V4L2Subdevice; >> +class SensorConfiguration; > Alphabatical order. > >> struct StreamConfiguration; >> struct V4L2SubdeviceFormat; >> >> @@ -44,6 +45,7 @@ public: >> const Size &resolution, >> StreamRole role); >> CameraConfiguration::Status validate(const CameraSensor *sensor, >> + std::optional<SensorConfiguration> &sensorConfig, >> StreamConfiguration *cfg); >> >> int configure(const StreamConfiguration &config,
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index c02c7cf3..42961c12 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -447,11 +447,12 @@ bool RkISP1CameraConfiguration::fitsAllPaths(const StreamConfiguration &cfg) StreamConfiguration config; config = cfg; - if (data_->mainPath_->validate(sensor, &config) != Valid) + if (data_->mainPath_->validate(sensor, sensorConfig, &config) != Valid) return false; config = cfg; - if (data_->selfPath_ && data_->selfPath_->validate(sensor, &config) != Valid) + if (data_->selfPath_ && + data_->selfPath_->validate(sensor, sensorConfig, &config) != Valid) return false; return true; @@ -468,6 +469,27 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() status = validateColorSpaces(ColorSpaceFlag::StreamsShareColorSpace); + /* + * Make sure that if a sensor configuration has been requested it + * is valid. + */ + if (sensorConfig) { + if (!sensorConfig->isValid()) { + LOG(RkISP1, Error) + << "Invalid sensor configuration request"; + + return Invalid; + } + + unsigned int bitDepth = sensorConfig->bitDepth; + if (bitDepth != 8 && bitDepth != 10 && bitDepth != 12) { + LOG(RkISP1, Error) + << "Invalid sensor configuration bit depth"; + + return Invalid; + } + } + /* Cap the number of entries to the available streams. */ if (config_.size() > pathCount) { config_.resize(pathCount); @@ -514,7 +536,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() /* Try to match stream without adjusting configuration. */ if (mainPathAvailable) { StreamConfiguration tryCfg = cfg; - if (data_->mainPath_->validate(sensor, &tryCfg) == Valid) { + if (data_->mainPath_->validate(sensor, sensorConfig, &tryCfg) == Valid) { mainPathAvailable = false; cfg = tryCfg; cfg.setStream(const_cast<Stream *>(&data_->mainPathStream_)); @@ -524,7 +546,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() if (selfPathAvailable) { StreamConfiguration tryCfg = cfg; - if (data_->selfPath_->validate(sensor, &tryCfg) == Valid) { + if (data_->selfPath_->validate(sensor, sensorConfig, &tryCfg) == Valid) { selfPathAvailable = false; cfg = tryCfg; cfg.setStream(const_cast<Stream *>(&data_->selfPathStream_)); @@ -535,7 +557,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() /* Try to match stream allowing adjusting configuration. */ if (mainPathAvailable) { StreamConfiguration tryCfg = cfg; - if (data_->mainPath_->validate(sensor, &tryCfg) == Adjusted) { + if (data_->mainPath_->validate(sensor, sensorConfig, &tryCfg) == Adjusted) { mainPathAvailable = false; cfg = tryCfg; cfg.setStream(const_cast<Stream *>(&data_->mainPathStream_)); @@ -546,7 +568,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() if (selfPathAvailable) { StreamConfiguration tryCfg = cfg; - if (data_->selfPath_->validate(sensor, &tryCfg) == Adjusted) { + if (data_->selfPath_->validate(sensor, sensorConfig, &tryCfg) == Adjusted) { selfPathAvailable = false; cfg = tryCfg; cfg.setStream(const_cast<Stream *>(&data_->selfPathStream_)); @@ -723,7 +745,13 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c) V4L2SubdeviceFormat format = config->sensorFormat(); LOG(RkISP1, Debug) << "Configuring sensor with " << format; - ret = sensor->setFormat(&format, config->combinedTransform()); + if (config->sensorConfig) + ret = sensor->applyConfiguration(*config->sensorConfig, + config->combinedTransform(), + &format); + else + ret = sensor->setFormat(&format, config->combinedTransform()); + if (ret < 0) return ret; diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp index da8d25c3..3b5bea96 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp @@ -251,8 +251,10 @@ RkISP1Path::generateConfiguration(const CameraSensor *sensor, const Size &size, return cfg; } -CameraConfiguration::Status RkISP1Path::validate(const CameraSensor *sensor, - StreamConfiguration *cfg) +CameraConfiguration::Status +RkISP1Path::validate(const CameraSensor *sensor, + std::optional<SensorConfiguration> &sensorConfig, + StreamConfiguration *cfg) { const std::vector<unsigned int> &mbusCodes = sensor->mbusCodes(); Size resolution = filterSensorResolution(sensor); @@ -282,9 +284,14 @@ CameraConfiguration::Status RkISP1Path::validate(const CameraSensor *sensor, continue; /* - * Store the raw format with the highest bits per pixel - * for later usage. + * If the bits per pixel is supplied from the sensor + * configuration, choose a raw format that complies with + * it. Otherwise, store the raw format with the highest + * bits per pixel for later usage. */ + if (sensorConfig && info.bitsPerPixel != sensorConfig->bitDepth) + continue; + if (info.bitsPerPixel > rawBitsPerPixel) { rawBitsPerPixel = info.bitsPerPixel; rawFormat = format; @@ -297,6 +304,9 @@ CameraConfiguration::Status RkISP1Path::validate(const CameraSensor *sensor, } } + if (sensorConfig && !found) + return CameraConfiguration::Invalid; + bool isRaw = PixelFormatInfo::info(cfg->pixelFormat).colourEncoding == PixelFormatInfo::ColourEncodingRAW; @@ -319,11 +329,39 @@ CameraConfiguration::Status RkISP1Path::validate(const CameraSensor *sensor, * size. */ uint32_t mbusCode = formatToMediaBus.at(cfg->pixelFormat); + Size rawSize = sensorConfig ? sensorConfig->outputSize + : cfg->size; + V4L2SubdeviceFormat sensorFormat = - sensor->getFormat({ mbusCode }, cfg->size); + sensor->getFormat({ mbusCode }, rawSize); + + if (sensorConfig && + sensorConfig->outputSize != sensorFormat.size) + return CameraConfiguration::Invalid; minResolution = sensorFormat.size; maxResolution = sensorFormat.size; + } else if (sensorConfig) { + /* + * We have already ensured 'rawFormat' has the matching bit + * depth with sensorConfig.bitDepth hence, only validate the + * sensorConfig's output size here. + */ + Size sensorSize = sensorConfig->outputSize; + + if (sensorSize > resolution) + return CameraConfiguration::Invalid; + + uint32_t mbusCode = formatToMediaBus.at(rawFormat); + V4L2SubdeviceFormat sensorFormat = + sensor->getFormat({ mbusCode }, sensorSize); + + if (sensorFormat.size != sensorSize) + return CameraConfiguration::Invalid; + + minResolution = minResolution_.expandedToAspectRatio(sensorSize); + maxResolution = maxResolution_.boundedTo(sensorSize) + .boundedToAspectRatio(sensorSize); } else { /* * Adjust the size based on the sensor resolution and absolute diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.h b/src/libcamera/pipeline/rkisp1/rkisp1_path.h index 777fcae7..ce9a5666 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.h +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.h @@ -27,6 +27,7 @@ namespace libcamera { class CameraSensor; class MediaDevice; class V4L2Subdevice; +class SensorConfiguration; struct StreamConfiguration; struct V4L2SubdeviceFormat; @@ -44,6 +45,7 @@ public: const Size &resolution, StreamRole role); CameraConfiguration::Status validate(const CameraSensor *sensor, + std::optional<SensorConfiguration> &sensorConfig, StreamConfiguration *cfg); int configure(const StreamConfiguration &config,
Integrate the RkISP1 pipeline handler to support sensor configuration provided by applications through CameraConfiguration::sensorConfig. The SensorConfiguration must be validated on both RkISP1Path (mainPath and selfPath), so the parameters of RkISP1Path::validate() have been updated to include sensorConfig. The camera configuration will be marked as invalid when the sensor configuration is supplied, if: - Invalid sensor configuration (SensorConfiguration::isValid()) - Bit depth not supported by RkISP1 pipeline - RAW stream configuration size doesn't matches sensor configuration output size - Sensor configuration output size is larger than maximum supported sensor configuration on RkISP1 pipeline - No matching sensor configuration output size supplied by the sensor Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> --- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 42 +++++++++++++--- src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 48 +++++++++++++++++-- src/libcamera/pipeline/rkisp1/rkisp1_path.h | 2 + 3 files changed, 80 insertions(+), 12 deletions(-)