Re: [PATCH 1/2 v3] v4l: add a media-bus API for configuring v4l2 subdev pixel and frame formats

2009-12-02 Thread Hans Verkuil
On Wednesday 02 December 2009 13:32:35 Guennadi Liakhovetski wrote:
> Hi Hans
>
> On Wed, 2 Dec 2009, Hans Verkuil wrote:
> > On Tuesday 01 December 2009 16:22:55 Guennadi Liakhovetski wrote:
> > > On Tue, 1 Dec 2009, Hans Verkuil wrote:
> > > > On Monday 30 November 2009 14:49:07 Guennadi Liakhovetski wrote:
> > > > > Right, how about this:
> > > > >
> > > > > /*
> > > > >  * These pixel codes uniquely identify data formats on the media
> > > > > bus. Mostly * they correspond to similarly named V4L2_PIX_FMT_*
> > > > > formats, format 0 is * reserved, V4L2_MBUS_FMT_FIXED shall be used
> > > > > by host-client pairs, where the * data format is fixed.
> > > > > Additionally, "2X8" means that one pixel is transferred * in two
> > > > > 8-bit samples, "BE" or "LE" specify in which order those samples
> > > > > are * transferred over the bus: "LE" means that the least
> > > > > significant bits are * 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.
> > > > >  */
> > > > > enum v4l2_mbus_pixelcode {
> > > > >   V4L2_MBUS_FMT_FIXED = 1,
> > > > >   V4L2_MBUS_FMT_YUYV_2X8_LE,
> > > > >   V4L2_MBUS_FMT_YVYU_2X8_LE,
> > > > >   V4L2_MBUS_FMT_UYVY_2X8_LE,
> > > > >   V4L2_MBUS_FMT_VYUY_2X8_LE,
> > > >
> > > > These possibly may need a comment saying that each Y/U/V sample is 8
> > > > bits wide. I'm not sure how far we want to go with systematic naming
> > > > schemes here. Adding a short comment if there is a possible ambiguity
> > > > is probably sufficient.
> > >
> > > Is there an ambiguity? Aren't these formats standardised?
> >
> > HDMI receivers/transmitters can do YUV with 8, 10 or 12 bits. So when you
> > say YUYV_2X8_LE do you mean that 10 bits are transported over two bytes,
> > or that a Y and a U (or V) are transferred one after another? From the
> > absence of a PADHI or PADLO I can infer that it is the latter, but it is
> > not exactly obvious.
> >
> > Actually, why not name these formats YUYV8, etc. and the order of the
> > bytes going over the bus is just the order of the text 'YUYV'. There is
> > not really any big or little endian issues since you just need to know
> > the sequence of Ys, Us and Vs.
>
> Ok, we could keep discussing these things for a while, but I don't think
> we have that time, and it's not _that_ important to me what these things
> will be called - will use whatever names there are.

I agree that this is dragging on a bit too long. The main reason is my busy 
schedule since normally we could hash this out quickly on irc. Apologies for 
that.

> Just to explain 2X8 - this notation comes from packing and means, to get
> one _pixel_ you need two 8-bit wide samples. With YUYV one pixel is
> defined as YU or YV, so, that gives you (at most) 8 bits per component.
>
> Ok, I'm planning to submit a version of this patch a bit later today with
> names like
>
> enum v4l2_mbus_pixelcode {
>   V4L2_MBUS_FMT_FIXED = 1,
>   V4L2_MBUS_FMT_YUYV8,
>   V4L2_MBUS_FMT_YVYU8,
>
> according to your last suggestion.
>
> > > Do we then have
> > > to explain what rgb555 means etc?
> > >
> > > > >   V4L2_MBUS_FMT_RGB555_2X8_PADHI_LE,
> > > > >   V4L2_MBUS_FMT_RGB555X_2X8_PADHI_LE,
> > > >
> > > > Shouldn't this be: V4L2_MBUS_FMT_RGB555_2X8_PADHI_BE? Since the 555X
> > > > format is just the big-endian variant of the RGB555 if I am not
> > > > mistaken.
> > >
> > > No, don't think so. As an RGB555X format it is sent in LE order, if you
> > > send RGB555X in BE order you get RGB555 (without an "X"). In fact,
> > > you'll never have a RGB555X_BE format, because, that's just the
> > > RGB555_LE. So, you may only have BE variants for formats, whoch
> > > byte-swapped variants do not have an own name.
> >
> > RGB 5:5:5 consists of 16 bits argg gggb ('a' is either padding or
> > an alpha bit).
>
> From what I read, RGB555 has high bit unused. With the alpha bit (or
> transparency bit) it's already RGBA or RGBT.
>
> > RGB 5:5:5 over an 8 bit data bus is either with the MSB byte first (big
> > endian aka RGB555X aka RGB555_2X8_PADHI_BE) or with the LSB byte first
> > (little endian aka RGB555 aka RGB555_2X8_PADHI_LE).
> >
> > The use of RGB555X in the pixel formats is a really ugly accident of
> > history. 'RGB555' is the name of the format, and _LE or _BE should define
> > what the order of the LSB and MSB over the data bus is.
>
> So, I'll make them
>
>   V4L2_MBUS_FMT_RGB555_2X8_PADHI_LE,
>   V4L2_MBUS_FMT_RGB555_2X8_PADHI_BE,
>
> and "555X" will just vanish?

Yup.

>
> > > > >   V4L2_MBUS_FMT_RGB565_2X8_LE,
> > > > >   V4L2_MBUS_FMT_RGB565X_2X8_LE,
> > > >
> > > > Ditto.
> > > >
> > > > >   V4L2_MBUS_FMT_SBGGR8_1X8,
> > > > >   V4L2_MBUS_FMT_SBGGR10_1X10,
> > > > >   V4L2_MBUS_FMT_GREY_1X8,
> > > >
> > > > This is also 8 bits per sample, right? This might

Re: [PATCH 1/2 v3] v4l: add a media-bus API for configuring v4l2 subdev pixel and frame formats

2009-12-02 Thread Guennadi Liakhovetski
Hi Hans

On Wed, 2 Dec 2009, Hans Verkuil wrote:

> On Tuesday 01 December 2009 16:22:55 Guennadi Liakhovetski wrote:
> > On Tue, 1 Dec 2009, Hans Verkuil wrote:
> > > On Monday 30 November 2009 14:49:07 Guennadi Liakhovetski wrote:
> > > > Right, how about this:
> > > >
> > > > /*
> > > >  * These pixel codes uniquely identify data formats on the media bus.
> > > > Mostly * they correspond to similarly named V4L2_PIX_FMT_* formats,
> > > > format 0 is * reserved, V4L2_MBUS_FMT_FIXED shall be used by
> > > > host-client pairs, where the * data format is fixed. Additionally,
> > > > "2X8" means that one pixel is transferred * in two 8-bit samples, "BE"
> > > > or "LE" specify in which order those samples are * transferred over the
> > > > bus: "LE" means that the least significant bits are * 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.
> > > >  */
> > > > enum v4l2_mbus_pixelcode {
> > > > V4L2_MBUS_FMT_FIXED = 1,
> > > > V4L2_MBUS_FMT_YUYV_2X8_LE,
> > > > V4L2_MBUS_FMT_YVYU_2X8_LE,
> > > > V4L2_MBUS_FMT_UYVY_2X8_LE,
> > > > V4L2_MBUS_FMT_VYUY_2X8_LE,
> > >
> > > These possibly may need a comment saying that each Y/U/V sample is 8 bits
> > > wide. I'm not sure how far we want to go with systematic naming schemes
> > > here. Adding a short comment if there is a possible ambiguity is probably
> > > sufficient.
> >
> > Is there an ambiguity? Aren't these formats standardised?
> 
> HDMI receivers/transmitters can do YUV with 8, 10 or 12 bits. So when you say
> YUYV_2X8_LE do you mean that 10 bits are transported over two bytes, or that
> a Y and a U (or V) are transferred one after another? From the absence of a 
> PADHI or PADLO I can infer that it is the latter, but it is not exactly 
> obvious.
> 
> Actually, why not name these formats YUYV8, etc. and the order of the bytes 
> going over the bus is just the order of the text 'YUYV'. There is not really 
> any big or little endian issues since you just need to know the sequence of 
> Ys, Us and Vs.

Ok, we could keep discussing these things for a while, but I don't think 
we have that time, and it's not _that_ important to me what these things 
will be called - will use whatever names there are.

Just to explain 2X8 - this notation comes from packing and means, to get 
one _pixel_ you need two 8-bit wide samples. With YUYV one pixel is 
defined as YU or YV, so, that gives you (at most) 8 bits per component.

Ok, I'm planning to submit a version of this patch a bit later today with 
names like

enum v4l2_mbus_pixelcode {
V4L2_MBUS_FMT_FIXED = 1,
V4L2_MBUS_FMT_YUYV8,
V4L2_MBUS_FMT_YVYU8,

according to your last suggestion.

> > Do we then have
> > to explain what rgb555 means etc?
> >
> > > > V4L2_MBUS_FMT_RGB555_2X8_PADHI_LE,
> > > > V4L2_MBUS_FMT_RGB555X_2X8_PADHI_LE,
> > >
> > > Shouldn't this be: V4L2_MBUS_FMT_RGB555_2X8_PADHI_BE? Since the 555X
> > > format is just the big-endian variant of the RGB555 if I am not mistaken.
> >
> > No, don't think so. As an RGB555X format it is sent in LE order, if you
> > send RGB555X in BE order you get RGB555 (without an "X"). In fact, you'll
> > never have a RGB555X_BE format, because, that's just the RGB555_LE. So,
> > you may only have BE variants for formats, whoch byte-swapped variants do
> > not have an own name.
> 
> RGB 5:5:5 consists of 16 bits argg gggb ('a' is either padding or an 
> alpha bit).

>From what I read, RGB555 has high bit unused. With the alpha bit (or 
transparency bit) it's already RGBA or RGBT.

> RGB 5:5:5 over an 8 bit data bus is either with the MSB byte first (big 
> endian 
> aka RGB555X aka RGB555_2X8_PADHI_BE) or with the LSB byte first (little 
> endian 
> aka RGB555 aka RGB555_2X8_PADHI_LE).
> 
> The use of RGB555X in the pixel formats is a really ugly accident of history. 
> 'RGB555' is the name of the format, and _LE or _BE should define what the 
> order of the LSB and MSB over the data bus is.

So, I'll make them

V4L2_MBUS_FMT_RGB555_2X8_PADHI_LE,
V4L2_MBUS_FMT_RGB555_2X8_PADHI_BE,

and "555X" will just vanish?

> > > > V4L2_MBUS_FMT_RGB565_2X8_LE,
> > > > V4L2_MBUS_FMT_RGB565X_2X8_LE,
> > >
> > > Ditto.
> > >
> > > > V4L2_MBUS_FMT_SBGGR8_1X8,
> > > > V4L2_MBUS_FMT_SBGGR10_1X10,
> > > > V4L2_MBUS_FMT_GREY_1X8,
> > >
> > > This is also 8 bits per sample, right? This might be renamed to
> > > GREY8_1X8.
> >
> > I named it after V4L2_PIX_FMT_GREY. If we ever get GREY7 or similar, I
> > think, we anyway will need a new fourcc code for it, then we'll call the
> > MBUS_FMT similarly.
> 
> Why not do it right from the start? Frankly, the PIX_FMT names aren't that 
> great. And since this will become a public API in the future I think it is 
> reasonable to spend some

Re: [PATCH 1/2 v3] v4l: add a media-bus API for configuring v4l2 subdev pixel and frame formats

2009-12-01 Thread Hans Verkuil
On Tuesday 01 December 2009 16:22:55 Guennadi Liakhovetski wrote:
> On Tue, 1 Dec 2009, Hans Verkuil wrote:
> > On Monday 30 November 2009 14:49:07 Guennadi Liakhovetski wrote:
> > > Right, how about this:
> > >
> > > /*
> > >  * These pixel codes uniquely identify data formats on the media bus.
> > > Mostly * they correspond to similarly named V4L2_PIX_FMT_* formats,
> > > format 0 is * reserved, V4L2_MBUS_FMT_FIXED shall be used by
> > > host-client pairs, where the * data format is fixed. Additionally,
> > > "2X8" means that one pixel is transferred * in two 8-bit samples, "BE"
> > > or "LE" specify in which order those samples are * transferred over the
> > > bus: "LE" means that the least significant bits are * 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.
> > >  */
> > > enum v4l2_mbus_pixelcode {
> > >   V4L2_MBUS_FMT_FIXED = 1,
> > >   V4L2_MBUS_FMT_YUYV_2X8_LE,
> > >   V4L2_MBUS_FMT_YVYU_2X8_LE,
> > >   V4L2_MBUS_FMT_UYVY_2X8_LE,
> > >   V4L2_MBUS_FMT_VYUY_2X8_LE,
> >
> > These possibly may need a comment saying that each Y/U/V sample is 8 bits
> > wide. I'm not sure how far we want to go with systematic naming schemes
> > here. Adding a short comment if there is a possible ambiguity is probably
> > sufficient.
>
> Is there an ambiguity? Aren't these formats standardised?

HDMI receivers/transmitters can do YUV with 8, 10 or 12 bits. So when you say
YUYV_2X8_LE do you mean that 10 bits are transported over two bytes, or that
a Y and a U (or V) are transferred one after another? From the absence of a 
PADHI or PADLO I can infer that it is the latter, but it is not exactly 
obvious.

Actually, why not name these formats YUYV8, etc. and the order of the bytes 
going over the bus is just the order of the text 'YUYV'. There is not really 
any big or little endian issues since you just need to know the sequence of 
Ys, Us and Vs.

> Do we then have
> to explain what rgb555 means etc?
>
> > >   V4L2_MBUS_FMT_RGB555_2X8_PADHI_LE,
> > >   V4L2_MBUS_FMT_RGB555X_2X8_PADHI_LE,
> >
> > Shouldn't this be: V4L2_MBUS_FMT_RGB555_2X8_PADHI_BE? Since the 555X
> > format is just the big-endian variant of the RGB555 if I am not mistaken.
>
> No, don't think so. As an RGB555X format it is sent in LE order, if you
> send RGB555X in BE order you get RGB555 (without an "X"). In fact, you'll
> never have a RGB555X_BE format, because, that's just the RGB555_LE. So,
> you may only have BE variants for formats, whoch byte-swapped variants do
> not have an own name.

RGB 5:5:5 consists of 16 bits argg gggb ('a' is either padding or an 
alpha bit).

RGB 5:5:5 over an 8 bit data bus is either with the MSB byte first (big endian 
aka RGB555X aka RGB555_2X8_PADHI_BE) or with the LSB byte first (little endian 
aka RGB555 aka RGB555_2X8_PADHI_LE).

The use of RGB555X in the pixel formats is a really ugly accident of history. 
'RGB555' is the name of the format, and _LE or _BE should define what the 
order of the LSB and MSB over the data bus is.

>
> > >   V4L2_MBUS_FMT_RGB565_2X8_LE,
> > >   V4L2_MBUS_FMT_RGB565X_2X8_LE,
> >
> > Ditto.
> >
> > >   V4L2_MBUS_FMT_SBGGR8_1X8,
> > >   V4L2_MBUS_FMT_SBGGR10_1X10,
> > >   V4L2_MBUS_FMT_GREY_1X8,
> >
> > This is also 8 bits per sample, right? This might be renamed to
> > GREY8_1X8.
>
> I named it after V4L2_PIX_FMT_GREY. If we ever get GREY7 or similar, I
> think, we anyway will need a new fourcc code for it, then we'll call the
> MBUS_FMT similarly.

Why not do it right from the start? Frankly, the PIX_FMT names aren't that 
great. And since this will become a public API in the future I think it is 
reasonable to spend some time on it (and it is the reason why I'm so picky 
about it :-) ).

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2 v3] v4l: add a media-bus API for configuring v4l2 subdev pixel and frame formats

2009-12-01 Thread Guennadi Liakhovetski
On Tue, 1 Dec 2009, Hans Verkuil wrote:

> On Monday 30 November 2009 14:49:07 Guennadi Liakhovetski wrote:
> >
> > Right, how about this:
> >
> > /*
> >  * These pixel codes uniquely identify data formats on the media bus.
> > Mostly * they correspond to similarly named V4L2_PIX_FMT_* formats, format
> > 0 is * reserved, V4L2_MBUS_FMT_FIXED shall be used by host-client pairs,
> > where the * data format is fixed. Additionally, "2X8" means that one pixel
> > is transferred * in two 8-bit samples, "BE" or "LE" specify in which order
> > those samples are * transferred over the bus: "LE" means that the least
> > significant bits are * 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.
> >  */
> > enum v4l2_mbus_pixelcode {
> > V4L2_MBUS_FMT_FIXED = 1,
> > V4L2_MBUS_FMT_YUYV_2X8_LE,
> > V4L2_MBUS_FMT_YVYU_2X8_LE,
> > V4L2_MBUS_FMT_UYVY_2X8_LE,
> > V4L2_MBUS_FMT_VYUY_2X8_LE,
> 
> These possibly may need a comment saying that each Y/U/V sample is 8 bits 
> wide. I'm not sure how far we want to go with systematic naming schemes here. 
> Adding a short comment if there is a possible ambiguity is probably 
> sufficient.

Is there an ambiguity? Aren't these formats standardised? Do we then have 
to explain what rgb555 means etc?

> > V4L2_MBUS_FMT_RGB555_2X8_PADHI_LE,
> > V4L2_MBUS_FMT_RGB555X_2X8_PADHI_LE,
> 
> Shouldn't this be: V4L2_MBUS_FMT_RGB555_2X8_PADHI_BE? Since the 555X format 
> is 
> just the big-endian variant of the RGB555 if I am not mistaken.

No, don't think so. As an RGB555X format it is sent in LE order, if you 
send RGB555X in BE order you get RGB555 (without an "X"). In fact, you'll 
never have a RGB555X_BE format, because, that's just the RGB555_LE. So, 
you may only have BE variants for formats, whoch byte-swapped variants do 
not have an own name.

> > V4L2_MBUS_FMT_RGB565_2X8_LE,
> > V4L2_MBUS_FMT_RGB565X_2X8_LE,
> 
> Ditto.
> 
> > V4L2_MBUS_FMT_SBGGR8_1X8,
> > V4L2_MBUS_FMT_SBGGR10_1X10,
> > V4L2_MBUS_FMT_GREY_1X8,
> 
> This is also 8 bits per sample, right? This might be renamed to GREY8_1X8.

I named it after V4L2_PIX_FMT_GREY. If we ever get GREY7 or similar, I 
think, we anyway will need a new fourcc code for it, then we'll call the 
MBUS_FMT similarly.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2 v3] v4l: add a media-bus API for configuring v4l2 subdev pixel and frame formats

2009-12-01 Thread Hans Verkuil
On Monday 30 November 2009 14:49:07 Guennadi Liakhovetski wrote:
> On Mon, 30 Nov 2009, Hans Verkuil wrote:
> > On Thu, 26 Nov 2009, Guennadi Liakhovetski wrote:
> > > > From 8b24c617e1ac4d324538a3eec476d48b85c2091f Mon Sep 17 00:00:00
> > > > 2001
> > >
> > > From: Guennadi Liakhovetski 
> > > Date: Thu, 26 Nov 2009 18:20:45 +0100
> > > Subject: [PATCH] v4l: add a media-bus API for configuring v4l2 subdev
> > > pixel and frame formats
> > >
> > > Video subdevices, like cameras, decoders, connect to video bridges over
> > > specialised busses. Data is being transferred over these busses in
> > > various formats, which only loosely correspond to fourcc codes,
> > > describing how video data is stored in RAM. This is not a one-to-one
> > > correspondence, therefore we cannot use fourcc codes to configure
> > > subdevice output data formats. This patch
> > > adds codes for several such on-the-bus formats and an API, similar to
> > > the familiar .s_fmt(), .g_fmt(), .try_fmt(), .enum_fmt() API for
> > > configuring those
> > > codes. After all users of the old API in struct v4l2_subdev_video_ops
> > > are converted, it will be removed. Also add helper routines to support
> > > generic pass-through mode for the soc-camera framework.
> > >
> > > Signed-off-by: Guennadi Liakhovetski 
> > > ---
> > >
> > > v2 -> v3: more comments:
> > >
> > > 1. moved enum v4l2_mbus_packing, enum v4l2_mbus_order, and struct
> > > v4l2_mbus_pixelfmt to soc-camera specific header, renamed them into
> > > soc-namespace.
> > >
> > > 2. commented enum v4l2_mbus_pixelcode and removed unused values.
> > >
> > > v1 -> v2: addressed comments from Hans, namely:
> > >
> > > 1. renamed image bus to media bus, now using "mbus" as a shorthand in
> > > function and data type names.
> > >
> > > 2. made media-bus helper functions soc-camera local.
> > >
> > > 3. moved colorspace from struct v4l2_mbus_pixelfmt to struct
> > > v4l2_mbus_framefmt.
> > >
> > > 4. added documentation for data types and enums.
> > >
> > > 5. added
> > >
> > >  V4L2_MBUS_FMT_FIXED = 1,
> > >
> > > format as the first in enum.
> > >
> > > soc-camera driver port will follow tomorrow.
> > >
> > > Thanks
> > > Guennadi
> > >
> > >
> > > drivers/media/video/Makefile   |2 +-
> > > drivers/media/video/soc_mediabus.c |  157
> > > 
> > > include/media/soc_mediabus.h   |   84 +++
> > > include/media/v4l2-mediabus.h  |   59 ++
> > > include/media/v4l2-subdev.h|   19 -
> >
> > 
> >
> > > diff --git a/include/media/v4l2-mediabus.h
> > > b/include/media/v4l2-mediabus.h new file mode 100644
> > > index 000..359840c
> > > --- /dev/null
> > > +++ b/include/media/v4l2-mediabus.h
> > > @@ -0,0 +1,59 @@
> > > +/*
> > > + * Media Bus API header
> > > + *
> > > + * Copyright (C) 2009, Guennadi Liakhovetski 
> > > + *
> > > + * This program is free software; you can redistribute it and/or
> > > modify + * it under the terms of the GNU General Public License version
> > > 2 as + * published by the Free Software Foundation.
> > > + */
> > > +
> > > +#ifndef V4L2_MEDIABUS_H
> > > +#define V4L2_MEDIABUS_H
> > > +
> > > +/*
> > > + * These pixel codes uniquely identify data formats on the media bus.
> > > Mostly
> > > + * they correspond to similarly named V4L2_PIX_FMT_* formats, format 0
> > > is + * reserved, V4L2_MBUS_FMT_FIXED shall be used by host-client
> > > pairs, where the
> > > + * data format is fixed. Additionally, "2X8" means that one pixel is
> > > transferred
> > > + * in two 8-bit samples, "BE" or "LE" specify in which order those
> > > samples + * should be stored in RAM, and "PADHI" and "PADLO" define
> > > which bits - low or
> > > + * high, in the incomplete high byte, are filled with padding bits.
> > > + */
> >
> > Hi Guennadi,
> >
> > This still needs some improvement. There are two things that need to be
> > changed here:
> >
> > 1) add the width of the data bus to the format name: so
> > V4L2_MBUS_FMT_YUYV becomes _FMT_YUYV_8. This is required since I know
> > several video receivers that can be programmed to use one of several data
> > widths (8, 10, 12 bits per color component). Perhaps _1X8 is even better
> > since that is nicely consistent with the 2X8 suffix that you already use.
> > The PADHI and PADLO naming convention
> > is fine and should be used whereever it is neeeded. Ditto for BE and LE.
>
> Ok, my first thought was "no, there only very seldom will be a need for a
> different padding / order / bus-width," but the second thought was "but
> if there ever will be one, you'll have to rename the original code too..."
>
> > 2) Rephrase '"BE" or "LE" specify in which order those samples should be
> > stored
> > in RAM' to:
> >
> > '"BE" or "LE" specify in which order those samples are transferred over
> > the bus:
> > BE means that the most significant bits are transferred first, "LE" means
> > that the least significant bits are transferred first.'
> >
> > This also m

Re: [PATCH 1/2 v3] v4l: add a media-bus API for configuring v4l2 subdev pixel and frame formats

2009-11-30 Thread Guennadi Liakhovetski
On Mon, 30 Nov 2009, Hans Verkuil wrote:

> On Thu, 26 Nov 2009, Guennadi Liakhovetski wrote:
> 
> > > From 8b24c617e1ac4d324538a3eec476d48b85c2091f Mon Sep 17 00:00:00 2001
> > From: Guennadi Liakhovetski 
> > Date: Thu, 26 Nov 2009 18:20:45 +0100
> > Subject: [PATCH] v4l: add a media-bus API for configuring v4l2 subdev pixel
> > and frame formats
> > 
> > Video subdevices, like cameras, decoders, connect to video bridges over
> > specialised busses. Data is being transferred over these busses in various
> > formats, which only loosely correspond to fourcc codes, describing how video
> > data is stored in RAM. This is not a one-to-one correspondence, therefore we
> > cannot use fourcc codes to configure subdevice output data formats. This
> > patch
> > adds codes for several such on-the-bus formats and an API, similar to the
> > familiar .s_fmt(), .g_fmt(), .try_fmt(), .enum_fmt() API for configuring
> > those
> > codes. After all users of the old API in struct v4l2_subdev_video_ops are
> > converted, it will be removed. Also add helper routines to support generic
> > pass-through mode for the soc-camera framework.
> > 
> > Signed-off-by: Guennadi Liakhovetski 
> > ---
> > 
> > v2 -> v3: more comments:
> > 
> > 1. moved enum v4l2_mbus_packing, enum v4l2_mbus_order, and struct
> > v4l2_mbus_pixelfmt to soc-camera specific header, renamed them into
> > soc-namespace.
> > 
> > 2. commented enum v4l2_mbus_pixelcode and removed unused values.
> > 
> > v1 -> v2: addressed comments from Hans, namely:
> > 
> > 1. renamed image bus to media bus, now using "mbus" as a shorthand in
> > function and data type names.
> > 
> > 2. made media-bus helper functions soc-camera local.
> > 
> > 3. moved colorspace from struct v4l2_mbus_pixelfmt to struct
> > v4l2_mbus_framefmt.
> > 
> > 4. added documentation for data types and enums.
> > 
> > 5. added
> > 
> >  V4L2_MBUS_FMT_FIXED = 1,
> > 
> > format as the first in enum.
> > 
> > soc-camera driver port will follow tomorrow.
> > 
> > Thanks
> > Guennadi
> > 
> > 
> > drivers/media/video/Makefile   |2 +-
> > drivers/media/video/soc_mediabus.c |  157
> > 
> > include/media/soc_mediabus.h   |   84 +++
> > include/media/v4l2-mediabus.h  |   59 ++
> > include/media/v4l2-subdev.h|   19 -
> 
> 
> 
> > diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h
> > new file mode 100644
> > index 000..359840c
> > --- /dev/null
> > +++ b/include/media/v4l2-mediabus.h
> > @@ -0,0 +1,59 @@
> > +/*
> > + * Media Bus API header
> > + *
> > + * Copyright (C) 2009, Guennadi Liakhovetski 
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + */
> > +
> > +#ifndef V4L2_MEDIABUS_H
> > +#define V4L2_MEDIABUS_H
> > +
> > +/*
> > + * These pixel codes uniquely identify data formats on the media bus.
> > Mostly
> > + * they correspond to similarly named V4L2_PIX_FMT_* formats, format 0 is
> > + * reserved, V4L2_MBUS_FMT_FIXED shall be used by host-client pairs, where
> > the
> > + * data format is fixed. Additionally, "2X8" means that one pixel is
> > transferred
> > + * in two 8-bit samples, "BE" or "LE" specify in which order those samples
> > + * should be stored in RAM, and "PADHI" and "PADLO" define which bits - low
> > or
> > + * high, in the incomplete high byte, are filled with padding bits.
> > + */
> 
> Hi Guennadi,
> 
> This still needs some improvement. There are two things that need to be
> changed here:
> 
> 1) add the width of the data bus to the format name: so V4L2_MBUS_FMT_YUYV
> becomes _FMT_YUYV_8. This is required since I know several video receivers
> that can be programmed to use one of several data widths (8, 10, 12 bits per
> color component). Perhaps _1X8 is even better since that is nicely consistent
> with the 2X8 suffix that you already use. The PADHI and PADLO naming
> convention
> is fine and should be used whereever it is neeeded. Ditto for BE and LE.

Ok, my first thought was "no, there only very seldom will be a need for a 
different padding / order / bus-width," but the second thought was "but 
if there ever will be one, you'll have to rename the original code too..."

> 2) Rephrase '"BE" or "LE" specify in which order those samples should be
> stored
> in RAM' to:
> 
> '"BE" or "LE" specify in which order those samples are transferred over the
> bus:
> BE means that the most significant bits are transferred first, "LE" means that
> the least significant bits are transferred first.'
> 
> This also means that formats like RGB555 need to be renamed to
> RGB555_2X8_PADHI_LE.
> 
> I think that this would make this list of pixelcodes unambiguous and
> understandable.

Right, how about this:

/*
 * These pixel codes uniquely identify data formats on the media bus. Mostly
 * they correspond to sim

Re: [PATCH 1/2 v3] v4l: add a media-bus API for configuring v4l2 subdev pixel and frame formats

2009-11-30 Thread Hans Verkuil

On Thu, 26 Nov 2009, Guennadi Liakhovetski wrote:


From 8b24c617e1ac4d324538a3eec476d48b85c2091f Mon Sep 17 00:00:00 2001

From: Guennadi Liakhovetski 
Date: Thu, 26 Nov 2009 18:20:45 +0100
Subject: [PATCH] v4l: add a media-bus API for configuring v4l2 subdev pixel and 
frame formats

Video subdevices, like cameras, decoders, connect to video bridges over
specialised busses. Data is being transferred over these busses in various
formats, which only loosely correspond to fourcc codes, describing how video
data is stored in RAM. This is not a one-to-one correspondence, therefore we
cannot use fourcc codes to configure subdevice output data formats. This patch
adds codes for several such on-the-bus formats and an API, similar to the
familiar .s_fmt(), .g_fmt(), .try_fmt(), .enum_fmt() API for configuring those
codes. After all users of the old API in struct v4l2_subdev_video_ops are
converted, it will be removed. Also add helper routines to support generic
pass-through mode for the soc-camera framework.

Signed-off-by: Guennadi Liakhovetski 
---

v2 -> v3: more comments:

1. moved enum v4l2_mbus_packing, enum v4l2_mbus_order, and struct
v4l2_mbus_pixelfmt to soc-camera specific header, renamed them into
soc-namespace.

2. commented enum v4l2_mbus_pixelcode and removed unused values.

v1 -> v2: addressed comments from Hans, namely:

1. renamed image bus to media bus, now using "mbus" as a shorthand in
function and data type names.

2. made media-bus helper functions soc-camera local.

3. moved colorspace from struct v4l2_mbus_pixelfmt to struct
v4l2_mbus_framefmt.

4. added documentation for data types and enums.

5. added

 V4L2_MBUS_FMT_FIXED = 1,

format as the first in enum.

soc-camera driver port will follow tomorrow.

Thanks
Guennadi


drivers/media/video/Makefile   |2 +-
drivers/media/video/soc_mediabus.c |  157 
include/media/soc_mediabus.h   |   84 +++
include/media/v4l2-mediabus.h  |   59 ++
include/media/v4l2-subdev.h|   19 -





diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h
new file mode 100644
index 000..359840c
--- /dev/null
+++ b/include/media/v4l2-mediabus.h
@@ -0,0 +1,59 @@
+/*
+ * Media Bus API header
+ *
+ * Copyright (C) 2009, Guennadi Liakhovetski 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef V4L2_MEDIABUS_H
+#define V4L2_MEDIABUS_H
+
+/*
+ * These pixel codes uniquely identify data formats on the media bus. Mostly
+ * they correspond to similarly named V4L2_PIX_FMT_* formats, format 0 is
+ * reserved, V4L2_MBUS_FMT_FIXED shall be used by host-client pairs, where the
+ * data format is fixed. Additionally, "2X8" means that one pixel is 
transferred
+ * in two 8-bit samples, "BE" or "LE" specify in which order those samples
+ * should be stored in RAM, and "PADHI" and "PADLO" define which bits - low or
+ * high, in the incomplete high byte, are filled with padding bits.
+ */


Hi Guennadi,

This still needs some improvement. There are two things that need to be changed 
here:

1) add the width of the data bus to the format name: so V4L2_MBUS_FMT_YUYV
becomes _FMT_YUYV_8. This is required since I know several video receivers
that can be programmed to use one of several data widths (8, 10, 12 bits per
color component). Perhaps _1X8 is even better since that is nicely consistent
with the 2X8 suffix that you already use. The PADHI and PADLO naming convention
is fine and should be used whereever it is neeeded. Ditto for BE and LE.

2) Rephrase '"BE" or "LE" specify in which order those samples should be stored
in RAM' to:

'"BE" or "LE" specify in which order those samples are transferred over the bus:
BE means that the most significant bits are transferred first, "LE" means that
the least significant bits are transferred first.'

This also means that formats like RGB555 need to be renamed to 
RGB555_2X8_PADHI_LE.

I think that this would make this list of pixelcodes unambiguous and
understandable.

Regards,

Hans


+enum v4l2_mbus_pixelcode {
+   V4L2_MBUS_FMT_FIXED = 1,
+   V4L2_MBUS_FMT_YUYV,
+   V4L2_MBUS_FMT_YVYU,
+   V4L2_MBUS_FMT_UYVY,
+   V4L2_MBUS_FMT_VYUY,
+   V4L2_MBUS_FMT_RGB555,
+   V4L2_MBUS_FMT_RGB555X,
+   V4L2_MBUS_FMT_RGB565,
+   V4L2_MBUS_FMT_RGB565X,
+   V4L2_MBUS_FMT_SBGGR8,
+   V4L2_MBUS_FMT_SBGGR10,
+   V4L2_MBUS_FMT_GREY,
+   V4L2_MBUS_FMT_Y10,
+   V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_BE,
+   V4L2_MBUS_FMT_SBGGR10_2X8_PADLO_BE,
+   V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_LE,
+   V4L2_MBUS_FMT_SBGGR10_2X8_PADLO_LE,
+};

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html