[libcamera-devel] Issues with ControlSerializer

[paul.elder at ideasonboard.com]

Hello,

While Jacopo was trying to send ControlInfoMaps over IPC and actually
try to run predicates on it, it came to our attention that
ControlInfoMaps are serialized by the ControlSerializer in such a way
that some assumptions are broken.

When the ControlSerializer serializes ControlInfoMaps, it discards the
ControlId's name, and the ControlInfo's default value. Everything else
(ControlId's id and type, and ControlInfo's min and max) are preserved.


Let's start with the discarding of the ControlId's name. The consequence
of this is that we are unable to use ControlInfoMap::find() with a
deserialized ControlId and expect it to work.

Let's look at an example. We pass a ControlInfoMap output argument to an
IPA call, from the pipeline handler, over IPC:

	ControlInfoMap ipaControls = {};
	ipa_->init(&ipaControls);

The IPA adds a control and returns it:

	ipaControls[&controls::ExposureTime] = ControlInfo(41, 33306, 0);

Back in the pipeline handler, we try to find the control:

	const auto exposureInfo = ipaControls_.find(&controls::ExposureTime);

Here, exposureInfo is a ControlId with id = EXPOSURE_TIME, name = "",
and type = int32_t. Meanwhile controls::ExposureTime is a
Control<int32_t> with id = EXPOSURE_TIME, name = "ExposureTime", and
type = int32_t. Clearly these two are not equal, as the name does not
match, so if we try to find the control from the ControlInfoMap that was
returned by the IPA, it will fail:

	if (exposureInfo == ipaControls_.end()) {
		LOG(IPU3, Error) << "Exposure control not initializaed by the IPA";
		return -EINVAL;
	}

Since the ControlId name is tied to the ControlId's id, I don't think
it's actually necessary to serialize the name as well. I think a
solution would be to provide a comparator for ControlIds, which ignores
the name and only considers the id and type. Is this a fine solution?


Next let's look at the discarding of the ControlInfo's default value.
The consequence of this is that we cannot rely on using the default
value to report any capabilities from the IPA, and we can only use
min/max. This may or may not be an issue, so I just want to prime a
discussion about this. It seems that the default value of a ControlInfo
is not important at all, as it is ignored by the serializer, and it is
also ignored by the comparator. So it seems to me that at the moment
nothing can expect the default value to hold any valuable information,
only suggestions. Do we want to change this, or leave it as-is?


Thanks,

Paul