| Message ID | 20201120124503.22718-1-sebastian.fricke.linux@gmail.com |
|---|---|
| Headers | show |
| Series |
|
| Related | show |
Hi Sebastian, Thanks for your patch. On 11/20/20 9:45 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 exceeded the maximum viable > resolution of the ISP. > This patch fixes the issue by first setting the ISP, checking if the > given resolution was taken and searching for another viable format that > is lower or equal to the ISP maximum. This description could be in the commit message, so we can save in the history the reasons for the change. > > --- > > Changelog: > > 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 > > [8:05:49.668381732] [9973] DEBUG Camera camera.cpp:771 streams configuration: (0) 1920x1920-NV12 > [8:05:49.669108269] [9973] INFO Camera camera.cpp:830 configuring streams: (0) 1920x1920-NV12 > ... > [8:05:49.670965005] [9974] DEBUG RkISP1 rkisp1.cpp:687 Configuring sensor with 4224x3136-SBGGR10_1X10 > [8:05:49.671126004] [9974] DEBUG RkISP1 rkisp1.cpp:694 ISP configured with 4032x3024-SBGGR10_1X10 > [8:05:49.671233045] [9974] INFO RkISP1 rkisp1.cpp:697 Configured resolution is greater than the maximum resolution for the ISP, trying to re-configure to a smaller valid sensor format > [8:05:49.671378294] [9974] DEBUG RkISP1 rkisp1.cpp:716 ISP re-configured with 2112x1568-SBGGR10_1X10 > [8:05:49.671510710] [9974] DEBUG RkISP1 rkisp1.cpp:724 Sensor configured with 2112x1568-SBGGR10_1X10 > [8:05:49.671676667] [9974] DEBUG RkISP1 rkisp1.cpp:731 ISP input pad configured with 2112x1568-SBGGR10_1X10 There are two prints here for the same ISP pad, please see my comments on 1/1. > [8:05:49.671787791] [9974] DEBUG RkISP1 rkisp1.cpp:735 Configuring ISP output pad with 2112x1568-YUYV8_2X8 > [8:05:49.671894540] [9974] DEBUG RkISP1 rkisp1.cpp:741 ISP output pad configured with 800x600-YUYV8_2X8 > [8:05:49.672107747] [9974] DEBUG RkISP1 rkisp1_path.cpp:120 Configured main resizer input pad with 800x600-YUYV8_2X8 > [8:05:49.672226163] [9974] DEBUG RkISP1 rkisp1_path.cpp:126 Configuring main resizer output pad with 1920x1920-YUYV8_2X8 > [8:05:49.672341662] [9974] DEBUG RkISP1 rkisp1_path.cpp:144 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 > > [8:07:05.725799205] [9977] DEBUG Camera camera.cpp:771 streams configuration: (0) 1920x1920-NV12 > [8:07:05.726242535] [9977] INFO Camera camera.cpp:830 configuring streams: (0) 900x600-NV12 > ... > [8:07:05.727718025] [9978] DEBUG RkISP1 rkisp1.cpp:687 Configuring sensor with 2112x1568-SBGGR10_1X10 > [8:07:05.727873482] [9978] DEBUG RkISP1 rkisp1.cpp:694 ISP configured with 2112x1568-SBGGR10_1X10 > [8:07:05.728056356] [9978] DEBUG RkISP1 rkisp1.cpp:724 Sensor configured with 2112x1568-SBGGR10_1X10 > [8:07:05.728233104] [9978] DEBUG RkISP1 rkisp1.cpp:731 ISP input pad configured with 2112x1568-SBGGR10_1X10 > [8:07:05.728340437] [9978] DEBUG RkISP1 rkisp1.cpp:735 Configuring ISP output pad with 2112x1568-YUYV8_2X8 > [8:07:05.728448353] [9978] DEBUG RkISP1 rkisp1.cpp:741 ISP output pad configured with 800x600-YUYV8_2X8 > [8:07:05.728567352] [9978] DEBUG RkISP1 rkisp1_path.cpp:120 Configured main resizer input pad with 800x600-YUYV8_2X8 > [8:07:05.728673809] [9978] DEBUG RkISP1 rkisp1_path.cpp:126 Configuring main resizer output pad with 900x600-YUYV8_2X8 > [8:07:05.728785517] [9978] DEBUG RkISP1 rkisp1_path.cpp:144 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 > > [8:08:02.372820979] [9981] DEBUG Camera camera.cpp:771 streams configuration: (0) 1920x1920-NV12 > [8:08:02.373280350] [9981] DEBUG RkISP1 rkisp1_path.cpp:100 Adjusting format from 4500x3500-NV12 to 4416x3312-NV12 > [8:08:02.373534098] [9981] DEBUG RkISP1 rkisp1_path.cpp:100 Adjusting format from 4500x3500-NV12 to 1920x1920-NV12 > [8:08:02.373767722] [9981] DEBUG RkISP1 rkisp1_path.cpp:100 Adjusting format from 4500x3500-NV12 to 4416x3312-NV12 > [8:08:02.374010095] [9981] DEBUG CameraSensor camera_sensor.cpp:408 'ov13850 1-0010': No supported format or size found > Camera configuration adjusted > [8:08:02.374449634] [9981] DEBUG CameraSensor camera_sensor.cpp:408 'ov13850 1-0010': No supported format or size found > [8:08:02.374591091] [9981] INFO Camera camera.cpp:830 configuring streams: (0) 4416x3312-NV12 > ... > [8:08:02.376301704] [9982] DEBUG RkISP1 rkisp1.cpp:687 Configuring sensor with 4224x3136-0x0000 > [8:08:02.376469994] [9982] DEBUG RkISP1 rkisp1.cpp:694 ISP configured with 4032x3024-SRGGB10_1X10 > [8:08:02.376576160] [9982] INFO RkISP1 rkisp1.cpp:697 Configured resolution is greater than the maximum resolution for the ISP, trying to re-configure to a smaller valid sensor format > [8:08:02.376710909] [9982] DEBUG RkISP1 rkisp1.cpp:716 ISP re-configured with 2112x1568-SBGGR10_1X10 > [8:08:02.376839241] [9982] DEBUG RkISP1 rkisp1.cpp:724 Sensor configured with 2112x1568-SBGGR10_1X10 > [8:08:02.377001115] [9982] DEBUG RkISP1 rkisp1.cpp:731 ISP input pad configured with 2112x1568-SBGGR10_1X10 > [8:08:02.377104364] [9982] DEBUG RkISP1 rkisp1.cpp:735 Configuring ISP output pad with 2112x1568-YUYV8_2X8 > [8:08:02.377208780] [9982] DEBUG RkISP1 rkisp1.cpp:741 ISP output pad configured with 800x600-YUYV8_2X8 > [8:08:02.377324863] [9982] DEBUG RkISP1 rkisp1_path.cpp:120 Configured main resizer input pad with 800x600-YUYV8_2X8 > [8:08:02.377425779] [9982] DEBUG RkISP1 rkisp1_path.cpp:126 Configuring main resizer output pad with 4416x3312-YUYV8_2X8 > [8:08:02.377533695] [9982] DEBUG RkISP1 rkisp1_path.cpp:144 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:10:41.140561164] [9989] DEBUG Camera camera.cpp:771 streams configuration: (0) 1920x1920-NV12 > [8:10:41.141348367] [9989] INFO Camera camera.cpp:830 configuring streams: (0) 40x30-NV12 > ... > [8:10:41.143016688] [9990] DEBUG RkISP1 rkisp1.cpp:687 Configuring sensor with 2112x1568-SBGGR10_1X10 > [8:10:41.143161062] [9990] DEBUG RkISP1 rkisp1.cpp:694 ISP configured with 2112x1568-SBGGR10_1X10 > [8:10:41.143283561] [9990] DEBUG RkISP1 rkisp1.cpp:724 Sensor configured with 2112x1568-SBGGR10_1X10 > [8:10:41.143442810] [9990] DEBUG RkISP1 rkisp1.cpp:731 ISP input pad configured with 2112x1568-SBGGR10_1X10 > [8:10:41.143544893] [9990] DEBUG RkISP1 rkisp1.cpp:735 Configuring ISP output pad with 2112x1568-YUYV8_2X8 > [8:10:41.143644934] [9990] DEBUG RkISP1 rkisp1.cpp:741 ISP output pad configured with 800x600-YUYV8_2X8 > [8:10:41.143756058] [9990] DEBUG RkISP1 rkisp1_path.cpp:120 Configured main resizer input pad with 800x600-YUYV8_2X8 > [8:10:41.143854640] [9990] DEBUG RkISP1 rkisp1_path.cpp:126 Configuring main resizer output pad with 40x30-YUYV8_2X8 > [8:10:41.143960223] [9990] DEBUG RkISP1 rkisp1_path.cpp:144 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:11:40.321895811] [9992] DEBUG Camera camera.cpp:771 streams configuration: (0) 1920x1920-NV12 > [8:11:40.322697014] [9992] INFO Camera camera.cpp:830 configuring streams: (0) 3450x2456-NV12 > ... > [8:11:40.324431543] [9993] DEBUG RkISP1 rkisp1.cpp:687 Configuring sensor with 4224x3136-SBGGR10_1X10 > [8:11:40.324579125] [9993] DEBUG RkISP1 rkisp1.cpp:694 ISP configured with 4032x3024-SBGGR10_1X10 > [8:11:40.324680041] [9993] INFO RkISP1 rkisp1.cpp:697 Configured resolution is greater than the maximum resolution for the ISP, trying to re-configure to a smaller valid sensor format > [8:11:40.324815082] [9993] DEBUG RkISP1 rkisp1.cpp:716 ISP re-configured with 2112x1568-SBGGR10_1X10 > [8:11:40.324946039] [9993] DEBUG RkISP1 rkisp1.cpp:724 Sensor configured with 2112x1568-SBGGR10_1X10 > [8:11:40.325111121] [9993] DEBUG RkISP1 rkisp1.cpp:731 ISP input pad configured with 2112x1568-SBGGR10_1X10 > [8:11:40.325216121] [9993] DEBUG RkISP1 rkisp1.cpp:735 Configuring ISP output pad with 2112x1568-YUYV8_2X8 > [8:11:40.325323453] [9993] DEBUG RkISP1 rkisp1.cpp:741 ISP output pad configured with 800x600-YUYV8_2X8 > [8:11:40.325441286] [9993] DEBUG RkISP1 rkisp1_path.cpp:120 Configured main resizer input pad with 800x600-YUYV8_2X8 > [8:11:40.325547451] [9993] DEBUG RkISP1 rkisp1_path.cpp:126 Configuring main resizer output pad with 3450x2456-YUYV8_2X8 > [8:11:40.325660909] [9993] DEBUG RkISP1 rkisp1_path.cpp:144 Configured main resizer output pad with 3450x2456-YUYV8_1_5X8 > Just a tip, when sending a single patch, I usually don't bother sending a cover letter, I would put all this information after the 3 dashes "---" in the patch (what is written after those 3 dashes will be ignored when git am file.patch) But maybe this is just my style, but I wanted to let you know you have this option :) Regards, Helen > ----- > > Sebastian Fricke (1): > pipeline: rkisp1: Fix sensor-ISP format mismatch > > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 46 ++++++++++++++++++++---- > 1 file changed, 40 insertions(+), 6 deletions(-) >