[libcamera-devel,09/11] apps: qcam: Add support for IPU3_Y10
diff mbox series

Message ID 20230318234014.29506-10-dan.scally@ideasonboard.com
State New
Headers show
Series
  • Support OV7251 in IPU3 pipeline and qcam
Related show

Commit Message

Daniel Scally March 18, 2023, 11:40 p.m. UTC 
Add support for the IPU3_Y10 format to the FormatConverter.

Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
---
 src/apps/qcam/format_converter.cpp | 50 ++++++++++++++++++++++++++++++
 src/apps/qcam/format_converter.h   |  2 ++
 2 files changed, 52 insertions(+)

Comments

Laurent Pinchart March 19, 2023, 11:34 p.m. UTC | #1 
Hi Dan,

Thank you for the patch.

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 ?

> Signed-off-by: Daniel Scally <dan.scally@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;
>  	};
>  }
>  
> @@ -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;
> +	}
> +}
> 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_;
Daniel Scally April 17, 2023, 9:59 a.m. UTC | #2 
Hi Laurent

On 19/03/2023 23:34, Laurent Pinchart wrote:
> Hi Dan,
>
> Thank you for the patch.
>
> 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.

>
>> Signed-off-by: Daniel Scally <dan.scally@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;
>>   	};
>>   }
>>   
>> @@ -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;
>> +	}
>> +}
>> 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_;
Laurent Pinchart April 18, 2023, 4:37 p.m. UTC | #3 
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@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_;

Patch
diff mbox series 

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;
 	};
 }
 
@@ -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;
+	}
+}
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_;