[{"id":25843,"web_url":"https://patchwork.libcamera.org/comment/25843/","msgid":"<20221121165919.spqizmh4ht7y4lsp@uno.localdomain>","date":"2022-11-21T16:59:19","subject":"Re: [libcamera-devel] [PATCH v2 0/5] Resolve invalid attempts to\n\tset sensor flip controls","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi David,\n\nOn Mon, Nov 21, 2022 at 03:44:16PM +0000, David Plowman via libcamera-devel wrote:\n> Hi again\n>\n> In v2 of this set, the first 3 patches are unchanged, so thanks for\n> some of the reviews there.\n>\n> In the light of the various discussions, I've updated the changes to\n> those other PHs (uvcvideo, simple and rkisp1). In all cases I'm being\n> more explicit that I want to preserve the previous behaviour before\n> this patch set was introduced. This may or may not (probably may not)\n> be the actual correct behaviour that people would want. So:\n>\n> uvcvideo: I've deleted the patch that updated this. These are\n> unaffected by CameraSensor changes so I think I'm better off just\n> leaving them alone.\n>\n> simple and rkisp1: Both of these, AIUI, use the CameraSensor class so\n> previously the flips were getting cleared. Therefore I've added\n> exactly this into the PHs so that the behaviour is unchanged.\n>\n> As we had discussed, I think there are concerns that the simple and\n> rkisp1 PHs implement incorrect behaviours in at least some respects,\n> but I think it's probably a separate job for someone working on those\n> platforms to investigate.\n\nAgreed, but I'm not sure it is worth to introduce the v4l2 device\nhelper for this temporary solution, as I'm not sure that's the\ndirection where we want to go (it would be different if the V4L2Device\nhelper is used by other PH to implement Transform support).\n\nAs said, my understanding is that the direction where we want to go is\nto reset flips to their driver defaults, and that should be happening\ntransparently in the CameraSensor::setFormat() call path...\n\nI don't think we risk regressions if we merge the first two patches\nonly, while we would solve your use case, so if others agree, I would\njust pick the first two ones..\n\nThanks\n j\n\n>\n> Does that make sense?\n>\n> Thanks!\n>\n> David\n>\n> David Plowman (5):\n> libcamera: bayer_format: Add toMbusCode method\n> libcamera: camera_sensor: Do not clear camera flips when listing\n> formats\n> libcamera: v4l2_device: Add setTransform method to set a device's flip\n> controls\n> libcamera: pipeline: simple: Set device's flip controls as previously\n> libcamera: pipeline: rkisp1: Set device's flip controls as previously\n>\n> include/libcamera/internal/bayer_format.h | 1 +\n> include/libcamera/internal/v4l2_device.h | 3 ++\n> src/libcamera/bayer_format.cpp | 11 +++++\n> src/libcamera/camera_sensor.cpp | 49 ++++++++++++++++++-----\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 3 ++\n> src/libcamera/pipeline/simple/simple.cpp | 3 ++\n> src/libcamera/v4l2_device.cpp | 37 +++++++++++++++++\n> 7 files changed, 97 insertions(+), 10 deletions(-)\n>\n> --\n> 2.30.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 9AFD7BD16B\n\tfor ;\n\tMon, 21 Nov 2022 16:59:22 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 24C7B6331A;\n\tMon, 21 Nov 2022 17:59:22 +0100 (CET)","from relay7-d.mail.gandi.net (relay7-d.mail.gandi.net\n\t[217.70.183.200])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B43E163097\n\tfor ;\n\tMon, 21 Nov 2022 17:59:20 +0100 (CET)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id 2B1EE20003;\n\tMon, 21 Nov 2022 16:59:19 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1669049962;\n\tbh=uLuSVrl4VUFfMIfbbDUbCk0fIjwuhkK7gTmDF4IBtAg=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=WIKcwYxl9+FvKv78CY7x3KydO0UsiGauomuT8U9QZcP2o1FmVkPKW58icAtzI0boJ\n\ttel7KU45KMQcaxfj6xWXx+wK9RHfFRCzfhsJMoEAWtQa68hnMeCcdZTbaSaFq821ud\n\tr/TN0ieAxsicPB9GpZ2xZgx1t4KUcCpKhrrl0uRRJS1dkOMyuCy0CF2Bk4a5pJyBNh\n\tlR4WNk4Px1uG7754fDFTjt2IZ8JuhkPiKkqCp2jttVMW60OcOvunKjIKhdthAUUjmt\n\tkYA0AXODb+09uLBZwZn11OpcTSsJrsRwD1klXp/mdaI4H7y7PMgJIXaI7XoWWKEA9m\n\tsC7Gyu13drXqQ==","Date":"Mon, 21 Nov 2022 17:59:19 +0100","To":"David Plowman ","Message-ID":"<20221121165919.spqizmh4ht7y4lsp@uno.localdomain>","References":"<20221121154421.11732-1-david.plowman@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20221121154421.11732-1-david.plowman@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v2 0/5] Resolve invalid attempts to\n\tset sensor flip controls","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","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25847,"web_url":"https://patchwork.libcamera.org/comment/25847/","msgid":"","date":"2022-11-22T11:11:03","subject":"Re: [libcamera-devel] [PATCH v2 0/5] Resolve invalid attempts to\n\tset sensor flip controls","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Jacopo\n\nOn Mon, 21 Nov 2022 at 16:59, Jacopo Mondi wrote:\n>\n> Hi David,\n>\n> On Mon, Nov 21, 2022 at 03:44:16PM +0000, David Plowman via libcamera-devel wrote:\n> > Hi again\n> >\n> > In v2 of this set, the first 3 patches are unchanged, so thanks for\n> > some of the reviews there.\n> >\n> > In the light of the various discussions, I've updated the changes to\n> > those other PHs (uvcvideo, simple and rkisp1). In all cases I'm being\n> > more explicit that I want to preserve the previous behaviour before\n> > this patch set was introduced. This may or may not (probably may not)\n> > be the actual correct behaviour that people would want. So:\n> >\n> > uvcvideo: I've deleted the patch that updated this. These are\n> > unaffected by CameraSensor changes so I think I'm better off just\n> > leaving them alone.\n> >\n> > simple and rkisp1: Both of these, AIUI, use the CameraSensor class so\n> > previously the flips were getting cleared. Therefore I've added\n> > exactly this into the PHs so that the behaviour is unchanged.\n> >\n> > As we had discussed, I think there are concerns that the simple and\n> > rkisp1 PHs implement incorrect behaviours in at least some respects,\n> > but I think it's probably a separate job for someone working on those\n> > platforms to investigate.\n>\n> Agreed, but I'm not sure it is worth to introduce the v4l2 device\n> helper for this temporary solution, as I'm not sure that's the\n> direction where we want to go (it would be different if the V4L2Device\n> helper is used by other PH to implement Transform support).\n>\n> As said, my understanding is that the direction where we want to go is\n> to reset flips to their driver defaults, and that should be happening\n> transparently in the CameraSensor::setFormat() call path...\n>\n> I don't think we risk regressions if we merge the first two patches\n> only, while we would solve your use case, so if others agree, I would\n> just pick the first two ones..\n\nThat certainly works for me. I think the result will be that the code,\nwhich is wrong already, will remain wrong in a slightly different way,\nbut I also suspect no one will notice until they start looking at\nthese specific cases (which would be the time to fix them).\n\nSo I'll re-post just the first two patches. And then there's still the\nsensor HBLANK to sort out... :)\n\nThanks!\nDavid\n\n>\n> Thanks\n> j\n>\n> >\n> > Does that make sense?\n> >\n> > Thanks!\n> >\n> > David\n> >\n> > David Plowman (5):\n> > libcamera: bayer_format: Add toMbusCode method\n> > libcamera: camera_sensor: Do not clear camera flips when listing\n> > formats\n> > libcamera: v4l2_device: Add setTransform method to set a device's flip\n> > controls\n> > libcamera: pipeline: simple: Set device's flip controls as previously\n> > libcamera: pipeline: rkisp1: Set device's flip controls as previously\n> >\n> > include/libcamera/internal/bayer_format.h | 1 +\n> > include/libcamera/internal/v4l2_device.h | 3 ++\n> > src/libcamera/bayer_format.cpp | 11 +++++\n> > src/libcamera/camera_sensor.cpp | 49 ++++++++++++++++++-----\n> > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 3 ++\n> > src/libcamera/pipeline/simple/simple.cpp | 3 ++\n> > src/libcamera/v4l2_device.cpp | 37 +++++++++++++++++\n> > 7 files changed, 97 insertions(+), 10 deletions(-)\n> >\n> > --\n> > 2.30.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 98032BDE6B\n\tfor ;\n\tTue, 22 Nov 2022 11:11:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0366763314;\n\tTue, 22 Nov 2022 12:11:19 +0100 (CET)","from mail-pj1-x102e.google.com (mail-pj1-x102e.google.com\n\t[IPv6:2607:f8b0:4864:20::102e])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id AA9FA603CE\n\tfor ;\n\tTue, 22 Nov 2022 12:11:15 +0100 (CET)","by mail-pj1-x102e.google.com with SMTP id ci10so6784706pjb.1\n\tfor ;\n\tTue, 22 Nov 2022 03:11:15 -0800 (PST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1669115479;\n\tbh=F/tXkQQn5eKb3v5GG1/1QpLpBS2lIYx82bQMRg4hyC8=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=31WUatTjKDNUqXxv15FTzlOhQJ9X0MyI3QosPcjFV5Exzfc8Yp2ryFU092xLVUUSR\n\t4P1aoQ8g0BCYjgZM79lZdBeRgK1v8StWlV3YYHYdonJA09iLcH66/qSFAS+kTf3K4M\n\tKslY+BV5sWy88ZJTqwP/L1zukL4jtNhy65Mf7M3etf+wV+230T3Zj2cRPDpeLFzt8x\n\t1EQfLv7SKYkyWEajC20+UN91XFyxNzoQGQWPHYVvFAt74nh4yWrk2mxDLy24dOd5On\n\tQ1zZ/UcrIZ8RekJGfbRaAeaE81BLnoAs4vkbhLYf2aMD/m3urwMW4xh77LctaUfUGJ\n\thKC37Dgfp45lQ==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\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=xYzQeyulpJbje3rWMXezuqO+GoWe0Tap8nK2Ja1hNO8=;\n\tb=GHQuQu9ZBXvgr0M4w86hTzkFzgxZm5YiyKE3EDsF8hF5enUgt9Pz2mzGbmVRw5L6wk\n\tpwWn3uBC6CNRyaeqC+qzwnqmkHD7fngAc5oY3zFMr1VcCf8UfNuFS9nBE4RqrUwBbC3p\n\tVeKk/uIZ3RGiufgbgbQgZIZ+qzSW0x+0eRDxSq3va+uxPf2TCAZYvfnRI1AIvXXhhKr2\n\tM2SwYw7nDdTGE8fdH3ClNp81QtSRRPZbID5J1ZlsYKo+WR2s0+97O6Oo21ULuiiHr+AQ\n\tUh571rBoT4C0I4/dL9MBZglLpL2EyvwkJxesETAotY2979OVxI0uJO8nZZwJvpL0DSv/\n\tC9Pg=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"GHQuQu9Z\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\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=xYzQeyulpJbje3rWMXezuqO+GoWe0Tap8nK2Ja1hNO8=;\n\tb=2jo3iqX3QYSvTa/5pma0XSZPxhup3rGUfJyHn+mMr2GcxU64OSHtAKXQxCfdwIcWtk\n\tmNGTMGRoE/U2I6fKAQZ89dky745PBCGL3jTsqlYecRruJCrQwhyDvPmUpZULJuKzkZQQ\n\tsSmXNDMH23HmxtCGaf/Qzewq+QfJeZD21QhrC3tK3RhCCz++t7R2LVnLPsKOTqNa8BZY\n\tqhwZvijNBe+M8yAp1QHdmUE/b4WfdtLdtcyq/0DXqJKKisgYjmwObDEPttPOHKUH2eW4\n\tAOX01nzE4VuICzUfbkjjWMnSFUYgN/U6XIP//xyCK9CKDbb5p20SIeJI55bHsKout4/y\n\trxdg==","X-Gm-Message-State":"ANoB5pk5rtx2OcnRuI4cOHdYDk9F0XMEinp63eTJMzM8+c2gNHjLj8NJ\n\tRBK7Vp9JVefgdSA90B+UIkaLdPTfYm4NqoplDdLuRQ==","X-Google-Smtp-Source":"AA0mqf5FsqyDh/23lvfW7RZLPHcIz6Gue4EQw+HDb2jHV3FnZxblmCPKH8m2yANRRKqQLH0ndF6P9qwm+PfmES4hFj0=","X-Received":"by 2002:a17:90a:a588:b0:218:8eb8:8502 with SMTP id\n\tb8-20020a17090aa58800b002188eb88502mr18315118pjq.179.1669115473978;\n\tTue, 22 Nov 2022 03:11:13 -0800 (PST)","MIME-Version":"1.0","References":"<20221121154421.11732-1-david.plowman@raspberrypi.com>\n\t<20221121165919.spqizmh4ht7y4lsp@uno.localdomain>","In-Reply-To":"<20221121165919.spqizmh4ht7y4lsp@uno.localdomain>","Date":"Tue, 22 Nov 2022 11:11:03 +0000","Message-ID":"","To":"Jacopo Mondi ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v2 0/5] Resolve invalid attempts to\n\tset sensor flip controls","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","From":"David Plowman via libcamera-devel ","Reply-To":"David Plowman ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]