[PATCH v1 2/2] libcamera: formats: Add PiSP specific image and config buffer formats
Jacopo Mondi
jacopo.mondi at ideasonboard.com
Thu Feb 29 08:40:32 CET 2024
Hi Naush
On Wed, Feb 28, 2024 at 05:19:47PM +0000, Naushir Patuck wrote:
> On Wed, 28 Feb 2024, 5:16 pm Jacopo Mondi, <jacopo.mondi at ideasonboard.com>
> wrote:
>
> > Hi Naush
> >
> > On Wed, Feb 28, 2024 at 04:45:54PM +0000, Naushir Patuck wrote:
> > > Hi Jacopo,
> > >
> > > > > > > -
> > > > > > > + { formats::BGGR16_PISP_COMP1, {
> > > > > > > + .name = "BGGR16_PISP_COMP1",
> > > > > > > + .format = formats::BGGR16_PISP_COMP1,
> > > > > > > + .v4l2Formats = {
> > V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR), },
> > > > > > > + .bitsPerPixel = 16,
> > > > > > > + .colourEncoding =
> > PixelFormatInfo::ColourEncodingRAW,
> > > > > > > + .packed = true,
> > > > > > > + .pixelsPerGroup = 2,
> > > > > > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},
> > > > > > > + } },
> > > > > >
> > > > > > For regular 16-bit formats this should be
> > > > > >
> > > > > > .pixelsPerGroup = 2,
> > > > > > .planes = {{ { 4, 1 }, { 0, 0 }, { 0, 0 } }},
> > > > > >
> > > > > > As 2 pixels of 16 bits each consume 4 bytes. Is this {2, 1}
> > > > > > intentional and due to compression ?
> > > > >
> > > > > Yes, I used {2, 1} to denote the 8-bit/sample after compression. Do
> > > > > you think I should change to {4, 1}?
> > > >
> > > > Depends on how many bytes the format actually consumes :)
> > > >
> > > > For 16bpp RAW formats, we have pixelsPerGroup = 2 and each sample
> > > > consumes 2 bytes.
> > > >
> > > > According to the pixelsPerGroup definition:
> > > >
> > > > * A pixel group is defined as the minimum number of pixels (including
> > padding)
> > > > * necessary in a row when the image has only one column of effective
> > pixels.
> > > >
> > > > So for a RAW Bayer formats, to get a complete full-color "superpixel"
> > > > we need at least two pixels per row.
> > > >
> > > > RG
> > > > GB
> > > >
> > > > Each "RG" or "GB" pair is composed by 2 16-bit samples, for a total of
> > > > 32bits of data, from which the {4, 1} value in planes[0]
> > > >
> > > > According to "Table 2", Chapter 5 of the PiSP datasheet, the
> > > > compressed formats are described as
> > > >
> > > > 8-bits per pixel single channel compressed with mode n, for n =
> > 1, 2, 3
> > > >
> > > > Which suggests each "RG" or "GB" pair is actually 2 samples of 8 bits
> > > > each, for a plane size of {2, 1}.
> > > >
> > > > Is this correct ? Is the actual .bitsPerPixel value for the compressed
> > > > formats 8 instead of 16 then ?
> > >
> > > This is correct, each sample is 8-bits so "RG"/"GB" pairs are 16-bits
> > each.
> > > So it sounds like I need to use .planes = {2, 1}, but .bitsPerPixel = 8
> > maybe?
> >
> > I now wonder if the '16' in the format names is correct then :)
> >
>
> It's meant to signify 16-bits compressed to 8-bits (in this case). Maybe a
> defined naming scheme is needed for such cases?
>
What about simply leaving the bitdpeth out from the format name like
it is done for the V4L2 pixel formats ?
#define V4L2_PIX_FMT_PISP_COMP1_RGGB v4l2_fourcc('P', 'C', '1', 'R') /* PiSP 8-bit mode 1 compressed RGGB bayer */
Something like
{ formats::BGGR_PISP_COMP1, {
.name = "BGGR_PISP_COMP1",
.format = formats::BGGR_PISP_COMP1,
.v4l2Formats = { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR), },
.bitsPerPixel = 8,
.colourEncoding = PixelFormatInfo::ColourEncodingRAW,
.packed = true,
.pixelsPerGroup = 2,
.planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},
} },
?
>
> > >
> > > Naush
> > >
> > > >
> > > > Thanks
> > > > j
> > > >
> > > > >
> > > > > Regards,
> > > > > Naush
> > > > >
> > > > >
> > > > > >
> > > > > > > + { formats::GBRG16_PISP_COMP1, {
> > > > > > > + .name = "GBRG16_PISP_COMP1",
> > > > > > > + .format = formats::GBRG16_PISP_COMP1,
> > > > > > > + .v4l2Formats = {
> > V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GBRG), },
> > > > > > > + .bitsPerPixel = 16,
> > > > > > > + .colourEncoding =
> > PixelFormatInfo::ColourEncodingRAW,
> > > > > > > + .packed = true,
> > > > > > > + .pixelsPerGroup = 2,
> > > > > > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},
> > > > > > > + } },
> > > > > > > + { formats::GRBG16_PISP_COMP1, {
> > > > > > > + .name = "GRBG16_PISP_COMP1",
> > > > > > > + .format = formats::GRBG16_PISP_COMP1,
> > > > > > > + .v4l2Formats = {
> > V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GRBG), },
> > > > > > > + .bitsPerPixel = 16,
> > > > > > > + .colourEncoding =
> > PixelFormatInfo::ColourEncodingRAW,
> > > > > > > + .packed = true,
> > > > > > > + .pixelsPerGroup = 2,
> > > > > > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},
> > > > > > > + } },
> > > > > > > + { formats::RGGB16_PISP_COMP1, {
> > > > > > > + .name = "RGGB16_PISP_COMP1",
> > > > > > > + .format = formats::RGGB16_PISP_COMP1,
> > > > > > > + .v4l2Formats = {
> > V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB), },
> > > > > > > + .bitsPerPixel = 16,
> > > > > > > + .colourEncoding =
> > PixelFormatInfo::ColourEncodingRAW,
> > > > > > > + .packed = true,
> > > > > > > + .pixelsPerGroup = 2,
> > > > > > > + .planes = {{ { 2, 1 }, { 0, 0 }, { 0, 0 } }},
> > > > > > > + } },
> > > > > > > /* Compressed formats. */
> > > > > > > { formats::MJPEG, {
> > > > > > > .name = "MJPEG",
> > > > > > > diff --git a/src/libcamera/formats.yaml
> > b/src/libcamera/formats.yaml
> > > > > > > index bde2cc803b98..f6df721243d0 100644
> > > > > > > --- a/src/libcamera/formats.yaml
> > > > > > > +++ b/src/libcamera/formats.yaml
> > > > > > > @@ -190,4 +190,20 @@ formats:
> > > > > > > - SBGGR10_IPU3:
> > > > > > > fourcc: DRM_FORMAT_SBGGR10
> > > > > > > mod: IPU3_FORMAT_MOD_PACKED
> > > > > > > +
> > > > > > > + - RGGB16_PISP_COMP1:
> > > > > > > + fourcc: DRM_FORMAT_SRGGB16
> > > > > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1
> > > > > > > + - GRBG16_PISP_COMP1:
> > > > > > > + fourcc: DRM_FORMAT_SGRBG16
> > > > > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1
> > > > > > > + - GBRG16_PISP_COMP1:
> > > > > > > + fourcc: DRM_FORMAT_SGBRG16
> > > > > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1
> > > > > > > + - BGGR16_PISP_COMP1:
> > > > > > > + fourcc: DRM_FORMAT_SBGGR16
> > > > > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1
> > > > > > > + - MONO_PISP_COMP1:
> > > > > > > + fourcc: DRM_FORMAT_R16
> > > > > > > + mod: PISP_FORMAT_MOD_COMPRESS_MODE1
> > > > > > > ...
> > > > > > > diff --git a/src/libcamera/v4l2_pixelformat.cpp
> > b/src/libcamera/v4l2_pixelformat.cpp
> > > > > > > index efb6f2940235..47baaf60199d 100644
> > > > > > > --- a/src/libcamera/v4l2_pixelformat.cpp
> > > > > > > +++ b/src/libcamera/v4l2_pixelformat.cpp
> > > > > > > @@ -207,6 +207,16 @@ const std::map<V4L2PixelFormat,
> > V4L2PixelFormat::Info> vpf2pf{
> > > > > > > { formats::SGRBG16, "16-bit Bayer GRGR/BGBG" } },
> > > > > > > { V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16),
> > > > > > > { formats::SRGGB16, "16-bit Bayer RGRG/GBGB" } },
> > > > > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_BGGR),
> > > > > > > + { formats::BGGR16_PISP_COMP1, "16-bit Bayer
> > BGBG/GRGR PiSP Compress Mode 1" } },
> > > > > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GBRG),
> > > > > > > + { formats::GBRG16_PISP_COMP1, "16-bit Bayer
> > GBGB/RGRG PiSP Compress Mode 1" } },
> > > > > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_GRBG),
> > > > > > > + { formats::GRBG16_PISP_COMP1, "16-bit Bayer
> > GRGR/BGBG PiSP Compress Mode 1" } },
> > > > > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_RGGB),
> > > > > > > + { formats::RGGB16_PISP_COMP1, "16-bit Bayer
> > RGRG/GBGB PiSP Compress Mode 1" } },
> > > > > > > + { V4L2PixelFormat(V4L2_PIX_FMT_PISP_COMP1_MONO),
> > > > > > > + { formats::MONO_PISP_COMP1, "16-bit Mono PiSP
> > Compress Mode 1" } },
> > > > > > >
> > > > > > > /* Compressed formats. */
> > > > > > > { V4L2PixelFormat(V4L2_PIX_FMT_MJPEG),
> > > > > > > diff --git a/src/libcamera/v4l2_subdevice.cpp
> > b/src/libcamera/v4l2_subdevice.cpp
> > > > > > > index 6d0785b7b484..aea90abaf9ef 100644
> > > > > > > --- a/src/libcamera/v4l2_subdevice.cpp
> > > > > > > +++ b/src/libcamera/v4l2_subdevice.cpp
> > > > > > > @@ -134,6 +134,10 @@ const std::map<uint32_t,
> > V4L2SubdeviceFormatInfo> formatInfoMap = {
> > > > > > > { MEDIA_BUS_FMT_SGBRG12_1X12, { 12, "SGBRG12_1X12",
> > PixelFormatInfo::ColourEncodingRAW } },
> > > > > > > { MEDIA_BUS_FMT_SGRBG12_1X12, { 12, "SGRBG12_1X12",
> > PixelFormatInfo::ColourEncodingRAW } },
> > > > > > > { MEDIA_BUS_FMT_SRGGB12_1X12, { 12, "SRGGB12_1X12",
> > PixelFormatInfo::ColourEncodingRAW } },
> > > > > > > + { MEDIA_BUS_FMT_SBGGR16_1X16, { 16, "SBGGR16_1x16",
> > PixelFormatInfo::ColourEncodingRAW } },
> > > > > > > + { MEDIA_BUS_FMT_SGBRG16_1X16, { 16, "SGBRG16_1x16",
> > PixelFormatInfo::ColourEncodingRAW } },
> > > > > > > + { MEDIA_BUS_FMT_SGRBG16_1X16, { 16, "SGRBG16_1x16",
> > PixelFormatInfo::ColourEncodingRAW } },
> > > > > > > + { MEDIA_BUS_FMT_SRGGB16_1X16, { 16, "SRGGB16_1x16",
> > PixelFormatInfo::ColourEncodingRAW } },
> > > > > > > /* \todo Clarify colour encoding for HSV formats */
> > > > > > > { MEDIA_BUS_FMT_AHSV8888_1X32, { 32, "AHSV8888_1X32",
> > PixelFormatInfo::ColourEncodingRGB } },
> > > > > > > { MEDIA_BUS_FMT_JPEG_1X8, { 8, "JPEG_1X8",
> > PixelFormatInfo::ColourEncodingYUV } },
> > > > > > > --
> > > > > > > 2.34.1
> > > > > > >
> >
More information about the libcamera-devel
mailing list