| Message ID | 20241206101344.767170-18-stefan.klug@ideasonboard.com |
|---|---|
| State | Superseded |
| Delegated to: | Paul Elder |
| Headers | show |
| Series |
|
| Related | show |
Hi Stefan On Fri, Dec 06, 2024 at 11:13:39AM +0100, Stefan Klug wrote: > In RkISPPath::validate() the sensor sizes are correctly filtered to the > ones supported by the ISP. But later in RkISPPipeline::configure() the > configured stream size is passed to sensor->getFormat() and the > CameraSensor class chooses the best sensor format for the requested > stream size. This can result in a sensor format that is too big for the > ISP. Fix that by supplying the maximum resolution supported by the ISP > to setFormat(). > > Fixes: 761545407c76 ("pipeline: rkisp1: Filter out sensor sizes not supported by the pipeline") Not a big deal as we don't do backports yet, but this requires "libcamera: camera_sensor: Add parameter to limit returned sensor size" I admit I'm not sure how we should be handling patch dependencies when we'll do backporting of fixes. Anyway, not an issue for now Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> > --- > Changes in v3: > - Added this patch > --- > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 3 ++- > src/libcamera/pipeline/rkisp1/rkisp1_path.h | 1 + > 2 files changed, 3 insertions(+), 1 deletion(-) > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > index 2baff6da7cb7..4dcc5a4fa6a1 100644 > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > @@ -677,7 +677,8 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() > [](const auto &value) { return value.second; }); > } > > - sensorFormat_ = sensor->getFormat(mbusCodes, maxSize); > + sensorFormat_ = sensor->getFormat(mbusCodes, maxSize, > + mainPath->maxResolution()); > > if (sensorFormat_.size.isNull()) > sensorFormat_.size = sensor->resolution(); > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.h b/src/libcamera/pipeline/rkisp1/rkisp1_path.h > index 45be8476616c..2a1ef0abe6d1 100644 > --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.h > +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.h > @@ -62,6 +62,7 @@ public: > > int queueBuffer(FrameBuffer *buffer) { return video_->queueBuffer(buffer); } > Signal<FrameBuffer *> &bufferReady() { return video_->bufferReady; } > + const Size &maxResolution() const { return maxResolution_; } > > private: > void populateFormats(); > -- > 2.43.0 >
On Fri, Dec 06, 2024 at 11:13:39AM +0100, Stefan Klug wrote: > In RkISPPath::validate() the sensor sizes are correctly filtered to the > ones supported by the ISP. But later in RkISPPipeline::configure() the > configured stream size is passed to sensor->getFormat() and the > CameraSensor class chooses the best sensor format for the requested > stream size. This can result in a sensor format that is too big for the > ISP. Fix that by supplying the maximum resolution supported by the ISP > to setFormat(). > > Fixes: 761545407c76 ("pipeline: rkisp1: Filter out sensor sizes not supported by the pipeline") > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> > --- > Changes in v3: > - Added this patch > --- > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 3 ++- > src/libcamera/pipeline/rkisp1/rkisp1_path.h | 1 + > 2 files changed, 3 insertions(+), 1 deletion(-) > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > index 2baff6da7cb7..4dcc5a4fa6a1 100644 > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > @@ -677,7 +677,8 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() > [](const auto &value) { return value.second; }); > } > > - sensorFormat_ = sensor->getFormat(mbusCodes, maxSize); > + sensorFormat_ = sensor->getFormat(mbusCodes, maxSize, > + mainPath->maxResolution()); > > if (sensorFormat_.size.isNull()) > sensorFormat_.size = sensor->resolution(); > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.h b/src/libcamera/pipeline/rkisp1/rkisp1_path.h > index 45be8476616c..2a1ef0abe6d1 100644 > --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.h > +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.h > @@ -62,6 +62,7 @@ public: > > int queueBuffer(FrameBuffer *buffer) { return video_->queueBuffer(buffer); } > Signal<FrameBuffer *> &bufferReady() { return video_->bufferReady; } > + const Size &maxResolution() const { return maxResolution_; } > > private: > void populateFormats(); > -- > 2.43.0 >
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 2baff6da7cb7..4dcc5a4fa6a1 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -677,7 +677,8 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() [](const auto &value) { return value.second; }); } - sensorFormat_ = sensor->getFormat(mbusCodes, maxSize); + sensorFormat_ = sensor->getFormat(mbusCodes, maxSize, + mainPath->maxResolution()); if (sensorFormat_.size.isNull()) sensorFormat_.size = sensor->resolution(); diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.h b/src/libcamera/pipeline/rkisp1/rkisp1_path.h index 45be8476616c..2a1ef0abe6d1 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.h +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.h @@ -62,6 +62,7 @@ public: int queueBuffer(FrameBuffer *buffer) { return video_->queueBuffer(buffer); } Signal<FrameBuffer *> &bufferReady() { return video_->bufferReady; } + const Size &maxResolution() const { return maxResolution_; } private: void populateFormats();
In RkISPPath::validate() the sensor sizes are correctly filtered to the ones supported by the ISP. But later in RkISPPipeline::configure() the configured stream size is passed to sensor->getFormat() and the CameraSensor class chooses the best sensor format for the requested stream size. This can result in a sensor format that is too big for the ISP. Fix that by supplying the maximum resolution supported by the ISP to setFormat(). Fixes: 761545407c76 ("pipeline: rkisp1: Filter out sensor sizes not supported by the pipeline") Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> --- Changes in v3: - Added this patch --- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 3 ++- src/libcamera/pipeline/rkisp1/rkisp1_path.h | 1 + 2 files changed, 3 insertions(+), 1 deletion(-)