[{"id":12328,"web_url":"https://patchwork.libcamera.org/comment/12328/","msgid":"<20200906004457.GS6319@pendragon.ideasonboard.com>","date":"2020-09-06T00:44:57","subject":"Re: [libcamera-devel] [PATCH v7 0/8] 2D transforms","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi David,\n\nOn Fri, Sep 04, 2020 at 11:30:34AM +0100, David Plowman wrote:\n> Hi everyone\n> \n> Thanks for the various comments from Kieran since last time. The\n> latest version I think incorporates that feedback - I moved the\n> combinedTransform_ field into the RPiCameraConfiguration, and removed\n> that extra one I had called userTransform_ because it's the same as\n> the field already named transform. Apart from that everything else is\n> unchanged, except for maybe a couple of improved comments. So still 8\n> separate commits as last time:\n> \n> 1. The revert commit - unchanged.\n> \n> 2. V4L2Device::controlInfo method - unchanged.\n> \n> 3. Transform enum - unchanged.\n> \n> 4. BayerFormat class - unchanged apart from cosmetic tidying as per\n> feedback.\n> \n> 5. Add transform to CameraConfiguration - unchanged.\n> \n> 6. This is where combinedTransform_ moves into the\n> RPiCameraConfiguration, and userTransform_ is deleted (being just a\n> duplicate of the transform field).\n> \n> 7 and 8. Unchanged.\n> \n> I think that's it!\n\nNice work, it's nice to see pieces falling into place. The BayerFormat\nclass is a good addition, I think we'll end up extending it to support\nPixelFormat in addition to V4L2PixelFormat.\n\nThe patches are mostly OK. I thought I could handle the last few fixups\nmyself when applying (after getting your feedback on a few questions),\nbut patch 4/8 needs a bit more refactoring, so if you don't mind a (I\nbelieve final) v8 would probably be best.\n\n> David Plowman (8):\n>   libcamera: pipeline: raspberrypi: Revert \"Set sensor default\n>     orientation before configure()\"\n>   libcamera: Allow access to v4l2_query_ext_ctrl structure for a V4L2\n>     control\n>   libcamera: Add Transform enum to represent 2D plane transforms.\n>   libcamera: Add BayerFormat type\n>   libcamera: Add user Transform to CameraConfiguration\n>   libcamera: raspberrypi: Set camera flips correctly from user transform\n>   libcamera: raspberrypi: Plumb user transform through to IPA\n>   libcamera: ipa: raspberrypi: ALSC: Handle user transform\n> \n>  include/libcamera/camera.h                    |   3 +\n>  include/libcamera/internal/bayer_format.h     |  60 ++++\n>  include/libcamera/internal/v4l2_device.h      |   2 +\n>  include/libcamera/meson.build                 |   1 +\n>  include/libcamera/transform.h                 |  78 +++++\n>  src/ipa/raspberrypi/controller/camera_mode.h  |   4 +\n>  src/ipa/raspberrypi/controller/rpi/alsc.cpp   |  13 +-\n>  src/ipa/raspberrypi/raspberrypi.cpp           |  48 +--\n>  src/libcamera/bayer_format.cpp                | 231 +++++++++++++\n>  src/libcamera/camera.cpp                      |  16 +-\n>  src/libcamera/meson.build                     |   2 +\n>  src/libcamera/pipeline/ipu3/ipu3.cpp          |   5 +\n>  .../pipeline/raspberrypi/raspberrypi.cpp      | 155 ++++++++-\n>  src/libcamera/pipeline/rkisp1/rkisp1.cpp      |   5 +\n>  src/libcamera/pipeline/simple/simple.cpp      |   5 +\n>  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp  |   5 +\n>  src/libcamera/pipeline/vimc/vimc.cpp          |   5 +\n>  src/libcamera/transform.cpp                   | 322 ++++++++++++++++++\n>  src/libcamera/v4l2_device.cpp                 |  15 +\n>  19 files changed, 941 insertions(+), 34 deletions(-)\n>  create mode 100644 include/libcamera/internal/bayer_format.h\n>  create mode 100644 include/libcamera/transform.h\n>  create mode 100644 src/libcamera/bayer_format.cpp\n>  create mode 100644 src/libcamera/transform.cpp","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 B6E3EBDB1C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSun,  6 Sep 2020 00:45:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4499C62B5D;\n\tSun,  6 Sep 2020 02:45:23 +0200 (CEST)","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 AA60461EA0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun,  6 Sep 2020 02:45:21 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1E190277;\n\tSun,  6 Sep 2020 02:45:21 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"oAFdn9rm\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1599353121;\n\tbh=jHUk6Vnqz+F5XFlxlC7+sTdu+vMfCqBS26z2rTSINBo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=oAFdn9rmseAiKRFSkV8oH3lRC3jhE6svSNvg0z6aHu+uOW74e9MeQpbYVtFZpNImI\n\tADd8s2RX1PjvyapDvrlvKBEldjqI+7i3mEkmJR8fFKcA0WzvPoMd4/fYeEGHZD0VUG\n\tBoQluAyfAKwe/c7aDAJk5v9HtDFnNZPG9z+vvlzQ=","Date":"Sun, 6 Sep 2020 03:44:57 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"David Plowman <david.plowman@raspberrypi.com>","Message-ID":"<20200906004457.GS6319@pendragon.ideasonboard.com>","References":"<20200904103042.1593-1-david.plowman@raspberrypi.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200904103042.1593-1-david.plowman@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v7 0/8] 2D transforms","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>","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":12337,"web_url":"https://patchwork.libcamera.org/comment/12337/","msgid":"<CAHW6GY+=1SQzsdMd7r4viNXx0hODpRPxLXmSK=9c0KdMgFj95Q@mail.gmail.com>","date":"2020-09-06T17:21:02","subject":"Re: [libcamera-devel] [PATCH v7 0/8] 2D transforms","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Laurent\n\nThanks very much for the review. I'll send out a v8 version tomorrow\nwith those various comments incorporated. There were just a couple of\nslight changes I made.\n\nFirstly if you recall this little snippet:\n\n> +     /*\n> +      * We also check if the sensor doesn't do h/vflips at all, in which\n> +      * case we clear them, and the application will have to do everything.\n> +      */\n> +     if (!data_->supportsFlips_ && !!combined) {\n> +             transform &= Transform::Transpose;\n> +             combined = Transform::Identity;\n> +             status = Adjusted;\n> +     }\n\nI think you're right that I got it slightly wrong, though I think it's\na bit subtle. The camera is set to implement the \"combined\" transform,\nso if it can't do any transforms then the only value for combined that\nit can perform correctly is the identity. If combined is something\nelse then we must change it to the identity, and the configuration is\ntherefore \"adjusted\". We set the user transform to be the transform\nthat gives a combined transform of the identity, which must therefore\nbe the inverse of the rotation (recall combined = user transform *\nrotation).\n\nFor example, in the case of the imx477 the rotation is 180 degrees. If\nthe camera couldn't do any flips (this is hypothetical) and the\napplication asked for an identity transform, then combined would be\nrot180 and would be coerced to the identity. The configuration would\nbe returned as \"adjusted\", and the application would discover its\ntransform had been changed to rot180 - informing it that it will get\nits images upside down compared to what it wanted. I think that makes\nsense. Anyway, I've fixed that and put in yet more comments.\nTransforms are always so head-scratching...\n\nThe other little detail was the effect of transposition on the BayerFormat.\n\nIf a camera ever did transposition you'd have to know where it comes\nrelative to the flips, otherwise the Bayer order will come out wrong -\nflips and transpose don't commute, of course. That does complicate\nthings somewhat, and I'm not really sure how you'd handle such a\nthing. So for now I've continued to ignore transpose, but added some\nwords to that effect - I hope that seems reasonable!\n\nThanks again\nDavid\n\nOn Sun, 6 Sep 2020 at 01:45, Laurent Pinchart\n<laurent.pinchart@ideasonboard.com> wrote:\n>\n> Hi David,\n>\n> On Fri, Sep 04, 2020 at 11:30:34AM +0100, David Plowman wrote:\n> > Hi everyone\n> >\n> > Thanks for the various comments from Kieran since last time. The\n> > latest version I think incorporates that feedback - I moved the\n> > combinedTransform_ field into the RPiCameraConfiguration, and removed\n> > that extra one I had called userTransform_ because it's the same as\n> > the field already named transform. Apart from that everything else is\n> > unchanged, except for maybe a couple of improved comments. So still 8\n> > separate commits as last time:\n> >\n> > 1. The revert commit - unchanged.\n> >\n> > 2. V4L2Device::controlInfo method - unchanged.\n> >\n> > 3. Transform enum - unchanged.\n> >\n> > 4. BayerFormat class - unchanged apart from cosmetic tidying as per\n> > feedback.\n> >\n> > 5. Add transform to CameraConfiguration - unchanged.\n> >\n> > 6. This is where combinedTransform_ moves into the\n> > RPiCameraConfiguration, and userTransform_ is deleted (being just a\n> > duplicate of the transform field).\n> >\n> > 7 and 8. Unchanged.\n> >\n> > I think that's it!\n>\n> Nice work, it's nice to see pieces falling into place. The BayerFormat\n> class is a good addition, I think we'll end up extending it to support\n> PixelFormat in addition to V4L2PixelFormat.\n>\n> The patches are mostly OK. I thought I could handle the last few fixups\n> myself when applying (after getting your feedback on a few questions),\n> but patch 4/8 needs a bit more refactoring, so if you don't mind a (I\n> believe final) v8 would probably be best.\n>\n> > David Plowman (8):\n> >   libcamera: pipeline: raspberrypi: Revert \"Set sensor default\n> >     orientation before configure()\"\n> >   libcamera: Allow access to v4l2_query_ext_ctrl structure for a V4L2\n> >     control\n> >   libcamera: Add Transform enum to represent 2D plane transforms.\n> >   libcamera: Add BayerFormat type\n> >   libcamera: Add user Transform to CameraConfiguration\n> >   libcamera: raspberrypi: Set camera flips correctly from user transform\n> >   libcamera: raspberrypi: Plumb user transform through to IPA\n> >   libcamera: ipa: raspberrypi: ALSC: Handle user transform\n> >\n> >  include/libcamera/camera.h                    |   3 +\n> >  include/libcamera/internal/bayer_format.h     |  60 ++++\n> >  include/libcamera/internal/v4l2_device.h      |   2 +\n> >  include/libcamera/meson.build                 |   1 +\n> >  include/libcamera/transform.h                 |  78 +++++\n> >  src/ipa/raspberrypi/controller/camera_mode.h  |   4 +\n> >  src/ipa/raspberrypi/controller/rpi/alsc.cpp   |  13 +-\n> >  src/ipa/raspberrypi/raspberrypi.cpp           |  48 +--\n> >  src/libcamera/bayer_format.cpp                | 231 +++++++++++++\n> >  src/libcamera/camera.cpp                      |  16 +-\n> >  src/libcamera/meson.build                     |   2 +\n> >  src/libcamera/pipeline/ipu3/ipu3.cpp          |   5 +\n> >  .../pipeline/raspberrypi/raspberrypi.cpp      | 155 ++++++++-\n> >  src/libcamera/pipeline/rkisp1/rkisp1.cpp      |   5 +\n> >  src/libcamera/pipeline/simple/simple.cpp      |   5 +\n> >  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp  |   5 +\n> >  src/libcamera/pipeline/vimc/vimc.cpp          |   5 +\n> >  src/libcamera/transform.cpp                   | 322 ++++++++++++++++++\n> >  src/libcamera/v4l2_device.cpp                 |  15 +\n> >  19 files changed, 941 insertions(+), 34 deletions(-)\n> >  create mode 100644 include/libcamera/internal/bayer_format.h\n> >  create mode 100644 include/libcamera/transform.h\n> >  create mode 100644 src/libcamera/bayer_format.cpp\n> >  create mode 100644 src/libcamera/transform.cpp\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","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 E6414BDB1D\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSun,  6 Sep 2020 17:21:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3CB1B62B66;\n\tSun,  6 Sep 2020 19:21:18 +0200 (CEST)","from mail-ot1-x336.google.com (mail-ot1-x336.google.com\n\t[IPv6:2607:f8b0:4864:20::336])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1C36460370\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun,  6 Sep 2020 19:21:16 +0200 (CEST)","by mail-ot1-x336.google.com with SMTP id g96so10393898otb.12\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 06 Sep 2020 10:21:15 -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=\"TgdQCpYU\"; 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=u1XjsOqvrmSzt/bLXgI8iDOBZrED5AuBikbJFxRIDsY=;\n\tb=TgdQCpYUg6mEJNseQsNGa6dYBsDhpjc3N9+WGbaqknREOBz3VAkDmwS6UQWA/xOyBw\n\tWgvsdx1nqJtLp5LoTBgbM9pUGbaj3XgbiProC2J1P12nIruxWTZqAR7yiP/OxCb7+/51\n\tvNQGa93MMDfAs1QSY1zfXfdML7Dz8hZuXpRqHP6F5vNal84ApYk3Nr8/lBs9uJAiruPD\n\tET0ktlh3ETdwJEqaSIpN8BukmmooMRJTtDjmDuirx44ztQmhVmXXZdFJDgDbJsfMAyK0\n\tmIxwF265HwBRPk+wNa89/0pNeQaGV8lS+9uD8lJYsj4HNT4/klMFvxyU6rb2YxBCtTC0\n\tFpdA==","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:cc;\n\tbh=u1XjsOqvrmSzt/bLXgI8iDOBZrED5AuBikbJFxRIDsY=;\n\tb=P1y7/oY3QBxfBucRQnRHHPIVdOPT2xNa32CGzy5Ol+U/TN9n0++5SSKGltt9kp4gB/\n\tlfS8ww+bbzHRPd29E327fhDLk20V6r82npk7FcwHDpKmKGRlbEuS7+pyMgbABmaJoYqx\n\tl3nW0Pyo+2ThVibYh00R56rj9/LZ2/xYqtfUw9EIc+ZFKJnORgJJFrTUtUyZfNGc3t3x\n\tUdVuJO+/BGYuANDJps9NUnbyYAJjMG//WrnJcGUJt14r8y4MEt38j83NNFtlOnvqQjAA\n\tTgzO01Iu33/R8ObzNfXwOyl7ZVGWqjILDVWlJp1U60Wnc4yiabhKLoY0CFONvYRVJCEq\n\taFmQ==","X-Gm-Message-State":"AOAM533d/07r4OE0TVZ1uvXjyh2pX/Okyx6IW0xx9wgQGJ8rFW09DePm\n\tx75U1ioY2t/o6qoQtQ+XqvJn1BHwXtSU3ygjXKJEX//phA6mmQ==","X-Google-Smtp-Source":"ABdhPJx5OJMIsgRlnfgPaRIoKy4B7C5LwntsO31+e4K+6/+a3bQFwIdJdNIikMM5BcN8DO1y0UsQqLLigaENtyUj6zU=","X-Received":"by 2002:a9d:d35:: with SMTP id 50mr12094545oti.166.1599412874583;\n\tSun, 06 Sep 2020 10:21:14 -0700 (PDT)","MIME-Version":"1.0","References":"<20200904103042.1593-1-david.plowman@raspberrypi.com>\n\t<20200906004457.GS6319@pendragon.ideasonboard.com>","In-Reply-To":"<20200906004457.GS6319@pendragon.ideasonboard.com>","From":"David Plowman <david.plowman@raspberrypi.com>","Date":"Sun, 6 Sep 2020 18:21:02 +0100","Message-ID":"<CAHW6GY+=1SQzsdMd7r4viNXx0hODpRPxLXmSK=9c0KdMgFj95Q@mail.gmail.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v7 0/8] 2D transforms","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>","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":12338,"web_url":"https://patchwork.libcamera.org/comment/12338/","msgid":"<20200906215401.GC6047@pendragon.ideasonboard.com>","date":"2020-09-06T21:54:01","subject":"Re: [libcamera-devel] [PATCH v7 0/8] 2D transforms","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Sun, Sep 06, 2020 at 06:21:02PM +0100, David Plowman wrote:\n> Hi Laurent\n> \n> Thanks very much for the review. I'll send out a v8 version tomorrow\n> with those various comments incorporated. There were just a couple of\n> slight changes I made.\n> \n> Firstly if you recall this little snippet:\n> \n> > +     /*\n> > +      * We also check if the sensor doesn't do h/vflips at all, in which\n> > +      * case we clear them, and the application will have to do everything.\n> > +      */\n> > +     if (!data_->supportsFlips_ && !!combined) {\n> > +             transform &= Transform::Transpose;\n> > +             combined = Transform::Identity;\n> > +             status = Adjusted;\n> > +     }\n> \n> I think you're right that I got it slightly wrong, though I think it's\n> a bit subtle. The camera is set to implement the \"combined\" transform,\n> so if it can't do any transforms then the only value for combined that\n> it can perform correctly is the identity. If combined is something\n> else then we must change it to the identity, and the configuration is\n> therefore \"adjusted\". We set the user transform to be the transform\n> that gives a combined transform of the identity, which must therefore\n> be the inverse of the rotation (recall combined = user transform *\n> rotation).\n> \n> For example, in the case of the imx477 the rotation is 180 degrees. If\n> the camera couldn't do any flips (this is hypothetical) and the\n> application asked for an identity transform, then combined would be\n> rot180 and would be coerced to the identity. The configuration would\n> be returned as \"adjusted\", and the application would discover its\n> transform had been changed to rot180 - informing it that it will get\n> its images upside down compared to what it wanted. I think that makes\n> sense. Anyway, I've fixed that and put in yet more comments.\n\nYes, I think it makes sense. Thanks for adding comments, this is a topic\nthat will really benefit from documentation.\n\n> Transforms are always so head-scratching...\n> \n> The other little detail was the effect of transposition on the BayerFormat.\n> \n> If a camera ever did transposition you'd have to know where it comes\n> relative to the flips, otherwise the Bayer order will come out wrong -\n> flips and transpose don't commute, of course. That does complicate\n> things somewhat, and I'm not really sure how you'd handle such a\n> thing. So for now I've continued to ignore transpose, but added some\n> words to that effect - I hope that seems reasonable!\n\nSounds good to me, given that we don't support any sensor that can\ntranspose the image.\n\n> On Sun, 6 Sep 2020 at 01:45, Laurent Pinchart wrote:\n> > On Fri, Sep 04, 2020 at 11:30:34AM +0100, David Plowman wrote:\n> > > Hi everyone\n> > >\n> > > Thanks for the various comments from Kieran since last time. The\n> > > latest version I think incorporates that feedback - I moved the\n> > > combinedTransform_ field into the RPiCameraConfiguration, and removed\n> > > that extra one I had called userTransform_ because it's the same as\n> > > the field already named transform. Apart from that everything else is\n> > > unchanged, except for maybe a couple of improved comments. So still 8\n> > > separate commits as last time:\n> > >\n> > > 1. The revert commit - unchanged.\n> > >\n> > > 2. V4L2Device::controlInfo method - unchanged.\n> > >\n> > > 3. Transform enum - unchanged.\n> > >\n> > > 4. BayerFormat class - unchanged apart from cosmetic tidying as per\n> > > feedback.\n> > >\n> > > 5. Add transform to CameraConfiguration - unchanged.\n> > >\n> > > 6. This is where combinedTransform_ moves into the\n> > > RPiCameraConfiguration, and userTransform_ is deleted (being just a\n> > > duplicate of the transform field).\n> > >\n> > > 7 and 8. Unchanged.\n> > >\n> > > I think that's it!\n> >\n> > Nice work, it's nice to see pieces falling into place. The BayerFormat\n> > class is a good addition, I think we'll end up extending it to support\n> > PixelFormat in addition to V4L2PixelFormat.\n> >\n> > The patches are mostly OK. I thought I could handle the last few fixups\n> > myself when applying (after getting your feedback on a few questions),\n> > but patch 4/8 needs a bit more refactoring, so if you don't mind a (I\n> > believe final) v8 would probably be best.\n> >\n> > > David Plowman (8):\n> > >   libcamera: pipeline: raspberrypi: Revert \"Set sensor default\n> > >     orientation before configure()\"\n> > >   libcamera: Allow access to v4l2_query_ext_ctrl structure for a V4L2\n> > >     control\n> > >   libcamera: Add Transform enum to represent 2D plane transforms.\n> > >   libcamera: Add BayerFormat type\n> > >   libcamera: Add user Transform to CameraConfiguration\n> > >   libcamera: raspberrypi: Set camera flips correctly from user transform\n> > >   libcamera: raspberrypi: Plumb user transform through to IPA\n> > >   libcamera: ipa: raspberrypi: ALSC: Handle user transform\n> > >\n> > >  include/libcamera/camera.h                    |   3 +\n> > >  include/libcamera/internal/bayer_format.h     |  60 ++++\n> > >  include/libcamera/internal/v4l2_device.h      |   2 +\n> > >  include/libcamera/meson.build                 |   1 +\n> > >  include/libcamera/transform.h                 |  78 +++++\n> > >  src/ipa/raspberrypi/controller/camera_mode.h  |   4 +\n> > >  src/ipa/raspberrypi/controller/rpi/alsc.cpp   |  13 +-\n> > >  src/ipa/raspberrypi/raspberrypi.cpp           |  48 +--\n> > >  src/libcamera/bayer_format.cpp                | 231 +++++++++++++\n> > >  src/libcamera/camera.cpp                      |  16 +-\n> > >  src/libcamera/meson.build                     |   2 +\n> > >  src/libcamera/pipeline/ipu3/ipu3.cpp          |   5 +\n> > >  .../pipeline/raspberrypi/raspberrypi.cpp      | 155 ++++++++-\n> > >  src/libcamera/pipeline/rkisp1/rkisp1.cpp      |   5 +\n> > >  src/libcamera/pipeline/simple/simple.cpp      |   5 +\n> > >  src/libcamera/pipeline/uvcvideo/uvcvideo.cpp  |   5 +\n> > >  src/libcamera/pipeline/vimc/vimc.cpp          |   5 +\n> > >  src/libcamera/transform.cpp                   | 322 ++++++++++++++++++\n> > >  src/libcamera/v4l2_device.cpp                 |  15 +\n> > >  19 files changed, 941 insertions(+), 34 deletions(-)\n> > >  create mode 100644 include/libcamera/internal/bayer_format.h\n> > >  create mode 100644 include/libcamera/transform.h\n> > >  create mode 100644 src/libcamera/bayer_format.cpp\n> > >  create mode 100644 src/libcamera/transform.cpp","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 828ADBDB1C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSun,  6 Sep 2020 21:54:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 04202629B6;\n\tSun,  6 Sep 2020 23:54:28 +0200 (CEST)","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 77CC560370\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun,  6 Sep 2020 23:54:26 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id AF3743E;\n\tSun,  6 Sep 2020 23:54:25 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"PfxyzZBy\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1599429265;\n\tbh=VSnbidf6E4f1kMa+CkHcIw+T3bnjPatFw1NzZCfznkI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=PfxyzZBy4/n7tkDZ8kgIOeTvaHfNGiIBlqCXHLjeJgG5dnuMsmuMO8tFTPj3PU4Dc\n\tByVGdnp3Ul2PMuO95+dmZTphdZjn9vziS8RXucnVRaEOuPe0zvWOtJzg80xQP9Fpow\n\t8oDJdyUpDOx5okHh1GD822iykQptjAmNrBqkZNRE=","Date":"Mon, 7 Sep 2020 00:54:01 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"David Plowman <david.plowman@raspberrypi.com>","Message-ID":"<20200906215401.GC6047@pendragon.ideasonboard.com>","References":"<20200904103042.1593-1-david.plowman@raspberrypi.com>\n\t<20200906004457.GS6319@pendragon.ideasonboard.com>\n\t<CAHW6GY+=1SQzsdMd7r4viNXx0hODpRPxLXmSK=9c0KdMgFj95Q@mail.gmail.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<CAHW6GY+=1SQzsdMd7r4viNXx0hODpRPxLXmSK=9c0KdMgFj95Q@mail.gmail.com>","Subject":"Re: [libcamera-devel] [PATCH v7 0/8] 2D transforms","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>","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]