[{"id":21575,"web_url":"https://patchwork.libcamera.org/comment/21575/","msgid":"","date":"2021-12-03T12:40:07","subject":"Re: [libcamera-devel] [PATCH v1] pipeline: raspberrypi: Return the\n\tsensor formats from generateConfiguration()","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 this patch!\n\nOn Fri, 3 Dec 2021 at 11:32, Naushir Patuck wrote:\n>\n> Return the available sensor PixelFormats and sizes from generateConfiguration()\n> if the StreamRole is set to StreamRole::Raw. The existing code returns the\n> PixelFormats and sizes for all other StreamRole types.\n>\n> Signed-off-by: Naushir Patuck \n> ---\n> .../pipeline/raspberrypi/raspberrypi.cpp | 21 ++++++++++++++-----\n> 1 file changed, 16 insertions(+), 5 deletions(-)\n>\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index 5b76916e9e98..cbfb58562626 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -595,12 +595,23 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,\n> return nullptr;\n> }\n>\n> - /* Translate the V4L2PixelFormat to PixelFormat. */\n> std::map> deviceFormats;\n> - for (const auto &format : fmts) {\n> - PixelFormat pf = format.first.toPixelFormat();\n> - if (pf.isValid())\n> - deviceFormats[pf] = format.second;\n> + if (role == StreamRole::Raw) {\n> + /* Translate the MBUS codes to a PixelFormat. */\n> + for (const auto &format : data->sensorFormats_) {\n> + PixelFormat pf = mbusCodeToPixelFormat(format.first,\n> + BayerFormat::Packing::CSI2);\n> + if (pf.isValid())\n> + deviceFormats.emplace(std::piecewise_construct, std::forward_as_tuple(pf),\n> + std::forward_as_tuple(format.second.begin(), format.second.end()));\n\nI won't even begin to pretend that I understand this! I guess I wonder\njust a little bit whether one could write it in a more \"obvious\" way\nthat the compiler could still optimise, but really I have no clue. So\nanyway:\n\nReviewed-by: David Plowman \n\nThanks!\nDavid\n\n> + }\n> + } else {\n> + /* Translate the V4L2PixelFormat to PixelFormat. */\n> + for (const auto &format : fmts) {\n> + PixelFormat pf = format.first.toPixelFormat();\n> + if (pf.isValid())\n> + deviceFormats[pf] = format.second;\n> + }\n> }\n>\n> /* Add the stream format based on the device node used for the use case. */\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 1898EBDB13\n\tfor ;\n\tFri, 3 Dec 2021 12:40:21 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 301D960832;\n\tFri, 3 Dec 2021 13:40:20 +0100 (CET)","from mail-wr1-x432.google.com (mail-wr1-x432.google.com\n\t[IPv6:2a00:1450:4864:20::432])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 747CF60725\n\tfor ;\n\tFri, 3 Dec 2021 13:40:18 +0100 (CET)","by mail-wr1-x432.google.com with SMTP id a18so5458968wrn.6\n\tfor ;\n\tFri, 03 Dec 2021 04:40:18 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"Oe2KjfI3\"; 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=EfvkTUu3ZZbD16iv/SbxeT/pMoS9ZUqf14r0sNNcD2E=;\n\tb=Oe2KjfI3qvDVRhBrOTmFHaREellOd/FyOFI+pnyCa9mL0FVilYeVNi4dNY3SOVXITh\n\tMgSXwM7Js1ebVJuzKS22DCZsAN5oROaVsNOpm4XZwKbjxDgNtdbPqMqlF0wUQRk/UIkS\n\tOFyMZZMiPId+B8AaJ0HW4a2iU1T9YiLgP5CDe35wv3/eT8yWNOy8mFjauRbL7hulBYQF\n\tpX2sa1ZCJPsrntWKnFug6iLtCX8jZI3mY6ajFCcuVvTwuQX98D+LR8rddVMpnxnS7U8T\n\tRKZthhEYHeceSFDIX0Kyvmssk+0vIKs67Hqq07mzHX4w3FZZ9EPSikO7GVL4StxQMxcJ\n\tjH+A==","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=EfvkTUu3ZZbD16iv/SbxeT/pMoS9ZUqf14r0sNNcD2E=;\n\tb=s+4qRrm8X3+Zr1gc5dz58yXsnhnDv8VHF9xnvpmXNob4Y7a3pH2wzM9LwItlDAkhSG\n\t8dA2lHMMrKX94FLAq56Y4Ba1S7mdbdwB+y1/s88O/GCLl9D+2qzN2dWH735ZpJ58bGE2\n\tA7pONqfADvPlQyRyY2NeZZn7ugLS2zlGBvgVNsbUv+tltYCqDZcYNjw8fgROmaQG09C7\n\tVjv3UMA6sffq+M+9YAsIirnKknK+Hnl2MsmbnQ9oasyGwQbLZbxRi/udWFShBcJSR9bb\n\tOj9Qli+FMNBoYH9bzGbPMFGZbSq1LpPK7jXe214S00HpkwJ729SWnieCwM+C3sZKsC3Y\n\tWS7A==","X-Gm-Message-State":"AOAM530cI09oh4+ebUndsJYEcu3f26P8vRPTEGT3kZMuB0Wl7CUtUSOR\n\tHInqW1Ex+klkse6Z40Or+U4fKJSAUFllOkUr3vH+wxwqqq8Iow==","X-Google-Smtp-Source":"ABdhPJy/MH7HpuZ2RA8ZTA0a56Dv3X+7BXz7g7/bqpIPAn68NNUGR5Ly/bKpd75nOySKCogf+TmwBw03D6KXy9yRz9Y=","X-Received":"by 2002:adf:f44e:: with SMTP id\n\tf14mr21896676wrp.37.1638535218053; \n\tFri, 03 Dec 2021 04:40:18 -0800 (PST)","MIME-Version":"1.0","References":"<20211203113205.2470651-1-naush@raspberrypi.com>","In-Reply-To":"<20211203113205.2470651-1-naush@raspberrypi.com>","From":"David Plowman ","Date":"Fri, 3 Dec 2021 12:40:07 +0000","Message-ID":"","To":"Naushir Patuck ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v1] pipeline: raspberrypi: Return the\n\tsensor formats from generateConfiguration()","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":21584,"web_url":"https://patchwork.libcamera.org/comment/21584/","msgid":"","date":"2021-12-03T17:45:53","subject":"Re: [libcamera-devel] [PATCH v1] pipeline: raspberrypi: Return the\n\tsensor formats from generateConfiguration()","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Actually I found myself looking at this bit of code again as part of\nmy occasional quest to figure out why the gstreamer element and v4l2\ncompatibility library work so poorly.\n\nI was wondering if we should limit the output sizes to the max sensor\nresolution here. The trouble with advertising 16384x16384 is that\napplications (cheese, for example) decide that we really want 8K video\nand unsurprisingly it's all a complete disaster.\n\nWhat do you think - is this the right place to tackle the problem? (Or\nif not, where else?)\n\nThough I've not checked, maybe this would help the v4l2 compatibility\nlayer too? (anyone know?)\n\nDavid\n\nOn Fri, 3 Dec 2021 at 12:40, David Plowman\n wrote:\n>\n> Hi Naush\n>\n> Thanks for this patch!\n>\n> On Fri, 3 Dec 2021 at 11:32, Naushir Patuck wrote:\n> >\n> > Return the available sensor PixelFormats and sizes from generateConfiguration()\n> > if the StreamRole is set to StreamRole::Raw. The existing code returns the\n> > PixelFormats and sizes for all other StreamRole types.\n> >\n> > Signed-off-by: Naushir Patuck \n> > ---\n> > .../pipeline/raspberrypi/raspberrypi.cpp | 21 ++++++++++++++-----\n> > 1 file changed, 16 insertions(+), 5 deletions(-)\n> >\n> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > index 5b76916e9e98..cbfb58562626 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > @@ -595,12 +595,23 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,\n> > return nullptr;\n> > }\n> >\n> > - /* Translate the V4L2PixelFormat to PixelFormat. */\n> > std::map> deviceFormats;\n> > - for (const auto &format : fmts) {\n> > - PixelFormat pf = format.first.toPixelFormat();\n> > - if (pf.isValid())\n> > - deviceFormats[pf] = format.second;\n> > + if (role == StreamRole::Raw) {\n> > + /* Translate the MBUS codes to a PixelFormat. */\n> > + for (const auto &format : data->sensorFormats_) {\n> > + PixelFormat pf = mbusCodeToPixelFormat(format.first,\n> > + BayerFormat::Packing::CSI2);\n> > + if (pf.isValid())\n> > + deviceFormats.emplace(std::piecewise_construct, std::forward_as_tuple(pf),\n> > + std::forward_as_tuple(format.second.begin(), format.second.end()));\n>\n> I won't even begin to pretend that I understand this! I guess I wonder\n> just a little bit whether one could write it in a more \"obvious\" way\n> that the compiler could still optimise, but really I have no clue. So\n> anyway:\n>\n> Reviewed-by: David Plowman \n>\n> Thanks!\n> David\n>\n> > + }\n> > + } else {\n> > + /* Translate the V4L2PixelFormat to PixelFormat. */\n> > + for (const auto &format : fmts) {\n> > + PixelFormat pf = format.first.toPixelFormat();\n> > + if (pf.isValid())\n> > + deviceFormats[pf] = format.second;\n> > + }\n> > }\n> >\n> > /* Add the stream format based on the device node used for the use case. */\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 23782BDB13\n\tfor ;\n\tFri, 3 Dec 2021 17:46:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6D07760832;\n\tFri, 3 Dec 2021 18:46:07 +0100 (CET)","from mail-wr1-x429.google.com (mail-wr1-x429.google.com\n\t[IPv6:2a00:1450:4864:20::429])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 45F1860725\n\tfor ;\n\tFri, 3 Dec 2021 18:46:05 +0100 (CET)","by mail-wr1-x429.google.com with SMTP id d9so7320433wrw.4\n\tfor ;\n\tFri, 03 Dec 2021 09:46:05 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"lQHvVVPu\"; 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=8mKJk7CSYzL+qwPzxn5fDrY4L9Oq+wMZsSCOY/D9MKA=;\n\tb=lQHvVVPu+DXE0w7kc1FceqD/kqC/QftAMV9EqxzUmesbqsVwLQIKIAAFVMt45BF3NV\n\tCcn23ahAllbxKAgxiRQIBLGM7FDIZ5oTb8XSrnz+iCTf6TyEFa1l7LEW6nNegdpozbuv\n\tV3h8QP/cfxEGmj6EVUoi6dgaYS775xJKineAoI2yU+XFnl3yriWvlvW+wFkGh/m4CCUj\n\tW3UEOjxkzb7PCWzbcv1lCUpx+H4WUrwm10viSH1QJXjpI6jU0JEWrYPCqqQCaIskpC0M\n\t72nOX9X8iOd/d8X6gWZrdSFb/WJq/e5+B0ScJAuBENeSj6tsqmuN18IvFSs67TcZSUSh\n\tpdyw==","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=8mKJk7CSYzL+qwPzxn5fDrY4L9Oq+wMZsSCOY/D9MKA=;\n\tb=BMlSYl66HoLaTHzbNzV6kOjHnYC5Y4CPcZ4YodqHobAq0HQgfByscpW9A3lRei1D1g\n\t9nOI9oi/orRqaY/6zrWCe4iI+eyYI5vocdx04eJSnByaN4tSeMoXB5wR4CqJWRmYNlGf\n\t33BhLKtXk5yPmOOodu2XzlH4VOrIndi7lb9AcN7dUoYBzl4iJIA28qkMvloM/ZqoFD6O\n\t2mjVvVwzk+QXz8WafZYgELt5T5kbwkAsp9yYJDbeLefklDjSJsGbEdPd/+5y8d0cYeQ2\n\tAMj5Xmryr7w77Zmu8/gMctrRLMHrv5i2iixiQ39yK5mI4iY2Br+StjobzHfV7Kp+V3sk\n\tMSYQ==","X-Gm-Message-State":"AOAM532dqDapSyk8edcH6/ZcIhD+PFjLOICrwKksCctlx1gXznjONaVi\n\t5UGwkmsiqfoQzgflPIM3Ylen7UO0EnxNEy2Omc3lAh/3IIQ=","X-Google-Smtp-Source":"ABdhPJzocHC/asARvuq3cfxaFteOgeJAOsFgggSErDk7juIeUkrSLSOEwmZdeWB5UiCZHLtedBY3v55NCVKltRqX2Cs=","X-Received":"by 2002:adf:e390:: with SMTP id\n\te16mr22969741wrm.494.1638553564818; \n\tFri, 03 Dec 2021 09:46:04 -0800 (PST)","MIME-Version":"1.0","References":"<20211203113205.2470651-1-naush@raspberrypi.com>\n\t","In-Reply-To":"","From":"David Plowman ","Date":"Fri, 3 Dec 2021 17:45:53 +0000","Message-ID":"","To":"Naushir Patuck ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v1] pipeline: raspberrypi: Return the\n\tsensor formats from generateConfiguration()","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":21586,"web_url":"https://patchwork.libcamera.org/comment/21586/","msgid":"<163855972787.3059017.3602628639535048817@Monstersaurus>","date":"2021-12-03T19:28:47","subject":"Re: [libcamera-devel] [PATCH v1] pipeline: raspberrypi: Return the\n\tsensor formats from generateConfiguration()","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting David Plowman (2021-12-03 17:45:53)\n> Actually I found myself looking at this bit of code again as part of\n> my occasional quest to figure out why the gstreamer element and v4l2\n> compatibility library work so poorly.\n> \n> I was wondering if we should limit the output sizes to the max sensor\n> resolution here. The trouble with advertising 16384x16384 is that\n> applications (cheese, for example) decide that we really want 8K video\n> and unsurprisingly it's all a complete disaster.\n> \n> What do you think - is this the right place to tackle the problem? (Or\n> if not, where else?)\n> \n> Though I've not checked, maybe this would help the v4l2 compatibility\n> layer too? (anyone know?)\n> \n> David\n> \n> On Fri, 3 Dec 2021 at 12:40, David Plowman\n> wrote:\n> >\n> > Hi Naush\n> >\n> > Thanks for this patch!\n> >\n> > On Fri, 3 Dec 2021 at 11:32, Naushir Patuck wrote:\n> > >\n> > > Return the available sensor PixelFormats and sizes from generateConfiguration()\n> > > if the StreamRole is set to StreamRole::Raw. The existing code returns the\n> > > PixelFormats and sizes for all other StreamRole types.\n> > >\n> > > Signed-off-by: Naushir Patuck \n> > > ---\n> > > .../pipeline/raspberrypi/raspberrypi.cpp | 21 ++++++++++++++-----\n> > > 1 file changed, 16 insertions(+), 5 deletions(-)\n> > >\n> > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > index 5b76916e9e98..cbfb58562626 100644\n> > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > @@ -595,12 +595,23 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,\n> > > return nullptr;\n> > > }\n> > >\n> > > - /* Translate the V4L2PixelFormat to PixelFormat. */\n> > > std::map> deviceFormats;\n> > > - for (const auto &format : fmts) {\n> > > - PixelFormat pf = format.first.toPixelFormat();\n> > > - if (pf.isValid())\n> > > - deviceFormats[pf] = format.second;\n> > > + if (role == StreamRole::Raw) {\n> > > + /* Translate the MBUS codes to a PixelFormat. */\n> > > + for (const auto &format : data->sensorFormats_) {\n> > > + PixelFormat pf = mbusCodeToPixelFormat(format.first,\n> > > + BayerFormat::Packing::CSI2);\n> > > + if (pf.isValid())\n> > > + deviceFormats.emplace(std::piecewise_construct, std::forward_as_tuple(pf),\n> > > + std::forward_as_tuple(format.second.begin(), format.second.end()));\n> >\n> > I won't even begin to pretend that I understand this! I guess I wonder\n> > just a little bit whether one could write it in a more \"obvious\" way\n> > that the compiler could still optimise, but really I have no clue. So\n> > anyway:\n> >\n> > Reviewed-by: David Plowman \n> >\n> > Thanks!\n> > David\n> >\n> > > + }\n> > > + } else {\n> > > + /* Translate the V4L2PixelFormat to PixelFormat. */\n> > > + for (const auto &format : fmts) {\n> > > + PixelFormat pf = format.first.toPixelFormat();\n> > > + if (pf.isValid())\n> > > + deviceFormats[pf] = format.second;\n> > > + }\n> > > }\n> > >\n> > > /* Add the stream format based on the device node used for the use case. */\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 1F103BF415\n\tfor ;\n\tFri, 3 Dec 2021 19:28:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7A52C60725;\n\tFri, 3 Dec 2021 20:28:52 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BA4CE60725\n\tfor ;\n\tFri, 3 Dec 2021 20:28:50 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 5EB46A59;\n\tFri, 3 Dec 2021 20:28:50 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"cctQUdE2\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1638559730;\n\tbh=eL4PHq8PGA/9Afzy7gwe+PM0PzZvvTBITl5knHbm5Ak=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=cctQUdE2tiL0obgWZpUicYf8HB68kQWEqv9dFUnqB+zs1p18KlKuExbsUMftGdYjX\n\t0GHfyzDf8qhgICN2Y92vbwjrb9alnvRKbUwHmyF4n4VBS8+tkR1yQZuGcwdJXthQvO\n\tbYx5+LdOTGvjQAv+gq4mr8Y82JfIrAattpPAqaEc=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20211203113205.2470651-1-naush@raspberrypi.com>\n\t\n\t","From":"Kieran Bingham ","To":"David Plowman ,\n\tNaushir Patuck ","Date":"Fri, 03 Dec 2021 19:28:47 +0000","Message-ID":"<163855972787.3059017.3602628639535048817@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v1] pipeline: raspberrypi: Return the\n\tsensor formats from generateConfiguration()","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":21601,"web_url":"https://patchwork.libcamera.org/comment/21601/","msgid":"","date":"2021-12-06T09:53:17","subject":"Re: [libcamera-devel] [PATCH v1] pipeline: raspberrypi: Return the\n\tsensor formats from generateConfiguration()","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi David,\n\nOn Fri, 3 Dec 2021 at 17:46, David Plowman \nwrote:\n\n> Actually I found myself looking at this bit of code again as part of\n> my occasional quest to figure out why the gstreamer element and v4l2\n> compatibility library work so poorly.\n>\n> I was wondering if we should limit the output sizes to the max sensor\n> resolution here. The trouble with advertising 16384x16384 is that\n> applications (cheese, for example) decide that we really want 8K video\n> and unsurprisingly it's all a complete disaster.\n>\n> What do you think - is this the right place to tackle the problem? (Or\n> if not, where else?)\n>\n> Though I've not checked, maybe this would help the v4l2 compatibility\n> layer too? (anyone know?)\n>\n\nYes, that does make sense. Once this goes in, I'll look to make that\nchange.\n\nRegards,\nNaush\n\n\n>\n> David\n>\n> On Fri, 3 Dec 2021 at 12:40, David Plowman\n> wrote:\n> >\n> > Hi Naush\n> >\n> > Thanks for this patch!\n> >\n> > On Fri, 3 Dec 2021 at 11:32, Naushir Patuck \n> wrote:\n> > >\n> > > Return the available sensor PixelFormats and sizes from\n> generateConfiguration()\n> > > if the StreamRole is set to StreamRole::Raw. The existing code returns\n> the\n> > > PixelFormats and sizes for all other StreamRole types.\n> > >\n> > > Signed-off-by: Naushir Patuck \n> > > ---\n> > > .../pipeline/raspberrypi/raspberrypi.cpp | 21 ++++++++++++++-----\n> > > 1 file changed, 16 insertions(+), 5 deletions(-)\n> > >\n> > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > index 5b76916e9e98..cbfb58562626 100644\n> > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > @@ -595,12 +595,23 @@ CameraConfiguration\n> *PipelineHandlerRPi::generateConfiguration(Camera *camera,\n> > > return nullptr;\n> > > }\n> > >\n> > > - /* Translate the V4L2PixelFormat to PixelFormat. */\n> > > std::map>\n> deviceFormats;\n> > > - for (const auto &format : fmts) {\n> > > - PixelFormat pf = format.first.toPixelFormat();\n> > > - if (pf.isValid())\n> > > - deviceFormats[pf] = format.second;\n> > > + if (role == StreamRole::Raw) {\n> > > + /* Translate the MBUS codes to a PixelFormat.\n> */\n> > > + for (const auto &format :\n> data->sensorFormats_) {\n> > > + PixelFormat pf =\n> mbusCodeToPixelFormat(format.first,\n> > > +\n> BayerFormat::Packing::CSI2);\n> > > + if (pf.isValid())\n> > > +\n> deviceFormats.emplace(std::piecewise_construct, std::forward_as_tuple(pf),\n> > > +\n> std::forward_as_tuple(format.second.begin(), format.second.end()));\n> >\n> > I won't even begin to pretend that I understand this! I guess I wonder\n> > just a little bit whether one could write it in a more \"obvious\" way\n> > that the compiler could still optimise, but really I have no clue. So\n> > anyway:\n> >\n> > Reviewed-by: David Plowman \n> >\n> > Thanks!\n> > David\n> >\n> > > + }\n> > > + } else {\n> > > + /* Translate the V4L2PixelFormat to\n> PixelFormat. */\n> > > + for (const auto &format : fmts) {\n> > > + PixelFormat pf =\n> format.first.toPixelFormat();\n> > > + if (pf.isValid())\n> > > + deviceFormats[pf] =\n> format.second;\n> > > + }\n> > > }\n> > >\n> > > /* Add the stream format based on the device node used\n> for the use case. */\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 1D2B3BF415\n\tfor ;\n\tMon, 6 Dec 2021 09:53:39 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5191D6086A;\n\tMon, 6 Dec 2021 10:53:38 +0100 (CET)","from mail-lj1-x230.google.com (mail-lj1-x230.google.com\n\t[IPv6:2a00:1450:4864:20::230])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 42C376011A\n\tfor ;\n\tMon, 6 Dec 2021 10:53:36 +0100 (CET)","by mail-lj1-x230.google.com with SMTP id v15so19933572ljc.0\n\tfor ;\n\tMon, 06 Dec 2021 01:53:36 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"HkT9yQ1J\"; 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=tD5jRO3tSlQNUwjm0vNewCcOdPEllJOze3MpjF038/U=;\n\tb=HkT9yQ1JwiOTjOOORx02907iimP+flrkk+79O19ic2JK/bnb8H/NhjjXXAMSGH+zFa\n\tqvtpVLBY0c5KuyI0WNkdvwzZ8oTpqV8K/JYS5mY1MgsAgXmM41WltYc+E7PWUranTk12\n\tgWm7YVkm8KgxdVf5KCiWbCaBh0lYBwrqCoi0j2XKsHNSuXKhg1vM2rCyCS+yTF3/PAkE\n\tF/38MrN7vGkKY+T0lF76qx4w6dQCX0cEPhtYuaK7HxhERyQlzwz6iLjkWgOxeSA7j5ZD\n\tX/wYTi+1Khl13iQo/Kw5dB7Yv851HxtRr+4et65Ba+8E99oxNyt71iYje2qkXT4UqI1X\n\tomiQ==","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=tD5jRO3tSlQNUwjm0vNewCcOdPEllJOze3MpjF038/U=;\n\tb=7LCJU9KJp46e/ktQ55gbAVhlw4n669rIMBGi6e6oXbqMV6hnriZUSEijTX/LHazmDV\n\t84KRHQJoa3nCxw1WsN5/HuJoKWtXJbXKhcw6dxlyo5GjA2TfrWxXZ5VeqB5xpVq5J6FW\n\t8H9H7nf82vFq8CyqQvdwJfV18SjIFPvFWqx62zQJyeTN5oBrJepfN2HxiydG2oZhACJ0\n\tbl+i1sGwMA4tcmh/qtHWVzcwK8x4+DZJuzAodRqtTc3LcKzMoUkkv1xeJgL2qkI4/Q8Y\n\tw48tWN9SVHxMIa6h3MPewUpohg7QrdSjQ1rZrFav9ja5gXuoxs42S5EIlYYijmvR3qE+\n\tgX0g==","X-Gm-Message-State":"AOAM533NNoTe8toC3hCnKB69rJ4eCxjJJ3RlQuwbRqpRlYj2MdkMhXic\n\t34epCTMQbEQzraYq2uwaC9l7PqXVXmBXoGzMV/Vvjw==","X-Google-Smtp-Source":"ABdhPJw3QvYNvXs5O9j59mn6uSi7gsOt+MDF4pEtB//lf3s3fSMlFC7a+pWuuIHMZBTh/T/Cce0VHFYfpPE2OhQtKXE=","X-Received":"by 2002:a2e:908b:: with SMTP id\n\tl11mr35596705ljg.208.1638784413853; \n\tMon, 06 Dec 2021 01:53:33 -0800 (PST)","MIME-Version":"1.0","References":"<20211203113205.2470651-1-naush@raspberrypi.com>\n\t\n\t","In-Reply-To":"","From":"Naushir Patuck ","Date":"Mon, 6 Dec 2021 09:53:17 +0000","Message-ID":"","To":"David Plowman ","Content-Type":"multipart/alternative; boundary=\"000000000000d9fbf305d2773a07\"","Subject":"Re: [libcamera-devel] [PATCH v1] pipeline: raspberrypi: Return the\n\tsensor formats from generateConfiguration()","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":21613,"web_url":"https://patchwork.libcamera.org/comment/21613/","msgid":"<163881119601.995700.3972360592151937221@Monstersaurus>","date":"2021-12-06T17:19:56","subject":"Re: [libcamera-devel] [PATCH v1] pipeline: raspberrypi: Return the\n\tsensor formats from generateConfiguration()","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Naushir Patuck (2021-12-03 11:32:05)\n> Return the available sensor PixelFormats and sizes from generateConfiguration()\n> if the StreamRole is set to StreamRole::Raw. The existing code returns the\n> PixelFormats and sizes for all other StreamRole types.\n> \n> Signed-off-by: Naushir Patuck \n> ---\n> .../pipeline/raspberrypi/raspberrypi.cpp | 21 ++++++++++++++-----\n> 1 file changed, 16 insertions(+), 5 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index 5b76916e9e98..cbfb58562626 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -595,12 +595,23 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,\n> return nullptr;\n> }\n> \n> - /* Translate the V4L2PixelFormat to PixelFormat. */\n> std::map> deviceFormats;\n> - for (const auto &format : fmts) {\n> - PixelFormat pf = format.first.toPixelFormat();\n> - if (pf.isValid())\n> - deviceFormats[pf] = format.second;\n> + if (role == StreamRole::Raw) {\n> + /* Translate the MBUS codes to a PixelFormat. */\n> + for (const auto &format : data->sensorFormats_) {\n> + PixelFormat pf = mbusCodeToPixelFormat(format.first,\n> + BayerFormat::Packing::CSI2);\n> + if (pf.isValid())\n> + deviceFormats.emplace(std::piecewise_construct, std::forward_as_tuple(pf),\n> + std::forward_as_tuple(format.second.begin(), format.second.end()));\n\nI can't tell right now if this could be simpler, but I think it's fine\nas it is if it works for you and produces the right result.\n\nI think this functionality could or rather /should/ be done in the\nCameraSensor class to return these formats to simplify this for each\npipeline handler, as they should all do this in the same way if they use\na CameraSensor.\n\nHowever, I think that move to CameraSensor and adapting of the other\nhandlers can be done on top.\n\nSo,\n\nReviewed-by: Kieran Bingham \n\n\n> + }\n> + } else {\n> + /* Translate the V4L2PixelFormat to PixelFormat. */\n> + for (const auto &format : fmts) {\n> + PixelFormat pf = format.first.toPixelFormat();\n> + if (pf.isValid())\n> + deviceFormats[pf] = format.second;\n> + }\n> }\n> \n> /* Add the stream format based on the device node used for the use case. */\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 24068BF415\n\tfor ;\n\tMon, 6 Dec 2021 17:20:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4112D6086C;\n\tMon, 6 Dec 2021 18:20:00 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2D9CB60725\n\tfor ;\n\tMon, 6 Dec 2021 18:19:59 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B02E4EE;\n\tMon, 6 Dec 2021 18:19:58 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"wirm3reY\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1638811198;\n\tbh=B/oNif7B22Lnx0yoYBUrlQAdToenP+3+UwyehrHHrXY=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=wirm3reY+LguIjPSBjbrSJZ6KR8AIlnTfWmNop1lP66bfkZCFYQMxku6LkW8FlL7+\n\tSR60xPFVYfQB3lo9HZP7cnISYgt0DGxokZBerF3ArXuZ41ZLoweBZkSvng3gO1u1Ji\n\tv9rkt6vMRgRhvIFx++h+vYGp3e+v2atQSx2dSQrc=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20211203113205.2470651-1-naush@raspberrypi.com>","References":"<20211203113205.2470651-1-naush@raspberrypi.com>","From":"Kieran Bingham ","To":"Naushir Patuck ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Mon, 06 Dec 2021 17:19:56 +0000","Message-ID":"<163881119601.995700.3972360592151937221@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v1] pipeline: raspberrypi: Return the\n\tsensor formats from generateConfiguration()","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":21614,"web_url":"https://patchwork.libcamera.org/comment/21614/","msgid":"","date":"2021-12-06T17:48:47","subject":"Re: [libcamera-devel] [PATCH v1] pipeline: raspberrypi: Return the\n\tsensor formats from generateConfiguration()","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hello,\n\nOn Mon, Dec 06, 2021 at 05:19:56PM +0000, Kieran Bingham wrote:\n> Quoting Naushir Patuck (2021-12-03 11:32:05)\n> > Return the available sensor PixelFormats and sizes from generateConfiguration()\n> > if the StreamRole is set to StreamRole::Raw. The existing code returns the\n> > PixelFormats and sizes for all other StreamRole types.\n> > \n> > Signed-off-by: Naushir Patuck \n> > ---\n> > .../pipeline/raspberrypi/raspberrypi.cpp | 21 ++++++++++++++-----\n> > 1 file changed, 16 insertions(+), 5 deletions(-)\n> > \n> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > index 5b76916e9e98..cbfb58562626 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > @@ -595,12 +595,23 @@ CameraConfiguration *PipelineHandlerRPi::generateConfiguration(Camera *camera,\n> > return nullptr;\n> > }\n> > \n> > - /* Translate the V4L2PixelFormat to PixelFormat. */\n> > std::map> deviceFormats;\n> > - for (const auto &format : fmts) {\n> > - PixelFormat pf = format.first.toPixelFormat();\n> > - if (pf.isValid())\n> > - deviceFormats[pf] = format.second;\n> > + if (role == StreamRole::Raw) {\n> > + /* Translate the MBUS codes to a PixelFormat. */\n> > + for (const auto &format : data->sensorFormats_) {\n> > + PixelFormat pf = mbusCodeToPixelFormat(format.first,\n> > + BayerFormat::Packing::CSI2);\n> > + if (pf.isValid())\n> > + deviceFormats.emplace(std::piecewise_construct, std::forward_as_tuple(pf),\n> > + std::forward_as_tuple(format.second.begin(), format.second.end()));\n> \n> I can't tell right now if this could be simpler, but I think it's fine\n> as it is if it works for you and produces the right result.\n\nThat's likely the shortest piece of code possible, even if not the most\nexplicit one (declaring an explicit intermediate std::vector\nwould be more readable I think).\n\n> I think this functionality could or rather /should/ be done in the\n> CameraSensor class to return these formats to simplify this for each\n> pipeline handler, as they should all do this in the same way if they use\n> a CameraSensor.\n\nExcept that how a media bus code is translated to a pixel format is\nspecific to the video device, it's not an intrinsic property of the\ncamera sensor.\n\n> However, I think that move to CameraSensor and adapting of the other\n> handlers can be done on top.\n> \n> So,\n> \n> Reviewed-by: Kieran Bingham \n\nReviewed-by: Laurent Pinchart \n\n> > + }\n> > + } else {\n> > + /* Translate the V4L2PixelFormat to PixelFormat. */\n> > + for (const auto &format : fmts) {\n> > + PixelFormat pf = format.first.toPixelFormat();\n> > + if (pf.isValid())\n> > + deviceFormats[pf] = format.second;\n> > + }\n> > }\n> > \n> > /* Add the stream format based on the device node used for the use case. */","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 99C05BDB13\n\tfor ;\n\tMon, 6 Dec 2021 17:49:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0B1EC6086C;\n\tMon, 6 Dec 2021 18:49:17 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9EE4F60725\n\tfor ;\n\tMon, 6 Dec 2021 18:49:15 +0100 (CET)","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 113EBEE;\n\tMon, 6 Dec 2021 18:49:15 +0100 (CET)"],"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=\"dAIQ2ATc\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1638812955;\n\tbh=HljDTkkmk+2KsbO8G2Msy1X9bE+2dorKORHtT1YsrdE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=dAIQ2ATcCNfyRq0i0FAZkKaeuAIS4mWiVn3DttOqS2P56MOYhPY5PG4hhD+xOhmfr\n\t2j5VKAvpbZy+OGz2NX03IFbN8I6pReMSpAv508ODUH9p0Q6hR0Z40z0XLSIbSG+y9/\n\ta2YHKgYTzcUDySD+tFv9Qe7PyxTNn3DcsU9LFfqU=","Date":"Mon, 6 Dec 2021 19:48:47 +0200","From":"Laurent Pinchart ","To":"Kieran Bingham ","Message-ID":"","References":"<20211203113205.2470651-1-naush@raspberrypi.com>\n\t<163881119601.995700.3972360592151937221@Monstersaurus>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<163881119601.995700.3972360592151937221@Monstersaurus>","Subject":"Re: [libcamera-devel] [PATCH v1] pipeline: raspberrypi: Return the\n\tsensor formats from generateConfiguration()","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":21624,"web_url":"https://patchwork.libcamera.org/comment/21624/","msgid":"","date":"2021-12-07T08:45:14","subject":"Re: [libcamera-devel] [PATCH v1] pipeline: raspberrypi: Return the\n\tsensor formats from generateConfiguration()","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi all,\n\nThank you for your reviews!\n\nOn Mon, 6 Dec 2021 at 17:49, Laurent Pinchart <\nlaurent.pinchart@ideasonboard.com> wrote:\n\n> Hello,\n>\n> On Mon, Dec 06, 2021 at 05:19:56PM +0000, Kieran Bingham wrote:\n> > Quoting Naushir Patuck (2021-12-03 11:32:05)\n> > > Return the available sensor PixelFormats and sizes from\n> generateConfiguration()\n> > > if the StreamRole is set to StreamRole::Raw. The existing code returns\n> the\n> > > PixelFormats and sizes for all other StreamRole types.\n> > >\n> > > Signed-off-by: Naushir Patuck \n> > > ---\n> > > .../pipeline/raspberrypi/raspberrypi.cpp | 21 ++++++++++++++-----\n> > > 1 file changed, 16 insertions(+), 5 deletions(-)\n> > >\n> > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > index 5b76916e9e98..cbfb58562626 100644\n> > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > @@ -595,12 +595,23 @@ CameraConfiguration\n> *PipelineHandlerRPi::generateConfiguration(Camera *camera,\n> > > return nullptr;\n> > > }\n> > >\n> > > - /* Translate the V4L2PixelFormat to PixelFormat. */\n> > > std::map>\n> deviceFormats;\n> > > - for (const auto &format : fmts) {\n> > > - PixelFormat pf = format.first.toPixelFormat();\n> > > - if (pf.isValid())\n> > > - deviceFormats[pf] = format.second;\n> > > + if (role == StreamRole::Raw) {\n> > > + /* Translate the MBUS codes to a PixelFormat.\n> */\n> > > + for (const auto &format :\n> data->sensorFormats_) {\n> > > + PixelFormat pf =\n> mbusCodeToPixelFormat(format.first,\n> > > +\n> BayerFormat::Packing::CSI2);\n> > > + if (pf.isValid())\n> > > +\n> deviceFormats.emplace(std::piecewise_construct, std::forward_as_tuple(pf),\n> > > +\n> std::forward_as_tuple(format.second.begin(), format.second.end()));\n> >\n> > I can't tell right now if this could be simpler, but I think it's fine\n> > as it is if it works for you and produces the right result.\n>\n> That's likely the shortest piece of code possible, even if not the most\n> explicit one (declaring an explicit intermediate std::vector\n> would be more readable I think).\n>\n\nThis is the only way I could think of constructing deviceFormat without an\nintermediate temporary std::vector. If others feel this construct is\nsomewhat\nobfuscated, I am happy to add a temporary for readability. If not, then\nfeel\nfree to merge this :-)\n\nRegards,\nNaush\n\n\n>\n> > I think this functionality could or rather /should/ be done in the\n> > CameraSensor class to return these formats to simplify this for each\n> > pipeline handler, as they should all do this in the same way if they use\n> > a CameraSensor.\n>\n> Except that how a media bus code is translated to a pixel format is\n> specific to the video device, it's not an intrinsic property of the\n> camera sensor.\n>\n> > However, I think that move to CameraSensor and adapting of the other\n> > handlers can be done on top.\n> >\n> > So,\n> >\n> > Reviewed-by: Kieran Bingham \n>\n> Reviewed-by: Laurent Pinchart \n>\n> > > + }\n> > > + } else {\n> > > + /* Translate the V4L2PixelFormat to\n> PixelFormat. */\n> > > + for (const auto &format : fmts) {\n> > > + PixelFormat pf =\n> format.first.toPixelFormat();\n> > > + if (pf.isValid())\n> > > + deviceFormats[pf] =\n> format.second;\n> > > + }\n> > > }\n> > >\n> > > /* Add the stream format based on the device node used\n> for the use case. */\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 0AEFFBDB13\n\tfor ;\n\tTue, 7 Dec 2021 08:45:33 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 626AB6059E;\n\tTue, 7 Dec 2021 09:45:32 +0100 (CET)","from mail-lj1-x22d.google.com (mail-lj1-x22d.google.com\n\t[IPv6:2a00:1450:4864:20::22d])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D1AAD6059E\n\tfor ;\n\tTue, 7 Dec 2021 09:45:30 +0100 (CET)","by mail-lj1-x22d.google.com with SMTP id u22so26020094lju.7\n\tfor ;\n\tTue, 07 Dec 2021 00:45:30 -0800 (PST)"],"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=\"b72sLlHn\"; 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=PVR0GjoFwSDqwOqdmPDFz/34PBaUH0jTHUBV1prHMq4=;\n\tb=b72sLlHnQ9/TsndX17bF03qx+LTOp/cGJWD2smXrZgDR/p3Hx9CxAbUSs/mXg+zxN+\n\t7YA8yaP4ko1pBxnEj59p1KZtyf9N2wsCZclFCfMXLbcp1gTOwnVA0y6tK+vJvvtK/mlP\n\teAi8IgKRcglcMuFKm734xG1bFAILRzSyyE3N9o2kP/BQjUVAXLw9DCFohA1wHlJMn4aK\n\typ6WJztb50BNWQ6h6KiNVzsYLdOgWxr6kP/vi5Yr85YcfWLIQ9fnsJcmnzw0XUWjOx2C\n\t0RsCJy+8eMUsbvpKjEwb2FhNF6mLmMhxIwcpNQANi7UJpsSBASrOd8lAtQX0QBY5CkuF\n\tLF5w==","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=PVR0GjoFwSDqwOqdmPDFz/34PBaUH0jTHUBV1prHMq4=;\n\tb=bSE0wH1r/ef5K0xJqvT2GjUyujyhLMsOIH3Kt9iHI3hGo+sGTg55YuzTeGwLsFxv46\n\ti5lLxsywiMsKnpmLwkvlBx9zI95ljQBR8jhacKIxdaGZOzMguXCiIFsfzpDCBjjiWiP3\n\tenynYgJxZ5cls1Df2MPteOE6nhrxPX8iVOofegkIJ0QBoYRtbjPEm+5y+fglJS9iduFn\n\t9rLZJIGFUORiZhUPS23OP4WMVOe7uXRck5njB2TJG5SgwBotEi8EthxmUGI05EZMoWAt\n\tqNRkWQ6f/hq4ZiaTJlevC63tBo5UtLU0yzw8s1AEmiFXwbD5toXIfudzAD0Bfr95U5p4\n\tXhRg==","X-Gm-Message-State":"AOAM530xaHMavWXwSCUfV1C0g2AV34eScLcONqwP3TKQ/UOVzeNdRk8j\n\tdL3LAQNpDJQuoTh4lBJgOLy/c9Cv1xcGuugeaaYtbqcU45SpPw==","X-Google-Smtp-Source":"ABdhPJzWo3xi4Bev8pZ+pUF37pprhaSdUKXOV6EEhkdk6+kbhW14q44MTYthJSxRSTTKWBGpPJpwCy+NSd8FziB7Go4=","X-Received":"by 2002:a2e:9450:: with SMTP id\n\to16mr39373894ljh.444.1638866730162; \n\tTue, 07 Dec 2021 00:45:30 -0800 (PST)","MIME-Version":"1.0","References":"<20211203113205.2470651-1-naush@raspberrypi.com>\n\t<163881119601.995700.3972360592151937221@Monstersaurus>\n\t","In-Reply-To":"","From":"Naushir Patuck ","Date":"Tue, 7 Dec 2021 08:45:14 +0000","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"multipart/alternative; boundary=\"00000000000049305b05d28a65af\"","Subject":"Re: [libcamera-devel] [PATCH v1] pipeline: raspberrypi: Return the\n\tsensor formats from generateConfiguration()","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\" "}}]