[{"id":20559,"web_url":"https://patchwork.libcamera.org/comment/20559/","msgid":"<163533706941.1184428.10511540727508627850@Monstersaurus>","date":"2021-10-27T12:17:49","subject":"Re: [libcamera-devel] [PATCH v3 6/9] pipeline: raspberrypi: Set\n\tpacking formats for the Unicam image node","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Naushir Patuck (2021-10-27 10:28:00)\n> Default to using CSI2 packed formats when setting up the Unicam image format,\n> but use an unpacked format if the user requests one through StreamConfiguration.\n> \n> Signed-off-by: Naushir Patuck \n> Reviewed-by: David Plowman \n> ---\n> src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 12 ++++++++----\n> 1 file changed, 8 insertions(+), 4 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index 48f561d31a31..1b78b5e74a63 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -96,14 +96,14 @@ BayerFormat mbusCodeToBayerFormat(unsigned int mbusCode)\n> return BayerFormat{};\n> }\n> \n> -V4L2DeviceFormat toV4L2DeviceFormat(V4L2SubdeviceFormat &format)\n> +V4L2DeviceFormat toV4L2DeviceFormat(V4L2SubdeviceFormat &format, BayerFormat::Packing packing)\n> {\n> V4L2DeviceFormat deviceFormat;\n> BayerFormat bayer = mbusCodeToBayerFormat(format.mbus_code);\n> \n> ASSERT(bayer.isValid());\n> \n> - bayer.packing = BayerFormat::Packing::CSI2Packed;\n> + bayer.packing = packing;\n\nOk, that really solidifies the answer to my earlier question ;-)\n\n\n> deviceFormat.fourcc = bayer.toV4L2PixelFormat();\n> deviceFormat.size = format.size;\n> return deviceFormat;\n> @@ -413,7 +413,8 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n> * the user request.\n> */\n> V4L2SubdeviceFormat sensorFormat = findBestFormat(data_->sensorFormats_, cfg.size);\n> - V4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(sensorFormat);\n> + V4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(sensorFormat,\n> + BayerFormat::Packing::CSI2Packed);\n\nI believe ::Packing is optional if you want to reduce line length. But\ncan also stay, it won't make a big difference to the line formatting.\n\n\n> int ret = data_->unicam_[Unicam::Image].dev()->tryFormat(&unicamFormat);\n> if (ret)\n> return Invalid;\n> @@ -631,6 +632,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n> for (auto const stream : data->streams_)\n> stream->reset();\n> \n> + BayerFormat::Packing packing = BayerFormat::Packing::CSI2Packed;\n\nSame.\n\n\nReviewed-by: Kieran Bingham \n\n> Size maxSize, sensorSize;\n> unsigned int maxIndex = 0;\n> bool rawStream = false;\n> @@ -649,6 +651,8 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n> */\n> sensorSize = cfg.size;\n> rawStream = true;\n> + /* Check if the user has explicitly set an unpacked format. */\n> + packing = BayerFormat::fromPixelFormat(cfg.pixelFormat).packing;\n> } else {\n> if (cfg.size > maxSize) {\n> maxSize = config->at(i).size;\n> @@ -667,7 +671,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n> * Unicam image output format. The ISP input format gets set at start,\n> * just in case we have swapped bayer orders due to flips.\n> */\n> - V4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(sensorFormat);\n> + V4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(sensorFormat, packing);\n> ret = data->unicam_[Unicam::Image].dev()->setFormat(&unicamFormat);\n> if (ret)\n> return ret;\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 227FCBF415\n\tfor ;\n\tWed, 27 Oct 2021 12:17:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7DED76487F;\n\tWed, 27 Oct 2021 14:17:53 +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 0588F60123\n\tfor ;\n\tWed, 27 Oct 2021 14:17:52 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 97697596;\n\tWed, 27 Oct 2021 14:17:51 +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=\"N7Wfps6S\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1635337071;\n\tbh=z61kCNFq/uJyOJNxPtJPCd6jMEFMNY7JLG6fHth54D4=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=N7Wfps6SlTcphJkVN7HCkIIoc7jZTFqVaMTBAteLLZeAknkJN6r3+DyGctAFd9Uxa\n\tVbUlqI1OO2PdT95dvbsZPMBnvr/kzin90KbtiFsRiHOEtRm8UosFQV+KwuDm77bWxi\n\tbBhJP0cklY8ZbrIM+0AeoYnBNKWZdZvW0KSzv45A=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20211027092803.3671096-7-naush@raspberrypi.com>","References":"<20211027092803.3671096-1-naush@raspberrypi.com>\n\t<20211027092803.3671096-7-naush@raspberrypi.com>","From":"Kieran Bingham ","To":"Naushir Patuck ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Wed, 27 Oct 2021 13:17:49 +0100","Message-ID":"<163533706941.1184428.10511540727508627850@Monstersaurus>","User-Agent":"alot/0.9.1","Subject":"Re: [libcamera-devel] [PATCH v3 6/9] pipeline: raspberrypi: Set\n\tpacking formats for the Unicam image node","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":20570,"web_url":"https://patchwork.libcamera.org/comment/20570/","msgid":"","date":"2021-10-27T12:39:11","subject":"Re: [libcamera-devel] [PATCH v3 6/9] pipeline: raspberrypi: Set\n\tpacking formats for the Unicam image node","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Kieran,\n\nThank you for your feedback.\n\nOn Wed, 27 Oct 2021 at 13:17, Kieran Bingham <\nkieran.bingham@ideasonboard.com> wrote:\n\n> Quoting Naushir Patuck (2021-10-27 10:28:00)\n> > Default to using CSI2 packed formats when setting up the Unicam image\n> format,\n> > but use an unpacked format if the user requests one through\n> StreamConfiguration.\n> >\n> > Signed-off-by: Naushir Patuck \n> > Reviewed-by: David Plowman \n> > ---\n> > src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 12 ++++++++----\n> > 1 file changed, 8 insertions(+), 4 deletions(-)\n> >\n> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > index 48f561d31a31..1b78b5e74a63 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > @@ -96,14 +96,14 @@ BayerFormat mbusCodeToBayerFormat(unsigned int\n> mbusCode)\n> > return BayerFormat{};\n> > }\n> >\n> > -V4L2DeviceFormat toV4L2DeviceFormat(V4L2SubdeviceFormat &format)\n> > +V4L2DeviceFormat toV4L2DeviceFormat(V4L2SubdeviceFormat &format,\n> BayerFormat::Packing packing)\n> > {\n> > V4L2DeviceFormat deviceFormat;\n> > BayerFormat bayer = mbusCodeToBayerFormat(format.mbus_code);\n> >\n> > ASSERT(bayer.isValid());\n> >\n> > - bayer.packing = BayerFormat::Packing::CSI2Packed;\n> > + bayer.packing = packing;\n>\n> Ok, that really solidifies the answer to my earlier question ;-)\n>\n>\n> > deviceFormat.fourcc = bayer.toV4L2PixelFormat();\n> > deviceFormat.size = format.size;\n> > return deviceFormat;\n> > @@ -413,7 +413,8 @@ CameraConfiguration::Status\n> RPiCameraConfiguration::validate()\n> > * the user request.\n> > */\n> > V4L2SubdeviceFormat sensorFormat =\n> findBestFormat(data_->sensorFormats_, cfg.size);\n> > - V4L2DeviceFormat unicamFormat =\n> toV4L2DeviceFormat(sensorFormat);\n> > + V4L2DeviceFormat unicamFormat =\n> toV4L2DeviceFormat(sensorFormat,\n> > +\n> BayerFormat::Packing::CSI2Packed);\n>\n> I believe ::Packing is optional if you want to reduce line length. But\n> can also stay, it won't make a big difference to the line formatting.\n>\n>\n> > int ret =\n> data_->unicam_[Unicam::Image].dev()->tryFormat(&unicamFormat);\n> > if (ret)\n> > return Invalid;\n> > @@ -631,6 +632,7 @@ int PipelineHandlerRPi::configure(Camera *camera,\n> CameraConfiguration *config)\n> > for (auto const stream : data->streams_)\n> > stream->reset();\n> >\n> > + BayerFormat::Packing packing = BayerFormat::Packing::CSI2Packed;\n>\n> Same.\n>\n\nYes, I can remove the extra scoping to be more consistent.\n\nNaush\n\n\n>\n>\n> Reviewed-by: Kieran Bingham \n>\n> > Size maxSize, sensorSize;\n> > unsigned int maxIndex = 0;\n> > bool rawStream = false;\n> > @@ -649,6 +651,8 @@ int PipelineHandlerRPi::configure(Camera *camera,\n> CameraConfiguration *config)\n> > */\n> > sensorSize = cfg.size;\n> > rawStream = true;\n> > + /* Check if the user has explicitly set an\n> unpacked format. */\n> > + packing =\n> BayerFormat::fromPixelFormat(cfg.pixelFormat).packing;\n> > } else {\n> > if (cfg.size > maxSize) {\n> > maxSize = config->at(i).size;\n> > @@ -667,7 +671,7 @@ int PipelineHandlerRPi::configure(Camera *camera,\n> CameraConfiguration *config)\n> > * Unicam image output format. The ISP input format gets set at\n> start,\n> > * just in case we have swapped bayer orders due to flips.\n> > */\n> > - V4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(sensorFormat);\n> > + V4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(sensorFormat,\n> packing);\n> > ret =\n> data->unicam_[Unicam::Image].dev()->setFormat(&unicamFormat);\n> > if (ret)\n> > return ret;\n> > --\n> > 2.25.1\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 279BABF415\n\tfor ;\n\tWed, 27 Oct 2021 12:39:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DD08C6487F;\n\tWed, 27 Oct 2021 14:39:29 +0200 (CEST)","from mail-lf1-x130.google.com (mail-lf1-x130.google.com\n\t[IPv6:2a00:1450:4864:20::130])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2B7B460123\n\tfor ;\n\tWed, 27 Oct 2021 14:39:28 +0200 (CEST)","by mail-lf1-x130.google.com with SMTP id x192so5799382lff.12\n\tfor ;\n\tWed, 27 Oct 2021 05:39:28 -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=\"djEKVPzp\"; 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=GUpVM0avMeRdpBEf/TI49DG/Ml0g0tK9nW9ngn4u90k=;\n\tb=djEKVPzpUoEGgH9ELg45fktjSu3Y6f9lVCQGabEMqImle3sBYtfTiJGtZoh89SjxSz\n\tD82/i4QUfz+jJJjQZWQ+KY2K51ZK3EDSA1A7UQ0pX4BzSpKNFeePUtGkWckfrxpwEh4f\n\tvuKYqMWD7HfIBpBtiLA4IEjQiHDsDAMnfq5YXkoTv51Zs3wlMSOCNf2ZFYAiyDzcsaMF\n\tHA+/U+diwCiA71G+ZIFqkSz+BddyOA3FBDrCCMwZiSd+XcgwLTF/oswL6+kRRz14w2Sn\n\tEfe5o6XmWooJmdjPqY/YfOKh1vyZwh9pqGOQSdqRe6MvFPjOfEAQ8RfxFAWimXeuUVRh\n\tTYqQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=GUpVM0avMeRdpBEf/TI49DG/Ml0g0tK9nW9ngn4u90k=;\n\tb=I8jDtRRptopD5+Ntw2qbXy1PYKRWFQ4/LvbHh+KnsjHUlKXemqE6pwJhjRIkwscY3S\n\tGkdAc2fNuSPIOwFd0oFsIlrQZBvG6hEZRPkHt/eG0GAOUn/Iu/sFVeZqTPnPO/63SiDB\n\tfmTMUTMABn+rLNLjJC5Q7dPEntapHj5oB6DroE5QxBLakirb/NA8pG85gA1k9byxm2yZ\n\tlCKDpgl2ulpnhgB+Rh8QgS8nOSCsHXFtVMXgSWR111Yn6uW7zhqJR7TFp8hzzyAaw+F3\n\tpW4mPiiqbpnFg6zEHz/whJ8khEXZ2DJ6O8Cfv302c3sUyEdkTN1vyLJMjrL5kxnpCtBk\n\tpCgQ==","X-Gm-Message-State":"AOAM533e/ZTyvYu0W1w2GNwO8/KEhD0tAhQTduoc9VIOQE5ZmwTJ1zAN\n\tx5uYN4/AuZntl1c/JcMVg8Q71+SKSlFEVFVrWBUdPTNhc3Z+lw==","X-Google-Smtp-Source":"ABdhPJyhSvaHOkRyp3fd1cJ7aES105bTh1zYEVx0nk1++Ra0TIhaOCeQcWmwwRWEH/JJPps3MzGEqgbT70BV1HpeUko=","X-Received":"by 2002:a05:6512:1281:: with SMTP id\n\tu1mr5511858lfs.611.1635338367608; \n\tWed, 27 Oct 2021 05:39:27 -0700 (PDT)","MIME-Version":"1.0","References":"<20211027092803.3671096-1-naush@raspberrypi.com>\n\t<20211027092803.3671096-7-naush@raspberrypi.com>\n\t<163533706941.1184428.10511540727508627850@Monstersaurus>","In-Reply-To":"<163533706941.1184428.10511540727508627850@Monstersaurus>","From":"Naushir Patuck ","Date":"Wed, 27 Oct 2021 13:39:11 +0100","Message-ID":"","To":"Kieran Bingham ","Content-Type":"multipart/alternative; boundary=\"0000000000007d411a05cf54e288\"","Subject":"Re: [libcamera-devel] [PATCH v3 6/9] pipeline: raspberrypi: Set\n\tpacking formats for the Unicam image node","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":20575,"web_url":"https://patchwork.libcamera.org/comment/20575/","msgid":"<163533902889.2186503.17140913769796637670@Monstersaurus>","date":"2021-10-27T12:50:28","subject":"Re: [libcamera-devel] [PATCH v3 6/9] pipeline: raspberrypi: Set\n\tpacking formats for the Unicam image node","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Naushir Patuck (2021-10-27 13:39:11)\n> Hi Kieran,\n> \n> Thank you for your feedback.\n> \n> On Wed, 27 Oct 2021 at 13:17, Kieran Bingham <\n> kieran.bingham@ideasonboard.com> wrote:\n> \n> > Quoting Naushir Patuck (2021-10-27 10:28:00)\n> > > Default to using CSI2 packed formats when setting up the Unicam image\n> > format,\n> > > but use an unpacked format if the user requests one through\n> > StreamConfiguration.\n> > >\n> > > Signed-off-by: Naushir Patuck \n> > > Reviewed-by: David Plowman \n> > > ---\n> > > src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 12 ++++++++----\n> > > 1 file changed, 8 insertions(+), 4 deletions(-)\n> > >\n> > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > index 48f561d31a31..1b78b5e74a63 100644\n> > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > @@ -96,14 +96,14 @@ BayerFormat mbusCodeToBayerFormat(unsigned int\n> > mbusCode)\n> > > return BayerFormat{};\n> > > }\n> > >\n> > > -V4L2DeviceFormat toV4L2DeviceFormat(V4L2SubdeviceFormat &format)\n> > > +V4L2DeviceFormat toV4L2DeviceFormat(V4L2SubdeviceFormat &format,\n> > BayerFormat::Packing packing)\n> > > {\n> > > V4L2DeviceFormat deviceFormat;\n> > > BayerFormat bayer = mbusCodeToBayerFormat(format.mbus_code);\n> > >\n> > > ASSERT(bayer.isValid());\n> > >\n> > > - bayer.packing = BayerFormat::Packing::CSI2Packed;\n> > > + bayer.packing = packing;\n> >\n> > Ok, that really solidifies the answer to my earlier question ;-)\n> >\n> >\n> > > deviceFormat.fourcc = bayer.toV4L2PixelFormat();\n> > > deviceFormat.size = format.size;\n> > > return deviceFormat;\n> > > @@ -413,7 +413,8 @@ CameraConfiguration::Status\n> > RPiCameraConfiguration::validate()\n> > > * the user request.\n> > > */\n> > > V4L2SubdeviceFormat sensorFormat =\n> > findBestFormat(data_->sensorFormats_, cfg.size);\n> > > - V4L2DeviceFormat unicamFormat =\n> > toV4L2DeviceFormat(sensorFormat);\n> > > + V4L2DeviceFormat unicamFormat =\n> > toV4L2DeviceFormat(sensorFormat,\n> > > +\n> > BayerFormat::Packing::CSI2Packed);\n> >\n> > I believe ::Packing is optional if you want to reduce line length. But\n> > can also stay, it won't make a big difference to the line formatting.\n> >\n> >\n> > > int ret =\n> > data_->unicam_[Unicam::Image].dev()->tryFormat(&unicamFormat);\n> > > if (ret)\n> > > return Invalid;\n> > > @@ -631,6 +632,7 @@ int PipelineHandlerRPi::configure(Camera *camera,\n> > CameraConfiguration *config)\n> > > for (auto const stream : data->streams_)\n> > > stream->reset();\n> > >\n> > > + BayerFormat::Packing packing = BayerFormat::Packing::CSI2Packed;\n> >\n> > Same.\n> >\n> \n> Yes, I can remove the extra scoping to be more consistent.\n> \n\nErm... given Laurent's just submitted patch, I might ahve to take back\nmy suggestions I'm afraid.\n\nLooks like this should be /with/ ::Packing::\n\n--\nKieran\n\n\n\n> Naush\n> \n> \n> >\n> >\n> > Reviewed-by: Kieran Bingham \n> >\n> > > Size maxSize, sensorSize;\n> > > unsigned int maxIndex = 0;\n> > > bool rawStream = false;\n> > > @@ -649,6 +651,8 @@ int PipelineHandlerRPi::configure(Camera *camera,\n> > CameraConfiguration *config)\n> > > */\n> > > sensorSize = cfg.size;\n> > > rawStream = true;\n> > > + /* Check if the user has explicitly set an\n> > unpacked format. */\n> > > + packing =\n> > BayerFormat::fromPixelFormat(cfg.pixelFormat).packing;\n> > > } else {\n> > > if (cfg.size > maxSize) {\n> > > maxSize = config->at(i).size;\n> > > @@ -667,7 +671,7 @@ int PipelineHandlerRPi::configure(Camera *camera,\n> > CameraConfiguration *config)\n> > > * Unicam image output format. The ISP input format gets set at\n> > start,\n> > > * just in case we have swapped bayer orders due to flips.\n> > > */\n> > > - V4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(sensorFormat);\n> > > + V4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(sensorFormat,\n> > packing);\n> > > ret =\n> > data->unicam_[Unicam::Image].dev()->setFormat(&unicamFormat);\n> > > if (ret)\n> > > return ret;\n> > > --\n> > > 2.25.1\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 68014BDB1C\n\tfor ;\n\tWed, 27 Oct 2021 12:50:33 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CD4CD6487F;\n\tWed, 27 Oct 2021 14:50:32 +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 22A7E60123\n\tfor ;\n\tWed, 27 Oct 2021 14:50:31 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C16BF596;\n\tWed, 27 Oct 2021 14:50:30 +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=\"ejsLQ0sN\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1635339030;\n\tbh=tF2Xo+qi5nQ+63z127j3COwd1V3oBSjesxQLjHC1YUo=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=ejsLQ0sNni6uGvt3sF3eqM7TvGCfYCehC38dig/vOjKDWHY/RJoNQP13uYwWFALaV\n\tem0yXIX1gaU0idMqJcSmiCTQvq/hwHyWyNLxfXXerM0alCdHJOzJgoXpNljpKj9lCY\n\t2dNwxRKMSVkviSVUxHnYwV1iIveRD88UGG4If8lo=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20211027092803.3671096-1-naush@raspberrypi.com>\n\t<20211027092803.3671096-7-naush@raspberrypi.com>\n\t<163533706941.1184428.10511540727508627850@Monstersaurus>\n\t","From":"Kieran Bingham ","To":"Naushir Patuck ","Date":"Wed, 27 Oct 2021 13:50:28 +0100","Message-ID":"<163533902889.2186503.17140913769796637670@Monstersaurus>","User-Agent":"alot/0.9.1","Subject":"Re: [libcamera-devel] [PATCH v3 6/9] pipeline: raspberrypi: Set\n\tpacking formats for the Unicam image node","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":20576,"web_url":"https://patchwork.libcamera.org/comment/20576/","msgid":"","date":"2021-10-27T12:51:57","subject":"Re: [libcamera-devel] [PATCH v3 6/9] pipeline: raspberrypi: Set\n\tpacking formats for the Unicam image node","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"On Wed, 27 Oct 2021 at 13:50, Kieran Bingham <\nkieran.bingham@ideasonboard.com> wrote:\n\n> Quoting Naushir Patuck (2021-10-27 13:39:11)\n> > Hi Kieran,\n> >\n> > Thank you for your feedback.\n> >\n> > On Wed, 27 Oct 2021 at 13:17, Kieran Bingham <\n> > kieran.bingham@ideasonboard.com> wrote:\n> >\n> > > Quoting Naushir Patuck (2021-10-27 10:28:00)\n> > > > Default to using CSI2 packed formats when setting up the Unicam image\n> > > format,\n> > > > but use an unpacked format if the user requests one through\n> > > StreamConfiguration.\n> > > >\n> > > > Signed-off-by: Naushir Patuck \n> > > > Reviewed-by: David Plowman \n> > > > ---\n> > > > src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 12 ++++++++----\n> > > > 1 file changed, 8 insertions(+), 4 deletions(-)\n> > > >\n> > > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > > index 48f561d31a31..1b78b5e74a63 100644\n> > > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > > @@ -96,14 +96,14 @@ BayerFormat mbusCodeToBayerFormat(unsigned int\n> > > mbusCode)\n> > > > return BayerFormat{};\n> > > > }\n> > > >\n> > > > -V4L2DeviceFormat toV4L2DeviceFormat(V4L2SubdeviceFormat &format)\n> > > > +V4L2DeviceFormat toV4L2DeviceFormat(V4L2SubdeviceFormat &format,\n> > > BayerFormat::Packing packing)\n> > > > {\n> > > > V4L2DeviceFormat deviceFormat;\n> > > > BayerFormat bayer = mbusCodeToBayerFormat(format.mbus_code);\n> > > >\n> > > > ASSERT(bayer.isValid());\n> > > >\n> > > > - bayer.packing = BayerFormat::Packing::CSI2Packed;\n> > > > + bayer.packing = packing;\n> > >\n> > > Ok, that really solidifies the answer to my earlier question ;-)\n> > >\n> > >\n> > > > deviceFormat.fourcc = bayer.toV4L2PixelFormat();\n> > > > deviceFormat.size = format.size;\n> > > > return deviceFormat;\n> > > > @@ -413,7 +413,8 @@ CameraConfiguration::Status\n> > > RPiCameraConfiguration::validate()\n> > > > * the user request.\n> > > > */\n> > > > V4L2SubdeviceFormat sensorFormat =\n> > > findBestFormat(data_->sensorFormats_, cfg.size);\n> > > > - V4L2DeviceFormat unicamFormat =\n> > > toV4L2DeviceFormat(sensorFormat);\n> > > > + V4L2DeviceFormat unicamFormat =\n> > > toV4L2DeviceFormat(sensorFormat,\n> > > > +\n> > > BayerFormat::Packing::CSI2Packed);\n> > >\n> > > I believe ::Packing is optional if you want to reduce line length. But\n> > > can also stay, it won't make a big difference to the line formatting.\n> > >\n> > >\n> > > > int ret =\n> > > data_->unicam_[Unicam::Image].dev()->tryFormat(&unicamFormat);\n> > > > if (ret)\n> > > > return Invalid;\n> > > > @@ -631,6 +632,7 @@ int PipelineHandlerRPi::configure(Camera *camera,\n> > > CameraConfiguration *config)\n> > > > for (auto const stream : data->streams_)\n> > > > stream->reset();\n> > > >\n> > > > + BayerFormat::Packing packing =\n> BayerFormat::Packing::CSI2Packed;\n> > >\n> > > Same.\n> > >\n> >\n> > Yes, I can remove the extra scoping to be more consistent.\n> >\n>\n> Erm... given Laurent's just submitted patch, I might ahve to take back\n> my suggestions I'm afraid.\n>\n> Looks like this should be /with/ ::Packing::\n>\n\nYes, I saw that change in my inbox just after replying to your message...\n\n\n>\n> --\n> Kieran\n>\n>\n>\n> > Naush\n> >\n> >\n> > >\n> > >\n> > > Reviewed-by: Kieran Bingham \n> > >\n> > > > Size maxSize, sensorSize;\n> > > > unsigned int maxIndex = 0;\n> > > > bool rawStream = false;\n> > > > @@ -649,6 +651,8 @@ int PipelineHandlerRPi::configure(Camera *camera,\n> > > CameraConfiguration *config)\n> > > > */\n> > > > sensorSize = cfg.size;\n> > > > rawStream = true;\n> > > > + /* Check if the user has explicitly set an\n> > > unpacked format. */\n> > > > + packing =\n> > > BayerFormat::fromPixelFormat(cfg.pixelFormat).packing;\n> > > > } else {\n> > > > if (cfg.size > maxSize) {\n> > > > maxSize = config->at(i).size;\n> > > > @@ -667,7 +671,7 @@ int PipelineHandlerRPi::configure(Camera *camera,\n> > > CameraConfiguration *config)\n> > > > * Unicam image output format. The ISP input format gets set\n> at\n> > > start,\n> > > > * just in case we have swapped bayer orders due to flips.\n> > > > */\n> > > > - V4L2DeviceFormat unicamFormat =\n> toV4L2DeviceFormat(sensorFormat);\n> > > > + V4L2DeviceFormat unicamFormat =\n> toV4L2DeviceFormat(sensorFormat,\n> > > packing);\n> > > > ret =\n> > > data->unicam_[Unicam::Image].dev()->setFormat(&unicamFormat);\n> > > > if (ret)\n> > > > return ret;\n> > > > --\n> > > > 2.25.1\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 9EAB2BDB1C\n\tfor ;\n\tWed, 27 Oct 2021 12:52:14 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5A7216487F;\n\tWed, 27 Oct 2021 14:52:14 +0200 (CEST)","from mail-lj1-x230.google.com (mail-lj1-x230.google.com\n\t[IPv6:2a00:1450:4864:20::230])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9418060123\n\tfor ;\n\tWed, 27 Oct 2021 14:52:13 +0200 (CEST)","by mail-lj1-x230.google.com with SMTP id u5so4499990ljo.8\n\tfor ;\n\tWed, 27 Oct 2021 05:52:13 -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=\"RQWvNqUh\"; 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=VXY1ZjAQ875+sq7tlABKBGKiH5a8kYRtAcC/Ah8JzKo=;\n\tb=RQWvNqUhB83sUWi4d6NQR6wlkqX7DQJpXoLarudWNYHlv/CPU9M3jy3fvJiUMZwptZ\n\tvu5LyeXWIEKX3Ppzh1CZ+RaMWpUlNjx3IXTrbH2Gy573ZNJzcWapJBQQprwCZeV3J0r+\n\t4JWJwc1vmt46cjrVAk1LFXfft0VhoHSmmDw/EgLYv06q38VYEg7UlzfYj3CA53jDXEct\n\tDO8vd8wemsVh/phNPkDWsFiscCpMN+yn1836zM0ZOA+UTViTPpypxasda3Dmat1MXzn/\n\tjTWpzAw25jrDgCBi9TrqYxz/SX1sv3FbU7fE0Ih4Izp9QEktHM7W8A4oKrhPzu3bZWeP\n\tfVwg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=VXY1ZjAQ875+sq7tlABKBGKiH5a8kYRtAcC/Ah8JzKo=;\n\tb=Xo14DXmH1rP/zfiGmx/gqNkB/4Ysx0Tq/aYT9ZYyACP+sH5vSPvVdthMX74pH2L7e8\n\t9N3jbDa+wtRUnU5/UZHoMMlSKpvRMP1ooMq8qy3k2x2JcAairAHv3hMocaV8FwT3k7RA\n\tSymLqPoHZmUP1oDvVhvUf2C6hXVKj1Ngr3UtswuXPdYrF1T3VYkhG4K1Y3Kc4vMBtYAp\n\t1uXNOZAgYlLPmPbSIP/+4HEUzZ0kDqUtxK2i/5oTtxs/OKsMVfWOsW/POZyQcq09uas7\n\ta6C+ZedNEZoCYt1WFwPCRevlKiAWP7sHujYqS+oa8kyC32qKpPHrZIwEI2aMa91KKaOC\n\tjUKA==","X-Gm-Message-State":"AOAM531K1eYnHJVPBENCKX6pM/uUrfOtH5pqfGbH2hIMIY+lk5wMyYKk\n\tQAs4LXCWUmhvvcH3QeQtBc3uhYe/bHakfaCdvNRVLqvI8NgZ6g==","X-Google-Smtp-Source":"ABdhPJxGSF2ITsa7RwBM3dTLQ538vfGzge4AZYtTq0+6UeYxjJTAOgZSsU49z+ut6bHeX0vcRMhefrhU/x96QwIismY=","X-Received":"by 2002:a2e:874d:: with SMTP id\n\tq13mr33219978ljj.16.1635339132960; \n\tWed, 27 Oct 2021 05:52:12 -0700 (PDT)","MIME-Version":"1.0","References":"<20211027092803.3671096-1-naush@raspberrypi.com>\n\t<20211027092803.3671096-7-naush@raspberrypi.com>\n\t<163533706941.1184428.10511540727508627850@Monstersaurus>\n\t\n\t<163533902889.2186503.17140913769796637670@Monstersaurus>","In-Reply-To":"<163533902889.2186503.17140913769796637670@Monstersaurus>","From":"Naushir Patuck ","Date":"Wed, 27 Oct 2021 13:51:57 +0100","Message-ID":"","To":"Kieran Bingham ","Content-Type":"multipart/alternative; boundary=\"0000000000001b965205cf55105b\"","Subject":"Re: [libcamera-devel] [PATCH v3 6/9] pipeline: raspberrypi: Set\n\tpacking formats for the Unicam image node","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":20588,"web_url":"https://patchwork.libcamera.org/comment/20588/","msgid":"","date":"2021-10-27T14:20:42","subject":"Re: [libcamera-devel] [PATCH v3 6/9] pipeline: raspberrypi: Set\n\tpacking formats for the Unicam image node","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 Wed, Oct 27, 2021 at 10:28:00AM +0100, Naushir Patuck wrote:\n> Default to using CSI2 packed formats when setting up the Unicam image format,\n> but use an unpacked format if the user requests one through StreamConfiguration.\n> \n> Signed-off-by: Naushir Patuck \n> Reviewed-by: David Plowman \n> ---\n> src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 12 ++++++++----\n> 1 file changed, 8 insertions(+), 4 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index 48f561d31a31..1b78b5e74a63 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -96,14 +96,14 @@ BayerFormat mbusCodeToBayerFormat(unsigned int mbusCode)\n> \treturn BayerFormat{};\n> }\n> \n> -V4L2DeviceFormat toV4L2DeviceFormat(V4L2SubdeviceFormat &format)\n> +V4L2DeviceFormat toV4L2DeviceFormat(V4L2SubdeviceFormat &format, BayerFormat::Packing packing)\n> {\n> \tV4L2DeviceFormat deviceFormat;\n> \tBayerFormat bayer = mbusCodeToBayerFormat(format.mbus_code);\n> \n> \tASSERT(bayer.isValid());\n> \n> -\tbayer.packing = BayerFormat::Packing::CSI2Packed;\n> +\tbayer.packing = packing;\n> \tdeviceFormat.fourcc = bayer.toV4L2PixelFormat();\n\nPacking is handled explicitly, so going through BayerFormat here is fine\nI think. The other calls to mbusCodeToBayerFormat() in 5/9 however\nbother me, especially when calling .toPixelFormat() or\n.toV4L2PixelFormat() immediately after. It would be good to wrap those\nin helper functions that explicitly handle packing, to ensure that it\nwill be considered everywhere.\n\nHave you tested both packed and unpacked formats by the way ?\n\n> \tdeviceFormat.size = format.size;\n> \treturn deviceFormat;\n> @@ -413,7 +413,8 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n> \t\t\t * the user request.\n> \t\t\t */\n> \t\t\tV4L2SubdeviceFormat sensorFormat = findBestFormat(data_->sensorFormats_, cfg.size);\n> -\t\t\tV4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(sensorFormat);\n> +\t\t\tV4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(sensorFormat,\n> +\t\t\t\t\t\t\t\t\t BayerFormat::Packing::CSI2Packed);\n> \t\t\tint ret = data_->unicam_[Unicam::Image].dev()->tryFormat(&unicamFormat);\n> \t\t\tif (ret)\n> \t\t\t\treturn Invalid;\n> @@ -631,6 +632,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n> \tfor (auto const stream : data->streams_)\n> \t\tstream->reset();\n> \n> +\tBayerFormat::Packing packing = BayerFormat::Packing::CSI2Packed;\n\nCould we default to packed formats when generating the configuration\ntoo ?\n\n> \tSize maxSize, sensorSize;\n> \tunsigned int maxIndex = 0;\n> \tbool rawStream = false;\n> @@ -649,6 +651,8 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n> \t\t\t */\n> \t\t\tsensorSize = cfg.size;\n> \t\t\trawStream = true;\n> +\t\t\t/* Check if the user has explicitly set an unpacked format. */\n> +\t\t\tpacking = BayerFormat::fromPixelFormat(cfg.pixelFormat).packing;\n> \t\t} else {\n> \t\t\tif (cfg.size > maxSize) {\n> \t\t\t\tmaxSize = config->at(i).size;\n> @@ -667,7 +671,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n> \t * Unicam image output format. The ISP input format gets set at start,\n> \t * just in case we have swapped bayer orders due to flips.\n> \t */\n> -\tV4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(sensorFormat);\n> +\tV4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(sensorFormat, packing);\n> \tret = data->unicam_[Unicam::Image].dev()->setFormat(&unicamFormat);\n> \tif (ret)\n> \t\treturn ret;","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 C8764BDB1C\n\tfor ;\n\tWed, 27 Oct 2021 14:21:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2F9CC64882;\n\tWed, 27 Oct 2021 16:21:08 +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 8797F60123\n\tfor ;\n\tWed, 27 Oct 2021 16:21:06 +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 F36C9292;\n\tWed, 27 Oct 2021 16:21:05 +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=\"sgTuFwLK\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1635344466;\n\tbh=2JtSpxRZwuRegb1Pbc2ybANKJ81jRNzfFwCE+HACooc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=sgTuFwLKq2yBqA0DkkBJ5jVKeU0P59Ibprn2s6Q29mrfZ+D0guFuPPKjA3RosXXwv\n\tHOLtIDYMONKpqXCJ0ZRYuJxujceLe89VwuyFG5eVw2m1OmWvAxGfep8ZjcIAxWfYbV\n\twVHaPHnpn2Bgo+xIcme2+P9BIE2aUf3gFKwgeVqM=","Date":"Wed, 27 Oct 2021 17:20:42 +0300","From":"Laurent Pinchart ","To":"Naushir Patuck ","Message-ID":"","References":"<20211027092803.3671096-1-naush@raspberrypi.com>\n\t<20211027092803.3671096-7-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20211027092803.3671096-7-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v3 6/9] pipeline: raspberrypi: Set\n\tpacking formats for the Unicam image node","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\" "}},{"id":20590,"web_url":"https://patchwork.libcamera.org/comment/20590/","msgid":"","date":"2021-10-27T14:32:13","subject":"Re: [libcamera-devel] [PATCH v3 6/9] pipeline: raspberrypi: Set\n\tpacking formats for the Unicam image node","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Laurent,\n\nThank you for the feedback for this and the other changes.\n\nOn Wed, 27 Oct 2021 at 15:21, Laurent Pinchart <\nlaurent.pinchart@ideasonboard.com> wrote:\n\n> Hi Naush,\n>\n> Thank you for the patch.\n>\n> On Wed, Oct 27, 2021 at 10:28:00AM +0100, Naushir Patuck wrote:\n> > Default to using CSI2 packed formats when setting up the Unicam image\n> format,\n> > but use an unpacked format if the user requests one through\n> StreamConfiguration.\n> >\n> > Signed-off-by: Naushir Patuck \n> > Reviewed-by: David Plowman \n> > ---\n> > src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 12 ++++++++----\n> > 1 file changed, 8 insertions(+), 4 deletions(-)\n> >\n> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > index 48f561d31a31..1b78b5e74a63 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > @@ -96,14 +96,14 @@ BayerFormat mbusCodeToBayerFormat(unsigned int\n> mbusCode)\n> > return BayerFormat{};\n> > }\n> >\n> > -V4L2DeviceFormat toV4L2DeviceFormat(V4L2SubdeviceFormat &format)\n> > +V4L2DeviceFormat toV4L2DeviceFormat(V4L2SubdeviceFormat &format,\n> BayerFormat::Packing packing)\n> > {\n> > V4L2DeviceFormat deviceFormat;\n> > BayerFormat bayer = mbusCodeToBayerFormat(format.mbus_code);\n> >\n> > ASSERT(bayer.isValid());\n> >\n> > - bayer.packing = BayerFormat::Packing::CSI2Packed;\n> > + bayer.packing = packing;\n> > deviceFormat.fourcc = bayer.toV4L2PixelFormat();\n>\n> Packing is handled explicitly, so going through BayerFormat here is fine\n> I think. The other calls to mbusCodeToBayerFormat() in 5/9 however\n> bother me, especially when calling .toPixelFormat() or\n> .toV4L2PixelFormat() immediately after. It would be good to wrap those\n> in helper functions that explicitly handle packing, to ensure that it\n> will be considered everywhere.\n>\n\n> Have you tested both packed and unpacked formats by the way ?\n>\n\nYes, I see your point. As per your comment in the previous change, I will\nlook to\nchange the mbus -> Bayer format conversion to be mbus -> V4L2PixelFormat,\nand\ndeal with the packing separately in a helper.\n\nWith the changes in this version of the series Packing/unpacking works as\nexpected,\nbut you are correct in that the change in 5/9 to use .toPixelFormat() in\ngenerateConfiguration()\nwill advertise an unpacked format to the application, which is not\nefficient I will rework that\nalong with the mbus -> V4L2PixelFormat conversion.\n\nThanks,\nNaush\n\n\n>\n> > deviceFormat.size = format.size;\n> > return deviceFormat;\n> > @@ -413,7 +413,8 @@ CameraConfiguration::Status\n> RPiCameraConfiguration::validate()\n> > * the user request.\n> > */\n> > V4L2SubdeviceFormat sensorFormat =\n> findBestFormat(data_->sensorFormats_, cfg.size);\n> > - V4L2DeviceFormat unicamFormat =\n> toV4L2DeviceFormat(sensorFormat);\n> > + V4L2DeviceFormat unicamFormat =\n> toV4L2DeviceFormat(sensorFormat,\n> > +\n> BayerFormat::Packing::CSI2Packed);\n> > int ret =\n> data_->unicam_[Unicam::Image].dev()->tryFormat(&unicamFormat);\n> > if (ret)\n> > return Invalid;\n> > @@ -631,6 +632,7 @@ int PipelineHandlerRPi::configure(Camera *camera,\n> CameraConfiguration *config)\n> > for (auto const stream : data->streams_)\n> > stream->reset();\n> >\n> > + BayerFormat::Packing packing = BayerFormat::Packing::CSI2Packed;\n>\n> Could we default to packed formats when generating the configuration\n> too ?\n>\n> > Size maxSize, sensorSize;\n> > unsigned int maxIndex = 0;\n> > bool rawStream = false;\n> > @@ -649,6 +651,8 @@ int PipelineHandlerRPi::configure(Camera *camera,\n> CameraConfiguration *config)\n> > */\n> > sensorSize = cfg.size;\n> > rawStream = true;\n> > + /* Check if the user has explicitly set an\n> unpacked format. */\n> > + packing =\n> BayerFormat::fromPixelFormat(cfg.pixelFormat).packing;\n> > } else {\n> > if (cfg.size > maxSize) {\n> > maxSize = config->at(i).size;\n> > @@ -667,7 +671,7 @@ int PipelineHandlerRPi::configure(Camera *camera,\n> CameraConfiguration *config)\n> > * Unicam image output format. The ISP input format gets set at\n> start,\n> > * just in case we have swapped bayer orders due to flips.\n> > */\n> > - V4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(sensorFormat);\n> > + V4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(sensorFormat,\n> packing);\n> > ret = data->unicam_[Unicam::Image].dev()->setFormat(&unicamFormat);\n> > if (ret)\n> > return ret;\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\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 3E3EFBF415\n\tfor ;\n\tWed, 27 Oct 2021 14:32:32 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7DE2B6487F;\n\tWed, 27 Oct 2021 16:32:31 +0200 (CEST)","from mail-lf1-x136.google.com (mail-lf1-x136.google.com\n\t[IPv6:2a00:1450:4864:20::136])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E212260123\n\tfor ;\n\tWed, 27 Oct 2021 16:32:29 +0200 (CEST)","by mail-lf1-x136.google.com with SMTP id p16so6657962lfa.2\n\tfor ;\n\tWed, 27 Oct 2021 07:32:29 -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=\"JiSkRkHO\"; 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=/S2MmGimJgnPsXYrOyVL0Pn4oAByTs5JRynEKNSGtG0=;\n\tb=JiSkRkHOEwffvgD7yEJiTq/PvAEfOQRgorMogBfrmy0rYVD24Ztn5iwzf0rKmYBLrV\n\tKOYxTIazuyvA5v/rDFoEcQJ2sSvmT4KKdzjKTey+PWMGudEReBSPb4l1+53FVsP6OR4M\n\tpMGb+lClGBvyV26yUbSkGIADFgSNPPkducPjyF1RZzh7q9vGyS3iJwt6+IVC6iIMy895\n\tvu2vnORBYs7pA9xoUqk9wDYoW+3icsTSz+VivQIzAyUAnO6UeJJrBfx/VmAPPGDpYlVc\n\tTgsjsnJAD8CP8jmUdOYUcUev5MbqTj1axnpLJsPqRY5Zxmc1Qt0HmXNeN6Gub5VNvG/9\n\tqdVQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=/S2MmGimJgnPsXYrOyVL0Pn4oAByTs5JRynEKNSGtG0=;\n\tb=wX0Q0roLtJCf8ffFFRaFxv0hgZm9/GlEZFY5xgK4HDGkctM4Pgq1EDmPzH3DvNXttU\n\tx1Oil/DgQT+BhX007mxadOqE8mnXew+nRqdWUbnJXU10IDASXmDOyjh6mpIBc6BtpY2x\n\tFeRcGZkVQxq3ColHLHd6fTh0xRCSH8tdUMK6NLXKgh3ip1pDZO5+zRwj3mt1ktwi2WX2\n\tK3b8cnNekqYIe4ONs3xfPxFmP5rYDt+R5eQT2WZFzeH9cuP2O0s6iXcd7cyjH46lxMYT\n\te56NQWm5dX7pOGFbsjaVmJvlBIsgEqR8lTrcMPvn4UZjTwlr/DjZFa2z7iqgipHizHqH\n\t1ntw==","X-Gm-Message-State":"AOAM530xkADIKfYfNZgxKZijZ9ZzmkjBRknbUeONSLwz0IB66tZeRG1K\n\tOFouCIfWvCEnEambDHSrxijs2GQ/aGp6OkLH4o09qA==","X-Google-Smtp-Source":"ABdhPJz0LPFBukgDm4h5TWWMv8lImD9FpF8mjdJwXCVDLcMxeKtWLe64q1Y42knxO6C3gekG54rorl7bDIbVA7e8i/g=","X-Received":"by 2002:a05:6512:2118:: with SMTP id\n\tq24mr12273033lfr.315.1635345149113; \n\tWed, 27 Oct 2021 07:32:29 -0700 (PDT)","MIME-Version":"1.0","References":"<20211027092803.3671096-1-naush@raspberrypi.com>\n\t<20211027092803.3671096-7-naush@raspberrypi.com>\n\t","In-Reply-To":"","From":"Naushir Patuck ","Date":"Wed, 27 Oct 2021 15:32:13 +0100","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"multipart/alternative; boundary=\"000000000000b2cb7a05cf5676a6\"","Subject":"Re: [libcamera-devel] [PATCH v3 6/9] pipeline: raspberrypi: Set\n\tpacking formats for the Unicam image node","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":20591,"web_url":"https://patchwork.libcamera.org/comment/20591/","msgid":"","date":"2021-10-27T14:59:19","subject":"Re: [libcamera-devel] [PATCH v3 6/9] pipeline: raspberrypi: Set\n\tpacking formats for the Unicam image node","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Naush,\n\nOn Wed, Oct 27, 2021 at 03:32:13PM +0100, Naushir Patuck wrote:\n> On Wed, 27 Oct 2021 at 15:21, Laurent Pinchart wrote:\n> > On Wed, Oct 27, 2021 at 10:28:00AM +0100, Naushir Patuck wrote:\n> > > Default to using CSI2 packed formats when setting up the Unicam image format,\n> > > but use an unpacked format if the user requests one through StreamConfiguration.\n> > >\n> > > Signed-off-by: Naushir Patuck \n> > > Reviewed-by: David Plowman \n> > > ---\n> > > src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 12 ++++++++----\n> > > 1 file changed, 8 insertions(+), 4 deletions(-)\n> > >\n> > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > index 48f561d31a31..1b78b5e74a63 100644\n> > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > @@ -96,14 +96,14 @@ BayerFormat mbusCodeToBayerFormat(unsigned int mbusCode)\n> > > return BayerFormat{};\n> > > }\n> > >\n> > > -V4L2DeviceFormat toV4L2DeviceFormat(V4L2SubdeviceFormat &format)\n> > > +V4L2DeviceFormat toV4L2DeviceFormat(V4L2SubdeviceFormat &format, BayerFormat::Packing packing)\n> > > {\n> > > V4L2DeviceFormat deviceFormat;\n> > > BayerFormat bayer = mbusCodeToBayerFormat(format.mbus_code);\n> > >\n> > > ASSERT(bayer.isValid());\n> > >\n> > > - bayer.packing = BayerFormat::Packing::CSI2Packed;\n> > > + bayer.packing = packing;\n> > > deviceFormat.fourcc = bayer.toV4L2PixelFormat();\n> >\n> > Packing is handled explicitly, so going through BayerFormat here is fine\n> > I think. The other calls to mbusCodeToBayerFormat() in 5/9 however\n> > bother me, especially when calling .toPixelFormat() or\n> > .toV4L2PixelFormat() immediately after. It would be good to wrap those\n> > in helper functions that explicitly handle packing, to ensure that it\n> > will be considered everywhere.\n> \n> > Have you tested both packed and unpacked formats by the way ?\n> \n> Yes, I see your point. As per your comment in the previous change, I will look to\n> change the mbus -> Bayer format conversion to be mbus -> V4L2PixelFormat, and\n> deal with the packing separately in a helper.\n\nTo be perfecly clear, I don't object on using BayerFormat if it can\nhelp, as long as we handle packing explicitly. If a direct conversion\nends up being simpler then let's go for that, but there's no strict need\nto drop BayerFormat usage if BayerFormat results in a cleaner\nimplementation.\n\n> With the changes in this version of the series Packing/unpacking works as expected,\n> but you are correct in that the change in 5/9 to use .toPixelFormat() in generateConfiguration()\n> will advertise an unpacked format to the application, which is not efficient I will rework that\n> along with the mbus -> V4L2PixelFormat conversion.\n> \n> > > deviceFormat.size = format.size;\n> > > return deviceFormat;\n> > > @@ -413,7 +413,8 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n> > > * the user request.\n> > > */\n> > > V4L2SubdeviceFormat sensorFormat = findBestFormat(data_->sensorFormats_, cfg.size);\n> > > - V4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(sensorFormat);\n> > > + V4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(sensorFormat,\n> > > + BayerFormat::Packing::CSI2Packed);\n> > > int ret = data_->unicam_[Unicam::Image].dev()->tryFormat(&unicamFormat);\n> > > if (ret)\n> > > return Invalid;\n> > > @@ -631,6 +632,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n> > > for (auto const stream : data->streams_)\n> > > stream->reset();\n> > >\n> > > + BayerFormat::Packing packing = BayerFormat::Packing::CSI2Packed;\n> > Could we default to packed formats when generating the configuration too ?\n> >\n> > > Size maxSize, sensorSize;\n> > > unsigned int maxIndex = 0;\n> > > bool rawStream = false;\n> > > @@ -649,6 +651,8 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n> > > */\n> > > sensorSize = cfg.size;\n> > > rawStream = true;\n> > > + /* Check if the user has explicitly set an unpacked format. */\n> > > + packing = BayerFormat::fromPixelFormat(cfg.pixelFormat).packing;\n> > > } else {\n> > > if (cfg.size > maxSize) {\n> > > maxSize = config->at(i).size;\n> > > @@ -667,7 +671,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n> > > * Unicam image output format. The ISP input format gets set at start,\n> > > * just in case we have swapped bayer orders due to flips.\n> > > */\n> > > - V4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(sensorFormat);\n> > > + V4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(sensorFormat, packing);\n> > > ret = data->unicam_[Unicam::Image].dev()->setFormat(&unicamFormat);\n> > > if (ret)\n> > > return ret;","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 048FBBDB1C\n\tfor ;\n\tWed, 27 Oct 2021 14:59:45 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5922164882;\n\tWed, 27 Oct 2021 16:59:44 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 54EED60123\n\tfor ;\n\tWed, 27 Oct 2021 16:59:43 +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 BABE792C;\n\tWed, 27 Oct 2021 16:59:42 +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=\"kWQW1uyr\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1635346782;\n\tbh=e83LvcRFi3KH3sEeQwivYSa8fSMMQek4P1PuQc+GYQQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=kWQW1uyr6xGEVVsHay8bn6QRdp5Ntpb7MIC/oeV+W2kRTudX3ZXFQMva5LaW7DJ6/\n\tReKQq6tniyh/tJ6nkeVj42VV+53vQ8mm4vVl0y0EUyFCFdin9PnS674D1cqhPBi/UN\n\taAa6OFV9X9P5DgpERhqePUK5OaJ9S5TXawtd1dCg=","Date":"Wed, 27 Oct 2021 17:59:19 +0300","From":"Laurent Pinchart ","To":"Naushir Patuck ","Message-ID":"","References":"<20211027092803.3671096-1-naush@raspberrypi.com>\n\t<20211027092803.3671096-7-naush@raspberrypi.com>\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v3 6/9] pipeline: raspberrypi: Set\n\tpacking formats for the Unicam image node","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":20592,"web_url":"https://patchwork.libcamera.org/comment/20592/","msgid":"","date":"2021-10-27T15:04:57","subject":"Re: [libcamera-devel] [PATCH v3 6/9] pipeline: raspberrypi: Set\n\tpacking formats for the Unicam image node","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Laurent,\n\n\nOn Wed, 27 Oct 2021 at 15:59, Laurent Pinchart <\nlaurent.pinchart@ideasonboard.com> wrote:\n\n> Hi Naush,\n>\n> On Wed, Oct 27, 2021 at 03:32:13PM +0100, Naushir Patuck wrote:\n> > On Wed, 27 Oct 2021 at 15:21, Laurent Pinchart wrote:\n> > > On Wed, Oct 27, 2021 at 10:28:00AM +0100, Naushir Patuck wrote:\n> > > > Default to using CSI2 packed formats when setting up the Unicam\n> image format,\n> > > > but use an unpacked format if the user requests one through\n> StreamConfiguration.\n> > > >\n> > > > Signed-off-by: Naushir Patuck \n> > > > Reviewed-by: David Plowman \n> > > > ---\n> > > > src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 12 ++++++++----\n> > > > 1 file changed, 8 insertions(+), 4 deletions(-)\n> > > >\n> > > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > > index 48f561d31a31..1b78b5e74a63 100644\n> > > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > > @@ -96,14 +96,14 @@ BayerFormat mbusCodeToBayerFormat(unsigned int\n> mbusCode)\n> > > > return BayerFormat{};\n> > > > }\n> > > >\n> > > > -V4L2DeviceFormat toV4L2DeviceFormat(V4L2SubdeviceFormat &format)\n> > > > +V4L2DeviceFormat toV4L2DeviceFormat(V4L2SubdeviceFormat &format,\n> BayerFormat::Packing packing)\n> > > > {\n> > > > V4L2DeviceFormat deviceFormat;\n> > > > BayerFormat bayer = mbusCodeToBayerFormat(format.mbus_code);\n> > > >\n> > > > ASSERT(bayer.isValid());\n> > > >\n> > > > - bayer.packing = BayerFormat::Packing::CSI2Packed;\n> > > > + bayer.packing = packing;\n> > > > deviceFormat.fourcc = bayer.toV4L2PixelFormat();\n> > >\n> > > Packing is handled explicitly, so going through BayerFormat here is\n> fine\n> > > I think. The other calls to mbusCodeToBayerFormat() in 5/9 however\n> > > bother me, especially when calling .toPixelFormat() or\n> > > .toV4L2PixelFormat() immediately after. It would be good to wrap those\n> > > in helper functions that explicitly handle packing, to ensure that it\n> > > will be considered everywhere.\n> >\n> > > Have you tested both packed and unpacked formats by the way ?\n> >\n> > Yes, I see your point. As per your comment in the previous change, I\n> will look to\n> > change the mbus -> Bayer format conversion to be mbus ->\n> V4L2PixelFormat, and\n> > deal with the packing separately in a helper.\n>\n> To be perfecly clear, I don't object on using BayerFormat if it can\n> help, as long as we handle packing explicitly. If a direct conversion\n> ends up being simpler then let's go for that, but there's no strict need\n> to drop BayerFormat usage if BayerFormat results in a cleaner\n> implementation.\n>\n\nAfter having prototyped a change with using V4L2DeviceFormat for the mbus\nconversion,\nI think it would be better to use BayerFormat in this case. BayerFormat\nmakes things a bit\neasier to deal with packing/unpacking formats as it is not a separate enum,\nbut just a property\nof the format. Given that we have no intention of supporting YUV sensor\n(this will come back to bite me :)\nin our pipeline handler, using BayerFormat should be ok for now.\n\nI will however, remove the table in the pipeline handler, and use the\nexisting table in BayerFormat.\nIn fact, this is what I had for v1 and v2. I'll also fix things so the\nexisting mbusCodeToBayerFormat()\nwill handle switching between packing/unpacking though a helper.\n\nThanks,\nNaush\n\n\n\n>\n> > With the changes in this version of the series Packing/unpacking works\n> as expected,\n> > but you are correct in that the change in 5/9 to use .toPixelFormat() in\n> generateConfiguration()\n> > will advertise an unpacked format to the application, which is not\n> efficient I will rework that\n> > along with the mbus -> V4L2PixelFormat conversion.\n> >\n> > > > deviceFormat.size = format.size;\n> > > > return deviceFormat;\n> > > > @@ -413,7 +413,8 @@ CameraConfiguration::Status\n> RPiCameraConfiguration::validate()\n> > > > * the user request.\n> > > > */\n> > > > V4L2SubdeviceFormat sensorFormat =\n> findBestFormat(data_->sensorFormats_, cfg.size);\n> > > > - V4L2DeviceFormat unicamFormat =\n> toV4L2DeviceFormat(sensorFormat);\n> > > > + V4L2DeviceFormat unicamFormat =\n> toV4L2DeviceFormat(sensorFormat,\n> > > > + BayerFormat::Packing::CSI2Packed);\n> > > > int ret =\n> data_->unicam_[Unicam::Image].dev()->tryFormat(&unicamFormat);\n> > > > if (ret)\n> > > > return Invalid;\n> > > > @@ -631,6 +632,7 @@ int PipelineHandlerRPi::configure(Camera\n> *camera, CameraConfiguration *config)\n> > > > for (auto const stream : data->streams_)\n> > > > stream->reset();\n> > > >\n> > > > + BayerFormat::Packing packing =\n> BayerFormat::Packing::CSI2Packed;\n> > > Could we default to packed formats when generating the configuration\n> too ?\n> > >\n> > > > Size maxSize, sensorSize;\n> > > > unsigned int maxIndex = 0;\n> > > > bool rawStream = false;\n> > > > @@ -649,6 +651,8 @@ int PipelineHandlerRPi::configure(Camera\n> *camera, CameraConfiguration *config)\n> > > > */\n> > > > sensorSize = cfg.size;\n> > > > rawStream = true;\n> > > > + /* Check if the user has explicitly set an\n> unpacked format. */\n> > > > + packing =\n> BayerFormat::fromPixelFormat(cfg.pixelFormat).packing;\n> > > > } else {\n> > > > if (cfg.size > maxSize) {\n> > > > maxSize = config->at(i).size;\n> > > > @@ -667,7 +671,7 @@ int PipelineHandlerRPi::configure(Camera\n> *camera, CameraConfiguration *config)\n> > > > * Unicam image output format. The ISP input format gets set\n> at start,\n> > > > * just in case we have swapped bayer orders due to flips.\n> > > > */\n> > > > - V4L2DeviceFormat unicamFormat =\n> toV4L2DeviceFormat(sensorFormat);\n> > > > + V4L2DeviceFormat unicamFormat =\n> toV4L2DeviceFormat(sensorFormat, packing);\n> > > > ret =\n> data->unicam_[Unicam::Image].dev()->setFormat(&unicamFormat);\n> > > > if (ret)\n> > > > return ret;\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\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 56A6BBF415\n\tfor ;\n\tWed, 27 Oct 2021 15:05:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9C5926487F;\n\tWed, 27 Oct 2021 17:05:14 +0200 (CEST)","from mail-lj1-x22b.google.com (mail-lj1-x22b.google.com\n\t[IPv6:2a00:1450:4864:20::22b])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E592460123\n\tfor ;\n\tWed, 27 Oct 2021 17:05:13 +0200 (CEST)","by mail-lj1-x22b.google.com with SMTP id h11so5251544ljk.1\n\tfor ;\n\tWed, 27 Oct 2021 08:05:13 -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=\"J5RwU9G4\"; 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=mjbSBjTLcgKT5QI8Iq+LGcMnU9+9geaCEu/AzxuVV6I=;\n\tb=J5RwU9G4ULluT2N8bBmUFpJVQoUKIYhol7VUSBxenKmBSQ9V2Vas4MfQg+D098QWgU\n\t2ets1kq3jCXoyx4IsI3bZS9U4YTpJOD8z17rZ/1rs5tfvOaHv8o2uenVV72bev9eRFk0\n\tZQeHbT5idfgvhnsbPgp8qNrhWp7+3TZsw+aDT4kc8M4Xkd4xnu38cHz4izsRof5TZ3oC\n\tBmPufole1RSItKldkmvbP2p+vZ7OXwlOyfS3fMrRQwuyuDlBq2ktHB8rnq2n1zIessS6\n\tUi8IXRvCYOgMKECRxPO5kHiebiHsybGFe19o7DhoeoggtICH8TiwTHZuIYRDdGWOsRNr\n\tQ2aw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=mjbSBjTLcgKT5QI8Iq+LGcMnU9+9geaCEu/AzxuVV6I=;\n\tb=xzth2gnWfKjYffOWq2HFbZQrCdR/Pfo76Thq2LVpqBQ9F6JSGYcJa3CjuANTG1rMW5\n\ttwf4w3xUbydKoxyN612OPGWDTsh342Q0OhNA3LD7WmMY2HlpoWUDPIqcvovge/eMpLVs\n\tmxyGfSTvfmDhG0ERIXztNAvHj1/yCDPpVDa//0dfXKQkDEpuUhpVK6+6gbo1rbNvkzqe\n\tB7X4qa9KgKkZWtbTd6htmMNetjSSpj/Yi2KwwxX4n9Xlyc6GF7L+V3/Wpr4GZCqtsEPN\n\tS2y5+/Dfw78uDIws+I8YhFdDWBeyzWyQ7c1RlqEkeIuQxQrtrim34ky9fUcA1OCjdTRS\n\tNoyg==","X-Gm-Message-State":"AOAM531qK3KNscpD6rMDHEQQbZFYJ0J/75VinFSjDGrHm+ISPcN7icnK\n\tSfjmHtyudUr8Cpkj+qqkdgoTmnK81pWiufOLvB/02PRZmJkTI7aL","X-Google-Smtp-Source":"ABdhPJxGilwwd4maqR4hHuL5I96oyOAiOxOOGQV2kf3LrU+I2sRVWkq8jh6lrbGiq42N1iA96YIm7ilvWkuLZhVQ7/s=","X-Received":"by 2002:a2e:874d:: with SMTP id\n\tq13mr33934654ljj.16.1635347113131; \n\tWed, 27 Oct 2021 08:05:13 -0700 (PDT)","MIME-Version":"1.0","References":"<20211027092803.3671096-1-naush@raspberrypi.com>\n\t<20211027092803.3671096-7-naush@raspberrypi.com>\n\t\n\t\n\t","In-Reply-To":"","From":"Naushir Patuck ","Date":"Wed, 27 Oct 2021 16:04:57 +0100","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"multipart/alternative; boundary=\"000000000000c352b405cf56eb0e\"","Subject":"Re: [libcamera-devel] [PATCH v3 6/9] pipeline: raspberrypi: Set\n\tpacking formats for the Unicam image node","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\" "}}]