Re: [libcamera-devel] [PATCH v2] drm/fourcc: Add bayer formats and modifiers

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Tomasz,

On Fri, Jun 19, 2020 at 01:06:55PM +0000, Tomasz Figa wrote:
> On Fri, May 22, 2020 at 01:52:01AM +0200, Niklas Söderlund wrote:
> > Bayer formats are used with cameras and contain green, red and blue
> > components, with alternating lines of red and green, and blue and green
> > pixels in different orders. For each block of 2x2 pixels there is one
> > pixel with a red filter, two with a green filter, and one with a blue
> > filter. The filters can be arranged in different patterns.
> > 
> > Add DRM fourcc formats to describe the most common Bayer formats. Also
> > add a modifiers to describe the custom packing layouts used by the Intel
> > IPU3 and in the MIPI (Mobile Industry Processor Interface) CSI-2
> > specification.
> > 
> > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> > ---
> > * Changes since v1
> > - Rename the defines from DRM_FORMAT_SRGGB8 to DRM_FORMAT_BAYER_RGGB8.
> > - Update the fourcc codes passed to fourcc_code() to avoid a conflict.
> > - Add diagrams for all Bayer formats memory layout.
> > - Update documentation.
> > ---
> >  include/uapi/drm/drm_fourcc.h | 205 ++++++++++++++++++++++++++++++++++
> >  1 file changed, 205 insertions(+)
> > 
> > diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> > index 8bc0b31597d80737..d07dd24b49bde6c1 100644
> > --- a/include/uapi/drm/drm_fourcc.h
> > +++ b/include/uapi/drm/drm_fourcc.h
> > @@ -285,6 +285,73 @@ extern "C" {
> >  #define DRM_FORMAT_YUV444	fourcc_code('Y', 'U', '2', '4') /* non-subsampled Cb (1) and Cr (2) planes */
> >  #define DRM_FORMAT_YVU444	fourcc_code('Y', 'V', '2', '4') /* non-subsampled Cr (1) and Cb (2) planes */
> >  
> > +/*
> > + * Bayer formats
> > + *
> > + * Bayer formats contain green, red and blue components, with alternating lines
> > + * of red and green, and blue and green pixels in different orders. For each
> > + * block of 2x2 pixels there is one pixel with a red filter, two with a green
> > + * filter, and one with a blue filter. The filters can be arranged in different
> > + * patterns.
> > + *
> > + * For example, RGGB:
> > + *	row0: RGRGRGRG...
> > + *	row1: GBGBGBGB...
> > + *	row2: RGRGRGRG...
> > + *	row3: GBGBGBGB...
> > + *	...
> > + *
> 
> I wonder if we're operating on the right level of abstraction within this
> proposal.
> 
> The sensor itself transfers only sequential pixels, as read
> out from its matrix. Whether a given pixel corresponds to a red, green
> or blue color filter actually depends on the filter layer, which could
> actually vary between integrations of the same sensor. (See Fujifilm
> X-Trans, which uses regular Sony sensors with their own filter pattern
> [1].)
> 
> Moreover, the sensor resolution is specified as the number of pixels
> horizontally and the number of lines horizontally, without considering
> the color pattern.
> 
> If we consider that, wouldn't the data stream coming from the sensor be
> essentially DRM_FORMAT_R8/R10/R12/etc.?
> 
> Then, on top of that, we would have the packing, which I believe is
> defined well in this document +/- being entangled with the Bayer
> pattern.
> 
> What do you think?
> 
> [1] https://en.wikipedia.org/wiki/Fujifilm_X-Trans_sensor

I think using DRM_FORMAT_R8/R10/R12/... is a good idea. Packing would
indeed be a modifier, and maybe the CFA could even be expressed
separately from the DRM format (4CC + modifier), through a libcamera
property.

-- 
Regards,

Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel