On Sunday, September 26, 2010 19:34:49 Laurent Pinchart wrote:
> Hi Hans,
>
> On Sunday 26 September 2010 19:29:53 Hans Verkuil wrote:
> > On Sunday, September 26, 2010 18:13:25 Laurent Pinchart wrote:
> > > Adding new pixel codes at the end of the enumeration will soon create a
> > > mess, so sort the pixel codes by type and then sort them alphabetically.
> > >
> > > As the codes are part of the kernel ABI their value can't change when a
> > > new code is inserted in the enumeration, so they are given an explicit
> > > numerical value. When inserting a new pixel code developers must use and
> > > update the V4L2_MBUS_FMT_LAST value.
> > >
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> > > ---
> > >
> > > include/linux/v4l2-mediabus.h | 54
> > > ++++++++++++++++++++++++---------------- 1 files changed, 32
> > > insertions(+), 22 deletions(-)
> > >
> > > diff --git a/include/linux/v4l2-mediabus.h
> > > b/include/linux/v4l2-mediabus.h index 127512a..bc637a5 100644
> > > --- a/include/linux/v4l2-mediabus.h
> > > +++ b/include/linux/v4l2-mediabus.h
> > > @@ -24,31 +24,41 @@
> > >
> > > * transferred first, "BE" means that the most significant bits are
> > > transferred * first, and "PADHI" and "PADLO" define which bits - low
> > > or high, in the * incomplete high byte, are filled with padding bits.
> > >
> > > + *
> > > + * The pixel codes are grouped by types and (mostly) sorted
> > > alphabetically. As + * their value can't change when a new pixel code is
> > > inserted in the + * enumeration, they are explicitly given a numerical
> > > value. When inserting a + * new pixel code use and update the
> > > V4L2_MBUS_FMT_LAST value.
> > >
> > > */
> > >
> > > enum v4l2_mbus_pixelcode {
> > >
> > > V4L2_MBUS_FMT_FIXED = 1,
> > >
> > > - V4L2_MBUS_FMT_YUYV8_2X8,
> > > - V4L2_MBUS_FMT_YVYU8_2X8,
> > > - V4L2_MBUS_FMT_UYVY8_2X8,
> > > - V4L2_MBUS_FMT_VYUY8_2X8,
> > > - V4L2_MBUS_FMT_RGB555_2X8_PADHI_LE,
> > > - V4L2_MBUS_FMT_RGB555_2X8_PADHI_BE,
> > > - V4L2_MBUS_FMT_RGB565_2X8_LE,
> > > - V4L2_MBUS_FMT_RGB565_2X8_BE,
> > > - V4L2_MBUS_FMT_SBGGR8_1X8,
> > > - V4L2_MBUS_FMT_SBGGR10_1X10,
> > > - V4L2_MBUS_FMT_GREY8_1X8,
> > > - V4L2_MBUS_FMT_Y10_1X10,
> > > - V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_LE,
> > > - V4L2_MBUS_FMT_SBGGR10_2X8_PADLO_LE,
> > > - V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_BE,
> > > - V4L2_MBUS_FMT_SBGGR10_2X8_PADLO_BE,
> > > - V4L2_MBUS_FMT_SGRBG8_1X8,
> > > - V4L2_MBUS_FMT_SBGGR12_1X12,
> > > - V4L2_MBUS_FMT_YUYV8_1_5X8,
> > > - V4L2_MBUS_FMT_YVYU8_1_5X8,
> > > - V4L2_MBUS_FMT_UYVY8_1_5X8,
> > > - V4L2_MBUS_FMT_VYUY8_1_5X8,
> > > + /* RGB */
> > > + V4L2_MBUS_FMT_RGB555_2X8_PADHI_BE = 7,
> > > + V4L2_MBUS_FMT_RGB555_2X8_PADHI_LE = 6,
> > > + V4L2_MBUS_FMT_RGB565_2X8_BE = 9,
> > > + V4L2_MBUS_FMT_RGB565_2X8_LE = 8,
> > > + /* YUV (including grey) */
> > > + V4L2_MBUS_FMT_GREY8_1X8 = 12,
> > > + V4L2_MBUS_FMT_Y10_1X10 = 13,
> > > + V4L2_MBUS_FMT_YUYV8_1_5X8 = 20,
> > > + V4L2_MBUS_FMT_YVYU8_1_5X8 = 21,
> > > + V4L2_MBUS_FMT_UYVY8_1_5X8 = 22,
> > > + V4L2_MBUS_FMT_VYUY8_1_5X8 = 23,
> > > + V4L2_MBUS_FMT_YUYV8_2X8 = 2,
> > > + V4L2_MBUS_FMT_UYVY8_2X8 = 4,
> > > + V4L2_MBUS_FMT_YVYU8_2X8 = 3,
> > > + V4L2_MBUS_FMT_VYUY8_2X8 = 5,
> > > + /* Bayer */
> > > + V4L2_MBUS_FMT_SBGGR8_1X8 = 10,
> > > + V4L2_MBUS_FMT_SBGGR10_1X10 = 11,
> > > + V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_BE = 16,
> > > + V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_LE = 14,
> > > + V4L2_MBUS_FMT_SBGGR10_2X8_PADLO_BE = 17,
> > > + V4L2_MBUS_FMT_SBGGR10_2X8_PADLO_LE = 15,
> > > + V4L2_MBUS_FMT_SBGGR12_1X12 = 19,
> > > + V4L2_MBUS_FMT_SGRBG8_1X8 = 18,
> >
> > Why on earth would you want to keep the original values? It was internal
> > only, so no need to stick to the old values.
>
> Good point :-)
>
> > Just let RGB start at 1000, YUV at 2000 and Bayer at 3000 or something like
> > that.
> >
> > > + /* Last - Update this when adding a new pixel code */
> > > + V4L2_MBUS_FMT_LAST = 24,
> >
> > Why would you need this?
>
> We've discussed this in the past. Keeping holes won't help, as we will have to
> insert formats in places where we won't have holes sooner or later. We agreed
> that it was better to keep the list organized by format, as that's easier to
> read. V4L2_MBUS_FMT_LAST is thus a shortcut to avoid searching for the highest
> format when adding a new one. As the constant itself isn't really needed, we
> can keep the highest allocated format code in a comment instead.
I would definitely make it a comment. Actually, a counter should be kept per
section. It might actually be useful in the future if you can do something like:
if ((mbus_fmt & MBUS_FMT_TYPE_MASK) == MBUS_FMT_TYPE_YUV)
Regards,
Hans
--
Hans Verkuil - video4linux developer - sponsored by TANDBERG, part of Cisco
--
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
