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

[Laurent Pinchart]

On Wed, Mar 13, 2024 at 06:16:15PM +0200, Tomi Valkeinen wrote:
> 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.

Sounds like a bug.

> 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...

-- 
Regards,

Laurent Pinchart