| Message ID | 20241009093919.391662-1-umang.jain@ideasonboard.com |
|---|---|
| State | Accepted |
| Commit | 3d0ca251e119936f7c21fc654659ba90ffc73d33 |
| Headers | show |
| Series |
|
| Related | show |
Hi Umang, Thank you for the patch. On Wed, Oct 09, 2024 at 03:09:19PM +0530, Umang Jain wrote: > Commit 761545407c76 ("pipeline: rkisp1: Filter out sensor sizes not > supported by the pipeline") introduced a mechanism to determine maximum > supported sensor resolution and filter out resolutions that cannot be > supported by the ISP. > > However, it missed to update the raw stream configuration path, where > it should have clamped the raw stream configuration size to the maximum > sensor supported resolution. > > This patch fixes the above issue and can be confirmed with IMX283 > on i.MX8MP: > > From: > ($) cam -c1 -srole=raw,width=5472,height=3072 > INFO Camera camera.cpp:1197 configuring streams: (0) 5472x3648-SRGGB12 > ERROR RkISP1 rkisp1_path.cpp:425 Unable to configure capture in 5472x3648-SRGGB12 > Failed to configure camera > Failed to start camera session > > To: > ($) cam -c1 -srole=raw,width=5472,height=3072 > INFO Camera camera.cpp:1197 configuring streams: (0) 4096x3072-SRGGB12 > cam0: Capture until user interrupts by SIGINT > 536.082380 (0.00 fps) cam0-stream0 seq: 000000 bytesused: 25165824 > 536.182378 (10.00 fps) cam0-stream0 seq: 000001 bytesused: 25165824 > 536.282375 (10.00 fps) cam0-stream0 seq: 000002 bytesused: 25165824 > ... > > Fixes: 761545407c76 ("pipeline: rkisp1: Filter out sensor sizes not supported by the pipeline") > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> Looks good to me. Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com> > --- > Changes in v2: > - Extend the comment on what 'resolution' var is and denote that > sensor->getFormat() will never return something greter than > 'resolution' > --- > src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp > index 3b5bea96..1999094e 100644 > --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp > +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp > @@ -326,9 +326,15 @@ RkISP1Path::validate(const CameraSensor *sensor, > if (isRaw) { > /* > * Use the sensor output size closest to the requested stream > - * size. > + * size while ensuring the output size doesn't exceed ISP limits. > + * > + * As 'resolution' is the largest sensor resolution > + * supported by the ISP, CameraSensor::getFormat() will never > + * return a V4L2SubdeviceFormat with a larger size. > */ > uint32_t mbusCode = formatToMediaBus.at(cfg->pixelFormat); > + cfg->size.boundTo(resolution); > + > Size rawSize = sensorConfig ? sensorConfig->outputSize > : cfg->size; > > -- > 2.45.2 >
Quoting Stefan Klug (2024-10-09 12:00:41) > Hi Umang, > > Thank you for the patch. > > On Wed, Oct 09, 2024 at 03:09:19PM +0530, Umang Jain wrote: > > Commit 761545407c76 ("pipeline: rkisp1: Filter out sensor sizes not > > supported by the pipeline") introduced a mechanism to determine maximum > > supported sensor resolution and filter out resolutions that cannot be > > supported by the ISP. > > > > However, it missed to update the raw stream configuration path, where > > it should have clamped the raw stream configuration size to the maximum > > sensor supported resolution. > > > > This patch fixes the above issue and can be confirmed with IMX283 > > on i.MX8MP: > > > > From: > > ($) cam -c1 -srole=raw,width=5472,height=3072 > > INFO Camera camera.cpp:1197 configuring streams: (0) 5472x3648-SRGGB12 > > ERROR RkISP1 rkisp1_path.cpp:425 Unable to configure capture in 5472x3648-SRGGB12 > > Failed to configure camera > > Failed to start camera session > > > > To: > > ($) cam -c1 -srole=raw,width=5472,height=3072 > > INFO Camera camera.cpp:1197 configuring streams: (0) 4096x3072-SRGGB12 > > cam0: Capture until user interrupts by SIGINT > > 536.082380 (0.00 fps) cam0-stream0 seq: 000000 bytesused: 25165824 > > 536.182378 (10.00 fps) cam0-stream0 seq: 000001 bytesused: 25165824 > > 536.282375 (10.00 fps) cam0-stream0 seq: 000002 bytesused: 25165824 > > ... > > > > Fixes: 761545407c76 ("pipeline: rkisp1: Filter out sensor sizes not supported by the pipeline") > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> > > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > > Looks good to me. > > Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com> > > > --- > > Changes in v2: > > - Extend the comment on what 'resolution' var is and denote that > > sensor->getFormat() will never return something greter than > > 'resolution' > > --- > > src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 8 +++++++- > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp > > index 3b5bea96..1999094e 100644 > > --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp > > @@ -326,9 +326,15 @@ RkISP1Path::validate(const CameraSensor *sensor, > > if (isRaw) { > > /* > > * Use the sensor output size closest to the requested stream > > - * size. > > + * size while ensuring the output size doesn't exceed ISP limits. > > + * > > + * As 'resolution' is the largest sensor resolution > > + * supported by the ISP, CameraSensor::getFormat() will never > > + * return a V4L2SubdeviceFormat with a larger size. > > */ > > uint32_t mbusCode = formatToMediaBus.at(cfg->pixelFormat); > > + cfg->size.boundTo(resolution); > > + I wonder if 'resolution' should be better named, but I think this is fine now. Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > Size rawSize = sensorConfig ? sensorConfig->outputSize > > : cfg->size; > > > > -- > > 2.45.2 > >
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp index 3b5bea96..1999094e 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp @@ -326,9 +326,15 @@ RkISP1Path::validate(const CameraSensor *sensor, if (isRaw) { /* * Use the sensor output size closest to the requested stream - * size. + * size while ensuring the output size doesn't exceed ISP limits. + * + * As 'resolution' is the largest sensor resolution + * supported by the ISP, CameraSensor::getFormat() will never + * return a V4L2SubdeviceFormat with a larger size. */ uint32_t mbusCode = formatToMediaBus.at(cfg->pixelFormat); + cfg->size.boundTo(resolution); + Size rawSize = sensorConfig ? sensorConfig->outputSize : cfg->size;
Commit 761545407c76 ("pipeline: rkisp1: Filter out sensor sizes not supported by the pipeline") introduced a mechanism to determine maximum supported sensor resolution and filter out resolutions that cannot be supported by the ISP. However, it missed to update the raw stream configuration path, where it should have clamped the raw stream configuration size to the maximum sensor supported resolution. This patch fixes the above issue and can be confirmed with IMX283 on i.MX8MP: From: ($) cam -c1 -srole=raw,width=5472,height=3072 INFO Camera camera.cpp:1197 configuring streams: (0) 5472x3648-SRGGB12 ERROR RkISP1 rkisp1_path.cpp:425 Unable to configure capture in 5472x3648-SRGGB12 Failed to configure camera Failed to start camera session To: ($) cam -c1 -srole=raw,width=5472,height=3072 INFO Camera camera.cpp:1197 configuring streams: (0) 4096x3072-SRGGB12 cam0: Capture until user interrupts by SIGINT 536.082380 (0.00 fps) cam0-stream0 seq: 000000 bytesused: 25165824 536.182378 (10.00 fps) cam0-stream0 seq: 000001 bytesused: 25165824 536.282375 (10.00 fps) cam0-stream0 seq: 000002 bytesused: 25165824 ... Fixes: 761545407c76 ("pipeline: rkisp1: Filter out sensor sizes not supported by the pipeline") Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> --- Changes in v2: - Extend the comment on what 'resolution' var is and denote that sensor->getFormat() will never return something greter than 'resolution' --- src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)