raspberrypi / rpicam-apps

BSD 2-Clause "Simplified" License
420 stars 228 forks source link

[BUG] Wrong mode selection regression #678

Closed mugnierb closed 6 months ago

mugnierb commented 6 months ago

Describe the bug Since v0.2.0+rpt20240418 (latest), rpicam-apps chooses a mode for vc4 that is not compatible with my sensor. I'm using my own sensor's driver for this test.

[2:38:27.119131966] [6034]  INFO Camera camera.cpp:1183 configuring streams: (0) 402x352-YUV420 (1) 640x480-R10_CSI2P
[2:38:27.119547700] [6037]  INFO RPI vc4.cpp:621 Sensor: /base/soc/i2c0mux/i2c@1/wolfy@10 - Selected sensor format: 640x480-SGBRG10_1X10 - Selected unicam format: 640x480-Y10P
[2:38:27.123569826] [6040]  WARN IPARPI ipa_base.cpp:1055 Could not set SHARPNESS - no sharpen algorithm
[2:38:27.123904765] [6040]  WARN IPARPI vc4.cpp:278 Could not set NOISE_REDUCTION_MODE - no SDN algorithm
[2:38:27.127907298] [6037] ERROR V4L2 v4l2_videodevice.cpp:1906 /dev/video0[13:cap]: Failed to start streaming: Invalid argument
ERROR: *** failed to start camera ***

640x480-Y10P should be 640x480-pGAA, like it was before.

[2:19:35.628599254] [5648]  INFO RPI vc4.cpp:570 Sensor: /base/soc/i2c0mux/i2c@1/wolfy@10 - Selected sensor format: 640x480-SGBRG10_1X10 - Selected unicam format: 640x480-pGAA

Bug report bug.txt

Additional context It was working fine in previous versions, effectively selecting 640x480-pGAA. It works if I build rpicam-apps against mainline libcamera, and also older libcamera (february build), and fails with only latest. The same happens on both rpi4 and rpi5.

Feel free to ask for more information. I'll try to debug this on my side to see if I can bisect this more.

naushir commented 6 months ago

Can you provide specific details on the camera, i.e. what make/model, is it a mono or Bayer sensor, etc? Also, can you provide the output of rpicam-hello --list-cameras please?

mugnierb commented 6 months ago

Unfortunately this sensor is not yet mainlined. I plan on doing so once ready. Here is the output of rpicam-hello --list-cameras.

Available cameras
-----------------
0 : st-vd55g1 [804x704 10-bit MONO] (/base/soc/i2c0mux/i2c@1/wolfy@10)
    Modes: 'R8' : 320x240 [315.16 fps - (82, 112)/640x480 crop]
                  640x480 [181.52 fps - (82, 112)/640x480 crop]
                  800x600 [149.77 fps - (2, 52)/800x600 crop]
                  800x704 [130.06 fps - (2, 0)/800x704 crop]
                  804x704 [130.06 fps - (0, 0)/804x704 crop]
           'R10_CSI2P' : 320x240 [315.16 fps - (82, 112)/640x480 crop]
                         640x480 [181.52 fps - (82, 112)/640x480 crop]
                         800x600 [149.77 fps - (2, 52)/800x600 crop]
                         800x704 [130.06 fps - (2, 0)/800x704 crop]
                         804x704 [130.06 fps - (0, 0)/804x704 crop]
           'SGBRG10_CSI2P' : 320x240 [315.16 fps - (82, 112)/640x480 crop]
                             640x480 [181.52 fps - (82, 112)/640x480 crop]
                             800x600 [149.77 fps - (2, 52)/800x600 crop]
                             800x704 [130.06 fps - (2, 0)/800x704 crop]
                             804x704 [130.06 fps - (0, 0)/804x704 crop]
           'SGBRG8' : 320x240 [315.16 fps - (82, 112)/640x480 crop]
                      640x480 [181.52 fps - (82, 112)/640x480 crop]
                      800x600 [149.77 fps - (2, 52)/800x600 crop]
                      800x704 [130.06 fps - (2, 0)/800x704 crop]
                      804x704 [130.06 fps - (0, 0)/804x704 crop]
naushir commented 6 months ago

I think I may see the problem. Your sensor driver is advertising Mono formats (R8 / R10_CSI2P) as well as Bayer formats (SGBRG10_CSI2P / SGBRG8). You should only have the Mono ones defined in the sensor - I assume this is a mono sensor? Otherwise, I can see the pipeline handler getting very confused with the mode selection routine.

mugnierb commented 6 months ago

Thanks a lot. You're correct, this is a mono sensor. Adding these modes are a hack for backward compatibility with very old kernel that only handle RGB. It works by removing the Bayer formats.

Any way not to confuse the pipeline like before ? So I could keep my Bayer formats.

naushir commented 6 months ago

Any way not to confuse the pipeline like before ? So I could keep my Bayer formats.

I think it's unlikely we can change anything in the pipeline handler to account for this without things going wrong elsewhere. But you might be able to get things to work if you manually choose the mode through the --mode or --viewfinder-mode command line argument in rpicam-apps. Could you give that a try?

mugnierb commented 6 months ago

Hum according to the --help :

  --mode arg                            Camera mode as W:H:bit-depth:packing, where packing is P (packed) or U 
                                        (unpacked)

I can't find a way to specify RGB or Mono. I tried this line that does not specify it

rpicam-hello --mode "640:480:10:P"

But got the same output.

I still wonder 2 things :

naushir commented 6 months ago

I can't find a way to specify RGB or Mono. I tried this line that does not specify it

There's no way to do this as we only ever expect either RGB or Mono formats set in a mode.

  • Why the issue is not happening at all in cam/qcam.

This is possibly because qcam/cam has no option to manually select a sensor mode from the application, whereas rpicam-apps does. It's quite hard to tell without going through with a debugger and seeing what code path the mode selection is taking.

  • Why was it handled correctly before. Even if supporting both RAW and Bayer formats is kind of hacky.

There's been quite a few changes to the mode selection code, probably fixing things along the way. Reverting whatever broke this use case will probably break other legitimate use cases.

One other thing to try would be to use the --no-raw argument to your command line. This switches off the mode selection in rpicam-apps and uses the pipeline handler to do the mode selection as is done in cam/qcam. If those two apps work as expected, the --no-raw option might also work.

mugnierb commented 6 months ago

--no-raw gives me the correct format

[4:43:02.553938509] [3773]  INFO RPI vc4.cpp:629 Sensor: /base/soc/i2c0mux/i2c@1/wolfy@10 - Selected sensor format: 640x480-SGBRG10_1X10 - Selected unicam format: 640x480-pGAA

Anyway, having both RGB and Mono modes is bugged. I think I'll remove hacky RGB modes and look for another way to handle older kernels and ISP drivers. I'll debug a bit more libcamera to see what changed in the mode selection code (btw if you have any pointers I'll glady accept them).

Thanks a lot for your support.

naushir commented 6 months ago

Great, you can find the rpicam-app code for mode selection here: https://github.com/raspberrypi/rpicam-apps/blob/c8351a09174f143424884081a69b6dd8018f4ba3/core/rpicam_app.cpp#L184

I'll close this down now, please do reopen if you find a fix.