RE: [PATCH RFC 1/5] V4L: Add V4L2_MBUS_FMT_S5C_UYVY_JPEG_1X8 media bus format

2012-11-06 Thread Vincent ABRIOU
Hi Sakari and Sylwester,

Sorry for the late answer.

 -Original Message-
 From: Sakari Ailus [mailto:sakari.ai...@iki.fi]
 Sent: Monday, October 15, 2012 8:36 PM
 To: Sylwester Nawrocki
 Cc: Vincent ABRIOU; linux-media@vger.kernel.org; a.ha...@samsung.com;
 Laurent Pinchart; hverk...@xs4all.nl; kyungmin.p...@samsung.com;
 sw0312@samsung.com; Nicolas THERY; Jean-Marc VOLLE; Pierre-yves
 TALOUD; Willy POISSON
 Subject: Re: [PATCH RFC 1/5] V4L: Add
 V4L2_MBUS_FMT_S5C_UYVY_JPEG_1X8 media bus format
 
 Hi Sylwester and Vincent,
 
 My apologies for the late reply on this topic. I've been quite busy lately.
 
 Sylwester Nawrocki wrote:
  On 10/09/2012 03:36 PM, Vincent ABRIOU wrote:
  Hi Sylwester,
 
  I'm wondering why don't you simply define
 V4L2_MBUS_FMT_UYVY_JPEG_1X8
  without any reference to your camera?
 
  Because it's not a plain UYVY/JPEG data. There is an additional
  meta-data that follows interleaved UYVY/JPEG. It's all on a single
  User Defined MIPI CSI-2 DT. In addition to that there is some more
  meta data transmitted on MIPI CSI-2 Embedded Data DT. If there was no
  meta-data present at the User Defined DT, then we could think about
  using generic
  V4L2_MBUS_FMT_UYVY_JPEG_1X8 pixel code and handling the meta-data
 on
  separate DT with the frame_desc calls.
 
  Anyway this S5C media bus format is an experimental thing and if there
  are cameras generating plain JPEG/YUV we need to search for better,
  more generic solution.
 
 Vincent: what's the frame layout that your sensor produces? There are two
 cases that could be easy (sort of, everything's relative) that I can see for 
 the
 standard interfaces.

The sensor I used interleaved Jpeg and YUV but I met 2 cases:
1/ sensor providing JPEG / YUV (using the same DT)
2/ sensor providing JPEG with a 1st DT, YUV with a 2nd DT and embedded data 
using a 3rd DT

 
 1. Different parts of the image are transmitted over different CSI-2 contexts.
 This way the receiver may separate them to separate memory regions, and
 the end result is a single multi-plane buffer.
 
 2. If the distance in octets of the intermittent image strides is constant, 
 then
 we could do some tricks with multi-plane buffers. The two planes of the
 buffer could be interleaved, with correct base addresses.
 
 I think Sylwester's case fits into neither of the above. A device-specific
 format does not resolve configuring the two formats.
 
 Both require adding plane-specific pixel codes, agreement over how frame
 descriptors are used for describing frames of multiple independent content
 planes, and how the multiple formats are configured on the sensor. Use of
 sub-subdevs come to mind for the last one as an alternative --- this issue
 stems from the fact we're using the same interface to model the bus and the
 image format on that bus. It no longer works out the way it used to when
 there are two formats on the same bus.
 

My point of view is that the only user's concern is how to retrieve the data, 
on which pad and how are they sorted?

The MBUS format to be use by the camera (and set by the user) should only 
define the overall structure of the CSI stream and could be generic (like 
V4L2_MBUS_FMT_UYVY_JPEG_1X8  or V4L2_MBUS_FMT_YUYV_JPEG_1X8 without any sensor 
model mentioned).
In the MBUS format naming, we don't care about describing the eventual metadata 
and other stream specificities link to the camera because the CSI stream 
details could be described in the v4l2_subdev_frame_format as initially 
proposed by Sakari (More flag descriptor could be added to describe the data to 
be transmitted).
It is then up to the CSI2 receiver to recover the CSI stream layout and to 
configure output pads and multi-plane buffer according to its capabilities. 
Multi-plan buffer attached to the pad need to have a plane-specific pixel code 
in order to warn user about the buffer content.
Then user could query the output pads of the CSI 2 receiver to discover on 
which pad the different pixel formats are outputted and how:

 Each of these probably require a separate RFC and someone to implement
 the changes.
 
 Kind regards,
 
 --
 Sakari Ailus
 sakari.ai...@iki.fi

Best Regards,

Vincent Abriou
--
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 RFC 1/5] V4L: Add V4L2_MBUS_FMT_S5C_UYVY_JPEG_1X8 media bus format

2012-10-15 Thread Sakari Ailus

Hi Sylwester and Vincent,

My apologies for the late reply on this topic. I've been quite busy lately.

Sylwester Nawrocki wrote:

On 10/09/2012 03:36 PM, Vincent ABRIOU wrote:

Hi Sylwester,

I'm wondering why don't you simply define V4L2_MBUS_FMT_UYVY_JPEG_1X8
without any reference to your camera?


Because it's not a plain UYVY/JPEG data. There is an additional meta-data
that follows interleaved UYVY/JPEG. It's all on a single User Defined
MIPI CSI-2 DT. In addition to that there is some more meta data transmitted
on MIPI CSI-2 Embedded Data DT. If there was no meta-data present at the
User Defined DT, then we could think about using generic
V4L2_MBUS_FMT_UYVY_JPEG_1X8 pixel code and handling the meta-data on
separate DT with the frame_desc calls.

Anyway this S5C media bus format is an experimental thing and if there are
cameras generating plain JPEG/YUV we need to search for better, more generic
solution.


Vincent: what's the frame layout that your sensor produces? There are two
cases that could be easy (sort of, everything's relative) that I can see for
the standard interfaces.

1. Different parts of the image are transmitted over different CSI-2
contexts. This way the receiver may separate them to separate memory
regions, and the end result is a single multi-plane buffer.

2. If the distance in octets of the intermittent image strides is constant,
then we could do some tricks with multi-plane buffers. The two planes of the
buffer could be interleaved, with correct base addresses.

I think Sylwester's case fits into neither of the above. A device-specific
format does not resolve configuring the two formats.

Both require adding plane-specific pixel codes, agreement over how frame
descriptors are used for describing frames of multiple independent content
planes, and how the multiple formats are configured on the sensor. Use of
sub-subdevs come to mind for the last one as an alternative --- this issue
stems from the fact we're using the same interface to model the bus and the
image format on that bus. It no longer works out the way it used to when
there are two formats on the same bus.

Each of these probably require a separate RFC and someone to implement the
changes.

Kind regards,

--
Sakari Ailus
sakari.ai...@iki.fi
--
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 RFC 1/5] V4L: Add V4L2_MBUS_FMT_S5C_UYVY_JPEG_1X8 media bus format

2012-10-09 Thread Vincent ABRIOU
Hi Sylwester,

I'm wondering why don't you simply define V4L2_MBUS_FMT_UYVY_JPEG_1X8
without any reference to your camera?
Indeed, many other cameras could support Jpeg interleaved with YUV and it will
avoid to define a new media bus type for every cameras integrated under V4L2
supporting JPEG/YUV interleaving feature.

Thank you for your feedback.

Regards,
--
Vincent Abriou


 -Original Message-
 From: linux-media-ow...@vger.kernel.org [mailto:linux-media-
 ow...@vger.kernel.org] On Behalf Of Sylwester Nawrocki
 Sent: Tuesday, September 25, 2012 4:40 PM
 To: Laurent Pinchart
 Cc: linux-media@vger.kernel.org; a.ha...@samsung.com; sakari.ai...@iki.fi;
 hverk...@xs4all.nl; kyungmin.p...@samsung.com;
 sw0312@samsung.com
 Subject: Re: [PATCH RFC 1/5] V4L: Add
 V4L2_MBUS_FMT_S5C_UYVY_JPEG_1X8 media bus format
 
 Hi Laurent,
 
 Thanks for your review.
 
 On 09/25/2012 01:42 PM, Laurent Pinchart wrote:
  On Monday 24 September 2012 16:55:42 Sylwester Nawrocki wrote:
  This patch adds media bus pixel code for the interleaved JPEG/UYVY
  image format used by S5C73MX Samsung cameras. This interleaved image
  data is transferred on MIPI-CSI2 bus as User Defined Byte-based Data.
 
  It also defines an experimental vendor and device specific media bus
  formats section and adds related DocBook documentation.
 
  Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com
  Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
  ---
   Documentation/DocBook/media/v4l/compat.xml |  4 ++
   Documentation/DocBook/media/v4l/subdev-formats.xml | 45
 +++
   include/linux/v4l2-mediabus.h  |  5 +++
   3 files changed, 54 insertions(+)
 
  diff --git a/Documentation/DocBook/media/v4l/compat.xml
  b/Documentation/DocBook/media/v4l/compat.xml index
 98e8d08..5d2480b
  100644
  --- a/Documentation/DocBook/media/v4l/compat.xml
  +++ b/Documentation/DocBook/media/v4l/compat.xml
  @@ -2605,6 +2605,10 @@ ioctls./para
   listitem
   paraSupport for frequency band enumeration:
  VIDIOC-ENUM-FREQ-BANDS; ioctl./para /listitem
  +listitem
  +paraVendor and device specific media bus pixel formats.
  +  xref linkend=v4l2-mbus-vendor-spec-fmts /./para
  +/listitem
 /itemizedlist
   /section
 
  diff --git a/Documentation/DocBook/media/v4l/subdev-formats.xml
  b/Documentation/DocBook/media/v4l/subdev-formats.xml index
  49c532e..d7aa870
  100644
  --- a/Documentation/DocBook/media/v4l/subdev-formats.xml
  +++ b/Documentation/DocBook/media/v4l/subdev-formats.xml
  @@ -2565,5 +2565,50 @@
 /tgroup
 /table
   /section
  +
  +section id=v4l2-mbus-vendor-spec-fmts
  +  titleVendor and Device Specific Formats/title
  +
  +  note
  +  title Experimental /title
 
  I don't think you need spaces across the title.
 
 Thanks for spotting this, I'll fix it and any other occurrences there.
 
  +  paraThis is an link linkend=experimentalexperimental/link
  +interface and may change in the future./para
  +  /note
  +
  +  para This section lists complex data formats that are either
  + vendor
  or
  +  device specific. These formats comprise raw and compressed image
 data
  +  and optional meta-data within a single frame.
 
  That's currently true, but we could have other strange vendor-specific
  formats that don't interleave raw and compressed frames.
 
 OK, let me remove that sentence then.
 
  +  /para
  +
  +  paraThe following table lists the existing vendor and device
  specific
  +  formats./para
  +
  +  table pgwide=0 frame=none
  id=v4l2-mbus-pixelcode-vendor-specific +titleVendor and
 device
  specific formats/title
  +  tgroup cols=3
  +colspec colname=id align=left /
  +colspec colname=code align=left/
  +colspec colname=remarks align=left/
  +thead
  +  row
  +entryIdentifier/entry
  +entryCode/entry
  +entryComments/entry
  +  /row
  +/thead
  +tbody valign=top
  +  row id=V4L2-MBUS-FMT-S5C-UYVY-JPG-1X8
  +entryV4L2_MBUS_FMT_S5C_UYVY_JPG_1X8/entry
  +entry0x8001/entry
  +entry
  +  Interleaved raw UYVY and JPEG image format with
 embedded
  +  meta-data, produced by S3C73M3 camera sensors.
  +/entry
  +  /row
  +/tbody
  +  /tgroup
  +  /table
  +/section
  +
 /section
   /section
  diff --git a/include/linux/v4l2-mediabus.h
  b/include/linux/v4l2-mediabus.h index 5ea7f75..b98c566 100644
  --- a/include/linux/v4l2-mediabus.h
  +++ b/include/linux/v4l2-mediabus.h
  @@ -92,6 +92,11 @@ enum v4l2_mbus_pixelcode {
 
 /* JPEG compressed formats - next is 0x4002 */
 V4L2_MBUS_FMT_JPEG_1X8 = 0x4001,
  +
  +  /* Vendor specific formats - next is 0x8002 */
 
  Anything wrong with 0x5000 as a base value ? :-)
 
 I think I was originally using this value but during discussions the 
 conclusion
 was to clearly separate this new range. I have no strong preference, I'm

Re: [PATCH RFC 1/5] V4L: Add V4L2_MBUS_FMT_S5C_UYVY_JPEG_1X8 media bus format

2012-10-09 Thread Sylwester Nawrocki
Hi Vincent,

On 10/09/2012 03:36 PM, Vincent ABRIOU wrote:
 Hi Sylwester,
 
 I'm wondering why don't you simply define V4L2_MBUS_FMT_UYVY_JPEG_1X8
 without any reference to your camera?

Because it's not a plain UYVY/JPEG data. There is an additional meta-data
that follows interleaved UYVY/JPEG. It's all on a single User Defined 
MIPI CSI-2 DT. In addition to that there is some more meta data transmitted 
on MIPI CSI-2 Embedded Data DT. If there was no meta-data present at the
User Defined DT, then we could think about using generic 
V4L2_MBUS_FMT_UYVY_JPEG_1X8 pixel code and handling the meta-data on 
separate DT with the frame_desc calls.

Anyway this S5C media bus format is an experimental thing and if there are
cameras generating plain JPEG/YUV we need to search for better, more generic
solution.

 Indeed, many other cameras could support Jpeg interleaved with YUV and it will
 avoid to define a new media bus type for every cameras integrated under V4L2
 supporting JPEG/YUV interleaving feature.

Yes, we also need some interface to configure both streams, e.g. image
resolutions independently. Still having separate pixel codes for pairs of
JPEG and something else seems not optimal. Nevertheless I can't think of
any better solution right now, so applications are able to configure
selected format at a camera subdev.

I'm also wondering what are the interleaving methods in case of various
cameras. Even though they interleave same 2 standard formats, the way how
the interleaving happens could differ, couldn't it ? Such information
probably cannot be easily contained in the meta-data, unless there is some
standard (header) format for it. 

 Thank you for your feedback.
 
 Regards,
 --
 Vincent Abriou

--

Regards,
Sylwester
--
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 RFC 1/5] V4L: Add V4L2_MBUS_FMT_S5C_UYVY_JPEG_1X8 media bus format

2012-09-25 Thread Laurent Pinchart
Hi Sylwester,

On Monday 24 September 2012 16:55:42 Sylwester Nawrocki wrote:
 This patch adds media bus pixel code for the interleaved JPEG/UYVY
 image format used by S5C73MX Samsung cameras. This interleaved image
 data is transferred on MIPI-CSI2 bus as User Defined Byte-based Data.
 
 It also defines an experimental vendor and device specific media bus
 formats section and adds related DocBook documentation.
 
 Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com
 Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
 ---
  Documentation/DocBook/media/v4l/compat.xml |  4 ++
  Documentation/DocBook/media/v4l/subdev-formats.xml | 45 +++
  include/linux/v4l2-mediabus.h  |  5 +++
  3 files changed, 54 insertions(+)
 
 diff --git a/Documentation/DocBook/media/v4l/compat.xml
 b/Documentation/DocBook/media/v4l/compat.xml index 98e8d08..5d2480b 100644
 --- a/Documentation/DocBook/media/v4l/compat.xml
 +++ b/Documentation/DocBook/media/v4l/compat.xml
 @@ -2605,6 +2605,10 @@ ioctls./para
  listitem
 paraSupport for frequency band enumeration: VIDIOC-ENUM-FREQ-BANDS;
 ioctl./para /listitem
 +listitem
 +   paraVendor and device specific media bus pixel formats.
 + xref linkend=v4l2-mbus-vendor-spec-fmts /./para
 +/listitem
/itemizedlist
  /section
 
 diff --git a/Documentation/DocBook/media/v4l/subdev-formats.xml
 b/Documentation/DocBook/media/v4l/subdev-formats.xml index 49c532e..d7aa870
 100644
 --- a/Documentation/DocBook/media/v4l/subdev-formats.xml
 +++ b/Documentation/DocBook/media/v4l/subdev-formats.xml
 @@ -2565,5 +2565,50 @@
   /tgroup
/table
  /section
 +
 +section id=v4l2-mbus-vendor-spec-fmts
 +  titleVendor and Device Specific Formats/title
 +
 +  note
 + title Experimental /title

I don't think you need spaces across the title.

 + paraThis is an link linkend=experimentalexperimental/link
 +interface and may change in the future./para
 +  /note
 +
 +  para This section lists complex data formats that are either vendor
 or
 + device specific. These formats comprise raw and compressed image data
 + and optional meta-data within a single frame.

That's currently true, but we could have other strange vendor-specific formats 
that don't interleave raw and compressed frames.

 +  /para
 +
 +  paraThe following table lists the existing vendor and device
 specific
 + formats./para
 +
 +  table pgwide=0 frame=none
 id=v4l2-mbus-pixelcode-vendor-specific +   titleVendor and device
 specific formats/title
 + tgroup cols=3
 +   colspec colname=id align=left /
 +   colspec colname=code align=left/
 +   colspec colname=remarks align=left/
 +   thead
 + row
 +   entryIdentifier/entry
 +   entryCode/entry
 +   entryComments/entry
 + /row
 +   /thead
 +   tbody valign=top
 + row id=V4L2-MBUS-FMT-S5C-UYVY-JPG-1X8
 +   entryV4L2_MBUS_FMT_S5C_UYVY_JPG_1X8/entry
 +   entry0x8001/entry
 +   entry
 + Interleaved raw UYVY and JPEG image format with embedded
 + meta-data, produced by S3C73M3 camera sensors.
 +   /entry
 + /row
 +   /tbody
 + /tgroup
 +  /table
 +/section
 +
/section
  /section
 diff --git a/include/linux/v4l2-mediabus.h b/include/linux/v4l2-mediabus.h
 index 5ea7f75..b98c566 100644
 --- a/include/linux/v4l2-mediabus.h
 +++ b/include/linux/v4l2-mediabus.h
 @@ -92,6 +92,11 @@ enum v4l2_mbus_pixelcode {
 
   /* JPEG compressed formats - next is 0x4002 */
   V4L2_MBUS_FMT_JPEG_1X8 = 0x4001,
 +
 + /* Vendor specific formats - next is 0x8002 */

Anything wrong with 0x5000 as a base value ? :-)

 +
 + /* S5C73M3 interleaved UYVY and JPEG */
 + V4L2_MBUS_FMT_S5C_UYVY_JPEG_1X8 = 0x8001,
  };
 
  /**

-- 
Regards,

Laurent Pinchart

--
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 RFC 1/5] V4L: Add V4L2_MBUS_FMT_S5C_UYVY_JPEG_1X8 media bus format

2012-09-25 Thread Sylwester Nawrocki
Hi Laurent,

Thanks for your review.

On 09/25/2012 01:42 PM, Laurent Pinchart wrote:
 On Monday 24 September 2012 16:55:42 Sylwester Nawrocki wrote:
 This patch adds media bus pixel code for the interleaved JPEG/UYVY
 image format used by S5C73MX Samsung cameras. This interleaved image
 data is transferred on MIPI-CSI2 bus as User Defined Byte-based Data.

 It also defines an experimental vendor and device specific media bus
 formats section and adds related DocBook documentation.

 Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com
 Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
 ---
  Documentation/DocBook/media/v4l/compat.xml |  4 ++
  Documentation/DocBook/media/v4l/subdev-formats.xml | 45 +++
  include/linux/v4l2-mediabus.h  |  5 +++
  3 files changed, 54 insertions(+)

 diff --git a/Documentation/DocBook/media/v4l/compat.xml
 b/Documentation/DocBook/media/v4l/compat.xml index 98e8d08..5d2480b 100644
 --- a/Documentation/DocBook/media/v4l/compat.xml
 +++ b/Documentation/DocBook/media/v4l/compat.xml
 @@ -2605,6 +2605,10 @@ ioctls./para
  listitem
paraSupport for frequency band enumeration: VIDIOC-ENUM-FREQ-BANDS;
 ioctl./para /listitem
 +listitem
 +  paraVendor and device specific media bus pixel formats.
 +xref linkend=v4l2-mbus-vendor-spec-fmts /./para
 +/listitem
/itemizedlist
  /section

 diff --git a/Documentation/DocBook/media/v4l/subdev-formats.xml
 b/Documentation/DocBook/media/v4l/subdev-formats.xml index 49c532e..d7aa870
 100644
 --- a/Documentation/DocBook/media/v4l/subdev-formats.xml
 +++ b/Documentation/DocBook/media/v4l/subdev-formats.xml
 @@ -2565,5 +2565,50 @@
  /tgroup
/table
  /section
 +
 +section id=v4l2-mbus-vendor-spec-fmts
 +  titleVendor and Device Specific Formats/title
 +
 +  note
 +title Experimental /title
 
 I don't think you need spaces across the title.

Thanks for spotting this, I'll fix it and any other occurrences there.

 +paraThis is an link linkend=experimentalexperimental/link
 +interface and may change in the future./para
 +  /note
 +
 +  para This section lists complex data formats that are either vendor
 or
 +device specific. These formats comprise raw and compressed image data
 +and optional meta-data within a single frame.
 
 That's currently true, but we could have other strange vendor-specific 
 formats 
 that don't interleave raw and compressed frames.

OK, let me remove that sentence then.

 +  /para
 +
 +  paraThe following table lists the existing vendor and device
 specific
 +formats./para
 +
 +  table pgwide=0 frame=none
 id=v4l2-mbus-pixelcode-vendor-specific +  titleVendor and device
 specific formats/title
 +tgroup cols=3
 +  colspec colname=id align=left /
 +  colspec colname=code align=left/
 +  colspec colname=remarks align=left/
 +  thead
 +row
 +  entryIdentifier/entry
 +  entryCode/entry
 +  entryComments/entry
 +/row
 +  /thead
 +  tbody valign=top
 +row id=V4L2-MBUS-FMT-S5C-UYVY-JPG-1X8
 +  entryV4L2_MBUS_FMT_S5C_UYVY_JPG_1X8/entry
 +  entry0x8001/entry
 +  entry
 +Interleaved raw UYVY and JPEG image format with embedded
 +meta-data, produced by S3C73M3 camera sensors.
 +  /entry
 +/row
 +  /tbody
 +/tgroup
 +  /table
 +/section
 +
/section
  /section
 diff --git a/include/linux/v4l2-mediabus.h b/include/linux/v4l2-mediabus.h
 index 5ea7f75..b98c566 100644
 --- a/include/linux/v4l2-mediabus.h
 +++ b/include/linux/v4l2-mediabus.h
 @@ -92,6 +92,11 @@ enum v4l2_mbus_pixelcode {

  /* JPEG compressed formats - next is 0x4002 */
  V4L2_MBUS_FMT_JPEG_1X8 = 0x4001,
 +
 +/* Vendor specific formats - next is 0x8002 */
 
 Anything wrong with 0x5000 as a base value ? :-)

I think I was originally using this value but during discussions
the conclusion was to clearly separate this new range. I have no
strong preference, I'm going to revert it to 0x5000 in the next
iteration, unless someone raises objections.

 +
 +/* S5C73M3 interleaved UYVY and JPEG */
 +V4L2_MBUS_FMT_S5C_UYVY_JPEG_1X8 = 0x8001,
  };

  /**
 

Regards,
-- 
Sylwester Nawrocki
Samsung Poland RD Center
--
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