[{"id":1558,"web_url":"https://patchwork.libcamera.org/comment/1558/","msgid":"<20190504223446.GB4922@pendragon.ideasonboard.com>","date":"2019-05-04T22:34:46","subject":"Re: [libcamera-devel] [PATCH 1/2] qcam: format_converter: Add\n\tformatFamily enum","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nThank you for the patch.\n\nOn Sat, May 04, 2019 at 04:33:53PM -0400, Paul Elder wrote:\n> It is not sustainable to add a new flag for every new video format\n> family (eg. YUYV, RGB, NV, MJPEG, etc), so add a formatFamily enum to\n> indicate these in the FormatConverter.\n> \n> Note that formats are grouped into families based on if they are able to\n> share a set of parameters for conversion.\n> \n> Signed-off-by: Paul Elder \n> ---\n> src/qcam/format_converter.cpp | 27 ++++++++++++++++-----------\n> src/qcam/format_converter.h | 11 ++++++++++-\n> 2 files changed, 26 insertions(+), 12 deletions(-)\n> \n> diff --git a/src/qcam/format_converter.cpp b/src/qcam/format_converter.cpp\n> index d9088c3..192767c 100644\n> --- a/src/qcam/format_converter.cpp\n> +++ b/src/qcam/format_converter.cpp\n> @@ -32,48 +32,48 @@ int FormatConverter::configure(unsigned int format, unsigned int width,\n> {\n> \tswitch (format) {\n> \tcase V4L2_PIX_FMT_BGR24:\n> -\t\tyuv_ = false;\n> +\t\tformatFamily_ = RGB;\n> \t\tr_pos_ = 2;\n> \t\tg_pos_ = 1;\n> \t\tb_pos_ = 0;\n> \t\tbpp_ = 3;\n> \t\tbreak;\n> \tcase V4L2_PIX_FMT_RGB24:\n> -\t\tyuv_ = false;\n> +\t\tformatFamily_ = RGB;\n> \t\tr_pos_ = 0;\n> \t\tg_pos_ = 1;\n> \t\tb_pos_ = 2;\n> \t\tbpp_ = 3;\n> \t\tbreak;\n> \tcase V4L2_PIX_FMT_ARGB32:\n> -\t\tyuv_ = false;\n> +\t\tformatFamily_ = RGB;\n> \t\tr_pos_ = 1;\n> \t\tg_pos_ = 2;\n> \t\tb_pos_ = 3;\n> \t\tbpp_ = 4;\n> \t\tbreak;\n> \tcase V4L2_PIX_FMT_VYUY:\n> -\t\tyuv_ = true;\n> +\t\tformatFamily_ = YUV;\n> \t\ty_pos_ = 1;\n> \t\tcb_pos_ = 2;\n> \t\tbreak;\n> \tcase V4L2_PIX_FMT_YVYU:\n> -\t\tyuv_ = true;\n> +\t\tformatFamily_ = YUV;\n> \t\ty_pos_ = 0;\n> \t\tcb_pos_ = 3;\n> \t\tbreak;\n> \tcase V4L2_PIX_FMT_UYVY:\n> -\t\tyuv_ = true;\n> +\t\tformatFamily_ = YUV;\n> \t\ty_pos_ = 1;\n> \t\tcb_pos_ = 0;\n> \t\tbreak;\n> \tcase V4L2_PIX_FMT_YUYV:\n> -\t\tyuv_ = true;\n> +\t\tformatFamily_ = YUV;\n> \t\ty_pos_ = 0;\n> \t\tcb_pos_ = 1;\n> \t\tbreak;\n> \tcase V4L2_PIX_FMT_MJPEG:\n> -\t\tyuv_ = false;\n> +\t\tformatFamily_ = MJPEG;\n> \t\tbreak;\n> \tdefault:\n> \t\treturn -EINVAL;\n> @@ -89,12 +89,17 @@ int FormatConverter::configure(unsigned int format, unsigned int width,\n> void FormatConverter::convert(const unsigned char *src, size_t size,\n> \t\t\t QImage *dst)\n> {\n> -\tif (format_ == V4L2_PIX_FMT_MJPEG)\n> +\tswitch (formatFamily_) {\n> +\tcase MJPEG:\n> \t\tdst->loadFromData(src, size, \"JPEG\");\n> -\telse if (yuv_)\n> +\t\tbreak;\n> +\tcase YUV:\n> \t\tconvertYUV(src, dst->bits());\n> -\telse\n> +\t\tbreak;\n> +\tcase RGB:\n> \t\tconvertRGB(src, dst->bits());\n> +\t\tbreak;\n> +\t};\n> }\n> \n> void FormatConverter::convertRGB(const unsigned char *src, unsigned char *dst)\n> diff --git a/src/qcam/format_converter.h b/src/qcam/format_converter.h\n> index 76cd9f1..9820982 100644\n> --- a/src/qcam/format_converter.h\n> +++ b/src/qcam/format_converter.h\n> @@ -14,6 +14,12 @@ class QImage;\n> class FormatConverter\n> {\n> public:\n> +\tenum formatFamily {\n\nAll type names should start with an uppercase letter, so this should be\nFormatFamily (don't forget to update the one in the commit message). The\nfield name below is fine.\n\nApart from this,\n\nReviewed-by: Laurent Pinchart \n\n> +\t\tMJPEG,\n> +\t\tRGB,\n> +\t\tYUV,\n> +\t};\n> +\n> \tint configure(unsigned int format, unsigned int width,\n> \t\t unsigned int height);\n> \n> @@ -27,12 +33,15 @@ private:\n> \tunsigned int width_;\n> \tunsigned int height_;\n> \n> +\tenum formatFamily formatFamily_;\n> +\n> +\t/* RGB parameters */\n> \tunsigned int bpp_;\n> \tunsigned int r_pos_;\n> \tunsigned int g_pos_;\n> \tunsigned int b_pos_;\n> \n> -\tbool yuv_;\n> +\t/* YUV parameters */\n> \tunsigned int y_pos_;\n> \tunsigned int cb_pos_;\n> };","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CA42C60E54\n\tfor ;\n\tSun, 5 May 2019 00:35:01 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C819FD5;\n\tSun, 5 May 2019 00:34:59 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1557009300;\n\tbh=y7OAmUGKs87kYio08Vb5f6ySmUSvGnNy9wmD8BZnDXA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=n2vM3qDzmWlsHRaTs5+50i8s1EkTNO0sDkUqM9RGRrq9nQwo4FU7rbW3yyGODbYlA\n\tdqJZHosEdOAX7cPWZxVYy5SBPzIvgbMVEmSTApJIyA8hCnuOPYwqkRcVQ2hmm01XM1\n\tIpL1qbzOf2+3vzOJhox00DlqCE7JiV/x7iNontrQ=","Date":"Sun, 5 May 2019 01:34:46 +0300","From":"Laurent Pinchart ","To":"Paul Elder ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190504223446.GB4922@pendragon.ideasonboard.com>","References":"<20190504203354.20870-1-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20190504203354.20870-1-paul.elder@ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 1/2] qcam: format_converter: Add\n\tformatFamily enum","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Sat, 04 May 2019 22:35:02 -0000"}}]