[PATCH/RFC 22/32] libcamera: Add CameraSensor implementation for raw V4L2 sensors

[Tomi Valkeinen]

On 13/03/2024 14:46, Tomi Valkeinen wrote:
> On 13/03/2024 14:20, Naushir Patuck wrote:
> 
>> ?> +               case MediaBusFormatInfo::Type::Metadata:
>>> +                       /*
>>> +                        * Skip metadata streams that are not sensor 
>>> embedded
>>> +                        * data. The source stream reports a generic 
>>> metadata
>>> +                        * format, check the sink stream for the 
>>> exact format.
>>> +                        */
>>> +                       formats = subdev_->formats(route.sink);
>>> +                       if (formats.size() != 1)
>>> +                               continue;
>>
>> Should this test be if (!formats.size()) insead?  It might be possible
>> to have multiple metadata types.
> 
> The driver in my branch is old and hacky. I should see what Laurent has 
> done with the imx219 in his branch, and possibly just take that one.
> 
> I think advertising only a single format makes sense here, as the 
> embedded format is defined by the video format.
> 
>>> +
>>> +                       if 
>>> (MediaBusFormatInfo::info(formats.cbegin()->first).type !=
>>> +                           MediaBusFormatInfo::Type::EmbeddedData)
>>> +                               continue;
>>
>> The IMX219 driver (from Tomi's kernel tree) advertises
>> MEDIA_BUS_FMT_META_8 / MEDIA_BUS_FMT_META_10 formats for the embedded
>> data stream, which translates to a type of
>> MediaBusFormatInfo::Type::Metadata.  Does the driver need updating, or
>> should this check include MediaBusFormatInfo::Type::Metadata?
> 
> Laurent's version should also report those same mbus formats. Hmm, oh, 
> but it uses MEDIA_BUS_FMT_CCS_EMBEDDED for the internal pad...

This looks a bit odd. The driver gives MEDIA_BUS_FMT_CCS_EMBEDDED when 
enumerating the mbus codes, but then always sets the format to META_8 or 
META_10. I'm guessing the driver is supposed to keep the internal pad's 
format as MEDIA_BUS_FMT_CCS_EMBEDDED, and only the external pad would 
use META_8/10...

  Tomi