[{"id":11173,"web_url":"https://patchwork.libcamera.org/comment/11173/","msgid":"<20200704185739.GG6018@pendragon.ideasonboard.com>","date":"2020-07-04T18:57:39","subject":"Re: [libcamera-devel] [PATCH v3 09/22] v4l2: v4l2_camera_proxy: Get\n\tstride and frameSize from stream config","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, Jul 04, 2020 at 10:31:27PM +0900, Paul Elder wrote:\n> The stride and frameSize should be obtained through StreamConfiguration\n> rather than PixelFormatInfo, as pipeline handlers might have different\n> values (eg. for alignment). Get the stride and frameSize values from\n> StreamConfiguration instead of from PixelFormatInfo.\n> \n> This also removes the need for V4L2CameraProxy::calculateSizeImage, so\n> remove it.\n> \n> Signed-off-by: Paul Elder \n\nReviewed-by: Laurent Pinchart \n\n> ---\n> New in v3\n> ---\n> src/v4l2/v4l2_camera_proxy.cpp | 42 ++++------------------------------\n> src/v4l2/v4l2_camera_proxy.h | 1 -\n> 2 files changed, 5 insertions(+), 38 deletions(-)\n> \n> diff --git a/src/v4l2/v4l2_camera_proxy.cpp b/src/v4l2/v4l2_camera_proxy.cpp\n> index 6a31415..9121d3d 100644\n> --- a/src/v4l2/v4l2_camera_proxy.cpp\n> +++ b/src/v4l2/v4l2_camera_proxy.cpp\n> @@ -71,7 +71,6 @@ int V4L2CameraProxy::open(V4L2CameraFile *file)\n> \n> \tvcam_->getStreamConfig(&streamConfig_);\n> \tsetFmtFromConfig(streamConfig_);\n> -\tsizeimage_ = calculateSizeImage(streamConfig_);\n> \n> \tfiles_.insert(file);\n> \n> @@ -166,30 +165,20 @@ bool V4L2CameraProxy::validateMemoryType(uint32_t memory)\n> void V4L2CameraProxy::setFmtFromConfig(StreamConfiguration &streamConfig)\n> {\n> \tconst PixelFormatInfo &info = PixelFormatInfo::info(streamConfig.pixelFormat);\n> -\tSize size = streamConfig.size;\n> \n> -\tcurV4L2Format_.fmt.pix.width = size.width;\n> -\tcurV4L2Format_.fmt.pix.height = size.height;\n> +\tcurV4L2Format_.fmt.pix.width = streamConfig.size.width;\n> +\tcurV4L2Format_.fmt.pix.height = streamConfig.size.height;\n> \tcurV4L2Format_.fmt.pix.pixelformat = info.v4l2Format;\n> \tcurV4L2Format_.fmt.pix.field = V4L2_FIELD_NONE;\n> -\tcurV4L2Format_.fmt.pix.bytesperline = info.stride(size.width, 0);\n> -\tcurV4L2Format_.fmt.pix.sizeimage = info.frameSize(size);\n> +\tcurV4L2Format_.fmt.pix.bytesperline = streamConfig.stride;\n> +\tcurV4L2Format_.fmt.pix.sizeimage = streamConfig.frameSize;\n> \tcurV4L2Format_.fmt.pix.colorspace = V4L2_COLORSPACE_SRGB;\n> \tcurV4L2Format_.fmt.pix.priv = V4L2_PIX_FMT_PRIV_MAGIC;\n> \tcurV4L2Format_.fmt.pix.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;\n> \tcurV4L2Format_.fmt.pix.quantization = V4L2_QUANTIZATION_DEFAULT;\n> \tcurV4L2Format_.fmt.pix.xfer_func = V4L2_XFER_FUNC_DEFAULT;\n> -}\n> -\n> -unsigned int V4L2CameraProxy::calculateSizeImage(StreamConfiguration &streamConfig)\n> -{\n> -\t/*\n> -\t * \\todo Merge this method with setFmtFromConfig (need imageSize to\n> -\t * support all libcamera formats first, or filter out MJPEG for now).\n> -\t */\n> -\tconst PixelFormatInfo &info = PixelFormatInfo::info(streamConfig.pixelFormat);\n> \n> -\treturn info.frameSize(streamConfig.size);\n> +\tsizeimage_ = streamConfig.frameSize;\n> }\n> \n> void V4L2CameraProxy::querycap(std::shared_ptr camera)\n> @@ -359,12 +348,6 @@ int V4L2CameraProxy::vidioc_s_fmt(V4L2CameraFile *file, struct v4l2_format *arg)\n> \tif (ret < 0)\n> \t\treturn -EINVAL;\n> \n> -\tunsigned int sizeimage = calculateSizeImage(streamConfig_);\n> -\tif (sizeimage == 0)\n> -\t\treturn -EINVAL;\n> -\n> -\tsizeimage_ = sizeimage;\n> -\n> \tsetFmtFromConfig(streamConfig_);\n> \n> \treturn 0;\n> @@ -505,21 +488,6 @@ int V4L2CameraProxy::vidioc_reqbufs(V4L2CameraFile *file, struct v4l2_requestbuf\n> \tif (ret < 0)\n> \t\treturn -EINVAL;\n> \n> -\tsizeimage_ = calculateSizeImage(streamConfig_);\n> -\t/*\n> -\t * If we return -EINVAL here then the application will think that we\n> -\t * don't support streaming mmap. Since we don't support readwrite and\n> -\t * userptr either, the application will get confused and think that\n> -\t * we don't support anything.\n> -\t * On the other hand, if the set format at the time of reqbufs has a\n> -\t * zero sizeimage we'll get a floating point exception when we try to\n> -\t * stream it.\n> -\t */\n> -\tif (sizeimage_ == 0)\n> -\t\tLOG(V4L2Compat, Warning)\n> -\t\t\t<< \"sizeimage of at least one format is zero. \"\n> -\t\t\t<< \"Streaming this format will cause a floating point exception.\";\n> -\n> \tsetFmtFromConfig(streamConfig_);\n> \n> \targ->count = streamConfig_.bufferCount;\n> diff --git a/src/v4l2/v4l2_camera_proxy.h b/src/v4l2/v4l2_camera_proxy.h\n> index 49184a1..e962694 100644\n> --- a/src/v4l2/v4l2_camera_proxy.h\n> +++ b/src/v4l2/v4l2_camera_proxy.h\n> @@ -40,7 +40,6 @@ private:\n> \tbool validateBufferType(uint32_t type);\n> \tbool validateMemoryType(uint32_t memory);\n> \tvoid setFmtFromConfig(StreamConfiguration &streamConfig);\n> -\tunsigned int calculateSizeImage(StreamConfiguration &streamConfig);\n> \tvoid querycap(std::shared_ptr camera);\n> \tvoid tryFormat(struct v4l2_format *arg);\n> \tenum v4l2_priority maxPriority();","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id A9976BD790\n\tfor ;\n\tSat, 4 Jul 2020 18:57:45 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 43830609C7;\n\tSat, 4 Jul 2020 20:57:45 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CC2FE609C7\n\tfor ;\n\tSat, 4 Jul 2020 20:57:43 +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 49355296;\n\tSat, 4 Jul 2020 20:57:43 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"r7VvJz7k\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1593889063;\n\tbh=QL/iZeQOfvoOZgT5BMXLQSZ82r+y+GVRkIVSE3Xhs1E=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=r7VvJz7kg7GBPbsexmxoVhmKUZEkKOiP+ipViTHA3p9lemFSW+0ZQAE8AuOAZRkui\n\tG5xAC94Bg4EXtH8nwS0vtRej7p7/SmHJ3vDdBwyFaHomhxExZlOvKD6jN7Ff0CB10F\n\tryqPYYSb8fzPmL0yP3G5sJLm5SU1lihuPQ9Py/eM=","Date":"Sat, 4 Jul 2020 21:57:39 +0300","From":"Laurent Pinchart ","To":"Paul Elder ","Message-ID":"<20200704185739.GG6018@pendragon.ideasonboard.com>","References":"<20200704133140.1738660-1-paul.elder@ideasonboard.com>\n\t<20200704133140.1738660-10-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200704133140.1738660-10-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 09/22] v4l2: v4l2_camera_proxy: Get\n\tstride and frameSize from stream config","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]