[{"id":20399,"web_url":"https://patchwork.libcamera.org/comment/20399/","msgid":"","date":"2021-10-22T15:22:28","subject":"Re: [libcamera-devel] [PATCH v2 3/6] pipeline: raspberrypi: Set\n\tpacking formats for the Unicam image node","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 the update.\n\nOn Fri, 22 Oct 2021 at 15:40, Naushir Patuck wrote:\n>\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\nYes, I think this is the right way to do it!\n\n>\n> Signed-off-by: Naushir Patuck \n> ---\n> src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 12 ++++++++++--\n> 1 file changed, 10 insertions(+), 2 deletions(-)\n>\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index a31b0f81eba7..45725527d66e 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -61,11 +61,14 @@ SensorFormats populateSensorFormats(std::unique_ptr &sensor)\n> return formats;\n> }\n>\n> -inline V4L2DeviceFormat toV4L2DeviceFormat(V4L2SubdeviceFormat &mode)\n> +inline V4L2DeviceFormat toV4L2DeviceFormat(V4L2SubdeviceFormat &mode, bool unpacked = false)\n\nI'm not totally sure I like the default value here - I kind of feel\npeople should be forced to think about it. But that's a teeny thing so\nI wouldn't worry.\n\nAnd also ignoring the very minor brain contortion involved in passing\n*un*-packed which gets flipped to set the *packed* field!\n\nReviewed-by: David Plowman \n\nThanks!\nDavid\n\n> {\n> V4L2DeviceFormat deviceFormat;\n> BayerFormat bayer = BayerFormat::fromMbusCode(mode.mbus_code);\n>\n> + bayer.packing = unpacked ? BayerFormat::Packing::None\n> + : BayerFormat::Packing::CSI2Packed;\n> +\n> deviceFormat.fourcc = bayer.toV4L2PixelFormat();\n> deviceFormat.size = mode.size;\n> return deviceFormat;\n> @@ -598,6 +601,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n> Size maxSize, sensorSize;\n> unsigned int maxIndex = 0;\n> bool rawStream = false;\n> + bool unpacked = false;\n>\n> /*\n> * Look for the RAW stream (if given) size as well as the largest\n> @@ -613,6 +617,10 @@ 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> + V4L2PixelFormat fourcc = V4L2PixelFormat::fromPixelFormat(cfg.pixelFormat);\n> + BayerFormat bayer = BayerFormat::fromV4L2PixelFormat(fourcc);\n> + unpacked = bayer.packing == BayerFormat::Packing::None;\n> } else {\n> if (cfg.size > maxSize) {\n> maxSize = config->at(i).size;\n> @@ -623,7 +631,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n>\n> /* First calculate the best sensor mode we can use based on the user request. */\n> V4L2SubdeviceFormat sensorFormat = findBestMode(data->sensorFormats_, rawStream ? sensorSize : maxSize);\n> - V4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(sensorFormat);\n> + V4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(sensorFormat, unpacked);\n>\n> ret = data->sensor_->setFormat(&sensorFormat);\n> if (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 BB2FDBF415\n\tfor ;\n\tFri, 22 Oct 2021 15:22:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 23E2068F59;\n\tFri, 22 Oct 2021 17:22:42 +0200 (CEST)","from mail-wm1-x32b.google.com (mail-wm1-x32b.google.com\n\t[IPv6:2a00:1450:4864:20::32b])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2FDD76012A\n\tfor ;\n\tFri, 22 Oct 2021 17:22:40 +0200 (CEST)","by mail-wm1-x32b.google.com with SMTP id\n\tu8-20020a05600c440800b0030d90076dabso9098614wmn.1\n\tfor ;\n\tFri, 22 Oct 2021 08:22:40 -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=\"EYN3nBf8\"; 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=5/fp/gm9nStyLHpwq7v4tdXLh5ewEqYVM+mAxCp1JNk=;\n\tb=EYN3nBf8bIONlUqbIDIMrgBsaTe93yWiCd43R263B8uCk7gm/tJKR3icwN6+PR0bPa\n\to87ZNN8TEL1pLmVrFAI7U26XJeR9Yd2iIRSGBzwmSc8/Zhdv+0f+tuEuMCcnayeSneZm\n\tlMIXU3voKaafDFX0GxpKH/aHuw0QyODsuhpofzNYs0JBdOI6PRXlEnUCvYRlV4cYe8pr\n\t1GzzFzYLhIJY/yqyWU9/MKfpqMxYqC9LqzWFYIJuLRxMIzqgRzsa7INf43yh42lJSRIn\n\tl04159PRCX/mBv98o9rRu+4WIQjk7PJbCvGhJ14dbJXFOQvoNNSBsRPi8Ht9+BLoXckr\n\tRM3w==","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=5/fp/gm9nStyLHpwq7v4tdXLh5ewEqYVM+mAxCp1JNk=;\n\tb=vOBgaWFTf+5a4RbTu6XJEsVWoaciedo8OKLuRSx8XtmpZc9ALTbGjBAOJtAU45tODJ\n\tunc7J2tBtvfjW/g4sgQZ6qtwup8ZKfxJAKKkaywDBJMP8QiSY8MEwvQrBFgs05HaxOz7\n\thJ9Gvd35mOVH4kQDJevEk3wxknjHswz/k7WbkP8LZ7g2qmiPSqAVJmyTO8V0VrvQXhZ0\n\t0AC8wi6UdxnbhwuAGT5mxxvOWNGg76QTCCdEPLgTwPpRbUIhYoElFr1mvmDKMagngR70\n\tgEjl2TAIA4mOtLw3i2Jx/w07dM2mD1sYx4ipyI3/1nNJFihLi+7ADcjDQ1eZNLe11OgQ\n\tsHPg==","X-Gm-Message-State":"AOAM533704pNGI7wnT+9XEmqF1TpX7IZeT/mayEU9Us+feyq4njITYos\n\tmUTxVAsgCzhKXYIApqafoeULePxmmMl77CzDgn5LlCGxWZs=","X-Google-Smtp-Source":"ABdhPJze9lVsNDJUme1g0Go8A2ZjJg3xAku/023029miQVGZtig6WqSpjGAAsdN+3W+6mfz0S2U4rmDrhNcXx5/dRN8=","X-Received":"by 2002:a7b:c858:: with SMTP id c24mr351305wml.171.1634916159804;\n\tFri, 22 Oct 2021 08:22:39 -0700 (PDT)","MIME-Version":"1.0","References":"<20211022143907.3089419-1-naush@raspberrypi.com>\n\t<20211022143907.3089419-4-naush@raspberrypi.com>","In-Reply-To":"<20211022143907.3089419-4-naush@raspberrypi.com>","From":"David Plowman ","Date":"Fri, 22 Oct 2021 16:22:28 +0100","Message-ID":"","To":"Naushir Patuck ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v2 3/6] 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":20402,"web_url":"https://patchwork.libcamera.org/comment/20402/","msgid":"","date":"2021-10-22T15:30:45","subject":"Re: [libcamera-devel] [PATCH v2 3/6] 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 David,\n\nThank you for your feedback.\n\nOn Fri, 22 Oct 2021 at 16:22, David Plowman \nwrote:\n\n> Hi Naush\n>\n> Thanks for the update.\n>\n> On Fri, 22 Oct 2021 at 15:40, Naushir Patuck \n> wrote:\n> >\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> Yes, I think this is the right way to do it!\n>\n> >\n> > Signed-off-by: Naushir Patuck \n> > ---\n> > src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 12 ++++++++++--\n> > 1 file changed, 10 insertions(+), 2 deletions(-)\n> >\n> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > index a31b0f81eba7..45725527d66e 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > @@ -61,11 +61,14 @@ SensorFormats\n> populateSensorFormats(std::unique_ptr &sensor)\n> > return formats;\n> > }\n> >\n> > -inline V4L2DeviceFormat toV4L2DeviceFormat(V4L2SubdeviceFormat &mode)\n> > +inline V4L2DeviceFormat toV4L2DeviceFormat(V4L2SubdeviceFormat &mode,\n> bool unpacked = false)\n>\n> I'm not totally sure I like the default value here - I kind of feel\n> people should be forced to think about it. But that's a teeny thing so\n> I wouldn't worry.\n>\n\nI can remove the default value in the next version, and switch the argument\nto be \"packed\".\n\nRegards,\nNaush\n\n\n>\n> And also ignoring the very minor brain contortion involved in passing\n> *un*-packed which gets flipped to set the *packed* field!\n>\n> Reviewed-by: David Plowman \n>\n> Thanks!\n> David\n>\n> > {\n> > V4L2DeviceFormat deviceFormat;\n> > BayerFormat bayer = BayerFormat::fromMbusCode(mode.mbus_code);\n> >\n> > + bayer.packing = unpacked ? BayerFormat::Packing::None\n> > + : BayerFormat::Packing::CSI2Packed;\n> > +\n> > deviceFormat.fourcc = bayer.toV4L2PixelFormat();\n> > deviceFormat.size = mode.size;\n> > return deviceFormat;\n> > @@ -598,6 +601,7 @@ int PipelineHandlerRPi::configure(Camera *camera,\n> CameraConfiguration *config)\n> > Size maxSize, sensorSize;\n> > unsigned int maxIndex = 0;\n> > bool rawStream = false;\n> > + bool unpacked = false;\n> >\n> > /*\n> > * Look for the RAW stream (if given) size as well as the largest\n> > @@ -613,6 +617,10 @@ 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> > + V4L2PixelFormat fourcc =\n> V4L2PixelFormat::fromPixelFormat(cfg.pixelFormat);\n> > + BayerFormat bayer =\n> BayerFormat::fromV4L2PixelFormat(fourcc);\n> > + unpacked = bayer.packing ==\n> BayerFormat::Packing::None;\n> > } else {\n> > if (cfg.size > maxSize) {\n> > maxSize = config->at(i).size;\n> > @@ -623,7 +631,7 @@ int PipelineHandlerRPi::configure(Camera *camera,\n> CameraConfiguration *config)\n> >\n> > /* First calculate the best sensor mode we can use based on the\n> user request. */\n> > V4L2SubdeviceFormat sensorFormat =\n> findBestMode(data->sensorFormats_, rawStream ? sensorSize : maxSize);\n> > - V4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(sensorFormat);\n> > + V4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(sensorFormat,\n> unpacked);\n> >\n> > ret = data->sensor_->setFormat(&sensorFormat);\n> > if (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 40DE2BF415\n\tfor ;\n\tFri, 22 Oct 2021 15:31:03 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EB94068F59;\n\tFri, 22 Oct 2021 17:31:02 +0200 (CEST)","from mail-lj1-x22e.google.com (mail-lj1-x22e.google.com\n\t[IPv6:2a00:1450:4864:20::22e])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 766266012A\n\tfor ;\n\tFri, 22 Oct 2021 17:31:01 +0200 (CEST)","by mail-lj1-x22e.google.com with SMTP id e19so2644634ljk.12\n\tfor ;\n\tFri, 22 Oct 2021 08:31:01 -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=\"jOsmvfn2\"; 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=wq6LrE/Zu6S8+Y8E+CKqHOTAS1G2pkIiQ7K6G3MKGBA=;\n\tb=jOsmvfn2MXmnEIAn/lb9URnKH8WlsqLNTBBsSvfubVZjwgh7Kvrt02bUDDmNMvD+tZ\n\tjTnjvsi2ULHQlTdzt0A9E8bRtnX2ex+XGeEJmKorGI53l7k0Jd2xvEWnqiaUfRaeyjA1\n\t37aCuFtLnNQfhEVxjLMmKUUFsQg6K1qvWOygeqEaFwtJldiE46AURH9aXdbcJ/5KZOyP\n\twUA4sPvjfTGFo6BLz83tpvRM24cfFAhnaLwmVvXwHAttGGtQ8gBKccm6foyz4anAfiQG\n\t5BITzGZAZHgoWyYXrYR504u8kX4rsJU8vcdZsSmcGPRXv1C4CS8Dx/5fwTVETDLSqNgx\n\t01fw==","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=wq6LrE/Zu6S8+Y8E+CKqHOTAS1G2pkIiQ7K6G3MKGBA=;\n\tb=QVWTtGYTMBautQiduJJTdjnezCLffzGLTm5jxls3suqrs+hAVFRvwLpAIPi5tMJsJ1\n\t4xUoYniXGBEcNdewU11Omdkq9BMNUFM3lhxv9c6kFtgHi3GjZ3uqQdk8vpbUHDuLkBWq\n\tOyOK/l89HUstGBJQBoTy8jLvQU57S99EfSEUSZqvTd081Qsb/AyO82uPMajnV6Zlm5LI\n\txGKO6lonV2NBtnHC0wcwMbe1UpeXt5TRupAqXlF8WMc8l+wglpXyWL1dyx8Qny1r6Y+U\n\teTvTMgXZ7wSTrgXL+gWrdgwPgBdPbeh4Cbd5gm3Iuf3v9FSRK1RInbv3RJNYHRiRVgPt\n\tO6DA==","X-Gm-Message-State":"AOAM533/zySyMgJ/ow8TtNErxaZ+wHuG9v4A3wCNth9iwuaLrvnWjU23\n\tntShHimjZ4wNYHIxQrO+KdC78Y5zEYcAFRYluZLbsttumvSLIw==","X-Google-Smtp-Source":"ABdhPJxI6u8bkZFLs/lQspfUKdpRQiYSP506imPd3SL92A0dRLIv+m3H2htmiswENXXdC526tIpp/grhRlo+T1WVmaI=","X-Received":"by 2002:a05:651c:1505:: with SMTP id\n\te5mr683567ljf.520.1634916660936; \n\tFri, 22 Oct 2021 08:31:00 -0700 (PDT)","MIME-Version":"1.0","References":"<20211022143907.3089419-1-naush@raspberrypi.com>\n\t<20211022143907.3089419-4-naush@raspberrypi.com>\n\t","In-Reply-To":"","From":"Naushir Patuck ","Date":"Fri, 22 Oct 2021 16:30:45 +0100","Message-ID":"","To":"David Plowman ","Content-Type":"multipart/alternative; boundary=\"000000000000d0130805cef2b2ab\"","Subject":"Re: [libcamera-devel] [PATCH v2 3/6] 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":20447,"web_url":"https://patchwork.libcamera.org/comment/20447/","msgid":"","date":"2021-10-25T16:59:19","subject":"Re: [libcamera-devel] [PATCH v2 3/6] 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 Fri, Oct 22, 2021 at 04:30:45PM +0100, Naushir Patuck wrote:\n> On Fri, 22 Oct 2021 at 16:22, David Plowman wrote:\n> > On Fri, 22 Oct 2021 at 15:40, Naushir Patuck wrote:\n> > >\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> > Yes, I think this is the right way to do it!\n> >\n> > >\n> > > Signed-off-by: Naushir Patuck \n> > > ---\n> > > src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 12 ++++++++++--\n> > > 1 file changed, 10 insertions(+), 2 deletions(-)\n> > >\n> > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > index a31b0f81eba7..45725527d66e 100644\n> > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > @@ -61,11 +61,14 @@ SensorFormats populateSensorFormats(std::unique_ptr &sensor)\n> > > return formats;\n> > > }\n> > >\n> > > -inline V4L2DeviceFormat toV4L2DeviceFormat(V4L2SubdeviceFormat &mode)\n> > > +inline V4L2DeviceFormat toV4L2DeviceFormat(V4L2SubdeviceFormat &mode, bool unpacked = false)\n\nBy the way, I'd drop the inline keyword and let the compiler decide.\n\n> > I'm not totally sure I like the default value here - I kind of feel\n> > people should be forced to think about it. But that's a teeny thing so\n> > I wouldn't worry.\n> \n> I can remove the default value in the next version, and switch the argument\n> to be \"packed\".\n\nHow about also replacing the bool argument with a BayerFormat::Packing ?\n\n> > And also ignoring the very minor brain contortion involved in passing\n> > *un*-packed which gets flipped to set the *packed* field!\n> >\n> > Reviewed-by: David Plowman \n> >\n> > > {\n> > > V4L2DeviceFormat deviceFormat;\n> > > BayerFormat bayer = BayerFormat::fromMbusCode(mode.mbus_code);\n> > >\n> > > + bayer.packing = unpacked ? BayerFormat::Packing::None\n> > > + : BayerFormat::Packing::CSI2Packed;\n> > > +\n> > > deviceFormat.fourcc = bayer.toV4L2PixelFormat();\n> > > deviceFormat.size = mode.size;\n> > > return deviceFormat;\n> > > @@ -598,6 +601,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n> > > Size maxSize, sensorSize;\n> > > unsigned int maxIndex = 0;\n> > > bool rawStream = false;\n> > > + bool unpacked = false;\n> > >\n> > > /*\n> > > * Look for the RAW stream (if given) size as well as the largest\n> > > @@ -613,6 +617,10 @@ 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> > > + V4L2PixelFormat fourcc = V4L2PixelFormat::fromPixelFormat(cfg.pixelFormat);\n> > > + BayerFormat bayer = BayerFormat::fromV4L2PixelFormat(fourcc);\n\nThis should go to a BayerFormat::fromPixelFormat() function, given that\nyou'd added BayerFormat::toPixelFormat() in 1/6.\n\n> > > + unpacked = bayer.packing == BayerFormat::Packing::None;\n> > > } else {\n> > > if (cfg.size > maxSize) {\n> > > maxSize = config->at(i).size;\n> > > @@ -623,7 +631,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n> > >\n> > > /* First calculate the best sensor mode we can use based on the user request. */\n> > > V4L2SubdeviceFormat sensorFormat = findBestMode(data->sensorFormats_, rawStream ? sensorSize : maxSize);\n> > > - V4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(sensorFormat);\n> > > + V4L2DeviceFormat unicamFormat = toV4L2DeviceFormat(sensorFormat, unpacked);\n> > >\n> > > ret = data->sensor_->setFormat(&sensorFormat);\n> > > if (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 B3152BDB1C\n\tfor ;\n\tMon, 25 Oct 2021 16:59:46 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 105B764875;\n\tMon, 25 Oct 2021 18:59:46 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1C93860125\n\tfor ;\n\tMon, 25 Oct 2021 18:59:44 +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 32F39E0A;\n\tMon, 25 Oct 2021 18:59: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=\"FGVbTOML\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1635181183;\n\tbh=Hcxy6O3ps6MnKpoPkGl5q6Gwfl59PetpU/hQlSMo+e4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=FGVbTOMLGYN8rwP3uErPIDYSxtVohuXg925hawMwJH0AoOFIxaa+rbXpyjOyBbQEY\n\tRDrXX0nO6+mrHvH8yj3J7fLY2n99vBw9cjfb9cj9OfELa6HeQVPDEnQTC/4s3xTYw9\n\tnrXx/SOphI3pWIm1GH7NlUVEtaBOOumvqUyOfk/M=","Date":"Mon, 25 Oct 2021 19:59:19 +0300","From":"Laurent Pinchart ","To":"Naushir Patuck ","Message-ID":"","References":"<20211022143907.3089419-1-naush@raspberrypi.com>\n\t<20211022143907.3089419-4-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 v2 3/6] 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":20510,"web_url":"https://patchwork.libcamera.org/comment/20510/","msgid":"","date":"2021-10-26T07:51:33","subject":"Re: [libcamera-devel] [PATCH v2 3/6] 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 your feedback.\n\nOn Mon, 25 Oct 2021 at 17:59, Laurent Pinchart <\nlaurent.pinchart@ideasonboard.com> wrote:\n\n> Hi Naush,\n>\n> Thank you for the patch.\n>\n> On Fri, Oct 22, 2021 at 04:30:45PM +0100, Naushir Patuck wrote:\n> > On Fri, 22 Oct 2021 at 16:22, David Plowman wrote:\n> > > On Fri, 22 Oct 2021 at 15:40, Naushir Patuck wrote:\n> > > >\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> > > Yes, I think this is the right way to do it!\n> > >\n> > > >\n> > > > Signed-off-by: Naushir Patuck \n> > > > ---\n> > > > src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 12 ++++++++++--\n> > > > 1 file changed, 10 insertions(+), 2 deletions(-)\n> > > >\n> > > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > > index a31b0f81eba7..45725527d66e 100644\n> > > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > > @@ -61,11 +61,14 @@ SensorFormats\n> populateSensorFormats(std::unique_ptr &sensor)\n> > > > return formats;\n> > > > }\n> > > >\n> > > > -inline V4L2DeviceFormat toV4L2DeviceFormat(V4L2SubdeviceFormat\n> &mode)\n> > > > +inline V4L2DeviceFormat toV4L2DeviceFormat(V4L2SubdeviceFormat\n> &mode, bool unpacked = false)\n>\n> By the way, I'd drop the inline keyword and let the compiler decide.\n>\n> > > I'm not totally sure I like the default value here - I kind of feel\n> > > people should be forced to think about it. But that's a teeny thing so\n> > > I wouldn't worry.\n> >\n> > I can remove the default value in the next version, and switch the\n> argument\n> > to be \"packed\".\n>\n> How about also replacing the bool argument with a BayerFormat::Packing ?\n>\n\nThat seems reasonable, and would really force the caller to make a decision.\n\n\n>\n> > > And also ignoring the very minor brain contortion involved in passing\n> > > *un*-packed which gets flipped to set the *packed* field!\n> > >\n> > > Reviewed-by: David Plowman \n> > >\n> > > > {\n> > > > V4L2DeviceFormat deviceFormat;\n> > > > BayerFormat bayer =\n> BayerFormat::fromMbusCode(mode.mbus_code);\n> > > >\n> > > > + bayer.packing = unpacked ? BayerFormat::Packing::None\n> > > > + : BayerFormat::Packing::CSI2Packed;\n> > > > +\n> > > > deviceFormat.fourcc = bayer.toV4L2PixelFormat();\n> > > > deviceFormat.size = mode.size;\n> > > > return deviceFormat;\n> > > > @@ -598,6 +601,7 @@ int PipelineHandlerRPi::configure(Camera\n> *camera, CameraConfiguration *config)\n> > > > Size maxSize, sensorSize;\n> > > > unsigned int maxIndex = 0;\n> > > > bool rawStream = false;\n> > > > + bool unpacked = false;\n> > > >\n> > > > /*\n> > > > * Look for the RAW stream (if given) size as well as the\n> largest\n> > > > @@ -613,6 +617,10 @@ 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> > > > + V4L2PixelFormat fourcc =\n> V4L2PixelFormat::fromPixelFormat(cfg.pixelFormat);\n> > > > + BayerFormat bayer =\n> BayerFormat::fromV4L2PixelFormat(fourcc);\n>\n> This should go to a BayerFormat::fromPixelFormat() function, given that\n> you'd added BayerFormat::toPixelFormat() in 1/6.\n>\n\nWill do!\n\nThanks,\nNaush\n\n\n>\n> > > > + unpacked = bayer.packing ==\n> BayerFormat::Packing::None;\n> > > > } else {\n> > > > if (cfg.size > maxSize) {\n> > > > maxSize = config->at(i).size;\n> > > > @@ -623,7 +631,7 @@ int PipelineHandlerRPi::configure(Camera\n> *camera, CameraConfiguration *config)\n> > > >\n> > > > /* First calculate the best sensor mode we can use based on\n> the user request. */\n> > > > V4L2SubdeviceFormat sensorFormat =\n> findBestMode(data->sensorFormats_, rawStream ? sensorSize : maxSize);\n> > > > - V4L2DeviceFormat unicamFormat =\n> toV4L2DeviceFormat(sensorFormat);\n> > > > + V4L2DeviceFormat unicamFormat =\n> toV4L2DeviceFormat(sensorFormat, unpacked);\n> > > >\n> > > > ret = data->sensor_->setFormat(&sensorFormat);\n> > > > if (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 937BEBDB1C\n\tfor ;\n\tTue, 26 Oct 2021 07:51:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C0BA764875;\n\tTue, 26 Oct 2021 09:51:50 +0200 (CEST)","from mail-lj1-x232.google.com (mail-lj1-x232.google.com\n\t[IPv6:2a00:1450:4864:20::232])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CC54660123\n\tfor ;\n\tTue, 26 Oct 2021 09:51:49 +0200 (CEST)","by mail-lj1-x232.google.com with SMTP id w23so13275530lje.7\n\tfor ;\n\tTue, 26 Oct 2021 00:51:49 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"qcJChMl3\"; 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=l5uvZvAraD6/Hjg1j/WvITEhknYQnVP7z+/IXl9/p/4=;\n\tb=qcJChMl3nVrqp5M19oHlduAn0T3fSTgFTLnJUl0WjqhB71Mxw2/aBSz5pa1sRZhVW0\n\t7WcTopKj864LotQueU84q0cKYsacDWMYItNRscOBo9a+xsketfGuY6ePt1I6i/8urVlp\n\trhTjP8jOxSQEcSKpaHix1R5E0Nb5ah4Lz6ceSdVX8gVWmYwerFx4AR+/7G7GQzEJLpog\n\tLrz0lVAm4WbN/J69cTlHzpchHvReaOBwCTRFZrScpn1uM3S6j5TDBytd/bM35xujopXA\n\t2d27nAhmBYyljWA5XQZNAg7IpA00yEvdmf9nDlOaTUX29EsRN0FylEyWCE21o110DxF0\n\tJC8w==","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=l5uvZvAraD6/Hjg1j/WvITEhknYQnVP7z+/IXl9/p/4=;\n\tb=h/N5igeDtE8MdhdgIcyz7cA23utQi6BY4idIo8SBI1Brd4BGW8EoSqeubG3ijLj1wa\n\tchfL6zs2ay50lLsBJW0RsAmh0d/HhYit5swGyCAbMi5dWY0jwjtGbGPBm3tkJt0Qp0wL\n\tNVIkFAw/stQZ1hMpfwS+eByrweXkSprVLQnYvxg6QCHiFsqn2vqsakNsq4JZ7zwkz9Oj\n\tcKACvZxMukXsBZFY/HZ9Fi3da79PtLmw6dV0Lh0c27KuFPM77/s/VLXEtQ1ekIqPoc+0\n\tSGT7ztTb4Mvq8LZefIuZIhs5rzfvJuWlrtVZdmCIdEAOPA8YX7n0Vy9Bk+0zlrx+WklX\n\t1YPw==","X-Gm-Message-State":"AOAM533ghTxr+ST8NrralYCnHpbxR/SQ4N5WL7BzNgw/7JGsKy++KG4p\n\tqRv4pvouaIjw7eEiDdKiGBnDIUJh/3zgfladE+wKfc+qr9Xstg==","X-Google-Smtp-Source":"ABdhPJzsE7eaH/yKOnlNutxIbha3FOY2CqiVPex5lLXpGafbk0SiHagxjDXNO0fTQFaoixQcG7L/ZwquwhyBlEbEftM=","X-Received":"by 2002:a2e:a595:: with SMTP id\n\tm21mr25359336ljp.372.1635234709185; \n\tTue, 26 Oct 2021 00:51:49 -0700 (PDT)","MIME-Version":"1.0","References":"<20211022143907.3089419-1-naush@raspberrypi.com>\n\t<20211022143907.3089419-4-naush@raspberrypi.com>\n\t\n\t\n\t","In-Reply-To":"","From":"Naushir Patuck ","Date":"Tue, 26 Oct 2021 08:51:33 +0100","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"multipart/alternative; boundary=\"000000000000f7346c05cf3cbfa6\"","Subject":"Re: [libcamera-devel] [PATCH v2 3/6] 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\" "}}]