[{"id":27442,"web_url":"https://patchwork.libcamera.org/comment/27442/","msgid":"","date":"2023-06-30T12:44:47","subject":"Re: [libcamera-devel] [RFC PATCH 1/5] rpi: cam_helper_imx708: Use\n\tSpan<> to pass PDAF data","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Umang,\n\nThank you for this work.\n\nOn Fri, 30 Jun 2023 at 13:03, Umang Jain wrote:\n>\n> Instead of passing raw buffer pointer and length, construct a Span<>\n> and pass it in parsePdafData().\n>\n> While at it, introduce a constexpr which denotes the scanline of the\n> PDAF data in the embedded data. Use that constexpr to compute the\n> offset of PDAF buffer in the embedded data.\n>\n> Np functional changes intended in this patch.\n>\n> Signed-off-by: Umang Jain \n> ---\n> src/ipa/rpi/cam_helper/cam_helper_imx708.cpp | 25 +++++++++++---------\n> 1 file changed, 14 insertions(+), 11 deletions(-)\n>\n> diff --git a/src/ipa/rpi/cam_helper/cam_helper_imx708.cpp b/src/ipa/rpi/cam_helper/cam_helper_imx708.cpp\n> index 641ba18f..b24ee643 100644\n> --- a/src/ipa/rpi/cam_helper/cam_helper_imx708.cpp\n> +++ b/src/ipa/rpi/cam_helper/cam_helper_imx708.cpp\n> @@ -42,6 +42,9 @@ constexpr std::initializer_list registerList =\n> { expHiReg, expLoReg, gainHiReg, gainLoReg, lineLengthHiReg,\n> lineLengthLoReg, frameLengthHiReg, frameLengthLoReg, temperatureReg };\n>\n> +/* PDAF data is expect to occupy the third scanline of embedded data. */\n> +constexpr uint32_t pdafLineOffsetImx708 = 2;\n\nCould this live in the private member area of the helper class? We have other\nconst values listed there. Also, I would remove the \"Imx708\" suffix.\n\nRegards,\nNaush\n\n> +\n> class CamHelperImx708 : public CamHelper\n> {\n> public:\n> @@ -75,7 +78,7 @@ private:\n> void populateMetadata(const MdParser::RegisterMap ®isters,\n> Metadata &metadata) const override;\n>\n> - static bool parsePdafData(const uint8_t *ptr, size_t len, unsigned bpp,\n> + static bool parsePdafData(libcamera::Span &pdafData, unsigned bpp,\n> PdafRegions &pdaf);\n>\n> bool parseAEHist(const uint8_t *ptr, size_t len, unsigned bpp);\n> @@ -116,17 +119,16 @@ void CamHelperImx708::prepare(libcamera::Span buffer, Metadata &m\n>\n> parseEmbeddedData(buffer, metadata);\n>\n> - /*\n> - * Parse PDAF data, which we expect to occupy the third scanline\n> - * of embedded data. As PDAF is quite sensor-specific, it's parsed here.\n> - */\n> + /* Parse sensor-specific PDAF data. */\n> size_t bytesPerLine = (mode_.width * mode_.bitdepth) >> 3;\n> + size_t pdafDataOffset = pdafLineOffsetImx708 * bytesPerLine;\n>\n> - if (buffer.size() > 2 * bytesPerLine) {\n> + if (buffer.size() > pdafDataOffset) {\n> PdafRegions pdaf;\n> - if (parsePdafData(&buffer[2 * bytesPerLine],\n> - buffer.size() - 2 * bytesPerLine,\n> - mode_.bitdepth, pdaf))\n> + libcamera::Span pdafData{ &buffer[pdafDataOffset],\n> + buffer.size() - pdafDataOffset };\n> +\n> + if (parsePdafData(pdafData, mode_.bitdepth, pdaf))\n> metadata.set(\"pdaf.regions\", pdaf);\n> }\n>\n> @@ -241,9 +243,10 @@ void CamHelperImx708::populateMetadata(const MdParser::RegisterMap ®isters,\n> metadata.set(\"device.status\", deviceStatus);\n> }\n>\n> -bool CamHelperImx708::parsePdafData(const uint8_t *ptr, size_t len,\n> - unsigned bpp, PdafRegions &pdaf)\n> +bool CamHelperImx708::parsePdafData(Span &data, unsigned bpp, PdafRegions &pdaf)\n> {\n> + const uint8_t *ptr = data.data();\n> + unsigned int len = data.size();\n> size_t step = bpp >> 1; /* bytes per PDAF grid entry */\n>\n> if (bpp < 10 || bpp > 12 || len < 194 * step || ptr[0] != 0 || ptr[1] >= 0x40) {\n> --\n> 2.39.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 08FC9BDB1D\n\tfor ;\n\tFri, 30 Jun 2023 12:45:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6F592628C0;\n\tFri, 30 Jun 2023 14:45:06 +0200 (CEST)","from mail-yw1-x1130.google.com (mail-yw1-x1130.google.com\n\t[IPv6:2607:f8b0:4864:20::1130])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 829AD60454\n\tfor ;\n\tFri, 30 Jun 2023 14:45:04 +0200 (CEST)","by mail-yw1-x1130.google.com with SMTP id\n\t00721157ae682-576a9507a9bso26223617b3.1\n\tfor ;\n\tFri, 30 Jun 2023 05:45:04 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1688129106;\n\tbh=bKHUK4sz30kP4qTjAE03Wc7KeFvD/y5yEU6ntqhcViY=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=eI7XaOTbXNqRkSvBg/o1ivYZnPlthXKwFHvBHCDO+D5/pavlftsGOQFv/alk1HBGM\n\t+sRvvidJWCxq9NEviaUV85GXwD1lSrhKs9TCjWMK5r3u38UtF6vHf9XQNfKVcFXzAP\n\tBu/YznaAcDDawlhY8DV2qaVanMhqGeG9vM5cq9BApcWYKgW+eGLKlmfgP5m/aJjehZ\n\tbp8lx6U3zUpGnIAPxqRYszx8Y4Yb9xn2181fnsJADadYIkQPcA2UrAlkdTniDrAsnz\n\t+1w7s7gaKDPn+A2weMU/I1wz1SQopkJ3NzgMdW9aixOYB8PzhRYiThuMcnSMEDgxeA\n\tFfdpEmvnn9s/g==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1688129103; x=1690721103;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=vSbFlIWsU7h2h+y/zIob92g4W0ZabA36nikONPUj+eY=;\n\tb=HZUKUKrMPdYEZL53D8Ji1KMgHZ4A8aEczzcxprvW3NQkw0bNeVQXdNGkVAtqGpQWtu\n\tkhsZARSZKDVJNtyLv9MI5D0uw3H8QZdRzWpZT3/na92Oa4vCdFLnsQ7oW8ncR2GUa3/h\n\tTSie45h42VNv/KT11wQYZC61w3n76DGqjzJwvA7U5xLmJDkrmgTB/cP8bmZCAnzt6ZZ/\n\tujx6v2ILDU7hGF/mL1sSjtppb5xFuUyQGvTfmEjcFPX0H1kF2WRyqbH8IWehTtfvrn9L\n\tTPdEVE3pLxkX5YOAxgvhFxBjuCAO4ta81jcbc3DMvGP0SjnqZaioSS4m5FPxuA34r0p7\n\tUxhA=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"HZUKUKrM\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20221208; t=1688129103; x=1690721103;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=vSbFlIWsU7h2h+y/zIob92g4W0ZabA36nikONPUj+eY=;\n\tb=YgJS/OXXgDRJua2nr3RRTUxE+lV3NIc+BINVOireYfIbmsgfyFY5Qj3mhjnEjFgmbX\n\tJAVVDArEsdxL6J856LjahQGZ/FvmEFWwR0KZqkJ0Ip5b/vvttbx9NVE2PnBzEtkuEobl\n\tlhrwzHR3E4/dY7ZzhsiKxbDTSWG9piT5MfyN+lQrn8hirz1+8TQB60Fo/wOjzYGPMRzD\n\tXwA6+78Dd2T9Am3lgofcOOUnTNnlxJQqvaV0b9RPi+4T+74XYZ1T4ed62jP+1EPDizA5\n\tv85wm7p3L4c/q33cY+lcQDQ5u8tGi+s5dyy7R2ELF6aSlXJNCBdZky0NzhQFpQ19tUtY\n\ty0uA==","X-Gm-Message-State":"ABy/qLbxUqBGoEKnv5boO2zmMH4PNGb9NJ86Sz04KPd2cPuBIMFlKPSI\n\t+WyoOcr5yQ68Vm8oq3U9Gj9yn579CSaeaBGwY8EHNQ==","X-Google-Smtp-Source":"APBJJlEeGSiBdQ9VAv1eNPmUNWborHbEesHXmxOFUDK12PkxytZUzLf+GxNjqyiqllqzj4LeXInZXEhXumXWleBgWho=","X-Received":"by 2002:a0d:eb4e:0:b0:570:8b1e:cdaf with SMTP id\n\tu75-20020a0deb4e000000b005708b1ecdafmr2608560ywe.22.1688129103275;\n\tFri, 30 Jun 2023 05:45:03 -0700 (PDT)","MIME-Version":"1.0","References":"<20230630120303.33023-1-umang.jain@ideasonboard.com>\n\t<20230630120303.33023-2-umang.jain@ideasonboard.com>","In-Reply-To":"<20230630120303.33023-2-umang.jain@ideasonboard.com>","Date":"Fri, 30 Jun 2023 13:44:47 +0100","Message-ID":"","To":"Umang Jain ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [RFC PATCH 1/5] rpi: cam_helper_imx708: Use\n\tSpan<> to pass PDAF data","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","From":"Naushir Patuck via libcamera-devel\n\t","Reply-To":"Naushir Patuck ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]