[libcamera-devel] [PATCH v4] pipeline-rkisp1-Fix-sensor-ISP-format-mismatch
Helen Koike
helen.koike at collabora.com
Mon Feb 15 14:00:05 CET 2021
Hi Sebastian,
On 2/7/21 9:58 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. Additionally, make sure that the media bus code
> of the format is valid on the ISP.
>
> 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 at posteo.net>
This patch doesn't compile, I believe things changed since you submitted.
Could you submit a fixed version please?
Thanks
Helen
> ---
>
> v1: https://lists.libcamera.org/pipermail/libcamera-devel/2020-November/015113.html
> v2: https://lists.libcamera.org/pipermail/libcamera-devel/2020-November/015136.html
> v3: https://patchwork.libcamera.org/patch/10660/
>
> Changelog:
>
> Changes since v3:
>
> * Introduce a helper method for finding the correct sensor format:
> `findBestFitFormat`
> * Besides checking for a valid format resolution also check for a
> matching media bus code
> * Return -EINVAL on an error instead of -1
> * Add documentation for the `findBestFitFormat` method
>
> 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
>
> [6:41:00.383092648] [28903] DEBUG Camera camera.cpp:831 streams configuration: (0) 1920x1920-NV12
> [6:41:00.383284855] [28903] INFO Camera camera.cpp:890 configuring streams: (0) 1920x1920-NV12
> ...
> [6:41:00.384053394] [28904] INFO RkISP1 rkisp1.cpp:573 Sensor format 4224x3136 is not supported by the ISP (maximum: 4032x3024), trying to re-configure to a smaller sensor format
> [6:41:00.384124851] [28904] DEBUG RkISP1 rkisp1.cpp:637 Sensor configured with 2112x1568-SBGGR10_1X10
> [6:41:00.384184351] [28904] DEBUG RkISP1 rkisp1.cpp:648 ISP input pad configured with 2112x1568-SBGGR10_1X10 crop (0x0)/2112x1568
> [6:41:00.384214684] [28904] DEBUG RkISP1 rkisp1.cpp:654 Configuring ISP output pad with 2112x1568-YUYV8_2X8 crop (0x0)/2112x1568
> [6:41:00.384252309] [28904] DEBUG RkISP1 rkisp1.cpp:666 ISP output pad configured with 2112x1568-YUYV8_2X8 crop (0x0)/2112x1568
> [6:41:00.384295184] [28904] DEBUG RkISP1 rkisp1_path.cpp:119 Configured main resizer input pad with 2112x1568-YUYV8_2X8 crop (0x0)/2112x1568
> [6:41:00.384324059] [28904] DEBUG RkISP1 rkisp1_path.cpp:125 Configuring main resizer output pad with 1920x1920-YUYV8_2X8
> [6:41:00.384351475] [28904] 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
>
> [6:51:11.037976397] [28922] DEBUG Camera camera.cpp:831 streams configuration: (0) 1920x1920-NV12
> [6:51:11.038439562] [28922] INFO Camera camera.cpp:890 configuring streams: (0) 900x600-NV12
> ...
> [6:51:11.040661843] [28923] DEBUG RkISP1 rkisp1.cpp:609 Sensor configured with 2112x1568-SBGGR10_1X10
> [6:51:11.040884092] [28923] DEBUG RkISP1 rkisp1.cpp:620 ISP input pad configured with 2112x1568-SBGGR10_1X10 crop (0x0)/2112x1568
> [6:51:11.041015633] [28923] DEBUG RkISP1 rkisp1.cpp:626 Configuring ISP output pad with 2112x1568-YUYV8_2X8 crop (0x0)/2112x1568
> [6:51:11.041182757] [28923] DEBUG RkISP1 rkisp1.cpp:638 ISP output pad configured with 2112x1568-YUYV8_2X8 crop (0x0)/2112x1568
> [6:51:11.041374089] [28923] DEBUG RkISP1 rkisp1_path.cpp:119 Configured main resizer input pad with 2112x1568-YUYV8_2X8 crop (0x0)/2112x1568
> [6:51:11.041498339] [28923] DEBUG RkISP1 rkisp1_path.cpp:125 Configuring main resizer output pad with 900x600-YUYV8_2X8
> [6:51:11.041619380] [28923] 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
>
> [6:52:41.156286265] [28926] DEBUG Camera camera.cpp:831 streams configuration: (0) 1920x1920-NV12
> [6:52:41.156612639] [28926] DEBUG RkISP1 rkisp1_path.cpp:94 Adjusting format from 4500x3500-NV12 to 4416x3312-NV12
> [6:52:41.156745055] [28926] DEBUG RkISP1 rkisp1_path.cpp:94 Adjusting format from 4500x3500-NV12 to 1920x1920-NV12
> [6:52:41.156849471] [28926] DEBUG RkISP1 rkisp1_path.cpp:94 Adjusting format from 4500x3500-NV12 to 4416x3312-NV12
> [6:52:41.156959137] [28926] DEBUG CameraSensor camera_sensor.cpp:560 'ov13850 1-0010': No supported format or size found
> Camera configuration adjusted
> [6:52:41.157176136] [28926] DEBUG CameraSensor camera_sensor.cpp:560 'ov13850 1-0010': No supported format or size found
> [6:52:41.157245844] [28926] INFO Camera camera.cpp:890 configuring streams: (0) 4416x3312-NV12
> ...
> [6:52:41.159387000] [28927] INFO RkISP1 rkisp1.cpp:585 Sensor format 4224x3136 is not supported by the ISP (maximum: 4032x3024), trying to re-configure to a smaller sensor format
> [6:52:41.159552958] [28927] DEBUG RkISP1 rkisp1.cpp:609 Sensor configured with 2112x1568-SBGGR10_1X10
> [6:52:41.159752749] [28927] DEBUG RkISP1 rkisp1.cpp:620 ISP input pad configured with 2112x1568-SBGGR10_1X10 crop (0x0)/2112x1568
> [6:52:41.159883123] [28927] DEBUG RkISP1 rkisp1.cpp:626 Configuring ISP output pad with 2112x1568-YUYV8_2X8 crop (0x0)/2112x1568
> [6:52:41.160129289] [28927] DEBUG RkISP1 rkisp1.cpp:638 ISP output pad configured with 2112x1568-YUYV8_2X8 crop (0x0)/2112x1568
> [6:52:41.160322079] [28927] DEBUG RkISP1 rkisp1_path.cpp:119 Configured main resizer input pad with 2112x1568-YUYV8_2X8 crop (0x0)/2112x1568
> [6:52:41.160451870] [28927] DEBUG RkISP1 rkisp1_path.cpp:125 Configuring main resizer output pad with 4416x3312-YUYV8_2X8
> [6:52:41.160574370] [28927] 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
>
> [6:53:43.547487714] [28930] DEBUG Camera camera.cpp:831 streams configuration: (0) 1920x1920-NV12
> [6:53:43.547902462] [28930] INFO Camera camera.cpp:890 configuring streams: (0) 40x30-NV12
> ...
> [6:53:43.550449033] [28931] DEBUG RkISP1 rkisp1.cpp:609 Sensor configured with 2112x1568-SBGGR10_1X10
> [6:53:43.550650282] [28931] DEBUG RkISP1 rkisp1.cpp:620 ISP input pad configured with 2112x1568-SBGGR10_1X10 crop (0x0)/2112x1568
> [6:53:43.550773657] [28931] DEBUG RkISP1 rkisp1.cpp:626 Configuring ISP output pad with 2112x1568-YUYV8_2X8 crop (0x0)/2112x1568
> [6:53:43.550932323] [28931] DEBUG RkISP1 rkisp1.cpp:638 ISP output pad configured with 2112x1568-YUYV8_2X8 crop (0x0)/2112x1568
> [6:53:43.551103822] [28931] DEBUG RkISP1 rkisp1_path.cpp:119 Configured main resizer input pad with 2112x1568-YUYV8_2X8 crop (0x0)/2112x1568
> [6:53:43.551222530] [28931] DEBUG RkISP1 rkisp1_path.cpp:125 Configuring main resizer output pad with 40x30-YUYV8_2X8
> [6:53:43.551334237] [28931] 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
>
> [6:54:43.212517584] [28933] DEBUG Camera camera.cpp:831 streams configuration: (0) 1920x1920-NV12
> [6:54:43.212946040] [28933] INFO Camera camera.cpp:890 configuring streams: (0) 3450x2456-NV12
> ...
> [6:54:43.215050739] [28934] INFO RkISP1 rkisp1.cpp:585 Sensor format 4224x3136 is not supported by the ISP (maximum: 4032x3024), trying to re-configure to a smaller sensor format
> [6:54:43.215213779] [28934] DEBUG RkISP1 rkisp1.cpp:609 Sensor configured with 2112x1568-SBGGR10_1X10
> [6:54:43.215416487] [28934] DEBUG RkISP1 rkisp1.cpp:620 ISP input pad configured with 2112x1568-SBGGR10_1X10 crop (0x0)/2112x1568
> [6:54:43.215545986] [28934] DEBUG RkISP1 rkisp1.cpp:626 Configuring ISP output pad with 2112x1568-YUYV8_2X8 crop (0x0)/2112x1568
> [6:54:43.215710485] [28934] DEBUG RkISP1 rkisp1.cpp:638 ISP output pad configured with 2112x1568-YUYV8_2X8 crop (0x0)/2112x1568
> [6:54:43.215891026] [28934] DEBUG RkISP1 rkisp1_path.cpp:119 Configured main resizer input pad with 2112x1568-YUYV8_2X8 crop (0x0)/2112x1568
> [6:54:43.216104233] [28934] DEBUG RkISP1 rkisp1_path.cpp:125 Configuring main resizer output pad with 3450x2456-YUYV8_2X8
> [6:54:43.216246274] [28934] DEBUG RkISP1 rkisp1_path.cpp:143 Configured main resizer output pad with 3450x2456-YUYV8_1_5X8
>
> ---
> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 67 ++++++++++++++++++++++++
> 1 file changed, 67 insertions(+)
>
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index e85979a7..55b1a10d 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -163,6 +163,8 @@ private:
> void paramReady(FrameBuffer *buffer);
> void statReady(FrameBuffer *buffer);
> void frameStart(uint32_t sequence);
> + int findBestFitFormat(V4L2SubdeviceFormat *format,
> + CameraSensor *sensor);
>
> int allocateBuffers(Camera *camera);
> int freeBuffers(Camera *camera);
> @@ -551,6 +553,67 @@ CameraConfiguration *PipelineHandlerRkISP1::generateConfiguration(Camera *camera
> return config;
> }
>
> +/**
> + * \brief Find the closest possible match to the desired configuration format,
> + * that is valid on the RkISP1.
> + * \param[out] format The configuration format for the image sensor
> + * \param[in] sensor The camera sensor object
> + * \return 0 on success, -1 on failure
> + */
> +int PipelineHandlerRkISP1::findBestFitFormat(
> + V4L2SubdeviceFormat *format, CameraSensor *sensor)
> +{
> + V4L2Subdevice::Formats ispFormats = isp_->formats(0);
> + if (ispFormats.empty()) {
> + LOG(RkISP1, Error) << "Unable to fetch ISP formats.";
> + 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;
> +
> + 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 avail. sensor resolution <= "
> + << format->toString();
> + return -1;
> + }
> + format->size = maxSize;
> + }
> +
> + auto mbusCodeMatch = ispFormats.find(format->mbus_code);
> + if (mbusCodeMatch == ispFormats.end()) {
> + format->mbus_code = 0;
> + for (unsigned int mbusCode : sensor->mbusCodes()) {
> + mbusCodeMatch = ispFormats.find(mbusCode);
> + if (mbusCodeMatch != ispFormats.end()) {
> + format->mbus_code = mbusCode;
> + break;
> + }
> + }
> + if (format->mbus_code == 0) {
> + LOG(RkISP1, Error) << "No valid sensor mbus-code found";
> + return -1;
> + }
> + }
> + return 0;
> +}
> +
> int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
> {
> RkISP1CameraConfiguration *config =
> @@ -570,6 +633,10 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
> V4L2SubdeviceFormat format = config->sensorFormat();
> LOG(RkISP1, Debug) << "Configuring sensor with " << format.toString();
>
> + ret = findBestFitFormat(&format, sensor);
> + if (ret < 0)
> + return -EINVAL;
> +
> ret = sensor->setFormat(&format);
> if (ret < 0)
> return ret;
>
More information about the libcamera-devel
mailing list