[libcamera-devel] [PATCH 09/11] apps: qcam: Add support for IPU3_Y10

[Laurent Pinchart]

Hi Dan,

On Mon, Apr 17, 2023 at 10:59:43AM +0100, Dan Scally wrote:
> On 19/03/2023 23:34, Laurent Pinchart wrote:
> > On Sat, Mar 18, 2023 at 11:40:12PM +0000, Daniel Scally via libcamera-devel wrote:
> >> Add support for the IPU3_Y10 format to the FormatConverter.
> >
> > Before reviewing this patch in details, I'd like to know if there would
> > be a way to use the ImgU to convert the IPU3-packed greyscale format to
> > a more standard format. Is this something you have considered ?
> 
> Sorry for the delay on this. I had concluded some time ago that this
> wasn't possible, so I hadn't particularly considered it lately. Since
> you ask I have been double checking and the conclusion seems to hold -
> I talked to the ipu3/cio2 maintainers and my understanding from those
> conversations is that the Imgu supports bayer input only, and the CIO2
> can only output packed data, so I think we are stuck with this or
> something like this.

OK, thanks for checking.

> >> Signed-off-by: Daniel Scally <dan.scally at ideasonboard.com>
> >> ---
> >>   src/apps/qcam/format_converter.cpp | 50 ++++++++++++++++++++++++++++++
> >>   src/apps/qcam/format_converter.h   |  2 ++
> >>   2 files changed, 52 insertions(+)
> >>
> >> diff --git a/src/apps/qcam/format_converter.cpp b/src/apps/qcam/format_converter.cpp
> >> index de76b32c..7f5648f3 100644
> >> --- a/src/apps/qcam/format_converter.cpp
> >> +++ b/src/apps/qcam/format_converter.cpp
> >> @@ -169,6 +169,10 @@ int FormatConverter::configure(const libcamera::PixelFormat &format,
> >>   		formatFamily_ = MJPEG;
> >>   		break;
> >>   
> >> +	case libcamera::formats::Y10_IPU3:
> >> +		formatFamily_ = IPU3Packed;
> >> +		break;
> >> +
> >>   	default:
> >>   		return -EINVAL;
> >>   	};
> >> @@ -199,6 +203,9 @@ void FormatConverter::convert(const Image *src, size_t size, QImage *dst)
> >>   	case YUVPlanar:
> >>   		convertYUVPlanar(src, dst->bits());
> >>   		break;
> >> +	case IPU3Packed:
> >> +		convertIPU3Packed(src, dst->bits(), size);
> >> +		break;

Will we need to soon write a generic-purpose format conversion library ?
:-(

> >>   	};
> >>   }
> >>   
> >> @@ -357,3 +364,46 @@ void FormatConverter::convertYUVSemiPlanar(const Image *srcImage, unsigned char
> >>   		}
> >>   	}
> >>   }
> >> +
> >> +void FormatConverter::convertIPU3Packed(const Image *srcImage, unsigned char *dst, size_t size)
> >> +{
> >> +	const unsigned char *src = srcImage->data(0).data();
> >> +	unsigned int bytesprocessed = 0;
> >> +	unsigned int lsb_shift;
> >> +	unsigned int msb_shift;
> >> +	unsigned int index;
> >> +	unsigned int x = 0;
> >> +	uint16_t pixel;
> >> +
> >> +	while (bytesprocessed < size) {
> >> +		for (unsigned int i = 0; i < 25; ++i) {
> >> +			index = (i * 10) / 8;
> >> +			lsb_shift = (i * 10) % 8;
> >> +			msb_shift = 8 - lsb_shift;
> >> +
> >> +			/*
> >> +			 * The IPU3-packed format can result in padding at the
> >> +			 * end of a 32-byte block if the last pixel in a row is
> >> +			 * within that block. Check whether we're on the line's
> >> +			 * last pixel and skip the rest of the block if so.
> >> +			 */
> >> +			if (x == width_) {
> >> +				x = 0;
> >> +				dst += width_ * 4;
> >> +				break;
> >> +			}
> >> +
> >> +			pixel = (((src + bytesprocessed)[index+1] << msb_shift) & 0x3ff)
> >> +			      | (((src + bytesprocessed)[index+0] >> lsb_shift) & 0x3ff);
> >> +
> >> +			dst[4 * x + 0] = (pixel >> 2) & 0xff;
> >> +			dst[4 * x + 1] = (pixel >> 2) & 0xff;
> >> +			dst[4 * x + 2] = (pixel >> 2) & 0xff;
> >> +			dst[4 * x + 3] = 0xff;
> >> +
> >> +			x++;
> >> +		}
> >> +
> >> +		bytesprocessed += 32;

There's probably room for improvement when it comes to efficiency here.
We can optimize the code later, but I wonder if 

			pixel = ((src[index+1] << msb_shift) & 0x3ff)
			      | ((src[index+0] >> lsb_shift) & 0x3ff);

		...

		bytesprocessed += 32;
		src += 32;

couldn't help already. Doing something similar with dst to avoid the 4*x
offset could also possibly help.

> >> +	}
> >> +}
> >> diff --git a/src/apps/qcam/format_converter.h b/src/apps/qcam/format_converter.h
> >> index 37dbfae2..940f8b6b 100644
> >> --- a/src/apps/qcam/format_converter.h
> >> +++ b/src/apps/qcam/format_converter.h
> >> @@ -31,12 +31,14 @@ private:
> >>   		YUVPacked,
> >>   		YUVPlanar,
> >>   		YUVSemiPlanar,
> >> +		IPU3Packed,
> >>   	};
> >>   
> >>   	void convertRGB(const Image *src, unsigned char *dst);
> >>   	void convertYUVPacked(const Image *src, unsigned char *dst);
> >>   	void convertYUVPlanar(const Image *src, unsigned char *dst);
> >>   	void convertYUVSemiPlanar(const Image *src, unsigned char *dst);
> >> +	void convertIPU3Packed(const Image *src, unsigned char *dst, size_t size);
> >>   
> >>   	libcamera::PixelFormat format_;
> >>   	unsigned int width_;

-- 
Regards,

Laurent Pinchart