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