| Message ID | 20201213134251.7574-1-sebastian.fricke.linux@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Sebastian, On 12/13/20 10:42 AM, Sebastian Fricke wrote: > This patch fixes a mismatch of image formats during the pipeline > creation of the RkISP1. The mismatch happens because the current code > does not check if the configured format exceeds the maximum viable > resolution of the ISP. > > Make sure to use a sensor format resolution that is smaller or equal to > the maximum allowed resolution for the RkISP1. The maximum resolution > is defined within the `rkisp1-common.h` file as: > define RKISP1_ISP_MAX_WIDTH 4032 > define RKISP1_ISP_MAX_HEIGHT 3024 > > Enumerate the frame-sizes of the ISP entity and compare the maximum with > the configured resolution. > > This means that some camera sensors can never operate with their maximum > resolution, for example on my OV13850 camera sensor, there are two > possible resolutions: 4224x3136 & 2112x1568, the first of those two will > never be picked as it surpasses the maximum of the ISP. > > Signed-off-by: Sebastian Fricke <sebastian.fricke.linux@gmail.com> > --- > > Attention: This patch depends on the following patch for the Linux > Kernel media subsystem. > https://patchwork.kernel.org/project/linux-media/patch/20201212185306.19135-1-sebastian.fricke.linux@gmail.com/ > > v1: https://lists.libcamera.org/pipermail/libcamera-devel/2020-November/015113.html > v2: https://lists.libcamera.org/pipermail/libcamera-devel/2020-November/015136.html > > Changelog: > > Changes since v2: > > * Replace the act of attempting to set the ISP format before negotiating > the actual format for both the ISP input pad and the sensor in order > to get the maximum frame size. With a logic that involves enumerating > the maximum size directly from the subdevice and using that size for > the negotiation process. > * Improve the log messages > > Changes since v1: > > * Change snake_case variable names to camelCase > * Use the request comment style > * Correct the scope of the newly implemented variables > * Correct the subject of the debug log for the ISP format configuration > * Update the comment above the ISP format configuration > * Check if the original format is not equal to the configured ISP format > instead of checking if it is greater, this denies a false positive > where the height exceeds the maximum while the width is smaller. > If the configured format does not exceed the maximum resolution of the > ISP, it will stay untouched so the inequality always means that we > have to reconfigure the format. > * Adjust the comparison of the ISP format size with the available sensor > formats, to detect a false-positive were the width is smaller while > the height exceeds the maximum > * Use the standard function `max` > > ----- > > The following tests were all able to create a working camera pipeline: > 1. Without stream configuration > 2. With a normal stream configuration > 3. With a stream configuration that exceeds the maximum of the ISP > 4. With a very small resolution stream configuration > 5. With a configuration that is closer to the upper than to the lower > resolution > > ----- > > 1. LIBCAMERA_LOG_LEVELS=0 cam -c 1 --capture=3 > > [7:56:14.564557994] [9200] DEBUG Camera camera.cpp:831 streams configuration: (0) 1920x1920-NV12 > [7:56:14.565294449] [9200] INFO Camera camera.cpp:890 configuring streams: (0) 1920x1920-NV12 > ... > [7:56:14.567584898] [9201] INFO RkISP1 rkisp1.cpp:689 Sensor format 4224x3136 is not supported by the ISP (maximum: 4032x3024), trying to re-configure to a smaller sensor format > [7:56:14.567738605] [9201] DEBUG RkISP1 rkisp1.cpp:713 Sensor configured with 2112x1568-SBGGR10_1X10 > [7:56:14.567941896] [9201] DEBUG RkISP1 rkisp1.cpp:724 ISP input pad configured with 2112x1568-SBGGR10_1X10 crop (0x0)/2112x1568 > [7:56:14.568196812] [9201] DEBUG RkISP1 rkisp1.cpp:730 Configuring ISP output pad with 2112x1568-YUYV8_2X8 crop (0x0)/2112x1568 > [7:56:14.568360144] [9201] DEBUG RkISP1 rkisp1.cpp:742 ISP output pad configured with 2112x1568-YUYV8_2X8 crop (0x0)/2112x1568 > [7:56:14.568529602] [9201] DEBUG RkISP1 rkisp1_path.cpp:119 Configured main resizer input pad with 2112x1568-YUYV8_2X8 crop (0x0)/2112x1568 > [7:56:14.568647435] [9201] DEBUG RkISP1 rkisp1_path.cpp:125 Configuring main resizer output pad with 1920x1920-YUYV8_2X8 > [7:56:14.568762351] [9201] DEBUG RkISP1 rkisp1_path.cpp:143 Configured main resizer output pad with 1920x1920-YUYV8_1_5X8 > > 2. LIBCAMERA_LOG_LEVELS=0 cam -c 1 --capture=3 -s width=900,height=600,pixelformat=NV12,role=video > > [7:57:58.012056371] [9204] DEBUG Camera camera.cpp:831 streams configuration: (0) 1920x1920-NV12 > [7:57:58.012524202] [9204] INFO Camera camera.cpp:890 configuring streams: (0) 900x600-NV12 > ... > [7:57:58.014565860] [9205] DEBUG RkISP1 rkisp1.cpp:713 Sensor configured with 2112x1568-SBGGR10_1X10 > [7:57:58.014776442] [9205] DEBUG RkISP1 rkisp1.cpp:724 ISP input pad configured with 2112x1568-SBGGR10_1X10 crop (0x0)/2112x1568 > [7:57:58.014913816] [9205] DEBUG RkISP1 rkisp1.cpp:730 Configuring ISP output pad with 2112x1568-YUYV8_2X8 crop (0x0)/2112x1568 > [7:57:58.015064607] [9205] DEBUG RkISP1 rkisp1.cpp:742 ISP output pad configured with 2112x1568-YUYV8_2X8 crop (0x0)/2112x1568 > [7:57:58.015230273] [9205] DEBUG RkISP1 rkisp1_path.cpp:119 Configured main resizer input pad with 2112x1568-YUYV8_2X8 crop (0x0)/2112x1568 > [7:57:58.015348689] [9205] DEBUG RkISP1 rkisp1_path.cpp:125 Configuring main resizer output pad with 900x600-YUYV8_2X8 > [7:57:58.015463314] [9205] DEBUG RkISP1 rkisp1_path.cpp:143 Configured main resizer output pad with 900x600-YUYV8_1_5X8 > > 3. LIBCAMERA_LOG_LEVELS=0 cam -c 1 --capture=3 -s width=4500,height=3500,pixelformat=NV12,role=video > > [7:59:52.764968202] [9210] DEBUG Camera camera.cpp:831 streams configuration: (0) 1920x1920-NV12 > [7:59:52.765415033] [9210] DEBUG RkISP1 rkisp1_path.cpp:94 Adjusting format from 4500x3500-NV12 to 4416x3312-NV12 > [7:59:52.765671115] [9210] DEBUG RkISP1 rkisp1_path.cpp:94 Adjusting format from 4500x3500-NV12 to 1920x1920-NV12 > [7:59:52.765905613] [9210] DEBUG RkISP1 rkisp1_path.cpp:94 Adjusting format from 4500x3500-NV12 to 4416x3312-NV12 > [7:59:52.766147987] [9210] DEBUG CameraSensor camera_sensor.cpp:436 'ov13850 1-0010': No supported format or size found > Camera configuration adjusted > [7:59:52.766558943] [9210] DEBUG CameraSensor camera_sensor.cpp:436 'ov13850 1-0010': No supported format or size found > [7:59:52.766702151] [9210] INFO Camera camera.cpp:890 configuring streams: (0) 4416x3312-NV12 > ... > [7:59:52.768968097] [9211] INFO RkISP1 rkisp1.cpp:689 Sensor format 4224x3136 is not supported by the ISP (maximum: 4032x3024), trying to re-configure to a smaller sensor format > [7:59:52.769120347] [9211] DEBUG RkISP1 rkisp1.cpp:713 Sensor configured with 2112x1568-SBGGR10_1X10 > [7:59:52.769320721] [9211] DEBUG RkISP1 rkisp1.cpp:724 ISP input pad configured with 2112x1568-SBGGR10_1X10 crop (0x0)/2112x1568 > [7:59:52.769455762] [9211] DEBUG RkISP1 rkisp1.cpp:730 Configuring ISP output pad with 2112x1568-YUYV8_2X8 crop (0x0)/2112x1568 > [7:59:52.769606552] [9211] DEBUG RkISP1 rkisp1.cpp:742 ISP output pad configured with 2112x1568-YUYV8_2X8 crop (0x0)/2112x1568 > [7:59:52.769771927] [9211] DEBUG RkISP1 rkisp1_path.cpp:119 Configured main resizer input pad with 2112x1568-YUYV8_2X8 crop (0x0)/2112x1568 > [7:59:52.769888301] [9211] DEBUG RkISP1 rkisp1_path.cpp:125 Configuring main resizer output pad with 4416x3312-YUYV8_2X8 > [7:59:52.770003217] [9211] DEBUG RkISP1 rkisp1_path.cpp:143 Configured main resizer output pad with 4416x3312-YUYV8_1_5X8 > > 4. LIBCAMERA_LOG_LEVELS=0 cam -c 1 --capture=3 -s width=40,height=30,pixelformat=NV12,role=video > > [8:03:00.496627233] [9218] DEBUG Camera camera.cpp:831 streams configuration: (0) 1920x1920-NV12 > [8:03:00.497419103] [9218] INFO Camera camera.cpp:890 configuring streams: (0) 40x30-NV12 > ... > [8:03:00.499715966] [9219] DEBUG RkISP1 rkisp1.cpp:713 Sensor configured with 2112x1568-SBGGR10_1X10 > [8:03:00.499924215] [9219] DEBUG RkISP1 rkisp1.cpp:724 ISP input pad configured with 2112x1568-SBGGR10_1X10 crop (0x0)/2112x1568 > [8:03:00.500168047] [9219] DEBUG RkISP1 rkisp1.cpp:730 Configuring ISP output pad with 2112x1568-YUYV8_2X8 crop (0x0)/2112x1568 > [8:03:00.500336630] [9219] DEBUG RkISP1 rkisp1.cpp:742 ISP output pad configured with 2112x1568-YUYV8_2X8 crop (0x0)/2112x1568 > [8:03:00.500513087] [9219] DEBUG RkISP1 rkisp1_path.cpp:119 Configured main resizer input pad with 2112x1568-YUYV8_2X8 crop (0x0)/2112x1568 > [8:03:00.500632962] [9219] DEBUG RkISP1 rkisp1_path.cpp:125 Configuring main resizer output pad with 40x30-YUYV8_2X8 > [8:03:00.500745836] [9219] DEBUG RkISP1 rkisp1_path.cpp:143 Configured main resizer output pad with 40x30-YUYV8_1_5X8 > > 5. LIBCAMERA_LOG_LEVELS=0 cam -c 1 --capture=3 -s width=3450,height=2456,pixelformat=NV12,role=video > > [8:01:23.689086810] [9215] DEBUG Camera camera.cpp:831 streams configuration: (0) 1920x1920-NV12 > [8:01:23.689893848] [9215] INFO Camera camera.cpp:890 configuring streams: (0) 3450x2456-NV12 > ... > [8:01:23.692257211] [9216] INFO RkISP1 rkisp1.cpp:689 Sensor format 4224x3136 is not supported by the ISP (maximum: 4032x3024), trying to re-configure to a smaller sensor format > [8:01:23.692415876] [9216] DEBUG RkISP1 rkisp1.cpp:713 Sensor configured with 2112x1568-SBGGR10_1X10 > [8:01:23.692615084] [9216] DEBUG RkISP1 rkisp1.cpp:724 ISP input pad configured with 2112x1568-SBGGR10_1X10 crop (0x0)/2112x1568 > [8:01:23.692753041] [9216] DEBUG RkISP1 rkisp1.cpp:730 Configuring ISP output pad with 2112x1568-YUYV8_2X8 crop (0x0)/2112x1568 > [8:01:23.692901499] [9216] DEBUG RkISP1 rkisp1.cpp:742 ISP output pad configured with 2112x1568-YUYV8_2X8 crop (0x0)/2112x1568 > [8:01:23.693066581] [9216] DEBUG RkISP1 rkisp1_path.cpp:119 Configured main resizer input pad with 2112x1568-YUYV8_2X8 crop (0x0)/2112x1568 > [8:01:23.693183831] [9216] DEBUG RkISP1 rkisp1_path.cpp:125 Configuring main resizer output pad with 3450x2456-YUYV8_2X8 > [8:01:23.693297289] [9216] DEBUG RkISP1 rkisp1_path.cpp:143 Configured main resizer output pad with 3450x2456-YUYV8_1_5X8 > > --- > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 34 +++++++++++++++++++++++- > 1 file changed, 33 insertions(+), 1 deletion(-) > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > index 4d98c902..d6fc624c 100644 > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > @@ -667,12 +667,44 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c) > if (ret) > return ret; > > + V4L2Subdevice::Formats ispFormats = isp_->formats(0); > + if (ispFormats.empty()) > + return -1; Returning -1 doesn't seem appropriated. Please check how other functions return errors. I would guess -EINVAL. > + > + /* > + * The maximum resolution is identical for all media bus codes on > + * the RkISP1 isp entity. Therefore take the first available resolution. > + */ > + Size ispMaximum = ispFormats.begin()->second[0].max; > + > /* > * Configure the format on the sensor output and propagate it through > * the pipeline. > */ > V4L2SubdeviceFormat format = config->sensorFormat(); > - LOG(RkISP1, Debug) << "Configuring sensor with " << format.toString(); > + > + if (ispMaximum.width < format.size.width || > + ispMaximum.height < format.size.height) { > + Size maxSize; > + LOG(RkISP1, Info) << "Sensor format " << format.size.toString() > + << " is not supported by the ISP (maximum: " > + << ispMaximum.toString() << "), trying to " > + << "re-configure to a smaller sensor format"; > + > + for (const Size &size : sensor->sizes()) { > + if (size.width > ispMaximum.width || > + size.height > ispMaximum.height) > + continue; > + maxSize = std::max(maxSize, size); > + } > + if (maxSize == Size(0, 0)) { > + LOG(RkISP1, Error) << "No available sensor resolution" > + "that is smaller or equal to " > + << format.toString(); > + return -1; Returning -1 doesn't seem appropriated. Please check how other functions return errors. I would guess -EINVAL. With this change: Reviewed-by: Helen Koike <helen.koike@collabora.com> Thanks Helen > + } > + format.size = maxSize; > + } > > ret = sensor->setFormat(&format); > if (ret < 0) >
Am 13.12.20 um 14:42 schrieb Sebastian Fricke: > This patch fixes a mismatch of image formats during the pipeline > creation of the RkISP1. The mismatch happens because the current code > does not check if the configured format exceeds the maximum viable > resolution of the ISP. > > Make sure to use a sensor format resolution that is smaller or equal to > the maximum allowed resolution for the RkISP1. The maximum resolution > is defined within the `rkisp1-common.h` file as: > define RKISP1_ISP_MAX_WIDTH 4032 > define RKISP1_ISP_MAX_HEIGHT 3024 > > Enumerate the frame-sizes of the ISP entity and compare the maximum with > the configured resolution. > > This means that some camera sensors can never operate with their maximum > resolution, for example on my OV13850 camera sensor, there are two > possible resolutions: 4224x3136 & 2112x1568, the first of those two will > never be picked as it surpasses the maximum of the ISP. > > Signed-off-by: Sebastian Fricke <sebastian.fricke.linux@gmail.com> > --- > > Attention: This patch depends on the following patch for the Linux > Kernel media subsystem. > https://patchwork.kernel.org/project/linux-media/patch/20201212185306.19135-1-sebastian.fricke.linux@gmail.com/ You can add a patch that uses the new VIDIOC_SUBDEV_ENUM_FRAME_SIZE in the end of the 'RkISP1CameraConfiguration::validate()' function instead of that hard-coded list: sensorFormat_ = sensor->getFormat({ MEDIA_BUS_FMT_SBGGR12_1X12, MEDIA_BUS_FMT_SGBRG12_1X12, MEDIA_BUS_FMT_SGRBG12_1X12, MEDIA_BUS_FMT_SRGGB12_1X12, MEDIA_BUS_FMT_SBGGR10_1X10, MEDIA_BUS_FMT_SGBRG10_1X10, MEDIA_BUS_FMT_SGRBG10_1X10, MEDIA_BUS_FMT_SRGGB10_1X10, MEDIA_BUS_FMT_SBGGR8_1X8, MEDIA_BUS_FMT_SGBRG8_1X8, MEDIA_BUS_FMT_SGRBG8_1X8, MEDIA_BUS_FMT_SRGGB8_1X8 }, maxSize); > > v1: https://lists.libcamera.org/pipermail/libcamera-devel/2020-November/015113.html > v2: https://lists.libcamera.org/pipermail/libcamera-devel/2020-November/015136.html > > Changelog: > > Changes since v2: > > * Replace the act of attempting to set the ISP format before negotiating > the actual format for both the ISP input pad and the sensor in order > to get the maximum frame size. With a logic that involves enumerating > the maximum size directly from the subdevice and using that size for > the negotiation process. > * Improve the log messages > > Changes since v1: > > * Change snake_case variable names to camelCase > * Use the request comment style > * Correct the scope of the newly implemented variables > * Correct the subject of the debug log for the ISP format configuration > * Update the comment above the ISP format configuration > * Check if the original format is not equal to the configured ISP format > instead of checking if it is greater, this denies a false positive > where the height exceeds the maximum while the width is smaller. > If the configured format does not exceed the maximum resolution of the > ISP, it will stay untouched so the inequality always means that we > have to reconfigure the format. > * Adjust the comparison of the ISP format size with the available sensor > formats, to detect a false-positive were the width is smaller while > the height exceeds the maximum > * Use the standard function `max` > > ----- > > The following tests were all able to create a working camera pipeline: > 1. Without stream configuration > 2. With a normal stream configuration > 3. With a stream configuration that exceeds the maximum of the ISP > 4. With a very small resolution stream configuration > 5. With a configuration that is closer to the upper than to the lower > resolution > > ----- > > 1. LIBCAMERA_LOG_LEVELS=0 cam -c 1 --capture=3 > > [7:56:14.564557994] [9200] DEBUG Camera camera.cpp:831 streams configuration: (0) 1920x1920-NV12 > [7:56:14.565294449] [9200] INFO Camera camera.cpp:890 configuring streams: (0) 1920x1920-NV12 > ... > [7:56:14.567584898] [9201] INFO RkISP1 rkisp1.cpp:689 Sensor format 4224x3136 is not supported by the ISP (maximum: 4032x3024), trying to re-configure to a smaller sensor format > [7:56:14.567738605] [9201] DEBUG RkISP1 rkisp1.cpp:713 Sensor configured with 2112x1568-SBGGR10_1X10 > [7:56:14.567941896] [9201] DEBUG RkISP1 rkisp1.cpp:724 ISP input pad configured with 2112x1568-SBGGR10_1X10 crop (0x0)/2112x1568 > [7:56:14.568196812] [9201] DEBUG RkISP1 rkisp1.cpp:730 Configuring ISP output pad with 2112x1568-YUYV8_2X8 crop (0x0)/2112x1568 > [7:56:14.568360144] [9201] DEBUG RkISP1 rkisp1.cpp:742 ISP output pad configured with 2112x1568-YUYV8_2X8 crop (0x0)/2112x1568 > [7:56:14.568529602] [9201] DEBUG RkISP1 rkisp1_path.cpp:119 Configured main resizer input pad with 2112x1568-YUYV8_2X8 crop (0x0)/2112x1568 > [7:56:14.568647435] [9201] DEBUG RkISP1 rkisp1_path.cpp:125 Configuring main resizer output pad with 1920x1920-YUYV8_2X8 > [7:56:14.568762351] [9201] DEBUG RkISP1 rkisp1_path.cpp:143 Configured main resizer output pad with 1920x1920-YUYV8_1_5X8 > > 2. LIBCAMERA_LOG_LEVELS=0 cam -c 1 --capture=3 -s width=900,height=600,pixelformat=NV12,role=video > > [7:57:58.012056371] [9204] DEBUG Camera camera.cpp:831 streams configuration: (0) 1920x1920-NV12 > [7:57:58.012524202] [9204] INFO Camera camera.cpp:890 configuring streams: (0) 900x600-NV12 > ... > [7:57:58.014565860] [9205] DEBUG RkISP1 rkisp1.cpp:713 Sensor configured with 2112x1568-SBGGR10_1X10 > [7:57:58.014776442] [9205] DEBUG RkISP1 rkisp1.cpp:724 ISP input pad configured with 2112x1568-SBGGR10_1X10 crop (0x0)/2112x1568 > [7:57:58.014913816] [9205] DEBUG RkISP1 rkisp1.cpp:730 Configuring ISP output pad with 2112x1568-YUYV8_2X8 crop (0x0)/2112x1568 > [7:57:58.015064607] [9205] DEBUG RkISP1 rkisp1.cpp:742 ISP output pad configured with 2112x1568-YUYV8_2X8 crop (0x0)/2112x1568 > [7:57:58.015230273] [9205] DEBUG RkISP1 rkisp1_path.cpp:119 Configured main resizer input pad with 2112x1568-YUYV8_2X8 crop (0x0)/2112x1568 > [7:57:58.015348689] [9205] DEBUG RkISP1 rkisp1_path.cpp:125 Configuring main resizer output pad with 900x600-YUYV8_2X8 > [7:57:58.015463314] [9205] DEBUG RkISP1 rkisp1_path.cpp:143 Configured main resizer output pad with 900x600-YUYV8_1_5X8 > > 3. LIBCAMERA_LOG_LEVELS=0 cam -c 1 --capture=3 -s width=4500,height=3500,pixelformat=NV12,role=video > > [7:59:52.764968202] [9210] DEBUG Camera camera.cpp:831 streams configuration: (0) 1920x1920-NV12 > [7:59:52.765415033] [9210] DEBUG RkISP1 rkisp1_path.cpp:94 Adjusting format from 4500x3500-NV12 to 4416x3312-NV12 > [7:59:52.765671115] [9210] DEBUG RkISP1 rkisp1_path.cpp:94 Adjusting format from 4500x3500-NV12 to 1920x1920-NV12 > [7:59:52.765905613] [9210] DEBUG RkISP1 rkisp1_path.cpp:94 Adjusting format from 4500x3500-NV12 to 4416x3312-NV12 > [7:59:52.766147987] [9210] DEBUG CameraSensor camera_sensor.cpp:436 'ov13850 1-0010': No supported format or size found Not sure, I think that instead of adjusting the sizes to 2112x1568 in "configure", they should be adjusted in the 'validate'. So we avoid reaching this case of "No supported format or size found". > Camera configuration adjusted > [7:59:52.766558943] [9210] DEBUG CameraSensor camera_sensor.cpp:436 'ov13850 1-0010': No supported format or size found > [7:59:52.766702151] [9210] INFO Camera camera.cpp:890 configuring streams: (0) 4416x3312-NV12 > ... > [7:59:52.768968097] [9211] INFO RkISP1 rkisp1.cpp:689 Sensor format 4224x3136 is not supported by the ISP (maximum: 4032x3024), trying to re-configure to a smaller sensor format > [7:59:52.769120347] [9211] DEBUG RkISP1 rkisp1.cpp:713 Sensor configured with 2112x1568-SBGGR10_1X10 > [7:59:52.769320721] [9211] DEBUG RkISP1 rkisp1.cpp:724 ISP input pad configured with 2112x1568-SBGGR10_1X10 crop (0x0)/2112x1568 > [7:59:52.769455762] [9211] DEBUG RkISP1 rkisp1.cpp:730 Configuring ISP output pad with 2112x1568-YUYV8_2X8 crop (0x0)/2112x1568 > [7:59:52.769606552] [9211] DEBUG RkISP1 rkisp1.cpp:742 ISP output pad configured with 2112x1568-YUYV8_2X8 crop (0x0)/2112x1568 > [7:59:52.769771927] [9211] DEBUG RkISP1 rkisp1_path.cpp:119 Configured main resizer input pad with 2112x1568-YUYV8_2X8 crop (0x0)/2112x1568 > [7:59:52.769888301] [9211] DEBUG RkISP1 rkisp1_path.cpp:125 Configuring main resizer output pad with 4416x3312-YUYV8_2X8 > [7:59:52.770003217] [9211] DEBUG RkISP1 rkisp1_path.cpp:143 Configured main resizer output pad with 4416x3312-YUYV8_1_5X8 > > 4. LIBCAMERA_LOG_LEVELS=0 cam -c 1 --capture=3 -s width=40,height=30,pixelformat=NV12,role=video > > [8:03:00.496627233] [9218] DEBUG Camera camera.cpp:831 streams configuration: (0) 1920x1920-NV12 > [8:03:00.497419103] [9218] INFO Camera camera.cpp:890 configuring streams: (0) 40x30-NV12 > ... > [8:03:00.499715966] [9219] DEBUG RkISP1 rkisp1.cpp:713 Sensor configured with 2112x1568-SBGGR10_1X10 > [8:03:00.499924215] [9219] DEBUG RkISP1 rkisp1.cpp:724 ISP input pad configured with 2112x1568-SBGGR10_1X10 crop (0x0)/2112x1568 > [8:03:00.500168047] [9219] DEBUG RkISP1 rkisp1.cpp:730 Configuring ISP output pad with 2112x1568-YUYV8_2X8 crop (0x0)/2112x1568 > [8:03:00.500336630] [9219] DEBUG RkISP1 rkisp1.cpp:742 ISP output pad configured with 2112x1568-YUYV8_2X8 crop (0x0)/2112x1568 > [8:03:00.500513087] [9219] DEBUG RkISP1 rkisp1_path.cpp:119 Configured main resizer input pad with 2112x1568-YUYV8_2X8 crop (0x0)/2112x1568 > [8:03:00.500632962] [9219] DEBUG RkISP1 rkisp1_path.cpp:125 Configuring main resizer output pad with 40x30-YUYV8_2X8 > [8:03:00.500745836] [9219] DEBUG RkISP1 rkisp1_path.cpp:143 Configured main resizer output pad with 40x30-YUYV8_1_5X8 > > 5. LIBCAMERA_LOG_LEVELS=0 cam -c 1 --capture=3 -s width=3450,height=2456,pixelformat=NV12,role=video > > [8:01:23.689086810] [9215] DEBUG Camera camera.cpp:831 streams configuration: (0) 1920x1920-NV12 > [8:01:23.689893848] [9215] INFO Camera camera.cpp:890 configuring streams: (0) 3450x2456-NV12 > ... > [8:01:23.692257211] [9216] INFO RkISP1 rkisp1.cpp:689 Sensor format 4224x3136 is not supported by the ISP (maximum: 4032x3024), trying to re-configure to a smaller sensor format > [8:01:23.692415876] [9216] DEBUG RkISP1 rkisp1.cpp:713 Sensor configured with 2112x1568-SBGGR10_1X10 > [8:01:23.692615084] [9216] DEBUG RkISP1 rkisp1.cpp:724 ISP input pad configured with 2112x1568-SBGGR10_1X10 crop (0x0)/2112x1568 > [8:01:23.692753041] [9216] DEBUG RkISP1 rkisp1.cpp:730 Configuring ISP output pad with 2112x1568-YUYV8_2X8 crop (0x0)/2112x1568 > [8:01:23.692901499] [9216] DEBUG RkISP1 rkisp1.cpp:742 ISP output pad configured with 2112x1568-YUYV8_2X8 crop (0x0)/2112x1568 > [8:01:23.693066581] [9216] DEBUG RkISP1 rkisp1_path.cpp:119 Configured main resizer input pad with 2112x1568-YUYV8_2X8 crop (0x0)/2112x1568 > [8:01:23.693183831] [9216] DEBUG RkISP1 rkisp1_path.cpp:125 Configuring main resizer output pad with 3450x2456-YUYV8_2X8 > [8:01:23.693297289] [9216] DEBUG RkISP1 rkisp1_path.cpp:143 Configured main resizer output pad with 3450x2456-YUYV8_1_5X8 > > --- > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 34 +++++++++++++++++++++++- > 1 file changed, 33 insertions(+), 1 deletion(-) > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > index 4d98c902..d6fc624c 100644 > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > @@ -667,12 +667,44 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c) > if (ret) > return ret; > > + V4L2Subdevice::Formats ispFormats = isp_->formats(0); > + if (ispFormats.empty()) > + return -1; > + > + /* > + * The maximum resolution is identical for all media bus codes on > + * the RkISP1 isp entity. Therefore take the first available resolution. > + */ > + Size ispMaximum = ispFormats.begin()->second[0].max; > + > /* > * Configure the format on the sensor output and propagate it through > * the pipeline. > */ > V4L2SubdeviceFormat format = config->sensorFormat(); > - LOG(RkISP1, Debug) << "Configuring sensor with " << format.toString(); > + > + if (ispMaximum.width < format.size.width || > + ispMaximum.height < format.size.height) { > + Size maxSize; > + LOG(RkISP1, Info) << "Sensor format " << format.size.toString() > + << " is not supported by the ISP (maximum: " > + << ispMaximum.toString() << "), trying to " > + << "re-configure to a smaller sensor format"; > + > + for (const Size &size : sensor->sizes()) { > + if (size.width > ispMaximum.width || > + size.height > ispMaximum.height) > + continue; > + maxSize = std::max(maxSize, size); Note that is it not enough to make sure that the sizes match but also that the mbus codes match. Thanks, Dafna > + } > + if (maxSize == Size(0, 0)) { > + LOG(RkISP1, Error) << "No available sensor resolution" > + "that is smaller or equal to " > + << format.toString(); > + return -1; > + } > + format.size = maxSize; > + } > > ret = sensor->setFormat(&format); > if (ret < 0) >
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 4d98c902..d6fc624c 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -667,12 +667,44 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c) if (ret) return ret; + V4L2Subdevice::Formats ispFormats = isp_->formats(0); + if (ispFormats.empty()) + return -1; + + /* + * The maximum resolution is identical for all media bus codes on + * the RkISP1 isp entity. Therefore take the first available resolution. + */ + Size ispMaximum = ispFormats.begin()->second[0].max; + /* * Configure the format on the sensor output and propagate it through * the pipeline. */ V4L2SubdeviceFormat format = config->sensorFormat(); - LOG(RkISP1, Debug) << "Configuring sensor with " << format.toString(); + + if (ispMaximum.width < format.size.width || + ispMaximum.height < format.size.height) { + Size maxSize; + LOG(RkISP1, Info) << "Sensor format " << format.size.toString() + << " is not supported by the ISP (maximum: " + << ispMaximum.toString() << "), trying to " + << "re-configure to a smaller sensor format"; + + for (const Size &size : sensor->sizes()) { + if (size.width > ispMaximum.width || + size.height > ispMaximum.height) + continue; + maxSize = std::max(maxSize, size); + } + if (maxSize == Size(0, 0)) { + LOG(RkISP1, Error) << "No available sensor resolution" + "that is smaller or equal to " + << format.toString(); + return -1; + } + format.size = maxSize; + } ret = sensor->setFormat(&format); if (ret < 0)
This patch fixes a mismatch of image formats during the pipeline creation of the RkISP1. The mismatch happens because the current code does not check if the configured format exceeds the maximum viable resolution of the ISP. Make sure to use a sensor format resolution that is smaller or equal to the maximum allowed resolution for the RkISP1. The maximum resolution is defined within the `rkisp1-common.h` file as: define RKISP1_ISP_MAX_WIDTH 4032 define RKISP1_ISP_MAX_HEIGHT 3024 Enumerate the frame-sizes of the ISP entity and compare the maximum with the configured resolution. This means that some camera sensors can never operate with their maximum resolution, for example on my OV13850 camera sensor, there are two possible resolutions: 4224x3136 & 2112x1568, the first of those two will never be picked as it surpasses the maximum of the ISP. Signed-off-by: Sebastian Fricke <sebastian.fricke.linux@gmail.com> --- Attention: This patch depends on the following patch for the Linux Kernel media subsystem. https://patchwork.kernel.org/project/linux-media/patch/20201212185306.19135-1-sebastian.fricke.linux@gmail.com/ v1: https://lists.libcamera.org/pipermail/libcamera-devel/2020-November/015113.html v2: https://lists.libcamera.org/pipermail/libcamera-devel/2020-November/015136.html Changelog: Changes since v2: * Replace the act of attempting to set the ISP format before negotiating the actual format for both the ISP input pad and the sensor in order to get the maximum frame size. With a logic that involves enumerating the maximum size directly from the subdevice and using that size for the negotiation process. * Improve the log messages Changes since v1: * Change snake_case variable names to camelCase * Use the request comment style * Correct the scope of the newly implemented variables * Correct the subject of the debug log for the ISP format configuration * Update the comment above the ISP format configuration * Check if the original format is not equal to the configured ISP format instead of checking if it is greater, this denies a false positive where the height exceeds the maximum while the width is smaller. If the configured format does not exceed the maximum resolution of the ISP, it will stay untouched so the inequality always means that we have to reconfigure the format. * Adjust the comparison of the ISP format size with the available sensor formats, to detect a false-positive were the width is smaller while the height exceeds the maximum * Use the standard function `max` ----- The following tests were all able to create a working camera pipeline: 1. Without stream configuration 2. With a normal stream configuration 3. With a stream configuration that exceeds the maximum of the ISP 4. With a very small resolution stream configuration 5. With a configuration that is closer to the upper than to the lower resolution ----- 1. LIBCAMERA_LOG_LEVELS=0 cam -c 1 --capture=3 [7:56:14.564557994] [9200] DEBUG Camera camera.cpp:831 streams configuration: (0) 1920x1920-NV12 [7:56:14.565294449] [9200] INFO Camera camera.cpp:890 configuring streams: (0) 1920x1920-NV12 ... [7:56:14.567584898] [9201] INFO RkISP1 rkisp1.cpp:689 Sensor format 4224x3136 is not supported by the ISP (maximum: 4032x3024), trying to re-configure to a smaller sensor format [7:56:14.567738605] [9201] DEBUG RkISP1 rkisp1.cpp:713 Sensor configured with 2112x1568-SBGGR10_1X10 [7:56:14.567941896] [9201] DEBUG RkISP1 rkisp1.cpp:724 ISP input pad configured with 2112x1568-SBGGR10_1X10 crop (0x0)/2112x1568 [7:56:14.568196812] [9201] DEBUG RkISP1 rkisp1.cpp:730 Configuring ISP output pad with 2112x1568-YUYV8_2X8 crop (0x0)/2112x1568 [7:56:14.568360144] [9201] DEBUG RkISP1 rkisp1.cpp:742 ISP output pad configured with 2112x1568-YUYV8_2X8 crop (0x0)/2112x1568 [7:56:14.568529602] [9201] DEBUG RkISP1 rkisp1_path.cpp:119 Configured main resizer input pad with 2112x1568-YUYV8_2X8 crop (0x0)/2112x1568 [7:56:14.568647435] [9201] DEBUG RkISP1 rkisp1_path.cpp:125 Configuring main resizer output pad with 1920x1920-YUYV8_2X8 [7:56:14.568762351] [9201] DEBUG RkISP1 rkisp1_path.cpp:143 Configured main resizer output pad with 1920x1920-YUYV8_1_5X8 2. LIBCAMERA_LOG_LEVELS=0 cam -c 1 --capture=3 -s width=900,height=600,pixelformat=NV12,role=video [7:57:58.012056371] [9204] DEBUG Camera camera.cpp:831 streams configuration: (0) 1920x1920-NV12 [7:57:58.012524202] [9204] INFO Camera camera.cpp:890 configuring streams: (0) 900x600-NV12 ... [7:57:58.014565860] [9205] DEBUG RkISP1 rkisp1.cpp:713 Sensor configured with 2112x1568-SBGGR10_1X10 [7:57:58.014776442] [9205] DEBUG RkISP1 rkisp1.cpp:724 ISP input pad configured with 2112x1568-SBGGR10_1X10 crop (0x0)/2112x1568 [7:57:58.014913816] [9205] DEBUG RkISP1 rkisp1.cpp:730 Configuring ISP output pad with 2112x1568-YUYV8_2X8 crop (0x0)/2112x1568 [7:57:58.015064607] [9205] DEBUG RkISP1 rkisp1.cpp:742 ISP output pad configured with 2112x1568-YUYV8_2X8 crop (0x0)/2112x1568 [7:57:58.015230273] [9205] DEBUG RkISP1 rkisp1_path.cpp:119 Configured main resizer input pad with 2112x1568-YUYV8_2X8 crop (0x0)/2112x1568 [7:57:58.015348689] [9205] DEBUG RkISP1 rkisp1_path.cpp:125 Configuring main resizer output pad with 900x600-YUYV8_2X8 [7:57:58.015463314] [9205] DEBUG RkISP1 rkisp1_path.cpp:143 Configured main resizer output pad with 900x600-YUYV8_1_5X8 3. LIBCAMERA_LOG_LEVELS=0 cam -c 1 --capture=3 -s width=4500,height=3500,pixelformat=NV12,role=video [7:59:52.764968202] [9210] DEBUG Camera camera.cpp:831 streams configuration: (0) 1920x1920-NV12 [7:59:52.765415033] [9210] DEBUG RkISP1 rkisp1_path.cpp:94 Adjusting format from 4500x3500-NV12 to 4416x3312-NV12 [7:59:52.765671115] [9210] DEBUG RkISP1 rkisp1_path.cpp:94 Adjusting format from 4500x3500-NV12 to 1920x1920-NV12 [7:59:52.765905613] [9210] DEBUG RkISP1 rkisp1_path.cpp:94 Adjusting format from 4500x3500-NV12 to 4416x3312-NV12 [7:59:52.766147987] [9210] DEBUG CameraSensor camera_sensor.cpp:436 'ov13850 1-0010': No supported format or size found Camera configuration adjusted [7:59:52.766558943] [9210] DEBUG CameraSensor camera_sensor.cpp:436 'ov13850 1-0010': No supported format or size found [7:59:52.766702151] [9210] INFO Camera camera.cpp:890 configuring streams: (0) 4416x3312-NV12 ... [7:59:52.768968097] [9211] INFO RkISP1 rkisp1.cpp:689 Sensor format 4224x3136 is not supported by the ISP (maximum: 4032x3024), trying to re-configure to a smaller sensor format [7:59:52.769120347] [9211] DEBUG RkISP1 rkisp1.cpp:713 Sensor configured with 2112x1568-SBGGR10_1X10 [7:59:52.769320721] [9211] DEBUG RkISP1 rkisp1.cpp:724 ISP input pad configured with 2112x1568-SBGGR10_1X10 crop (0x0)/2112x1568 [7:59:52.769455762] [9211] DEBUG RkISP1 rkisp1.cpp:730 Configuring ISP output pad with 2112x1568-YUYV8_2X8 crop (0x0)/2112x1568 [7:59:52.769606552] [9211] DEBUG RkISP1 rkisp1.cpp:742 ISP output pad configured with 2112x1568-YUYV8_2X8 crop (0x0)/2112x1568 [7:59:52.769771927] [9211] DEBUG RkISP1 rkisp1_path.cpp:119 Configured main resizer input pad with 2112x1568-YUYV8_2X8 crop (0x0)/2112x1568 [7:59:52.769888301] [9211] DEBUG RkISP1 rkisp1_path.cpp:125 Configuring main resizer output pad with 4416x3312-YUYV8_2X8 [7:59:52.770003217] [9211] DEBUG RkISP1 rkisp1_path.cpp:143 Configured main resizer output pad with 4416x3312-YUYV8_1_5X8 4. LIBCAMERA_LOG_LEVELS=0 cam -c 1 --capture=3 -s width=40,height=30,pixelformat=NV12,role=video [8:03:00.496627233] [9218] DEBUG Camera camera.cpp:831 streams configuration: (0) 1920x1920-NV12 [8:03:00.497419103] [9218] INFO Camera camera.cpp:890 configuring streams: (0) 40x30-NV12 ... [8:03:00.499715966] [9219] DEBUG RkISP1 rkisp1.cpp:713 Sensor configured with 2112x1568-SBGGR10_1X10 [8:03:00.499924215] [9219] DEBUG RkISP1 rkisp1.cpp:724 ISP input pad configured with 2112x1568-SBGGR10_1X10 crop (0x0)/2112x1568 [8:03:00.500168047] [9219] DEBUG RkISP1 rkisp1.cpp:730 Configuring ISP output pad with 2112x1568-YUYV8_2X8 crop (0x0)/2112x1568 [8:03:00.500336630] [9219] DEBUG RkISP1 rkisp1.cpp:742 ISP output pad configured with 2112x1568-YUYV8_2X8 crop (0x0)/2112x1568 [8:03:00.500513087] [9219] DEBUG RkISP1 rkisp1_path.cpp:119 Configured main resizer input pad with 2112x1568-YUYV8_2X8 crop (0x0)/2112x1568 [8:03:00.500632962] [9219] DEBUG RkISP1 rkisp1_path.cpp:125 Configuring main resizer output pad with 40x30-YUYV8_2X8 [8:03:00.500745836] [9219] DEBUG RkISP1 rkisp1_path.cpp:143 Configured main resizer output pad with 40x30-YUYV8_1_5X8 5. LIBCAMERA_LOG_LEVELS=0 cam -c 1 --capture=3 -s width=3450,height=2456,pixelformat=NV12,role=video [8:01:23.689086810] [9215] DEBUG Camera camera.cpp:831 streams configuration: (0) 1920x1920-NV12 [8:01:23.689893848] [9215] INFO Camera camera.cpp:890 configuring streams: (0) 3450x2456-NV12 ... [8:01:23.692257211] [9216] INFO RkISP1 rkisp1.cpp:689 Sensor format 4224x3136 is not supported by the ISP (maximum: 4032x3024), trying to re-configure to a smaller sensor format [8:01:23.692415876] [9216] DEBUG RkISP1 rkisp1.cpp:713 Sensor configured with 2112x1568-SBGGR10_1X10 [8:01:23.692615084] [9216] DEBUG RkISP1 rkisp1.cpp:724 ISP input pad configured with 2112x1568-SBGGR10_1X10 crop (0x0)/2112x1568 [8:01:23.692753041] [9216] DEBUG RkISP1 rkisp1.cpp:730 Configuring ISP output pad with 2112x1568-YUYV8_2X8 crop (0x0)/2112x1568 [8:01:23.692901499] [9216] DEBUG RkISP1 rkisp1.cpp:742 ISP output pad configured with 2112x1568-YUYV8_2X8 crop (0x0)/2112x1568 [8:01:23.693066581] [9216] DEBUG RkISP1 rkisp1_path.cpp:119 Configured main resizer input pad with 2112x1568-YUYV8_2X8 crop (0x0)/2112x1568 [8:01:23.693183831] [9216] DEBUG RkISP1 rkisp1_path.cpp:125 Configuring main resizer output pad with 3450x2456-YUYV8_2X8 [8:01:23.693297289] [9216] DEBUG RkISP1 rkisp1_path.cpp:143 Configured main resizer output pad with 3450x2456-YUYV8_1_5X8 --- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 34 +++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-)