[{"id":17331,"web_url":"https://patchwork.libcamera.org/comment/17331/","msgid":"","date":"2021-05-27T13:40:25","subject":"Re: [libcamera-devel] [PATCH 0/2] Raspberrypi: support per-mode\n\tcamera sensitivities","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Replying to myself...\n\nOn Thu, 27 May 2021 at 09:45, David Plowman\n wrote:\n>\n> Hi\n>\n> Here's an implementation of \"per-mode camera sensitivities\", following\n> on from a recent discussion email. This is taking the \"don't lie, even\n> a little\" approach, which I am inclined to prefer. There are two\n> patches as follows, though still some outstanding questions.\n>\n> 1. The first patch adds a \"sensitivity\" field to our camera mode\n> structure. The main question here is how to set this value correctly.\n>\n> - For now, I've just used a virtual function in the CamHelper. This is\n> dead simple and it works. (Though the sensor that needs this\n> function is still unreleased.)\n>\n> - But should I be using the sensor database? I'd have to add fields\n> for \"full resolution sensitivity\", \"2x2 binned sensitivity\" as a\n> minimum, though why stop there? There could be many different\n> binning modes! Also, I'd have to get this through to the IPA. Would\n> that mean writing a serialiser/deserialiser? Some clues on how to do\n> that would be helpful, I looked at ipa_data_serializer.cpp but it\n> was quite intimidating...\n\nHaving stared at this a bit more, I see one could move structs into\nthe core.mojom file (like IPACameraSensorInfo), where presumably\nthings get taken care of for you. Or you could add a\nserialiser/deserialiser in ipa_data_serializer.cpp. I notice that\nDEFINE_POD_SERIALIZER only works with numeric types but presumably\nit's only a small tweak to make it work for anything that is \"standard\nlayout\"? Would that work?\n\nThough I'm still not liking this solution too much because it's\nunclear how many \"mode sensitivities\" one would need to add, or indeed\nif mode sensitivities might differ for reasons other than binning...\n(and there's always \"way out there\" stuff like HDR modes)\n\nDavid\n\n>\n> - Or would we be better getting the value from the driver? That might\n> need a new read-only V4L2 control. And how would we pass that to the\n> IPA, in the IPACameraSensorInfo? It already has mode-dependent\n> fields...\n>\n> 2. The second patch takes care of the changing sensitivity in the\n> AEC/AGC, when the SwitchMode method is called.\n>\n> Finally, I think there's still a problem with this approach. Suppose\n> you wanted to run preview, then do a capture with the same output\n> brightness, but selecting the exposure and gain explicitly for\n> yourself. You simply can't do this without knowing the sensitivity of\n> the camera modes. So how could we report this? Note that signalling it\n> in metadata with completed requests is too late - the typical pattern\n> would be:\n>\n> Stop camera -> Reconfigure camera -> Recalculate exposure/gain ->\n> Start camera\n>\n> Opinions both sought and appreciated!\n>\n> Best regards\n> David\n>\n> David Plowman (2):\n> ipa: raspberrypi: Add sensitivity field to camera mode\n> ipa: raspberrypi: AGC: handle modes with different sensitivities\n>\n> src/ipa/raspberrypi/cam_helper.cpp | 5 ++++\n> src/ipa/raspberrypi/cam_helper.hpp | 3 +++\n> src/ipa/raspberrypi/controller/camera_mode.h | 2 ++\n> src/ipa/raspberrypi/controller/rpi/agc.cpp | 25 ++++++++++++++++----\n> src/ipa/raspberrypi/controller/rpi/agc.hpp | 1 +\n> src/ipa/raspberrypi/raspberrypi.cpp | 12 ++++++++++\n> 6 files changed, 43 insertions(+), 5 deletions(-)\n>\n> --\n> 2.20.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 31742C3203\n\tfor ;\n\tThu, 27 May 2021 13:40:40 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 884B6602AE;\n\tThu, 27 May 2021 15:40:39 +0200 (CEST)","from mail-wm1-x329.google.com (mail-wm1-x329.google.com\n\t[IPv6:2a00:1450:4864:20::329])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6D924602AA\n\tfor ;\n\tThu, 27 May 2021 15:40:37 +0200 (CEST)","by mail-wm1-x329.google.com with SMTP id b191so244733wmd.1\n\tfor ;\n\tThu, 27 May 2021 06:40:37 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"ocuj6Ak4\"; 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\tbh=u2p7MW0v+z1TQUU4V6uTq3zceGg+YXG6o4ZriCzXuYA=;\n\tb=ocuj6Ak40hScUYxn7TYEIDBfiG1hJcsyFruZ00OOb6ud6iRta1NbSmtJoB7ROPKm2B\n\tCd0/4/iwFMNRREVHhTz8jRGdQNSDrccf34j0ze/Dz6FXdjJSa7V0+awZ6YUJVnQQy2Ln\n\tyMJJm8S56aTnLT7C6IcTsZc5RfiTZYK7iwJ3FplMqhmZ9ABnmduuUOPmIr4ttr+F40J7\n\tG8MyObOszffwCU872ky7Y7XmcMfvSNvO9MAKW0Qhdxkmf+UN99+dZiA19fteHhEaJ03D\n\tGhMxSHmYiI8WeDGdz3++GWX7DoeRbB8pgyqWduBmrAsxfB+GUm0Iv4Ep3Og03+KY/jRw\n\t3Giw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to;\n\tbh=u2p7MW0v+z1TQUU4V6uTq3zceGg+YXG6o4ZriCzXuYA=;\n\tb=SZCLe9KwdUr8syApLo5Xy34EkrdCwqGmk6ZvFE4jqez6zNPjAPcx2AGoVLJlxphkxC\n\tvhDzgwjvTc4NTNR5quPuICFYSvtBtJU9tpTAbFIrGonPVwYBOgVauNQ3RVNXuiZse2vh\n\tsstk50pbreGxk53QG8K25//gZAjHIYyiIey12ZBy7vRD4B0orMV85z5DSUNeiRvxGZmR\n\tpazWp8gmRhvGihtv1KGSK0iMnueHNDrIPC3uxjdKWqlCyvnCpomg1wn+S40M1fKHFxW+\n\tzycTHtUQJpMaZOy5eXI93AHLOvPxFqVuhRa5M496rvePZJzomeZdaGSZLbiG5R4f9Voh\n\ttfVg==","X-Gm-Message-State":"AOAM530BNIugnPf97rQrt1Yh9Sc99O6rbv61tw3oVVDHw1OR6Uz0+ZHa\n\txUcNYuHX2bdd/JO/9R7ABeaFvaok6KIDKqNl+Cws3P9wXNv6Pw==","X-Google-Smtp-Source":"ABdhPJwQ+dZ7eQpHiEOhL5OJfSHAvafcGavGtw441W3B7YMqkZVTKKr9vD4eYIqqMx9s/VU/xJDODdyExQb1PmtczNo=","X-Received":"by 2002:a1c:6a0e:: with SMTP id\n\tf14mr8614541wmc.114.1622122836591; \n\tThu, 27 May 2021 06:40:36 -0700 (PDT)","MIME-Version":"1.0","References":"<20210527084540.9983-1-david.plowman@raspberrypi.com>","In-Reply-To":"<20210527084540.9983-1-david.plowman@raspberrypi.com>","From":"David Plowman ","Date":"Thu, 27 May 2021 14:40:25 +0100","Message-ID":"","To":"libcamera devel ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH 0/2] Raspberrypi: support per-mode\n\tcamera sensitivities","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":17385,"web_url":"https://patchwork.libcamera.org/comment/17385/","msgid":"","date":"2021-06-02T11:13:32","subject":"Re: [libcamera-devel] [PATCH 0/2] Raspberrypi: support per-mode\n\tcamera sensitivities","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Another reply to myself... :)\n\nOn Thu, 27 May 2021 at 14:40, David Plowman\n wrote:\n>\n> Replying to myself...\n>\n> On Thu, 27 May 2021 at 09:45, David Plowman\n> wrote:\n> >\n> > Hi\n> >\n> > Here's an implementation of \"per-mode camera sensitivities\", following\n> > on from a recent discussion email. This is taking the \"don't lie, even\n> > a little\" approach, which I am inclined to prefer. There are two\n> > patches as follows, though still some outstanding questions.\n> >\n> > 1. The first patch adds a \"sensitivity\" field to our camera mode\n> > structure. The main question here is how to set this value correctly.\n> >\n> > - For now, I've just used a virtual function in the CamHelper. This is\n> > dead simple and it works. (Though the sensor that needs this\n> > function is still unreleased.)\n> >\n> > - But should I be using the sensor database? I'd have to add fields\n> > for \"full resolution sensitivity\", \"2x2 binned sensitivity\" as a\n> > minimum, though why stop there? There could be many different\n> > binning modes! Also, I'd have to get this through to the IPA. Would\n> > that mean writing a serialiser/deserialiser? Some clues on how to do\n> > that would be helpful, I looked at ipa_data_serializer.cpp but it\n> > was quite intimidating...\n>\n> Having stared at this a bit more, I see one could move structs into\n> the core.mojom file (like IPACameraSensorInfo), where presumably\n> things get taken care of for you. Or you could add a\n> serialiser/deserialiser in ipa_data_serializer.cpp. I notice that\n> DEFINE_POD_SERIALIZER only works with numeric types but presumably\n> it's only a small tweak to make it work for anything that is \"standard\n> layout\"? Would that work?\n>\n> Though I'm still not liking this solution too much because it's\n> unclear how many \"mode sensitivities\" one would need to add, or indeed\n> if mode sensitivities might differ for reasons other than binning...\n> (and there's always \"way out there\" stuff like HDR modes)\n>\n> David\n>\n> >\n> > - Or would we be better getting the value from the driver? That might\n> > need a new read-only V4L2 control. And how would we pass that to the\n> > IPA, in the IPACameraSensorInfo? It already has mode-dependent\n> > fields...\n> >\n> > 2. The second patch takes care of the changing sensitivity in the\n> > AEC/AGC, when the SwitchMode method is called.\n> >\n> > Finally, I think there's still a problem with this approach. Suppose\n> > you wanted to run preview, then do a capture with the same output\n> > brightness, but selecting the exposure and gain explicitly for\n> > yourself. You simply can't do this without knowing the sensitivity of\n> > the camera modes. So how could we report this? Note that signalling it\n> > in metadata with completed requests is too late - the typical pattern\n> > would be:\n> >\n> > Stop camera -> Reconfigure camera -> Recalculate exposure/gain ->\n> > Start camera\n\nI'd also be interested in people's thoughts on this question - how to\ntell the application what the mode sensitivity is. Could it be\nreturned in the camera configuration perhaps?\n\nThanks\nDavid\n\n> >\n> > Opinions both sought and appreciated!\n> >\n> > Best regards\n> > David\n> >\n> > David Plowman (2):\n> > ipa: raspberrypi: Add sensitivity field to camera mode\n> > ipa: raspberrypi: AGC: handle modes with different sensitivities\n> >\n> > src/ipa/raspberrypi/cam_helper.cpp | 5 ++++\n> > src/ipa/raspberrypi/cam_helper.hpp | 3 +++\n> > src/ipa/raspberrypi/controller/camera_mode.h | 2 ++\n> > src/ipa/raspberrypi/controller/rpi/agc.cpp | 25 ++++++++++++++++----\n> > src/ipa/raspberrypi/controller/rpi/agc.hpp | 1 +\n> > src/ipa/raspberrypi/raspberrypi.cpp | 12 ++++++++++\n> > 6 files changed, 43 insertions(+), 5 deletions(-)\n> >\n> > --\n> > 2.20.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 DCF40C3206\n\tfor ;\n\tWed, 2 Jun 2021 11:13:46 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5B2EB602A8;\n\tWed, 2 Jun 2021 13:13:46 +0200 (CEST)","from mail-wm1-x334.google.com (mail-wm1-x334.google.com\n\t[IPv6:2a00:1450:4864:20::334])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id AD51E602A4\n\tfor ;\n\tWed, 2 Jun 2021 13:13:44 +0200 (CEST)","by mail-wm1-x334.google.com with SMTP id h3so1016625wmq.3\n\tfor ;\n\tWed, 02 Jun 2021 04:13:44 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"kXDYjFPd\"; 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\tbh=ORVdP5j9OOn5CYpwhYkhJhzYtcrWQHpC0gTy9mzx7gA=;\n\tb=kXDYjFPdkSnh8UE4ogERGLcjNIAtyaywhroSriDt3YUMiFpDt40zrgTkAXo/ZkuoqH\n\t+Z56qJ0Bh3cPnZtZ77CBJR80bE4NrNq1MiG4Arr2t1ql2FQ2pYWxzpdop+zl3yeq90Ml\n\tVmO6n8pH94Sftid3yVdakIz5EUsjPKom7FSAr8BFbZy1W5POWQaNG4f1igcK6UUmdJD7\n\tOvYKVnc20C3oPGJhLoTVyvTNyzNCko0KuH/xsjCK9UUZs1MKN8I1Bpefb7/U0zhWWxc/\n\t/VIJDVUzVrPYaymDgkqMLmlSTqlec00WO/j/m39EdHeGvjEaJwyKFFnnXyfCBhclflVa\n\tK0uw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to;\n\tbh=ORVdP5j9OOn5CYpwhYkhJhzYtcrWQHpC0gTy9mzx7gA=;\n\tb=RlhhItA9pks+BhEn35DlUOKsnR+mXAvQIyc7wDGrjnsVi/OFhJ8dubVIvUrMLCaqf+\n\tYcAAiWYSyfNhvWVrrzKJ1E0hDP+NMdLsbCDxv/XhBAew5FHZo52TOlJStPyhs1d4Taxx\n\taW4P+KvOxCmApGRsT30J+F5yKjFpqOIfuLjnJnPQjMCkP9YOA9RqXtB7npQINK/BFnrT\n\tkxK5ZKHZDz95nNdBOCaGzOAwzq7FW5QuvIkX8wTVmiUktBiYYiW5RAB0r81oI3YDSbTI\n\tC5Do3aPEfK5AQUY5i1CJW342Bh1/AE2fMPjH0T5LPH05xqhrq5CQFJv1OnpIsEBdLeO/\n\tO9uQ==","X-Gm-Message-State":"AOAM533X6EzWs3ghzHRPCC7Xya46gWO83x+2lcN+qVBfyDUxauyJx9bh\n\t7SD80epYl88GlO91MSI7Prg6bE0Xx3+AasNXLG7VGAAutYljjg==","X-Google-Smtp-Source":"ABdhPJwyQX/HJHNDAIV0Ll7XJKOJIj497lLPK0HLPQhBAND71aol11+Zua8EQcj6WRHeOnz8ny2PJvaPV8ozltqgZGw=","X-Received":"by 2002:a1c:df85:: with SMTP id\n\tw127mr4666261wmg.130.1622632424030; \n\tWed, 02 Jun 2021 04:13:44 -0700 (PDT)","MIME-Version":"1.0","References":"<20210527084540.9983-1-david.plowman@raspberrypi.com>\n\t","In-Reply-To":"","From":"David Plowman ","Date":"Wed, 2 Jun 2021 12:13:32 +0100","Message-ID":"","To":"libcamera devel ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH 0/2] Raspberrypi: support per-mode\n\tcamera sensitivities","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\" "}}]