[{"id":17528,"web_url":"https://patchwork.libcamera.org/comment/17528/","msgid":"","date":"2021-06-14T16:32:35","subject":"Re: [libcamera-devel] [PATCH 2/6] ipa: raspberrypi: Set default\n\tvalues for member variables of MdParser","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Naush\n\nThanks for fixing this, I should probably have spotted it when I (I\nthink it was me) made the Span change.\n\nOn Mon, 14 Jun 2021 at 10:53, Naushir Patuck wrote:\n>\n> Set some sensible default values for member variables of the MdParser class.\n>\n> Remove buffer_size_bytes_ along with some related asserts as this class now uses\n> libcamera::Span for buffer handling, and buffer_size_bytes_ is unused.\n>\n> Signed-off-by: Naushir Patuck \n> ---\n> src/ipa/raspberrypi/cam_helper_imx219.cpp | 1 -\n> src/ipa/raspberrypi/cam_helper_imx477.cpp | 1 -\n> src/ipa/raspberrypi/md_parser.hpp | 4 ++--\n> 3 files changed, 2 insertions(+), 4 deletions(-)\n>\n> diff --git a/src/ipa/raspberrypi/cam_helper_imx219.cpp b/src/ipa/raspberrypi/cam_helper_imx219.cpp\n> index e550fba62cde..ec218dce5456 100644\n> --- a/src/ipa/raspberrypi/cam_helper_imx219.cpp\n> +++ b/src/ipa/raspberrypi/cam_helper_imx219.cpp\n> @@ -128,7 +128,6 @@ MdParser::Status MdParserImx219::Parse(libcamera::Span buffer)\n> * registers.\n> */\n> assert(bits_per_pixel_);\n> - assert(num_lines_ || buffer_size_bytes_);\n\nI wonder if we should delete line_length_bytes_ and num_lines_ too?\nAfter all, the caller always knows the line length of the buffer so\ncan adjust the Span accordingly, and it would generally make this\nstuff a bit simpler. But no urgency, maybe that's more trouble than\nit's worth at this point... so anyway:\n\nReviewed-by: David Plowman \n\nThanks\nDavid\n\n> /* Need to be ordered */\n> uint32_t regs[3] = { GAIN_REG, EXPHI_REG, EXPLO_REG };\n> reg_offsets_[0] = reg_offsets_[1] = reg_offsets_[2] = -1;\n> diff --git a/src/ipa/raspberrypi/cam_helper_imx477.cpp b/src/ipa/raspberrypi/cam_helper_imx477.cpp\n> index a4a58c15467d..25b36bce0dac 100644\n> --- a/src/ipa/raspberrypi/cam_helper_imx477.cpp\n> +++ b/src/ipa/raspberrypi/cam_helper_imx477.cpp\n> @@ -117,7 +117,6 @@ MdParser::Status MdParserImx477::Parse(libcamera::Span buffer)\n> * registers.\n> */\n> assert(bits_per_pixel_);\n> - assert(num_lines_ || buffer_size_bytes_);\n> /* Need to be ordered */\n> uint32_t regs[4] = {\n> EXPHI_REG,\n> diff --git a/src/ipa/raspberrypi/md_parser.hpp b/src/ipa/raspberrypi/md_parser.hpp\n> index 86e0577614e0..25ba0e7c9400 100644\n> --- a/src/ipa/raspberrypi/md_parser.hpp\n> +++ b/src/ipa/raspberrypi/md_parser.hpp\n> @@ -68,7 +68,8 @@ public:\n> ERROR = 2\n> };\n>\n> - MdParser() : reset_(true)\n> + MdParser()\n> + : reset_(true), bits_per_pixel_(0), num_lines_(0), line_length_bytes_(0)\n> {\n> }\n>\n> @@ -103,7 +104,6 @@ protected:\n> int bits_per_pixel_;\n> unsigned int num_lines_;\n> unsigned int line_length_bytes_;\n> - unsigned int buffer_size_bytes_;\n> };\n>\n> /*\n> --\n> 2.25.1\n>","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 C3B3EBD78E\n\tfor ;\n\tMon, 14 Jun 2021 16:32:48 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 13F6268932;\n\tMon, 14 Jun 2021 18:32:48 +0200 (CEST)","from mail-wm1-x32d.google.com (mail-wm1-x32d.google.com\n\t[IPv6:2a00:1450:4864:20::32d])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7005F602A1\n\tfor ;\n\tMon, 14 Jun 2021 18:32:47 +0200 (CEST)","by mail-wm1-x32d.google.com with SMTP id\n\tl7-20020a05600c1d07b02901b0e2ebd6deso337085wms.1\n\tfor ;\n\tMon, 14 Jun 2021 09:32:47 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"DzIlk4l1\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=lGMihZo+6zrWd1gOC9ZeTbHcA1Q4ZVT7tWAupFm/5LY=;\n\tb=DzIlk4l1sA1tDrwrsY66lri9Ej36/0PKUAK/y5DwGqhsFETWPMCN36QG8zmsbjQ+zd\n\tgv8m6A+AXzWQQZoof0SNaJT6b45UId38qmog7ttBD9dFRJxEpe5pUjeNCFhqJ61TLUHw\n\troKX6G87cx9DQ4yX2ad3Cq5MzvCmto1EW1ET+m5MN1XVndsY+QjUImLyllxzC9zgi30q\n\taUdSpyO11aEa4UBYBr93/Qr92EhjADIFSu8OIDUzZMPr+DNLHdD/TJXKMjih7OupmwYq\n\t2nK7audS0mWHOaGUAFiCxxdbSVLAcViUkyzxwt/s7x0ryBc9Q7fgN5d1VtR694FoEBr0\n\tkCBg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=lGMihZo+6zrWd1gOC9ZeTbHcA1Q4ZVT7tWAupFm/5LY=;\n\tb=TUqeyHGoCbZ99ICeC8XybjLemHZdRNS0ymIHe/dPk+OaF9U55hvvu2ZBHJnscmX72X\n\tR2g2GK8uztCdlii0MLbS4LZNUHa+dyG3+sBWLXc0fEfDqxqZP0WlnxpEIpRts+5rFL5k\n\t6XIG93i7hjzR/nyZLTWdmu3JBItYE6M2GFM+tB7HB7u8V1gcZnyYtMN04XqhVOzi9Q8t\n\t7ND68XN0UO1BlZtO9k2vqdojFsEi6iicaTdJV0yH/ulSRM507knKYlty1t6drfXDLFl9\n\tMCZPtuobX6XDVSe0L87u5eg0uJhbmBdBNhyKHsTJQwg8Txo9Ys8QQgIDWFiHO1bE2rn8\n\tkKxw==","X-Gm-Message-State":"AOAM532JOiGj/JJxqd8nCkNe20earo9qZdPvAaMOLIZxmT+szZYeph0+\n\t2hf0fwYIXn9ZbNhFttlQdWg9ylpsggNywP6Xd1YiaA==","X-Google-Smtp-Source":"ABdhPJxVXyWvbhhvh0sQdLY1p4GAvaFcsUoLMtRAGIL5KqXHnaUzOIT/T2W3jtvCl+h0t/sDAz5PAcaWl+V41WL3GKc=","X-Received":"by 2002:a1c:a382:: with SMTP id\n\tm124mr17013712wme.40.1623688367073; \n\tMon, 14 Jun 2021 09:32:47 -0700 (PDT)","MIME-Version":"1.0","References":"<20210614095340.3051816-1-naush@raspberrypi.com>\n\t<20210614095340.3051816-3-naush@raspberrypi.com>","In-Reply-To":"<20210614095340.3051816-3-naush@raspberrypi.com>","From":"David Plowman ","Date":"Mon, 14 Jun 2021 17:32:35 +0100","Message-ID":"","To":"Naushir Patuck ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH 2/6] ipa: raspberrypi: Set default\n\tvalues for member variables of MdParser","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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":17543,"web_url":"https://patchwork.libcamera.org/comment/17543/","msgid":"","date":"2021-06-15T02:39:48","subject":"Re: [libcamera-devel] [PATCH 2/6] ipa: raspberrypi: Set default\n\tvalues for member variables of MdParser","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Naush,\n\nThank you for the patch.\n\nOn Mon, Jun 14, 2021 at 10:53:36AM +0100, Naushir Patuck wrote:\n> Set some sensible default values for member variables of the MdParser class.\n> \n> Remove buffer_size_bytes_ along with some related asserts as this class now uses\n> libcamera::Span for buffer handling, and buffer_size_bytes_ is unused.\n> \n> Signed-off-by: Naushir Patuck \n\nReviewed-by: Laurent Pinchart \n\n> ---\n> src/ipa/raspberrypi/cam_helper_imx219.cpp | 1 -\n> src/ipa/raspberrypi/cam_helper_imx477.cpp | 1 -\n> src/ipa/raspberrypi/md_parser.hpp | 4 ++--\n> 3 files changed, 2 insertions(+), 4 deletions(-)\n> \n> diff --git a/src/ipa/raspberrypi/cam_helper_imx219.cpp b/src/ipa/raspberrypi/cam_helper_imx219.cpp\n> index e550fba62cde..ec218dce5456 100644\n> --- a/src/ipa/raspberrypi/cam_helper_imx219.cpp\n> +++ b/src/ipa/raspberrypi/cam_helper_imx219.cpp\n> @@ -128,7 +128,6 @@ MdParser::Status MdParserImx219::Parse(libcamera::Span buffer)\n> \t\t * registers.\n> \t\t */\n> \t\tassert(bits_per_pixel_);\n> -\t\tassert(num_lines_ || buffer_size_bytes_);\n> \t\t/* Need to be ordered */\n> \t\tuint32_t regs[3] = { GAIN_REG, EXPHI_REG, EXPLO_REG };\n> \t\treg_offsets_[0] = reg_offsets_[1] = reg_offsets_[2] = -1;\n> diff --git a/src/ipa/raspberrypi/cam_helper_imx477.cpp b/src/ipa/raspberrypi/cam_helper_imx477.cpp\n> index a4a58c15467d..25b36bce0dac 100644\n> --- a/src/ipa/raspberrypi/cam_helper_imx477.cpp\n> +++ b/src/ipa/raspberrypi/cam_helper_imx477.cpp\n> @@ -117,7 +117,6 @@ MdParser::Status MdParserImx477::Parse(libcamera::Span buffer)\n> \t\t * registers.\n> \t\t */\n> \t\tassert(bits_per_pixel_);\n> -\t\tassert(num_lines_ || buffer_size_bytes_);\n> \t\t/* Need to be ordered */\n> \t\tuint32_t regs[4] = {\n> \t\t\tEXPHI_REG,\n> diff --git a/src/ipa/raspberrypi/md_parser.hpp b/src/ipa/raspberrypi/md_parser.hpp\n> index 86e0577614e0..25ba0e7c9400 100644\n> --- a/src/ipa/raspberrypi/md_parser.hpp\n> +++ b/src/ipa/raspberrypi/md_parser.hpp\n> @@ -68,7 +68,8 @@ public:\n> \t\tERROR = 2\n> \t};\n> \n> -\tMdParser() : reset_(true)\n> +\tMdParser()\n> +\t\t: reset_(true), bits_per_pixel_(0), num_lines_(0), line_length_bytes_(0)\n> \t{\n> \t}\n> \n> @@ -103,7 +104,6 @@ protected:\n> \tint bits_per_pixel_;\n> \tunsigned int num_lines_;\n> \tunsigned int line_length_bytes_;\n> -\tunsigned int buffer_size_bytes_;\n> };\n> \n> /*","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 D83EBBD78E\n\tfor ;\n\tTue, 15 Jun 2021 02:40:11 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B813368932;\n\tTue, 15 Jun 2021 04:40:10 +0200 (CEST)","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 6961E68925\n\tfor ;\n\tTue, 15 Jun 2021 04:40:09 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id BEDF7E9;\n\tTue, 15 Jun 2021 04:40:08 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"p5J0RaQ0\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1623724808;\n\tbh=ZVUvxnq3vSKH2+pxtwcqMndA9MTkbmfDRQzcu3MBTf0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=p5J0RaQ04QSDD0KaL863m7N0oJEq8G06kkSioQUROIFhATBxW7XQf0cisfuWTTfGp\n\txKR5xTrTjRB4Tt3dlfPktuz2F/GL3TLF9lAoMcRkSPKaqV7Dbg2JteDg8woQ7mTK07\n\tqf/r0+WPHMba2AdyIDldHIHbWBNiAvE6mW8vW04s=","Date":"Tue, 15 Jun 2021 05:39:48 +0300","From":"Laurent Pinchart ","To":"Naushir Patuck ","Message-ID":"","References":"<20210614095340.3051816-1-naush@raspberrypi.com>\n\t<20210614095340.3051816-3-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210614095340.3051816-3-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH 2/6] ipa: raspberrypi: Set default\n\tvalues for member variables of MdParser","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]