[{"id":1561,"web_url":"https://patchwork.libcamera.org/comment/1561/","msgid":"<20190506203118.GD5021@pendragon.ideasonboard.com>","date":"2019-05-06T20:31:18","subject":"Re: [libcamera-devel] [PATCH v2 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 Mon, May 06, 2019 at 04:15:27PM -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> Reviewed-by: Laurent Pinchart \n> ---\n> Changes in v2: rename enum formatFamily to enum FormatFamily\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..c18871e 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> +\t\tMJPEG,\n> +\t\tRGB,\n> +\t\tYUV,\n> +\t};\n> +\n\nYou only use this enum inside the class, you could move it to the\nprivate section.\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[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A65EC60E63\n\tfor ;\n\tMon, 6 May 2019 22:31:32 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 41916115F;\n\tMon, 6 May 2019 22:31:32 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1557174692;\n\tbh=H1UcL6Nj/XFcCQO5wIYNi5G/UuLpyBoK2gNrRzpsjdA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=FkHNwH5N+LpzkYnmUEUxuG2yN+w4e7YL5FVyvocLgvnjdElaB1Xgout0gqL/+g+kD\n\tTqn781+Wgpm18I9yc5HjW1EbthXV+AV9gczzOhJ04AgYi/SfoOLuMYrCTnuRgZuj3x\n\trCdyeJdIh0L4ri3e9C2A7mJzLB6WNWW4wyA4jXFo=","Date":"Mon, 6 May 2019 23:31:18 +0300","From":"Laurent Pinchart ","To":"Paul Elder ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190506203118.GD5021@pendragon.ideasonboard.com>","References":"<20190506201528.24445-1-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20190506201528.24445-1-paul.elder@ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v2 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":"Mon, 06 May 2019 20:31:32 -0000"}}]