[{"id":27955,"web_url":"https://patchwork.libcamera.org/comment/27955/","msgid":"<2gqk6xpg3g4e3p6fn5okvmlvjplfqr7rzn7vq3jbxzog5vfgsp@jx2ngvqb7cin>","date":"2023-10-12T10:05:09","subject":"Re: [libcamera-devel] [PATCH 14/20] pipeline: rpi: Move flip\n\thandling validation code","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Naush\n\nOn Fri, Oct 06, 2023 at 02:19:54PM +0100, Naushir Patuck via libcamera-devel wrote:\n> Move the handling of Bayer order changes due to flips entirely into\n> platformValidate(). This removes the need for this code to be split\n> between platformValidate() and validate() as it is right now.\n\nDoesn't this mean the same code has to be repeated in both platform's\npipelines ?\n\n>\n> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n> Reviewed-by: David Plowman <david.plowman@raspberrypi.com>\n> ---\n>  .../pipeline/rpi/common/pipeline_base.cpp          | 14 --------------\n>  src/libcamera/pipeline/rpi/vc4/vc4.cpp             | 12 ++++++++++--\n>  2 files changed, 10 insertions(+), 16 deletions(-)\n>\n> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> index 825eae80d014..7437d38dc9ba 100644\n> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> @@ -242,20 +242,6 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n>  \t\tif (ret)\n>  \t\t\treturn Invalid;\n>\n> -\t\t/*\n> -\t\t * Some sensors change their Bayer order when they are h-flipped\n> -\t\t * or v-flipped, according to the transform. If this one does, we\n> -\t\t * must advertise the transformed Bayer order in the raw stream.\n> -\t\t * Note how we must fetch the \"native\" (i.e. untransformed) Bayer\n> -\t\t * order, because the sensor may currently be flipped!\n> -\t\t */\n> -\t\tBayerFormat bayer = BayerFormat::fromPixelFormat(raw.cfg->pixelFormat);\n> -\t\tif (data_->flipsAlterBayerOrder_) {\n> -\t\t\tbayer.order = data_->nativeBayerOrder_;\n> -\t\t\tbayer = bayer.transform(combinedTransform_);\n> -\t\t}\n> -\t\traw.cfg->pixelFormat = bayer.toPixelFormat();\n> -\n>  \t\tif (RPi::PipelineHandlerBase::updateStreamConfig(raw.cfg, raw.format))\n>  \t\t\tstatus = Adjusted;\n>  \t}\n> diff --git a/src/libcamera/pipeline/rpi/vc4/vc4.cpp b/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> index 233473e2fe2b..4b42ddc71115 100644\n> --- a/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> +++ b/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> @@ -411,9 +411,17 @@ CameraConfiguration::Status Vc4CameraData::platformValidate(RPi::RPiCameraConfig\n>  \t\tStreamConfiguration *rawStream = rawStreams[0].cfg;\n>  \t\tBayerFormat rawBayer = BayerFormat::fromMbusCode(rpiConfig->sensorFormat_.mbus_code);\n>\n> -\t\t/* Handle flips to make sure to match the RAW stream format. */\n> -\t\tif (flipsAlterBayerOrder_)\n> +\t\t/*\n> +\t\t * Some sensors change their Bayer order when they are h-flipped\n> +\t\t * or v-flipped, according to the transform. If this one does, we\n> +\t\t * must advertise the transformed Bayer order in the raw stream.\n> +\t\t * Note how we must fetch the \"native\" (i.e. untransformed) Bayer\n> +\t\t * order, because the sensor may currently be flipped!\n> +\t\t */\n> +\t\tif (flipsAlterBayerOrder_) {\n> +\t\t\trawBayer.order = nativeBayerOrder_;\n>  \t\t\trawBayer = rawBayer.transform(rpiConfig->combinedTransform_);\n> +\t\t}\n>\n>  \t\t/* Apply the user requested packing. */\n>  \t\trawBayer.packing = BayerFormat::fromPixelFormat(rawStream->pixelFormat).packing;\n> --\n> 2.34.1\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 D91D1C3272\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 12 Oct 2023 10:05:13 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8E9A56297E;\n\tThu, 12 Oct 2023 12:05:13 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id EA4E76296D\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 12 Oct 2023 12:05:11 +0200 (CEST)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id CD7B57FC;\n\tThu, 12 Oct 2023 12:05:08 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1697105113;\n\tbh=rdz6+1FDt0NRSHApCAKJotCBPuN91ejsjElkuuxjizE=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=GLYdnBkdOaminvY37CEWMWQ1pyXtfYnHBNo4+Zd7zfGn3kuxKWKiQFarrEkzC+USE\n\t1TnmG7tFr1V7f3pxvv0Z5kbUkC8Job0JpDJmGCnvzkepRsX//vPs1QWQO2EntjmntS\n\tllr0owgHLayKxUoMLoThYXAVgkqf8dgzjYrkMHz0754ahaSNBgFtOLD1+utkZB5pOO\n\tLYtf7n8mRuBQKwBI7iamaIHY/pbANrjjGYz0BrLJtZx6qXwTqqWy6YWtRuKQI10n55\n\t54ZQWLaYkBoV53NcQXQP6wejk0bFAFiKxwwUkXLQUAHKjlEkUNCaK294NJukVgvqZ1\n\tIDg9MIGE+nprA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1697105108;\n\tbh=rdz6+1FDt0NRSHApCAKJotCBPuN91ejsjElkuuxjizE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=jT8bILezQ38Gr30vdxftXsWI50M5i7QTogSNzskpzBkRF6WvX+UgJN76voGpWm+Qf\n\tExoLlh9+27o2rwZim68MwmhwHDuvPwl8UCFhNfWkCW0Q189DNHMjwJyXt7wv40o7bm\n\tydvVhsUjhYrY5CdIz3EvmLCXKtmYYT0voG+OzkBs="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"jT8bILez\"; dkim-atps=neutral","Date":"Thu, 12 Oct 2023 12:05:09 +0200","To":"Naushir Patuck <naush@raspberrypi.com>","Message-ID":"<2gqk6xpg3g4e3p6fn5okvmlvjplfqr7rzn7vq3jbxzog5vfgsp@jx2ngvqb7cin>","References":"<20231006132000.23504-1-naush@raspberrypi.com>\n\t<20231006132000.23504-15-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20231006132000.23504-15-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH 14/20] pipeline: rpi: Move flip\n\thandling validation code","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Jacopo Mondi via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":27965,"web_url":"https://patchwork.libcamera.org/comment/27965/","msgid":"<CAEmqJPr=MrAWXUzPBgMw1a8YyY0Ptv7WW-=eP33nV6uGAi76dg@mail.gmail.com>","date":"2023-10-12T12:07:13","subject":"Re: [libcamera-devel] [PATCH 14/20] pipeline: rpi: Move flip\n\thandling validation code","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"On Thu, 12 Oct 2023 at 11:05, Jacopo Mondi\n<jacopo.mondi@ideasonboard.com> wrote:\n>\n> Hi Naush\n>\n> On Fri, Oct 06, 2023 at 02:19:54PM +0100, Naushir Patuck via libcamera-devel wrote:\n> > Move the handling of Bayer order changes due to flips entirely into\n> > platformValidate(). This removes the need for this code to be split\n> > between platformValidate() and validate() as it is right now.\n>\n> Doesn't this mean the same code has to be repeated in both platform's\n> pipelines ?\n\nSadly yes :(\n\nI haven't figured out a clean/tidy way to do this in pipeline_base and\nremove the duplication.  Things get a bit more complicated when user\npacking requests interfere with the raw output as well, and packing\nconstraints are platform specific (e.g. no CSI2 packing on Pi5).  I'll\ncontinue working on getting a cleaner solution, but depending on the\ntimeframe, it might have to be put on top of this work.\n\nRegards,\nNaush\n\n>\n> >\n> > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n> > Reviewed-by: David Plowman <david.plowman@raspberrypi.com>\n> > ---\n> >  .../pipeline/rpi/common/pipeline_base.cpp          | 14 --------------\n> >  src/libcamera/pipeline/rpi/vc4/vc4.cpp             | 12 ++++++++++--\n> >  2 files changed, 10 insertions(+), 16 deletions(-)\n> >\n> > diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > index 825eae80d014..7437d38dc9ba 100644\n> > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > @@ -242,20 +242,6 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n> >               if (ret)\n> >                       return Invalid;\n> >\n> > -             /*\n> > -              * Some sensors change their Bayer order when they are h-flipped\n> > -              * or v-flipped, according to the transform. If this one does, we\n> > -              * must advertise the transformed Bayer order in the raw stream.\n> > -              * Note how we must fetch the \"native\" (i.e. untransformed) Bayer\n> > -              * order, because the sensor may currently be flipped!\n> > -              */\n> > -             BayerFormat bayer = BayerFormat::fromPixelFormat(raw.cfg->pixelFormat);\n> > -             if (data_->flipsAlterBayerOrder_) {\n> > -                     bayer.order = data_->nativeBayerOrder_;\n> > -                     bayer = bayer.transform(combinedTransform_);\n> > -             }\n> > -             raw.cfg->pixelFormat = bayer.toPixelFormat();\n> > -\n> >               if (RPi::PipelineHandlerBase::updateStreamConfig(raw.cfg, raw.format))\n> >                       status = Adjusted;\n> >       }\n> > diff --git a/src/libcamera/pipeline/rpi/vc4/vc4.cpp b/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> > index 233473e2fe2b..4b42ddc71115 100644\n> > --- a/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> > +++ b/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> > @@ -411,9 +411,17 @@ CameraConfiguration::Status Vc4CameraData::platformValidate(RPi::RPiCameraConfig\n> >               StreamConfiguration *rawStream = rawStreams[0].cfg;\n> >               BayerFormat rawBayer = BayerFormat::fromMbusCode(rpiConfig->sensorFormat_.mbus_code);\n> >\n> > -             /* Handle flips to make sure to match the RAW stream format. */\n> > -             if (flipsAlterBayerOrder_)\n> > +             /*\n> > +              * Some sensors change their Bayer order when they are h-flipped\n> > +              * or v-flipped, according to the transform. If this one does, we\n> > +              * must advertise the transformed Bayer order in the raw stream.\n> > +              * Note how we must fetch the \"native\" (i.e. untransformed) Bayer\n> > +              * order, because the sensor may currently be flipped!\n> > +              */\n> > +             if (flipsAlterBayerOrder_) {\n> > +                     rawBayer.order = nativeBayerOrder_;\n> >                       rawBayer = rawBayer.transform(rpiConfig->combinedTransform_);\n> > +             }\n> >\n> >               /* Apply the user requested packing. */\n> >               rawBayer.packing = BayerFormat::fromPixelFormat(rawStream->pixelFormat).packing;\n> > --\n> > 2.34.1\n> >","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 D2B91C3272\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 12 Oct 2023 12:07:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1D3536297B;\n\tThu, 12 Oct 2023 14:07:43 +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 7DB106296D\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 12 Oct 2023 14:07:41 +0200 (CEST)","by mail-yw1-x1130.google.com with SMTP id\n\t00721157ae682-59f6492b415so7316227b3.0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 12 Oct 2023 05:07:41 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1697112463;\n\tbh=lpmmv7g/B83bNjDhdiYOisjYMgQbPbROR7XIQP+5QJs=;\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=k207vMm1mcL3+IXMk9DZCeSTbh6wq5W2LwJQSHHllWfaRhHNs95b6FpEMu421Atc0\n\tIJrxdBFGJC5s2bV7NqrZX549G1ixgb4AA8JVy/1pFU7v+V4LFBu7XcagKyTqqb5qK4\n\tk8TBEH0lfSZ3GaywvrpnhXJVSnxDprC/66wXaA2NLx1vV9Hmo5Y503cODve+ifQbL8\n\tzq6GhU3cIiM0tKDq/N1aqzSaQa5L8gtPI/ozp1Ll8b14gDGbUOgX6Lw6OeyPtbLMg1\n\tMPdB91St3aIxFTu7p0oofztnfLqr54wckGGTmgi32PGysRswwjOK47YidOXtB8qJx1\n\tLdPI5qahbnppg==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1697112460; x=1697717260;\n\tdarn=lists.libcamera.org; \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=UQCXYTWY2eOB9n3MaV4ZGtQnipW3JW2fjCtOKPM3Sfg=;\n\tb=KthtQR7rkSsIdncWU94Jflsk08j0eVrk8UVSrRjj0jEPrg4Rh5z2G0StLvu2hxynN0\n\tx3iDOnnI1PuJyQN6SclG4ab6Ci4cPR74B5VBOmEx24GMNcDXqf+MdQIoh6J1L+Cc/Ffa\n\t3qsnmuRhh2DuFNwiZEtitIPCQAYD1pIqw4NfZQYI0dj+WcWFcaho/fn2SYwj+vw+Oq+o\n\t4cc4hJJ9/vWe4q5/2dwxhVyfJ/2ejy0+r7PbgoQrFRwObED1t+6WGGhGu/dS4N/hL/sm\n\tZ2ou2iA9lRQet11hOeXnBAyaFMy5NqS4w14RobjJ3+lRQIzKxw2riZ5wFCqrDsA70ggC\n\tpy/Q=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"KthtQR7r\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1697112460; x=1697717260;\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=UQCXYTWY2eOB9n3MaV4ZGtQnipW3JW2fjCtOKPM3Sfg=;\n\tb=gEgM7Pdjf/1wdptgZ0+xVgGCobKBgnyPK1Qj4YagxNnKaeQhxz6X1LnIgQEIdEDDGM\n\tYaamX98nM0MPDis1YsoidTjBGhCk0qRns2IAf9J/7qjHXsVzIF9W+x4ElICD9MEGRayA\n\tkgbtSd3O+7kawsFrnAiCpzySXZdNrgVB6TvIC/W6gSQ03QZSePZ48gjMTcWibT0JreGn\n\t+e42HvxR/6WCnlytxNyFXxmMQeD2e7J2tefD9c+IeiSR9QdjXD7AOTAF/Rnfd/6NFMWV\n\tYFTVDGn3FylVk5UNaZa3dWtkkBg8blpJnWDaiwJGoEgjlqNPYK2y2TfpZ5S4m7YgvCa8\n\t+iWA==","X-Gm-Message-State":"AOJu0YymFX/fExid3jeDGsNX5+Q7jGVXqgPUMOhQAPfb7cRSfeEEAgze\n\tXvMnbGWjme+jHxjKnoBrN+b854VGlh/eiwLuF0oNU3d5TmCk7wcpW5g=","X-Google-Smtp-Source":"AGHT+IET5afLW4vIR8aOW+DwL1kiFvpVwVVfVsJeZz/Sqg8G+rt+qdxT5piCOnm3AcLPGKDTleQDI0RbyTjzWpVWSWY=","X-Received":"by 2002:a81:6dd6:0:b0:5a7:d088:2c5d with SMTP id\n\ti205-20020a816dd6000000b005a7d0882c5dmr3669202ywc.26.1697112460278;\n\tThu, 12 Oct 2023 05:07:40 -0700 (PDT)","MIME-Version":"1.0","References":"<20231006132000.23504-1-naush@raspberrypi.com>\n\t<20231006132000.23504-15-naush@raspberrypi.com>\n\t<2gqk6xpg3g4e3p6fn5okvmlvjplfqr7rzn7vq3jbxzog5vfgsp@jx2ngvqb7cin>","In-Reply-To":"<2gqk6xpg3g4e3p6fn5okvmlvjplfqr7rzn7vq3jbxzog5vfgsp@jx2ngvqb7cin>","Date":"Thu, 12 Oct 2023 13:07:13 +0100","Message-ID":"<CAEmqJPr=MrAWXUzPBgMw1a8YyY0Ptv7WW-=eP33nV6uGAi76dg@mail.gmail.com>","To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH 14/20] pipeline: rpi: Move flip\n\thandling validation code","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Naushir Patuck via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Naushir Patuck <naush@raspberrypi.com>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":27966,"web_url":"https://patchwork.libcamera.org/comment/27966/","msgid":"<CAEmqJPquFBdBoe9N9_W00oUMw1SjjGVQ9cO-gpp8MwCdOAFR9Q@mail.gmail.com>","date":"2023-10-12T12:55:00","subject":"Re: [libcamera-devel] [PATCH 14/20] pipeline: rpi: Move flip\n\thandling validation code","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"On Thu, 12 Oct 2023 at 13:07, Naushir Patuck <naush@raspberrypi.com> wrote:\n>\n> On Thu, 12 Oct 2023 at 11:05, Jacopo Mondi\n> <jacopo.mondi@ideasonboard.com> wrote:\n> >\n> > Hi Naush\n> >\n> > On Fri, Oct 06, 2023 at 02:19:54PM +0100, Naushir Patuck via libcamera-devel wrote:\n> > > Move the handling of Bayer order changes due to flips entirely into\n> > > platformValidate(). This removes the need for this code to be split\n> > > between platformValidate() and validate() as it is right now.\n> >\n> > Doesn't this mean the same code has to be repeated in both platform's\n> > pipelines ?\n>\n> Sadly yes :(\n>\n> I haven't figured out a clean/tidy way to do this in pipeline_base and\n> remove the duplication.  Things get a bit more complicated when user\n> packing requests interfere with the raw output as well, and packing\n> constraints are platform specific (e.g. no CSI2 packing on Pi5).  I'll\n> continue working on getting a cleaner solution, but depending on the\n> timeframe, it might have to be put on top of this work.\n>\n\nAnd now after spending a bit more time looking at it, I think the\nsolution is actually simpler than I thought!  I have a candidate fix\nthat can be used to replace this patch.  Should I send it in a v2\ntogether with all the other minor fixes you noted?\n\n> Regards,\n> Naush\n>\n> >\n> > >\n> > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n> > > Reviewed-by: David Plowman <david.plowman@raspberrypi.com>\n> > > ---\n> > >  .../pipeline/rpi/common/pipeline_base.cpp          | 14 --------------\n> > >  src/libcamera/pipeline/rpi/vc4/vc4.cpp             | 12 ++++++++++--\n> > >  2 files changed, 10 insertions(+), 16 deletions(-)\n> > >\n> > > diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > > index 825eae80d014..7437d38dc9ba 100644\n> > > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > > @@ -242,20 +242,6 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n> > >               if (ret)\n> > >                       return Invalid;\n> > >\n> > > -             /*\n> > > -              * Some sensors change their Bayer order when they are h-flipped\n> > > -              * or v-flipped, according to the transform. If this one does, we\n> > > -              * must advertise the transformed Bayer order in the raw stream.\n> > > -              * Note how we must fetch the \"native\" (i.e. untransformed) Bayer\n> > > -              * order, because the sensor may currently be flipped!\n> > > -              */\n> > > -             BayerFormat bayer = BayerFormat::fromPixelFormat(raw.cfg->pixelFormat);\n> > > -             if (data_->flipsAlterBayerOrder_) {\n> > > -                     bayer.order = data_->nativeBayerOrder_;\n> > > -                     bayer = bayer.transform(combinedTransform_);\n> > > -             }\n> > > -             raw.cfg->pixelFormat = bayer.toPixelFormat();\n> > > -\n> > >               if (RPi::PipelineHandlerBase::updateStreamConfig(raw.cfg, raw.format))\n> > >                       status = Adjusted;\n> > >       }\n> > > diff --git a/src/libcamera/pipeline/rpi/vc4/vc4.cpp b/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> > > index 233473e2fe2b..4b42ddc71115 100644\n> > > --- a/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> > > +++ b/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> > > @@ -411,9 +411,17 @@ CameraConfiguration::Status Vc4CameraData::platformValidate(RPi::RPiCameraConfig\n> > >               StreamConfiguration *rawStream = rawStreams[0].cfg;\n> > >               BayerFormat rawBayer = BayerFormat::fromMbusCode(rpiConfig->sensorFormat_.mbus_code);\n> > >\n> > > -             /* Handle flips to make sure to match the RAW stream format. */\n> > > -             if (flipsAlterBayerOrder_)\n> > > +             /*\n> > > +              * Some sensors change their Bayer order when they are h-flipped\n> > > +              * or v-flipped, according to the transform. If this one does, we\n> > > +              * must advertise the transformed Bayer order in the raw stream.\n> > > +              * Note how we must fetch the \"native\" (i.e. untransformed) Bayer\n> > > +              * order, because the sensor may currently be flipped!\n> > > +              */\n> > > +             if (flipsAlterBayerOrder_) {\n> > > +                     rawBayer.order = nativeBayerOrder_;\n> > >                       rawBayer = rawBayer.transform(rpiConfig->combinedTransform_);\n> > > +             }\n> > >\n> > >               /* Apply the user requested packing. */\n> > >               rawBayer.packing = BayerFormat::fromPixelFormat(rawStream->pixelFormat).packing;\n> > > --\n> > > 2.34.1\n> > >","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 A4E45BD808\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 12 Oct 2023 12:55:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 099316297E;\n\tThu, 12 Oct 2023 14:55:29 +0200 (CEST)","from mail-yw1-x112a.google.com (mail-yw1-x112a.google.com\n\t[IPv6:2607:f8b0:4864:20::112a])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DBA576296D\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 12 Oct 2023 14:55:27 +0200 (CEST)","by mail-yw1-x112a.google.com with SMTP id\n\t00721157ae682-5a7c011e113so12818887b3.1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 12 Oct 2023 05:55:27 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1697115329;\n\tbh=9aY19xJ5uEBF3LyKubzNeZurqrhHxQV6ELP/rEOjx9I=;\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=LJiWgosnukViuWT8pA5GQnXc4uwyllx2wtzDSksHsd8AmIYdGh+9fJwmtRHDo7saF\n\t9mQ075F0dDo5IF0pShRn0KlIZo6EFaK+tZh1hd4ZkK5bsbJMfrkZEHsvrhvNcQ5jZH\n\tY2rQmUC49R5I6jBxU5R9VBWjMXtLxzDqF7oNbVUQcSWgYNLh6HrT/JUNGByoAiJIhb\n\te8icQcDCECCUdvq6xQq89VNxtOSK1lZNt9hrLNDEc4sX3vY9e8W38UyaaSPkS1WZU5\n\te7NbuYxtU9zSHmwDfnIHN/u82ZR/4/K9Oh9Ocrxrz7v1CDToU8tcdjg3CIwQGoGt6O\n\tyySkSVSwOle8g==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1697115326; x=1697720126;\n\tdarn=lists.libcamera.org; \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=8ltev1VNPY8hdqWfxTNcWtMkbt0iIWSXhIgZi5wJCNE=;\n\tb=inc4/0H2iN5MaoB6tKxdb9oCEnT0M2VVbpRjwrukGEvsvLVd9ohMTnrkDcrToHTmIH\n\tfE/V10L6dE2q43PXSxTPQLF4yqZAEM2mKLNfBsR82iEGJr+L8ehuw7f/lxpFtvm1CB+G\n\tCjxBpa402jvptATgHs4PBu3gD3wYejNCuphOflZ/CddKf1LzS0C+V3efH0mR36uHZg4j\n\tnqrHH3q5ookZjLVLWn6g9ewlOn/Vf0FcTGklg8MA8M1971fyoWrFiZFb7K1hd+EZFrkQ\n\tYKlsiKl9bfNfvyHCsrif4m4Xch0Rrd3JI/kFgmXZPB8kJLCxDvQMtfGTrET7Rq79FM2L\n\tHYsg=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"inc4/0H2\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1697115326; x=1697720126;\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=8ltev1VNPY8hdqWfxTNcWtMkbt0iIWSXhIgZi5wJCNE=;\n\tb=fiP6NQ4MwR2UAOK3wXOZnz8vJdgrY16puP1nFS26wWZpFaUJ5FUT7YCyFUNxaVKIls\n\tSid51GTKa2W1ZYeh98sHESVf3anU3XETYCOmlcVbeh93HOtf1fRk9Zrk+vr/Iz4qbjZB\n\tdv616+Dp+qPSacSzcgyvBZGgLpSb3Wl1hZf8kkBeVPTA3hqoh9NZSZkj4AxGOstJqjN/\n\tfKclI47ngipb6oYl/0x/OoV/Wu4WWF8r7Oc34Ndv6Z4FpJ9Mt1tnqExVEuJ11zbvjbFN\n\tnnoveJi8lRuQUIv9srJ5b5WXFKB5nJy1IcbX1zn1VX6C3jYBxoEMkQqCFIl0WtNNWwYd\n\trOQw==","X-Gm-Message-State":"AOJu0YzRNpy8HRO6oYT0h/QaWnmCDcJFEBk0N9k3qIn7IRlggN6zzmnh\n\tPSD3fPVtKAy8Tx+0PvLL7dT7vftxW5alq2AAAXR5FBtQVguutfb0Jns=","X-Google-Smtp-Source":"AGHT+IFIzV3Dp/0k018CtH/nCAtjOxUsho5yY/fKrGLAgXyqaJGc659Oyw4X1U/COIdS2YqPKO+6G1vt8Fk5MQk7zjM=","X-Received":"by 2002:a81:4306:0:b0:5a7:a81d:e410 with SMTP id\n\tq6-20020a814306000000b005a7a81de410mr10207331ywa.18.1697115326698;\n\tThu, 12 Oct 2023 05:55:26 -0700 (PDT)","MIME-Version":"1.0","References":"<20231006132000.23504-1-naush@raspberrypi.com>\n\t<20231006132000.23504-15-naush@raspberrypi.com>\n\t<2gqk6xpg3g4e3p6fn5okvmlvjplfqr7rzn7vq3jbxzog5vfgsp@jx2ngvqb7cin>\n\t<CAEmqJPr=MrAWXUzPBgMw1a8YyY0Ptv7WW-=eP33nV6uGAi76dg@mail.gmail.com>","In-Reply-To":"<CAEmqJPr=MrAWXUzPBgMw1a8YyY0Ptv7WW-=eP33nV6uGAi76dg@mail.gmail.com>","Date":"Thu, 12 Oct 2023 13:55:00 +0100","Message-ID":"<CAEmqJPquFBdBoe9N9_W00oUMw1SjjGVQ9cO-gpp8MwCdOAFR9Q@mail.gmail.com>","To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH 14/20] pipeline: rpi: Move flip\n\thandling validation code","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Naushir Patuck via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Naushir Patuck <naush@raspberrypi.com>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":27968,"web_url":"https://patchwork.libcamera.org/comment/27968/","msgid":"<uquaojk3f6vteqmb5fwz75itkyuovuyt6y6yvvrf3dyow4rmcc@gzksny545wzh>","date":"2023-10-12T14:40:26","subject":"Re: [libcamera-devel] [PATCH 14/20] pipeline: rpi: Move flip\n\thandling validation code","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Naush\n\nOn Thu, Oct 12, 2023 at 01:55:00PM +0100, Naushir Patuck wrote:\n> On Thu, 12 Oct 2023 at 13:07, Naushir Patuck <naush@raspberrypi.com> wrote:\n> >\n> > On Thu, 12 Oct 2023 at 11:05, Jacopo Mondi\n> > <jacopo.mondi@ideasonboard.com> wrote:\n> > >\n> > > Hi Naush\n> > >\n> > > On Fri, Oct 06, 2023 at 02:19:54PM +0100, Naushir Patuck via libcamera-devel wrote:\n> > > > Move the handling of Bayer order changes due to flips entirely into\n> > > > platformValidate(). This removes the need for this code to be split\n> > > > between platformValidate() and validate() as it is right now.\n> > >\n> > > Doesn't this mean the same code has to be repeated in both platform's\n> > > pipelines ?\n> >\n> > Sadly yes :(\n> >\n> > I haven't figured out a clean/tidy way to do this in pipeline_base and\n> > remove the duplication.  Things get a bit more complicated when user\n> > packing requests interfere with the raw output as well, and packing\n> > constraints are platform specific (e.g. no CSI2 packing on Pi5).  I'll\n> > continue working on getting a cleaner solution, but depending on the\n> > timeframe, it might have to be put on top of this work.\n> >\n>\n> And now after spending a bit more time looking at it, I think the\n> solution is actually simpler than I thought!  I have a candidate fix\n> that can be used to replace this patch.  Should I send it in a v2\n> together with all the other minor fixes you noted?\n>\n\nIf you want, I certainly won't oppose. My proposal to fix minors when\napplying still holds, but if you consider worth sending a v2 I'm all\nfor it!\n\nThanks\n  j\n\n> > Regards,\n> > Naush\n> >\n> > >\n> > > >\n> > > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n> > > > Reviewed-by: David Plowman <david.plowman@raspberrypi.com>\n> > > > ---\n> > > >  .../pipeline/rpi/common/pipeline_base.cpp          | 14 --------------\n> > > >  src/libcamera/pipeline/rpi/vc4/vc4.cpp             | 12 ++++++++++--\n> > > >  2 files changed, 10 insertions(+), 16 deletions(-)\n> > > >\n> > > > diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > > > index 825eae80d014..7437d38dc9ba 100644\n> > > > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > > > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > > > @@ -242,20 +242,6 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n> > > >               if (ret)\n> > > >                       return Invalid;\n> > > >\n> > > > -             /*\n> > > > -              * Some sensors change their Bayer order when they are h-flipped\n> > > > -              * or v-flipped, according to the transform. If this one does, we\n> > > > -              * must advertise the transformed Bayer order in the raw stream.\n> > > > -              * Note how we must fetch the \"native\" (i.e. untransformed) Bayer\n> > > > -              * order, because the sensor may currently be flipped!\n> > > > -              */\n> > > > -             BayerFormat bayer = BayerFormat::fromPixelFormat(raw.cfg->pixelFormat);\n> > > > -             if (data_->flipsAlterBayerOrder_) {\n> > > > -                     bayer.order = data_->nativeBayerOrder_;\n> > > > -                     bayer = bayer.transform(combinedTransform_);\n> > > > -             }\n> > > > -             raw.cfg->pixelFormat = bayer.toPixelFormat();\n> > > > -\n> > > >               if (RPi::PipelineHandlerBase::updateStreamConfig(raw.cfg, raw.format))\n> > > >                       status = Adjusted;\n> > > >       }\n> > > > diff --git a/src/libcamera/pipeline/rpi/vc4/vc4.cpp b/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> > > > index 233473e2fe2b..4b42ddc71115 100644\n> > > > --- a/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> > > > +++ b/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> > > > @@ -411,9 +411,17 @@ CameraConfiguration::Status Vc4CameraData::platformValidate(RPi::RPiCameraConfig\n> > > >               StreamConfiguration *rawStream = rawStreams[0].cfg;\n> > > >               BayerFormat rawBayer = BayerFormat::fromMbusCode(rpiConfig->sensorFormat_.mbus_code);\n> > > >\n> > > > -             /* Handle flips to make sure to match the RAW stream format. */\n> > > > -             if (flipsAlterBayerOrder_)\n> > > > +             /*\n> > > > +              * Some sensors change their Bayer order when they are h-flipped\n> > > > +              * or v-flipped, according to the transform. If this one does, we\n> > > > +              * must advertise the transformed Bayer order in the raw stream.\n> > > > +              * Note how we must fetch the \"native\" (i.e. untransformed) Bayer\n> > > > +              * order, because the sensor may currently be flipped!\n> > > > +              */\n> > > > +             if (flipsAlterBayerOrder_) {\n> > > > +                     rawBayer.order = nativeBayerOrder_;\n> > > >                       rawBayer = rawBayer.transform(rpiConfig->combinedTransform_);\n> > > > +             }\n> > > >\n> > > >               /* Apply the user requested packing. */\n> > > >               rawBayer.packing = BayerFormat::fromPixelFormat(rawStream->pixelFormat).packing;\n> > > > --\n> > > > 2.34.1\n> > > >","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 93ECEBD808\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 12 Oct 2023 14:40:31 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6E2886297B;\n\tThu, 12 Oct 2023 16:40:30 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 37B136296D\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 12 Oct 2023 16:40:29 +0200 (CEST)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id BDAB77FC;\n\tThu, 12 Oct 2023 16:40:25 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1697121630;\n\tbh=tQ+ta+ECA9LLvXXnUgHQ3zB7HkrItyt00Xo46If/0t0=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=BBbHQ0DALI+HGeoQTBqHUf0H6mtlFCnDD+6+cB7uvGPx40TjjKtJXvtXUd9LIzzX0\n\tXce2aKurFXWuiFEpwBrDVtygfarwDLUplpr99j6CJG9dgXHtwtUs3LUt12xN7f5Y9F\n\tFwPWkZNrswG27xYT0lqobFzxfvaZhMFiN/sEpEhL+0CioBVQIPIlG8z9HFN3yaIRmA\n\t4sXmLG2DKqxYlxuob4NnpuQtTKI4PY6C9IwcFiElyX1Q0gLYk854espQgQ08jxy5/Y\n\tc2qxYoRKfFn/48HDohf12UlasgcUNymxsqRS4dghbWADuDYAs+fMeK1B7gFOkHGpwk\n\tY70GyXeIxrXvQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1697121625;\n\tbh=tQ+ta+ECA9LLvXXnUgHQ3zB7HkrItyt00Xo46If/0t0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=HJVoR74Wc7DFkZrg7IW07m7ps4vYKNSN3gk4ou6yVKFozZSzdE727sptT6q+NtCIh\n\tACw3A6cxf5K4eI7LdOwGpu8guvoiq/iwr87bf/EowQ4/pCPTRGcRrD+rryza/BItMr\n\tpEyRoZG+Ri4a9EZvAtqd01CuRb8+L4nd5LO23pAs="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"HJVoR74W\"; dkim-atps=neutral","Date":"Thu, 12 Oct 2023 16:40:26 +0200","To":"Naushir Patuck <naush@raspberrypi.com>","Message-ID":"<uquaojk3f6vteqmb5fwz75itkyuovuyt6y6yvvrf3dyow4rmcc@gzksny545wzh>","References":"<20231006132000.23504-1-naush@raspberrypi.com>\n\t<20231006132000.23504-15-naush@raspberrypi.com>\n\t<2gqk6xpg3g4e3p6fn5okvmlvjplfqr7rzn7vq3jbxzog5vfgsp@jx2ngvqb7cin>\n\t<CAEmqJPr=MrAWXUzPBgMw1a8YyY0Ptv7WW-=eP33nV6uGAi76dg@mail.gmail.com>\n\t<CAEmqJPquFBdBoe9N9_W00oUMw1SjjGVQ9cO-gpp8MwCdOAFR9Q@mail.gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<CAEmqJPquFBdBoe9N9_W00oUMw1SjjGVQ9cO-gpp8MwCdOAFR9Q@mail.gmail.com>","Subject":"Re: [libcamera-devel] [PATCH 14/20] pipeline: rpi: Move flip\n\thandling validation code","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Jacopo Mondi via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Cc":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":27969,"web_url":"https://patchwork.libcamera.org/comment/27969/","msgid":"<CAEmqJPr37gOfMfnp8-1GcOPN3yLpCR-MDBOepVqZX7w0pJV=-A@mail.gmail.com>","date":"2023-10-12T14:48:31","subject":"Re: [libcamera-devel] [PATCH 14/20] pipeline: rpi: Move flip\n\thandling validation code","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"On Thu, 12 Oct 2023 at 15:40, Jacopo Mondi\n<jacopo.mondi@ideasonboard.com> wrote:\n>\n> Hi Naush\n>\n> On Thu, Oct 12, 2023 at 01:55:00PM +0100, Naushir Patuck wrote:\n> > On Thu, 12 Oct 2023 at 13:07, Naushir Patuck <naush@raspberrypi.com> wrote:\n> > >\n> > > On Thu, 12 Oct 2023 at 11:05, Jacopo Mondi\n> > > <jacopo.mondi@ideasonboard.com> wrote:\n> > > >\n> > > > Hi Naush\n> > > >\n> > > > On Fri, Oct 06, 2023 at 02:19:54PM +0100, Naushir Patuck via libcamera-devel wrote:\n> > > > > Move the handling of Bayer order changes due to flips entirely into\n> > > > > platformValidate(). This removes the need for this code to be split\n> > > > > between platformValidate() and validate() as it is right now.\n> > > >\n> > > > Doesn't this mean the same code has to be repeated in both platform's\n> > > > pipelines ?\n> > >\n> > > Sadly yes :(\n> > >\n> > > I haven't figured out a clean/tidy way to do this in pipeline_base and\n> > > remove the duplication.  Things get a bit more complicated when user\n> > > packing requests interfere with the raw output as well, and packing\n> > > constraints are platform specific (e.g. no CSI2 packing on Pi5).  I'll\n> > > continue working on getting a cleaner solution, but depending on the\n> > > timeframe, it might have to be put on top of this work.\n> > >\n> >\n> > And now after spending a bit more time looking at it, I think the\n> > solution is actually simpler than I thought!  I have a candidate fix\n> > that can be used to replace this patch.  Should I send it in a v2\n> > together with all the other minor fixes you noted?\n> >\n>\n> If you want, I certainly won't oppose. My proposal to fix minors when\n> applying still holds, but if you consider worth sending a v2 I'm all\n> for it!\n\nI've got a v2 ready to go, so it's no problem.  I'll wait for further\ncomments from other folks and post this tomorrow (Friday) morning if\nthat's ok?\n\nRegards,\nNaush\n\n\n>\n> Thanks\n>   j\n>\n> > > Regards,\n> > > Naush\n> > >\n> > > >\n> > > > >\n> > > > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n> > > > > Reviewed-by: David Plowman <david.plowman@raspberrypi.com>\n> > > > > ---\n> > > > >  .../pipeline/rpi/common/pipeline_base.cpp          | 14 --------------\n> > > > >  src/libcamera/pipeline/rpi/vc4/vc4.cpp             | 12 ++++++++++--\n> > > > >  2 files changed, 10 insertions(+), 16 deletions(-)\n> > > > >\n> > > > > diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > > > > index 825eae80d014..7437d38dc9ba 100644\n> > > > > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > > > > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > > > > @@ -242,20 +242,6 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n> > > > >               if (ret)\n> > > > >                       return Invalid;\n> > > > >\n> > > > > -             /*\n> > > > > -              * Some sensors change their Bayer order when they are h-flipped\n> > > > > -              * or v-flipped, according to the transform. If this one does, we\n> > > > > -              * must advertise the transformed Bayer order in the raw stream.\n> > > > > -              * Note how we must fetch the \"native\" (i.e. untransformed) Bayer\n> > > > > -              * order, because the sensor may currently be flipped!\n> > > > > -              */\n> > > > > -             BayerFormat bayer = BayerFormat::fromPixelFormat(raw.cfg->pixelFormat);\n> > > > > -             if (data_->flipsAlterBayerOrder_) {\n> > > > > -                     bayer.order = data_->nativeBayerOrder_;\n> > > > > -                     bayer = bayer.transform(combinedTransform_);\n> > > > > -             }\n> > > > > -             raw.cfg->pixelFormat = bayer.toPixelFormat();\n> > > > > -\n> > > > >               if (RPi::PipelineHandlerBase::updateStreamConfig(raw.cfg, raw.format))\n> > > > >                       status = Adjusted;\n> > > > >       }\n> > > > > diff --git a/src/libcamera/pipeline/rpi/vc4/vc4.cpp b/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> > > > > index 233473e2fe2b..4b42ddc71115 100644\n> > > > > --- a/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> > > > > +++ b/src/libcamera/pipeline/rpi/vc4/vc4.cpp\n> > > > > @@ -411,9 +411,17 @@ CameraConfiguration::Status Vc4CameraData::platformValidate(RPi::RPiCameraConfig\n> > > > >               StreamConfiguration *rawStream = rawStreams[0].cfg;\n> > > > >               BayerFormat rawBayer = BayerFormat::fromMbusCode(rpiConfig->sensorFormat_.mbus_code);\n> > > > >\n> > > > > -             /* Handle flips to make sure to match the RAW stream format. */\n> > > > > -             if (flipsAlterBayerOrder_)\n> > > > > +             /*\n> > > > > +              * Some sensors change their Bayer order when they are h-flipped\n> > > > > +              * or v-flipped, according to the transform. If this one does, we\n> > > > > +              * must advertise the transformed Bayer order in the raw stream.\n> > > > > +              * Note how we must fetch the \"native\" (i.e. untransformed) Bayer\n> > > > > +              * order, because the sensor may currently be flipped!\n> > > > > +              */\n> > > > > +             if (flipsAlterBayerOrder_) {\n> > > > > +                     rawBayer.order = nativeBayerOrder_;\n> > > > >                       rawBayer = rawBayer.transform(rpiConfig->combinedTransform_);\n> > > > > +             }\n> > > > >\n> > > > >               /* Apply the user requested packing. */\n> > > > >               rawBayer.packing = BayerFormat::fromPixelFormat(rawStream->pixelFormat).packing;\n> > > > > --\n> > > > > 2.34.1\n> > > > >","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 01A58C3272\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 12 Oct 2023 14:49:00 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 720B16297B;\n\tThu, 12 Oct 2023 16:49:00 +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 ABED46296D\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 12 Oct 2023 16:48:58 +0200 (CEST)","by mail-yw1-x1130.google.com with SMTP id\n\t00721157ae682-5a7ad24b3aaso13226347b3.2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 12 Oct 2023 07:48:58 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1697122140;\n\tbh=hvXk/ZHipxQuXONXkPctVavS0zc6Lsdqp+9AMwzWd4E=;\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=t0MzbiWVDX4zjgm+9LWyV2S6kaf5PscqnvSR+UxKtcQgCy/mdhbRspr5JRnaaQEd8\n\t2qsjdh81ASMIskRjj5VauEVVrKRl2Wv4ZS+WlefCZDNY0F6h5JQ0GByJLu4FNbmxry\n\tU11+JhuDKbMOUay8zO9mtyRrDZTZ97A2MdE2EbxVZZJD2aBPeksCwDBPwjVTtWv/iZ\n\tu5P5IfOCp/n+3eJNsuPhjEkMknNdTec9ye2APwTk2zIkTMjiaDJL31ee/Gxxih4vWv\n\t3HsaxoDfvGH+LOshdbMIjmB1h1i2AVD7nvUpx4Drqw3XBD1tzixkpJ28N2eT4K1UDl\n\taj8YHVtyU6JBA==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1697122137; x=1697726937;\n\tdarn=lists.libcamera.org; \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=AXnx9XpQl9KdCjhXQf/zHThnRsGE8V55ho2s8AlApzU=;\n\tb=avnpzVRGzl2eAIoTK00bw/oTvL+tQTDTRb+X0LdWzlBbcP03DJ+371wDq1+P3YONxv\n\tWTYlFOIRVxBHUHUFL693F4mXmxXHp9tVuQz4+VTySazeGa6bwB4nwFkL4tSwJAp3b/GW\n\tWmFKOscVKYviW9+7Ss3qBoD7TJDDylSOdfMQF/vj9Non1ZLzHnoxWPHHS6lhOFqBS3f1\n\tBz/AK1UsNVnTqGxbOXEL4kNhKxBcaaVfelE/EZd7tu0KBTI9sCmJ8fKoV2WdgwPg15oK\n\t2G1fHflgwRgThoQ3qilqsGWIP9EnotLW0RSoHd7bwA7SoqH0Xojhebohx+gMZ/KvbQpK\n\tvRVw=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"avnpzVRG\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1697122137; x=1697726937;\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=AXnx9XpQl9KdCjhXQf/zHThnRsGE8V55ho2s8AlApzU=;\n\tb=WJPHC0vExBnJQCty4eXmu/s3iG4cuq1oD0amKkPXRG3ECW7DDVa0MdHkElDl0INcbI\n\tOvibqM/jNB7GdKCBiznaw3rsKFI2VOWsKYCm6Lj2/6uxT3Xv7Hbefy0kj4reIWRZCbPO\n\tU6EcZkaB969pIT8nNJf6N5I3f9Gx+2frR60pB97g+L2zygnIMd+ko6xyedCrvNcUVA11\n\t6aA0zthic1gq7Vjh+RFC+paHn5TlR1pnYpxJ5ANNQU/2qKE2QA7uJIZPPqRXb0BhMJxJ\n\tJTCz4BcyW4NYiJOV+GWiH/lk8GZH1jpir8ujHZHHNyR3ugB1CprECohYhaXZbnCtULsM\n\toFrA==","X-Gm-Message-State":"AOJu0Yw1CfCc43alzYtsBumFyhErULf8EV+bAM4+wk8wCWwvyVhp3Nuq\n\t7rezVXogz2tSPMZOXIl5dbcZ++jJ2Tdbm/WsRlkQNLKyRRY4aWOK5gs=","X-Google-Smtp-Source":"AGHT+IGFpzM21mSN2/eCWVz1v0qybVQQkzD36kG9xdmTv4YxtCLj/10lIud19Y0HDjrINWkKZr47xd7V7mBVZV6weOc=","X-Received":"by 2002:a5b:343:0:b0:d42:d029:ff99 with SMTP id\n\tq3-20020a5b0343000000b00d42d029ff99mr21112910ybp.55.1697122137393;\n\tThu, 12 Oct 2023 07:48:57 -0700 (PDT)","MIME-Version":"1.0","References":"<20231006132000.23504-1-naush@raspberrypi.com>\n\t<20231006132000.23504-15-naush@raspberrypi.com>\n\t<2gqk6xpg3g4e3p6fn5okvmlvjplfqr7rzn7vq3jbxzog5vfgsp@jx2ngvqb7cin>\n\t<CAEmqJPr=MrAWXUzPBgMw1a8YyY0Ptv7WW-=eP33nV6uGAi76dg@mail.gmail.com>\n\t<CAEmqJPquFBdBoe9N9_W00oUMw1SjjGVQ9cO-gpp8MwCdOAFR9Q@mail.gmail.com>\n\t<uquaojk3f6vteqmb5fwz75itkyuovuyt6y6yvvrf3dyow4rmcc@gzksny545wzh>","In-Reply-To":"<uquaojk3f6vteqmb5fwz75itkyuovuyt6y6yvvrf3dyow4rmcc@gzksny545wzh>","Date":"Thu, 12 Oct 2023 15:48:31 +0100","Message-ID":"<CAEmqJPr37gOfMfnp8-1GcOPN3yLpCR-MDBOepVqZX7w0pJV=-A@mail.gmail.com>","To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH 14/20] pipeline: rpi: Move flip\n\thandling validation code","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Naushir Patuck via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Naushir Patuck <naush@raspberrypi.com>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]