[{"id":29342,"web_url":"https://patchwork.libcamera.org/comment/29342/","msgid":"","date":"2024-04-25T11:37:48","subject":"Re: [PATCH] pipeline: rpi: Avoid duplicating size range for the same\n\tpixel format","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 this patch.\n\nOn Thu, 25 Apr 2024 at 11:52, David Plowman\n wrote:\n>\n> Some V4L2 formats translate to the same pixel format, e.g. YU12 and\n> YM12 both produce YUV420. In this case our ISP driver advertises the\n> same size range for both, but we must not record the same thing twice\n> for the same pixel format (which will cause a failure later on).\n>\n> Instead, ignore the V4l2 format if the pixel format has already been\n> seen.\n>\n> Signed-off-by: David Plowman \n\nReviewed-by: Naushir Patuck \n\n> ---\n> src/libcamera/pipeline/rpi/common/pipeline_base.cpp | 6 +++++-\n> 1 file changed, 5 insertions(+), 1 deletion(-)\n>\n> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> index 7e420b3f..0034e06d 100644\n> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> @@ -474,7 +474,11 @@ PipelineHandlerBase::generateConfiguration(Camera *camera, Span */\n> for (const auto &format : fmts) {\n> PixelFormat pf = format.first.toPixelFormat();\n> - if (pf.isValid()) {\n> + /*\n> + * Some V4L2 formats translate to the same pixel format (e.g. YU12, YM12\n> + * both give YUV420). We must avoid duplicating the range in this case.\n> + */\n> + if (pf.isValid() && deviceFormats.find(pf) == deviceFormats.end()) {\n> const SizeRange &ispSizes = format.second[0];\n> deviceFormats[pf].emplace_back(ispSizes.min, sensorSize,\n> ispSizes.hStep, ispSizes.vStep);\n> --\n> 2.39.2\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 2132BBE08B\n\tfor ;\n\tThu, 25 Apr 2024 11:38:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id F065E61B43;\n\tThu, 25 Apr 2024 13:38:25 +0200 (CEST)","from mail-yw1-x1133.google.com (mail-yw1-x1133.google.com\n\t[IPv6:2607:f8b0:4864:20::1133])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3DE3361A9B\n\tfor ;\n\tThu, 25 Apr 2024 13:38:24 +0200 (CEST)","by mail-yw1-x1133.google.com with SMTP id\n\t00721157ae682-61b3be24dd9so7681297b3.0\n\tfor ;\n\tThu, 25 Apr 2024 04:38:24 -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=\"KnqA/BRG\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1714045103; x=1714649903;\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=cdXZ0Q60l5WTqQz2zmpvUM/YqwK0PM9BomJQubmLupM=;\n\tb=KnqA/BRG3g+eX/d59zeuxnJWSOLeNrEu6FzM0AialGeg3OQPNlXu4M+eJVm0XMAwWh\n\tnlT9gahUAyr0OpzUUHerhwqKyeEvY3CMWi0Z6NlatVNxPgSY2T1S8pPma6PB8nuNbusm\n\tvC0u4KYJOSlgcNf9tKE57OvPBJ6MpuaijJHznG4op02qZO7WYcti+XLYn1m3UcMgPvqS\n\tAhrU5qnaVUw3YkENeZDK0r5kkNFBP0W6YdT8sRhdEFOYh28tcjwUIyxUmVwAGEZjnWAe\n\tGa3wa6r8Fw5fMrkVf4g+pRuQxRSknrwkY5h9jitLBrAgxYy8Js8hsDi/Lw6CeFFAcZM3\n\tkMDg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1714045103; x=1714649903;\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=cdXZ0Q60l5WTqQz2zmpvUM/YqwK0PM9BomJQubmLupM=;\n\tb=Vp9GTlXIpJC2PMvrLtv33Dbz/8bGvdv9pLgYydf+9oaGXsEHfIZxuXabrr/vDyN22Q\n\tgAIrMq1OGFLy+asGIdEckKt/pFTTJUS5vyzUjxl7gFNp9L9wh6rw0AXJPrmrW0SJlNm0\n\tOJigMK8N2c7IrgVptGwo2b6uLLtuWMVZdhVfO+xZvkk+YebjPuiSpsJCgpCncLY/KzUt\n\tLdYMLUcN5gJDOQvX/MTeMzfhDIh5cvG1CFHOflkfpRLVN5r7NSi6qxyuWc166OUWZQGb\n\thT/NWdN1/4Rrs67AQvHIB9W4c/+81kwXwc2gkf6hUjtwSpF4PhIWFM7ZXdi6/ltYUJvk\n\taPag==","X-Gm-Message-State":"AOJu0YwFp6fJzDJ7IS7kRiGa+DBXBOVQrOVEfQTXq/U7Q3HcOJAHN/jf\n\tAHf2bZc1SUJa1lWB5CYhuy2gWzSiUbFBQsLHt20Lcj0VHEj2bHSfWdRpKdlwpxIUfOxm1BLiKl1\n\t1LzInZdwBUMKEPJOUVPIp2Qg6dRCavPny6gSppA==","X-Google-Smtp-Source":"AGHT+IH71TaO6aju938oqeP4m/RsoeJ9M6ph1xxLMQlUU+YwhuKBgTrohJJ1OtcJdyHGwnw3k/XVjhh3M8u+MTbxztY=","X-Received":"by 2002:a0d:d5c2:0:b0:61a:e7a7:b3df with SMTP id\n\tx185-20020a0dd5c2000000b0061ae7a7b3dfmr1861543ywd.15.1714045103090;\n\tThu, 25 Apr 2024 04:38:23 -0700 (PDT)","MIME-Version":"1.0","References":"<20240425105200.41167-1-david.plowman@raspberrypi.com>","In-Reply-To":"<20240425105200.41167-1-david.plowman@raspberrypi.com>","From":"Naushir Patuck ","Date":"Thu, 25 Apr 2024 12:37:48 +0100","Message-ID":"","Subject":"Re: [PATCH] pipeline: rpi: Avoid duplicating size range for the same\n\tpixel format","To":"David Plowman ","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"UTF-8\"","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":29349,"web_url":"https://patchwork.libcamera.org/comment/29349/","msgid":"<7udwbb6zg6dixh7jwsm3l5uw2rmwh4vhsmobso7vgjawg55zdz@t6y5f566clr5>","date":"2024-04-26T13:06:04","subject":"Re: [PATCH] pipeline: rpi: Avoid duplicating size range for the same\n\tpixel format","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi David\n\nOn Thu, Apr 25, 2024 at 12:37:48PM +0100, Naushir Patuck wrote:\n> Hi David,\n>\n> Thank you for this patch.\n>\n> On Thu, 25 Apr 2024 at 11:52, David Plowman\n> wrote:\n> >\n> > Some V4L2 formats translate to the same pixel format, e.g. YU12 and\n> > YM12 both produce YUV420. In this case our ISP driver advertises the\n> > same size range for both, but we must not record the same thing twice\n> > for the same pixel format (which will cause a failure later on).\n\nAm I wrong or this mostly (only) apply to contiguous and non-contiguous\nformat versions ?\n\n> >\n> > Instead, ignore the V4l2 format if the pixel format has already been\n> > seen.\n> >\n> > Signed-off-by: David Plowman \n>\n> Reviewed-by: Naushir Patuck \n\n>\n> > ---\n> > src/libcamera/pipeline/rpi/common/pipeline_base.cpp | 6 +++++-\n> > 1 file changed, 5 insertions(+), 1 deletion(-)\n> >\n> > diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > index 7e420b3f..0034e06d 100644\n> > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > @@ -474,7 +474,11 @@ PipelineHandlerBase::generateConfiguration(Camera *camera, Span > */\n> > for (const auto &format : fmts) {\n> > PixelFormat pf = format.first.toPixelFormat();\n> > - if (pf.isValid()) {\n> > + /*\n> > + * Some V4L2 formats translate to the same pixel format (e.g. YU12, YM12\n> > + * both give YUV420). We must avoid duplicating the range in this case.\n> > + */\n> > + if (pf.isValid() && deviceFormats.find(pf) == deviceFormats.end()) {\n\nOr\n if (pf.isValid() && !deviceFormats.count(pf)) {\n\n\nThe change is anyway good!\nReviewed-by: Jacopo Mondi \n\n> > const SizeRange &ispSizes = format.second[0];\n> > deviceFormats[pf].emplace_back(ispSizes.min, sensorSize,\n> > ispSizes.hStep, ispSizes.vStep);\n> > --\n> > 2.39.2\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 DBAACBE08B\n\tfor ;\n\tFri, 26 Apr 2024 13:06:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C2EA363416;\n\tFri, 26 Apr 2024 15:06:09 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C0D6F61A9B\n\tfor ;\n\tFri, 26 Apr 2024 15:06:07 +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 601E0CC8;\n\tFri, 26 Apr 2024 15:05:14 +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=\"VzUjD80w\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1714136714;\n\tbh=inJ/EwWu3rZuXbGzO8N0DQ6JvFKbyOoedqguQqZPPXs=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=VzUjD80wRk/XJIsSgxyBCld/KUEdidhKEiAa7Uhoe812WMuqbJzl+kpCrPu5ruB/A\n\t1bjKuNOSgYQyiRK1+QBwNKtB/XKmjxjcBBwFIS+Kutbj4lVJjS28wBt/56XI04Hnk2\n\t04pL5/Ybbk1Ftw2z9tyMab/24XJ+UuTCDZkd7FjI=","Date":"Fri, 26 Apr 2024 15:06:04 +0200","From":"Jacopo Mondi ","To":"Naushir Patuck ","Cc":"David Plowman , \n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH] pipeline: rpi: Avoid duplicating size range for the same\n\tpixel format","Message-ID":"<7udwbb6zg6dixh7jwsm3l5uw2rmwh4vhsmobso7vgjawg55zdz@t6y5f566clr5>","References":"<20240425105200.41167-1-david.plowman@raspberrypi.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","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":29351,"web_url":"https://patchwork.libcamera.org/comment/29351/","msgid":"","date":"2024-04-26T13:16:03","subject":"Re: [PATCH] pipeline: rpi: Avoid duplicating size range for the same\n\tpixel format","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Jacopo\n\nThanks for reviewing the patch!\n\nOn Fri, 26 Apr 2024 at 14:06, Jacopo Mondi\n wrote:\n>\n> Hi David\n>\n> On Thu, Apr 25, 2024 at 12:37:48PM +0100, Naushir Patuck wrote:\n> > Hi David,\n> >\n> > Thank you for this patch.\n> >\n> > On Thu, 25 Apr 2024 at 11:52, David Plowman\n> > wrote:\n> > >\n> > > Some V4L2 formats translate to the same pixel format, e.g. YU12 and\n> > > YM12 both produce YUV420. In this case our ISP driver advertises the\n> > > same size range for both, but we must not record the same thing twice\n> > > for the same pixel format (which will cause a failure later on).\n>\n> Am I wrong or this mostly (only) apply to contiguous and non-contiguous\n> format versions ?\n\nI think that's right. I quoted the YU12/YM12 example, but we have the\nsame thing with NV12 contiguous/non-contiguous and probably a few\nothers as well.\n\n>\n> > >\n> > > Instead, ignore the V4l2 format if the pixel format has already been\n> > > seen.\n> > >\n> > > Signed-off-by: David Plowman \n> >\n> > Reviewed-by: Naushir Patuck \n>\n> >\n> > > ---\n> > > src/libcamera/pipeline/rpi/common/pipeline_base.cpp | 6 +++++-\n> > > 1 file changed, 5 insertions(+), 1 deletion(-)\n> > >\n> > > diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > > index 7e420b3f..0034e06d 100644\n> > > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > > @@ -474,7 +474,11 @@ PipelineHandlerBase::generateConfiguration(Camera *camera, Span > > */\n> > > for (const auto &format : fmts) {\n> > > PixelFormat pf = format.first.toPixelFormat();\n> > > - if (pf.isValid()) {\n> > > + /*\n> > > + * Some V4L2 formats translate to the same pixel format (e.g. YU12, YM12\n> > > + * both give YUV420). We must avoid duplicating the range in this case.\n> > > + */\n> > > + if (pf.isValid() && deviceFormats.find(pf) == deviceFormats.end()) {\n>\n> Or\n> if (pf.isValid() && !deviceFormats.count(pf)) {\n\nTrue, I mostly don't think to use count because it's not such a good\nway of testing \"are there any?\". But for \"are there none?\" of course\nit's fine!\n\nThanks!\nDavid\n\n>\n>\n> The change is anyway good!\n> Reviewed-by: Jacopo Mondi \n>\n> > > const SizeRange &ispSizes = format.second[0];\n> > > deviceFormats[pf].emplace_back(ispSizes.min, sensorSize,\n> > > ispSizes.hStep, ispSizes.vStep);\n> > > --\n> > > 2.39.2\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 4E6EFC3220\n\tfor ;\n\tFri, 26 Apr 2024 13:16:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4326563418;\n\tFri, 26 Apr 2024 15:16:17 +0200 (CEST)","from mail-oi1-x22f.google.com (mail-oi1-x22f.google.com\n\t[IPv6:2607:f8b0:4864:20::22f])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6E566633ED\n\tfor ;\n\tFri, 26 Apr 2024 15:16:15 +0200 (CEST)","by mail-oi1-x22f.google.com with SMTP id\n\t5614622812f47-3c70e46f3caso1345112b6e.2\n\tfor ;\n\tFri, 26 Apr 2024 06:16:15 -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=\"eVRH3xQ4\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1714137374; x=1714742174;\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=qSSZGHf9A78X65Jt6fWjVzJbma9Ict/A/4NK2W5i+mE=;\n\tb=eVRH3xQ4/ZKsyBh4NMiEVTAsx3tdUK7utmIicSnL+uKqoYKImWtoOn8GNDnmX2iaj5\n\tfRdiKJ8zHfYH1IwgUq/d0jQPNc2q/1LzlxP/wEW+Qm+jwCMe12Vj93p7H7JFLDz6Px1+\n\tnDehSBENwrHRNAoKerZa3Vl4fy3nsIBWdepJpzmjkZuC29hgeqWAT4GTqTWhvCybjk0f\n\t217WzPoCqncehBXbB8Xoqcur7jYdIZnm9eB/0jhLC/2Q4lOu/+wTalmBM7u7f+VWbgGM\n\ta4QEnGPKhuHeNEQqEF2lgAD9CWo0SOw91Put/LO4AFmxi7fmsvH4o72l5K1kDdmzCHbl\n\timHg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1714137374; x=1714742174;\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=qSSZGHf9A78X65Jt6fWjVzJbma9Ict/A/4NK2W5i+mE=;\n\tb=OYqKAJoE5s9DGiXKJwnd2bZhInXs0d/WCBK+7/P6N5PUUp+ngPnbH5/vcV6LkmRFdy\n\t59xrxSaVfchq9MM+2F44iK4fFoZ0+QQfm1Eyb+GajEsJF0KyRyF3NnNGX4oZ6NOJuhYo\n\tJFw8RdTEaitfCRMmZyfwkDozr/W5aR422bepSQlUdvo+qlwt92KyvjRSiaz2oTzPY9Zj\n\t36xQnh95VWJx7Ce94uXYthg75oK/AIvnYl7Acya6ZLR3LZYfnjtAd9DTA11Xc5nohsmE\n\t1bt5O0eUDg97NdwHJoN7WJfERn+6sK27tLMkfMojxkM+YnQpIE/AK787qCP3sikruP58\n\tMhDw==","X-Forwarded-Encrypted":"i=1;\n\tAJvYcCWBDYU1GaRQSqm5UKh8HyH4DFtk8Y0jWrvOqlG9kSswExP/o0uxALNx4doEfCAkq3pQsQPgsEXg5aEFL51oyRn95WXg81Rb/rjJlseSq2SJj1Fg9g==","X-Gm-Message-State":"AOJu0YyuHZzSZtda21Zx9UXBWELN4kd37NS/cJEOGNA3wBy3hzSA1B7Z\n\tVNdWNJXSKhH7DmMuyMhCnjNM2idD7ebFnCb4nbe16/qLTHK7FDVgyQe9TQIlm2Av5juGGCFfr9c\n\tnnxsNcyqyurOWtfQ2qkDGxAJI5d0n+qLAED3XWA==","X-Google-Smtp-Source":"AGHT+IFZ1ujDGKLj7BwRQTvEtHSVBOo228rdQZEJk8dTeo/skXR3i74DEm7wc7TrMg/wB4g7reoKYLNlUxAXPou8uR8=","X-Received":"by 2002:a05:6808:4388:b0:3c8:4bb6:2fe1 with SMTP id\n\tdz8-20020a056808438800b003c84bb62fe1mr2632576oib.45.1714137373982;\n\tFri, 26 Apr 2024 06:16:13 -0700 (PDT)","MIME-Version":"1.0","References":"<20240425105200.41167-1-david.plowman@raspberrypi.com>\n\t\n\t<7udwbb6zg6dixh7jwsm3l5uw2rmwh4vhsmobso7vgjawg55zdz@t6y5f566clr5>","In-Reply-To":"<7udwbb6zg6dixh7jwsm3l5uw2rmwh4vhsmobso7vgjawg55zdz@t6y5f566clr5>","From":"David Plowman ","Date":"Fri, 26 Apr 2024 14:16:03 +0100","Message-ID":"","Subject":"Re: [PATCH] pipeline: rpi: Avoid duplicating size range for the same\n\tpixel format","To":"Jacopo Mondi ","Cc":"Naushir Patuck ,\n\tlibcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"UTF-8\"","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":29472,"web_url":"https://patchwork.libcamera.org/comment/29472/","msgid":"<20240509095227.GB955@pendragon.ideasonboard.com>","date":"2024-05-09T09:52:27","subject":"Re: [PATCH] pipeline: rpi: Avoid duplicating size range for the same\n\tpixel format","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hello,\n\nOn Fri, Apr 26, 2024 at 02:16:03PM +0100, David Plowman wrote:\n> On Fri, 26 Apr 2024 at 14:06, Jacopo Mondi wrote:\n> > On Thu, Apr 25, 2024 at 12:37:48PM +0100, Naushir Patuck wrote:\n> > > On Thu, 25 Apr 2024 at 11:52, David Plowman wrote:\n> > > >\n> > > > Some V4L2 formats translate to the same pixel format, e.g. YU12 and\n> > > > YM12 both produce YUV420. In this case our ISP driver advertises the\n> > > > same size range for both, but we must not record the same thing twice\n> > > > for the same pixel format (which will cause a failure later on).\n> >\n> > Am I wrong or this mostly (only) apply to contiguous and non-contiguous\n> > format versions ?\n> \n> I think that's right. I quoted the YU12/YM12 example, but we have the\n> same thing with NV12 contiguous/non-contiguous and probably a few\n> others as well.\n> \n> > > > Instead, ignore the V4l2 format if the pixel format has already been\n> > > > seen.\n> > > >\n> > > > Signed-off-by: David Plowman \n> > >\n> > > Reviewed-by: Naushir Patuck \n> >\n> > > > ---\n> > > > src/libcamera/pipeline/rpi/common/pipeline_base.cpp | 6 +++++-\n> > > > 1 file changed, 5 insertions(+), 1 deletion(-)\n> > > >\n> > > > diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > > > index 7e420b3f..0034e06d 100644\n> > > > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > > > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > > > @@ -474,7 +474,11 @@ PipelineHandlerBase::generateConfiguration(Camera *camera, Span > > > */\n> > > > for (const auto &format : fmts) {\n> > > > PixelFormat pf = format.first.toPixelFormat();\n> > > > - if (pf.isValid()) {\n> > > > + /*\n> > > > + * Some V4L2 formats translate to the same pixel format (e.g. YU12, YM12\n> > > > + * both give YUV420). We must avoid duplicating the range in this case.\n> > > > + */\n> > > > + if (pf.isValid() && deviceFormats.find(pf) == deviceFormats.end()) {\n> >\n> > Or\n> > if (pf.isValid() && !deviceFormats.count(pf)) {\n> \n> True, I mostly don't think to use count because it's not such a good\n> way of testing \"are there any?\". But for \"are there none?\" of course\n> it's fine!\n\nI'm fine either way. std::map::contains() will be nice, once we move to\nC++20.\n\nReviewed-by: Laurent Pinchart \n\n> > The change is anyway good!\n> > Reviewed-by: Jacopo Mondi \n> >\n> > > > const SizeRange &ispSizes = format.second[0];\n> > > > deviceFormats[pf].emplace_back(ispSizes.min, sensorSize,\n> > > > ispSizes.hStep, ispSizes.vStep);","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 C2793C3226\n\tfor ;\n\tThu, 9 May 2024 09:52:39 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EB44A63462;\n\tThu, 9 May 2024 11:52:38 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1288563461\n\tfor ;\n\tThu, 9 May 2024 11:52:37 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 52836524;\n\tThu, 9 May 2024 11:52:33 +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=\"rRLUjdDi\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1715248353;\n\tbh=SBXlJBGhS2laFx/IbRFyr7y1Ka1c1yzgr9tXSi69ERI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=rRLUjdDi6XniTLBYfJm7vM+qQt9h2WcSqcILClz9MJy6Z5AWQ57qmMZUj1Mx0Mr7J\n\tAKrwTXo43W+72bOZnMk4+scULfjcMNytNnqa9qoZRdJ8YerWfSMxpMk1BiRWzOWtb1\n\ttOjdvkz2rmur86/UdoUkl29bY53OEkek02Hg2BpY=","Date":"Thu, 9 May 2024 12:52:27 +0300","From":"Laurent Pinchart ","To":"David Plowman ","Cc":"Jacopo Mondi ,\n\tNaushir Patuck ,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH] pipeline: rpi: Avoid duplicating size range for the same\n\tpixel format","Message-ID":"<20240509095227.GB955@pendragon.ideasonboard.com>","References":"<20240425105200.41167-1-david.plowman@raspberrypi.com>\n\t\n\t<7udwbb6zg6dixh7jwsm3l5uw2rmwh4vhsmobso7vgjawg55zdz@t6y5f566clr5>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","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\" "}}]