[PATCH] Revert "libcamera: rkisp1: Eliminate hard-coded resizer limits"

[Quentin Schulz]

Hi Jacopo,

On 4/4/25 8:56 AM, Jacopo Mondi wrote:
> Hi Quentin
> 
> On Thu, Apr 03, 2025 at 08:09:00PM +0200, Quentin Schulz wrote:
>> From: Quentin Schulz <quentin.schulz at cherry.de>
>>
>> This reverts commit e85c7ddd38ce8456ab01c2a73baf9e788f6a462e.
>>
>> Linux kernel predating 6.4 (specifically commit 7cfb35d3a800 ("media:
>> rkisp1: Implement ENUM_FRAMESIZES") do not have the ioctl in rkisp1
>> driver required to dynamically query the resizer limits.
>>
>> Because of that, maxResolution and minResolution are both {0, 0}
>> (default value for Size objects) which means filterSensorResolution()
>> will create an entry for the sensor in sensorSizesMap_ but because the
>> sensor resolution cannot fit inside the min and max resolution of the
>> rkisp1, no size is put into this entry in sensorSizesMap_.
>> On the next call to filterSensorResolution(),
>> sensorSizesMap_.find(sensor) will return the entry but when attempting
>> to call back() on iter->second, it'll trigger an assert because the size
>> array is empty.
>>
>> Linux kernel 6.1 is supported until December 2027, so it seems premature
>> to get rid of those hard-coded resizer limits before this happens.
>>
>> Let's keep the hard-coded resizer limits by default, they can still be
>> queried if necessary.
> 
> So I presume you hit
> 
>          LOG(RkISP1, Info)
>                  << "Failed to enumerate supported formats and sizes, using defaults";
> 

Correct. On v0.4.0 (latest version in Buildroot):

# cam -c1 -C15 -F --stream pixelformat=NV12,height=1440,width=1920 
--strict-formats
[0:02:21.164554400] [2523]  INFO Camera camera_manager.cpp:327 libcamera 
v0.4.0
[0:02:21.189272566] [2524] ERROR V4L2 v4l2_videodevice.cpp:1208 
/dev/video0[17:cap]: Unable to enumerate frame sizes: Inappropriate 
ioctl for device
[0:02:21.189448733] [2524]  INFO RkISP1 rkisp1_path.cpp:88 Failed to 
enumerate supported formats and sizes, using defaults
[0:02:21.189828483] [2524] ERROR V4L2 v4l2_videodevice.cpp:1208 
/dev/video1[19:cap]: Unable to enumerate frame sizes: Inappropriate 
ioctl for device
[0:02:21.189896150] [2524]  INFO RkISP1 rkisp1_path.cpp:88 Failed to 
enumerate supported formats and sizes, using defaults
[0:02:21.197233900] [2524]  WARN CameraSensor 
camera_sensor_legacy.cpp:501 'ov5675 2-0036': No sensor delays found in 
static properties. Assuming unverified defaults.
[0:02:21.206403316] [2524]  WARN RkISP1Agc agc.cpp:46 'AeMeteringMode' 
parameter not found in tuning file
[0:02:21.206555566] [2524]  WARN RkISP1Agc agc.cpp:69 No metering modes 
read from tuning file; defaulting to matrix
[0:02:21.206653858] [2524] ERROR Interpolator interpolator.h:50 yaml 
object must be a list
[0:02:21.206697025] [2524]  WARN RkISP1Awb awb.cpp:61 Failed to parse 
'colourGains' parameter from tuning file; manual colour temperature will 
not work properly
/home/qschulz/work/cobra/build-pp1516-debug/host/opt/ext-toolchain/aarch64-none-linux-gnu/include/c++/14.2.1/bits/stl_vector.h:1237: 
std::vector<_Tp, _Alloc>::reference std::vector<_Tp, _Alloc>::back() 
[with _Tp = libcamera::Size; _Alloc = std::allocator<libcamera::Size>; 
reference = libcamera::Size&]: Assertion '!this->empty()' failed.
Aborted

>>
>> Fixes: 761545407c76 ("pipeline: rkisp1: Filter out sensor sizes not supported by the pipeline")
> 
> Not sure about the fixes tag here, but indeed if this fixes an issue
> on an older but not-yet-dead kernel at the cheap cost of a having a
> few defaults here, then:
> 

Fair, me neither. It's just that the part that triggers the assert 
without this revert is added in that patch.

I'm not too bothered if there isn't a Fixes: tag, so up to you.

Cheers,
Quentin