Hi Philipp,
On Wednesday 27 August 2014 11:30:14 Philipp Zabel wrote:
> Am Dienstag, den 26.08.2014, 12:01 +0200 schrieb Laurent Pinchart:
> [...]
>
> > > +};
> > > +
> > > +const struct v4l2_pixfmt *v4l2_pixfmt_by_fourcc(u32 fourcc)
> > > +{
> > > + int i;
> >
> > The loop counter is always positive, it can be an unsigned int.
>
> I'll change that.
>
> > > + for (i = 0; i < ARRAY_SIZE(v4l2_pixfmts); i++) {
> > > + if (v4l2_pixfmts[i].pixelformat == fourcc)
> > > + return v4l2_pixfmts + i;
> > > + }
> >
> > We currently have 123 pixel formats defined, and that number will keep
> > increasing. I wonder if something more efficient than an O(n) array lookup
> > would be worth it.
>
> How about a function similar to soc_mbus_find_fmtdesc that uses an array
> provided by the driver:
>
> const struct v4l2_pixfmt_info *v4l2_find_pixfmt(u32 pixelformat,
> const struct v4l2_pixfmt_info *array, unsigned int len)
> {
> unsigned int i;
>
> for (i = 0; i < len; i++) {
> if (pixelformat == array[i].pixelformat)
> return array + i;
> }
>
> return NULL;
> }
>
> And a function to fill this driver specific array from the global array
> once:
>
> void v4l2_init_pixfmt_array(struct v4l2_pixfmt_info *array, int len)
> {
> unsigned int i;
>
> for (i = 0; i < len; i++)
> array[i] = *v4l2_pixfmt_by_fourcc(array[i].pixelformat);
> }
>
> A driver could then do the following:
>
> static struct v4l2_pixfmt_info driver_formats[] = {
> { .pixelformat = V4L2_PIX_FMT_YUYV },
> { .pixelformat = V4L2_PIX_FMT_YUV420 },
> };
>
> int driver_probe(...)
> {
> ...
> v4l2_init_pixfmt_array(driver_formats,
> ARRAY_SIZE(driver_formats));
> ...
> }
Good question. This option consumes more memory, and prevents the driver-
specific format info arrays to be const, which bothers me a bit. On the other
hand it allows drivers to override some of the default values for odd cases. I
won't nack this approach, but I'm wondering whether a better solution wouldn't
be possible. Hans, Mauro, Guennadi, any opinion ?
> [...]
>
> > > +unsigned int v4l2_sizeimage(const struct v4l2_pixfmt *fmt, unsigned int
> > > width,
> > > + unsigned int height)
> > > +{
> >
> > A small comment would be useful here to explain why we don't round up in
> > the second case.
>
> Agreed, I think the YUV410 case is a good example for this.
>
> [...]
>
> > > +/**
> > > + * struct v4l2_pixfmt - internal V4L2 pixel format description
> >
> > Maybe struct v4l2_pixfmt_info ?
>
> That's fine with me.
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
