[libcamera-devel] [PATCH v2 2/2] qcam: format_converter: Add NV formats support
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Mon May 6 22:30:17 CEST 2019
Hi Paul,
Thank you for the patch.
On Mon, May 06, 2019 at 04:15:28PM -0400, Paul Elder wrote:
> Add support for some NV formats:
> - V4L2_PIX_FMT_NV12, V4L2_PIX_FMT_NV21
> - V4L2_PIX_FMT_NV16, V4L2_PIX_FMT_NV61
> - V4L2_PIX_FMT_NV24, V4L2_PIX_FMT_NV42
>
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> ---
> Changes in v2:
> - reorder functions in format_converter.cpp to match format_converter.h
> (move yuv_to_rgb() to be before all specialized convert functions)
> - renamed NV conversion parameters from xDownSample_ and yDownSample_ to
> horzSubSample_ and vertSubSample_, respectively
> - unrolled the loop and simplify some of the calculations in convertNV()
> and achieved a 1.67 speedup compared to v1
>
> src/qcam/format_converter.cpp | 99 +++++++++++++++++++++++++++++++----
> src/qcam/format_converter.h | 7 +++
> 2 files changed, 96 insertions(+), 10 deletions(-)
>
> diff --git a/src/qcam/format_converter.cpp b/src/qcam/format_converter.cpp
> index 192767c..eab222f 100644
> --- a/src/qcam/format_converter.cpp
> +++ b/src/qcam/format_converter.cpp
> @@ -72,6 +72,42 @@ int FormatConverter::configure(unsigned int format, unsigned int width,
> y_pos_ = 0;
> cb_pos_ = 1;
> break;
> + case V4L2_PIX_FMT_NV12:
> + formatFamily_ = NV;
> + horzSubSample_ = 2;
> + vertSubSample_ = 2;
> + nvSwap_ = false;
> + break;
> + case V4L2_PIX_FMT_NV21:
> + formatFamily_ = NV;
> + horzSubSample_ = 2;
> + vertSubSample_ = 2;
> + nvSwap_ = true;
> + break;
> + case V4L2_PIX_FMT_NV16:
> + formatFamily_ = NV;
> + horzSubSample_ = 2;
> + vertSubSample_ = 1;
> + nvSwap_ = false;
> + break;
> + case V4L2_PIX_FMT_NV61:
> + formatFamily_ = NV;
> + horzSubSample_ = 2;
> + vertSubSample_ = 1;
> + nvSwap_ = true;
> + break;
> + case V4L2_PIX_FMT_NV24:
> + formatFamily_ = NV;
> + horzSubSample_ = 1;
> + vertSubSample_ = 1;
> + nvSwap_ = false;
> + break;
> + case V4L2_PIX_FMT_NV42:
> + formatFamily_ = NV;
> + horzSubSample_ = 1;
> + vertSubSample_ = 1;
> + nvSwap_ = true;
> + break;
> case V4L2_PIX_FMT_MJPEG:
> formatFamily_ = MJPEG;
> break;
> @@ -99,9 +135,62 @@ void FormatConverter::convert(const unsigned char *src, size_t size,
> case RGB:
> convertRGB(src, dst->bits());
> break;
> + case NV:
> + convertNV(src, dst->bits());
> + break;
You maye want to reorder the cases to match the order of the functions
as well as the order of the enum.
> };
> }
>
> +static void yuv_to_rgb(int y, int u, int v, int *r, int *g, int *b)
> +{
> + int c = y - 16;
> + int d = u - 128;
> + int e = v - 128;
> + *r = CLIP(( 298 * c + 409 * e + 128) >> RGBSHIFT);
> + *g = CLIP(( 298 * c - 100 * d - 208 * e + 128) >> RGBSHIFT);
> + *b = CLIP(( 298 * c + 516 * d + 128) >> RGBSHIFT);
> +}
> +
> +void FormatConverter::convertNV(const unsigned char *src, unsigned char *dst)
> +{
> + unsigned int x, y;
No need to define those variables at the top level scope, you can define
them inside the for statement (as in "for (unsigne int x = 0; ...)").
> + int r, g, b;
> +
> + unsigned int c_stride = width_ * (2 / horzSubSample_);
> + unsigned int c_inc = horzSubSample_ == 1 ? 2 : 0;
> + unsigned int cb_pos = nvSwap_ ? 1 : 0;
> + unsigned int cr_pos = nvSwap_ ? 0 : 1;
> + unsigned int base = width_ * height_;
Small improvement I believe, you could add
const unsigned char *src_c = src + width_ * height_;
and remove the base variable.
> +
> + for (y = 0; y < height_; y++) {
> + unsigned char *src_y = (unsigned char *)src + y * width_;
Don't blindly cast a const pointer to non-const, that's asking for
trouble (furthermore, C++-type casts should otherwise have been used,
with const_cast<unsigned char*>(src)). YOu can declare src_y as const
unsigned char * and everything should be fine.
> + unsigned char *src_cb = (unsigned char *)src + base + (y / vertSubSample_) * c_stride + cb_pos;
While exceptions are allowed, we still try to cap the line length to 80
characters when it doesn't hinder readability.
> + unsigned char *src_cr = (unsigned char *)src + base + (y / vertSubSample_) * c_stride + cr_pos;
> +
> + for (x = 0; x < width_; x += 2) {
> + yuv_to_rgb(*src_y, *src_cb, *src_cr, &r, &g, &b);
> + dst[0] = b;
> + dst[1] = g;
> + dst[2] = r;
> + dst[3] = 0xff;
> + src_y++;
> + src_cb += c_inc;
> + src_cr += c_inc;
> + dst += 4;
> +
> + yuv_to_rgb(*src_y, *src_cb, *src_cr, &r, &g, &b);
> + dst[0] = b;
> + dst[1] = g;
> + dst[2] = r;
> + dst[3] = 0xff;
> + src_y++;
> + src_cb += 2;
> + src_cr += 2;
> + dst += 4;
> + }
> + }
> +}
> +
> void FormatConverter::convertRGB(const unsigned char *src, unsigned char *dst)
> {
> unsigned int x, y;
> @@ -124,16 +213,6 @@ void FormatConverter::convertRGB(const unsigned char *src, unsigned char *dst)
> }
> }
>
> -static void yuv_to_rgb(int y, int u, int v, int *r, int *g, int *b)
> -{
> - int c = y - 16;
> - int d = u - 128;
> - int e = v - 128;
> - *r = CLIP(( 298 * c + 409 * e + 128) >> RGBSHIFT);
> - *g = CLIP(( 298 * c - 100 * d - 208 * e + 128) >> RGBSHIFT);
> - *b = CLIP(( 298 * c + 516 * d + 128) >> RGBSHIFT);
> -}
> -
> void FormatConverter::convertYUV(const unsigned char *src, unsigned char *dst)
> {
> unsigned int src_x, src_y, dst_x, dst_y;
> diff --git a/src/qcam/format_converter.h b/src/qcam/format_converter.h
> index c18871e..ad67081 100644
> --- a/src/qcam/format_converter.h
> +++ b/src/qcam/format_converter.h
> @@ -16,6 +16,7 @@ class FormatConverter
> public:
> enum FormatFamily {
> MJPEG,
> + NV,
> RGB,
> YUV,
> };
> @@ -26,6 +27,7 @@ public:
> void convert(const unsigned char *src, size_t size, QImage *dst);
>
> private:
> + void convertNV(const unsigned char *src, unsigned char *dst);
> void convertRGB(const unsigned char *src, unsigned char *dst);
> void convertYUV(const unsigned char *src, unsigned char *dst);
>
> @@ -35,6 +37,11 @@ private:
>
> enum FormatFamily formatFamily_;
>
> + /* NV parameters */
> + unsigned int horzSubSample_;
> + unsigned int vertSubSample_;
> + bool nvSwap_;
> +
> /* RGB parameters */
> unsigned int bpp_;
> unsigned int r_pos_;
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list