| Message ID | 20241004122103.89807-2-umang.jain@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Umang On Fri, Oct 04, 2024 at 05:51:03PM 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 > - 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 | 47 +++++++++++++++++-- > src/libcamera/pipeline/rkisp1/rkisp1_path.h | 2 + > 3 files changed, 79 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 feb6d89f..c51554bc 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,13 +329,40 @@ CameraConfiguration::Status RkISP1Path::validate(const CameraSensor *sensor, > * size while ensuring the output size doesn't exceed ISP limits. > */ > uint32_t mbusCode = formatToMediaBus.at(cfg->pixelFormat); > + Size rawSize = sensorConfig ? sensorConfig->outputSize > + : cfg->size; > cfg->size.boundTo(resolution); > > V4L2SubdeviceFormat sensorFormat = > - sensor->getFormat({ mbusCode }, cfg->size); > + sensor->getFormat({ mbusCode }, rawSize); > + > + if (sensorConfig && > + reqCfg.size != sensorConfig->outputSize) Should this be reqCfg or sensorFormat ? If checking sensorConfig->outputSize agains "reqCfg" you're validating that the requested RAW stream configuration has the desired sensor output size. If checking sensorFormat->size instead you're validating that the requested sensorConfig->outputSize is supported by the sensor natively. I think this should be the latter, if reqCfg->size doesn't match the sensorConfig->outputSize it will later be adjusted to it, and I think that's fine (as long as we return Adjusted), but we should make sure sensorFormat->size is exactly what has been requested with sensorConfig ? > + 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; Ah ok, I was about to suggest to move this just after initializing 'resolution', but in case of isRaw we don't go through the ISP, so the ISP max input constraint doesn't apply > + > + uint32_t mbusCode = formatToMediaBus.at(rawFormat); > + V4L2SubdeviceFormat sensorFormat = > + sensor->getFormat({ mbusCode }, sensorSize); > + > + if (sensorFormat.size != sensorSize) > + return CameraConfiguration::Invalid; That's what I was suggesting to do in the raw case as well One small push and we should be there! Thanks j > + > + 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.2 >
Hi Jacopo On 08/10/24 2:24 pm, Jacopo Mondi wrote: > Hi Umang > > On Fri, Oct 04, 2024 at 05:51:03PM 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 >> - 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 | 47 +++++++++++++++++-- >> src/libcamera/pipeline/rkisp1/rkisp1_path.h | 2 + >> 3 files changed, 79 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 feb6d89f..c51554bc 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,13 +329,40 @@ CameraConfiguration::Status RkISP1Path::validate(const CameraSensor *sensor, >> * size while ensuring the output size doesn't exceed ISP limits. >> */ >> uint32_t mbusCode = formatToMediaBus.at(cfg->pixelFormat); >> + Size rawSize = sensorConfig ? sensorConfig->outputSize >> + : cfg->size; >> cfg->size.boundTo(resolution); >> >> V4L2SubdeviceFormat sensorFormat = >> - sensor->getFormat({ mbusCode }, cfg->size); >> + sensor->getFormat({ mbusCode }, rawSize); >> + >> + if (sensorConfig && >> + reqCfg.size != sensorConfig->outputSize) > Should this be reqCfg or sensorFormat ? > > If checking sensorConfig->outputSize agains "reqCfg" you're validating > that the requested RAW stream configuration has the desired sensor > output size. > > If checking sensorFormat->size instead you're validating that the > requested sensorConfig->outputSize is supported by the sensor > natively. > > I think this should be the latter, if reqCfg->size doesn't match the > sensorConfig->outputSize it will later be adjusted to it, and I think > that's fine (as long as we return Adjusted), but we should make sure > sensorFormat->size is exactly what has been requested with > sensorConfig ? you're absolutely correct here. It should be sensorFormat, not refCfg, as reqCfg can get adjusted. Will address in v5. > >> + 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; > > Ah ok, I was about to suggest to move this just after initializing > 'resolution', but in case of isRaw we don't go through the ISP, so the > ISP max input constraint doesn't apply > >> + >> + uint32_t mbusCode = formatToMediaBus.at(rawFormat); >> + V4L2SubdeviceFormat sensorFormat = >> + sensor->getFormat({ mbusCode }, sensorSize); >> + >> + if (sensorFormat.size != sensorSize) >> + return CameraConfiguration::Invalid; > That's what I was suggesting to do in the raw case as well > > One small push and we should be there! > Thanks > j > >> + >> + 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.2 >>
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 feb6d89f..c51554bc 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,13 +329,40 @@ CameraConfiguration::Status RkISP1Path::validate(const CameraSensor *sensor, * size while ensuring the output size doesn't exceed ISP limits. */ uint32_t mbusCode = formatToMediaBus.at(cfg->pixelFormat); + Size rawSize = sensorConfig ? sensorConfig->outputSize + : cfg->size; cfg->size.boundTo(resolution); V4L2SubdeviceFormat sensorFormat = - sensor->getFormat({ mbusCode }, cfg->size); + sensor->getFormat({ mbusCode }, rawSize); + + if (sensorConfig && + reqCfg.size != sensorConfig->outputSize) + 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 | 47 +++++++++++++++++-- src/libcamera/pipeline/rkisp1/rkisp1_path.h | 2 + 3 files changed, 79 insertions(+), 12 deletions(-)