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 time on it (and it is the reason why I'm so picky 
 about it :-) ).

The whole then becomes:

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

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 g.liakhovet...@gmx.de
   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 g.liakhovet...@gmx.de
   ---
  
   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 -
 
  cut
 
   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 g.liakhovet...@gmx.de
   + *
   + * 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 similarly 

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 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-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 g.liakhovet...@gmx.de
  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 g.liakhovet...@gmx.de
  ---
  
  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 -
 
 cut
 
  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 g.liakhovet...@gmx.de
  + *
  + * 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 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