Theoretical assert when filtering resolutions on RkISP

[Quentin Schulz]

Hi all,

Looking at the code that triggered an assert when running an "old" 
kernel (5.10 for me but should happen until 6.4) on a Rockchip board 
(PX30 based)[1] I think I've identified another way to trigger that 
assert (on recent kernels).

I'm not sure it's reasonable to expect it to happen and I for sure do 
not know what to do, safely.

So here goes nothing:

https://git.libcamera.org/libcamera/libcamera.git/tree/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp#n139

"""
Size RkISP1Path::filterSensorResolution(const CameraSensor *sensor)
{
	auto iter = sensorSizesMap_.find(sensor);
	if (iter != sensorSizesMap_.end())
		return iter->second.back();

	std::vector<Size> &sizes = sensorSizesMap_[sensor];
	for (unsigned int code : sensor->mbusCodes()) {
		for (const Size &size : sensor->sizes(code)) {
			if (size.width > maxResolution_.width ||
			    size.height > maxResolution_.height)
				continue;

			sizes.push_back(size);
		}
	}

	/* Sort in increasing order and remove duplicates. */
	std::sort(sizes.begin(), sizes.end());
	auto last = std::unique(sizes.begin(), sizes.end());
	sizes.erase(last, sizes.end());

	return sizes.back();
}
"""

sensorSizesmap_ is an std::map<const CameraSensor *, std::vector<Size>>.

On first call to this function for sensor X, the first part till the 
for-loop is "skipped" because the sensor isn't present in the map. Then with

sensorSizesMap_[sensor];

it creates a new entry in the map, c.f. 
https://en.cppreference.com/w/cpp/container/map/operator_at

"""
Returns a reference to the value that is mapped to a key equivalent to 
key or x respectively, performing an insertion if such key does not 
already exist.
"""

So now we have a new entry in the map but without anything in 
map->second (or rather an empty std::vector).

Then we filter the sensor resolutions based on the min and max 
resolutions supported by the pipeline. However, it could (at least 
theoretically) be possible to have no possible subset of sensor 
resolution for the pipeline, in which case sizes vector is empty.

I have no clue if that would trigger an issue in the three lines after 
the end of the for loop but it'll for sure trigger an assert in the 
return statement as sizes.back() will assert on !this->empty() that I 
got in [2].

[1] 
https://lists.libcamera.org/pipermail/libcamera-devel/2025-April/049691.html
[2] 
https://lists.libcamera.org/pipermail/libcamera-devel/2025-April/049699.html

An option to not trigger the assert (here) would be to remove the sensor 
from the map if no compatible resolution is found, and then return.... 
what exactly? It seems callers of filterSensorResolution() always expect 
a resolution to be found. Maybe 0 for maxResolution and (unsigned int)-1 
for minResolution? Not sure if the rest of the code is able to handle 
that though. We could also simply insert the same dummy max and min 
resolutions for the sensor instead of removing the sensor from the map 
(same outcome, but we then avoid running through the forloop every time 
someone asks to filter the resolutions for that sensor.

Cheers,
Quentin