[PATCH] apps: cam: Fix colorSpace access crash in KMSSink::configure

[Milan Zamazal]

Hi Kieran,

Kieran Bingham <kieran.bingham at ideasonboard.com> writes:

> Quoting Milan Zamazal (2025-03-10 11:06:30)
>> cfg.colorSpace may be unset in KMSSink::configure, resulting in a crash
>> when it is accessed.  If cfg.colorSpace is unset, simply return, the
>> same way as when YcbcrEncoding is set to None.
>
> I think this is something that we should ensure is trapped by
> lc-compliance in fact.
>
> I believe pipeline handlers /must/ always set the correct colorSpace
> after validate - so it's incorrect for applications to ever hit an
> undefined color space ...

I'm not sure whether all the pipelines do that; at least `simple'
doesn't.  I can fix `simple' pipeline but maybe some others have the
problem too.

> Of course crashing isn't nice either ... 

Let's have an assertion there then to still expose the problem while
crashing in a civilized way?

> but is this occuring in SoftISP/simple pipeline handler ?

Yes.  It started happening to me once I reinstalled my development
system to Fedora 41.  I can't see any obvious reason why it crashes now
and not before (maybe some change in gcc? -- using 14.2.1).

>> Signed-off-by: Milan Zamazal <mzamazal at redhat.com>
>> ---
>>  src/apps/cam/kms_sink.cpp | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>> 
>> diff --git a/src/apps/cam/kms_sink.cpp b/src/apps/cam/kms_sink.cpp
>> index 672c985a..aa9459cf 100644
>> --- a/src/apps/cam/kms_sink.cpp
>> +++ b/src/apps/cam/kms_sink.cpp
>> @@ -153,7 +153,8 @@ int KMSSink::configure(const libcamera::CameraConfiguration &config)
>>         colorEncoding_ = std::nullopt;
>>         colorRange_ = std::nullopt;
>>  
>> -       if (cfg.colorSpace->ycbcrEncoding == libcamera::ColorSpace::YcbcrEncoding::None)
>> +       if (!cfg.colorSpace ||
>> +           cfg.colorSpace->ycbcrEncoding == libcamera::ColorSpace::YcbcrEncoding::None)
>>                 return 0;
>>  
>>         /*
>> -- 
>> 2.48.1
>>