[{"id":12843,"web_url":"https://patchwork.libcamera.org/comment/12843/","msgid":"<20200928232451.GD14614@pendragon.ideasonboard.com>","date":"2020-09-28T23:24:51","subject":"Re: [libcamera-devel] [PATCH 3/3] libcamera: ipu3: Adjust comment\n\ton ImgU configuration","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nThank you for the patch.\n\nOn Thu, Sep 24, 2020 at 04:51:43PM +0200, Jacopo Mondi wrote:\n> Be more precise in commenting why the ImgU shall not be configured\n> if only the RAW stream is requested.\n> \n> As an example, if the ImgU gets unecessary configured:\n> cam -srole=viewfinder -c2 -C -> WORKS\n> cam -srole=stillraw -c2 -C -> WORKS\n> cam -srole=viewfinder -c2 -C -> Failed to queue buffer 0: Invalid argument\n> \n> Since commit 1bd2e981a209 (\"libcamera: ipu3: Fix RAW+YUV capture\")\n> the ImgU configuration procedure also correctly implements the\n> assumption that at least one of the YUV output is being operated. If\n> that's not the case, as in the RAW-only capture use case, the code\n> tries to access a non-existing configuration. One more reason to\n> exit early if no YUV stream is requested.\n> \n> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> ---\n>  src/libcamera/pipeline/ipu3/ipu3.cpp | 6 +++---\n>  1 file changed, 3 insertions(+), 3 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 477f85a45755..2f90ad2e2780 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -459,9 +459,9 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n>  \t\treturn ret;\n>  \n>  \t/*\n> -\t * If the ImgU gets configured with proper IF, BDS and GDC sizes, it\n> -\t * is then expected that frames are dequeued from its main output\n> -\t * otherwise the system stalls.\n> +\t * If the ImgU gets configured it is then expected that buffers are\n> +\t * queued to its outputs, otherwise the next capture session\n> +\t * that uses the ImgU fails at queueing buffers at its input.\n\n\"it is then expected\" sounds like we expect this, while I believe you\nmean the driver expects (or seems to expect) this. I'd phrase the\ncomment as\n\n\t * If the ImgU gets configured, its driver seems to expect that\n\t * buffers will be queued to its outputs, as otherwise the next\n\t * capture session that uses the ImgU fails when queueing\n\t * buffers to its input.\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\n\n>  \t *\n>  \t * If no ImgU configuration has been computed, it means only a RAW\n>  \t * stream has been requested: return here to skip the ImgU configuration","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 F0E35C3B5B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 28 Sep 2020 23:25:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C0CDE60BF7;\n\tTue, 29 Sep 2020 01:25: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 13BDB60394\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 29 Sep 2020 01:25:27 +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 99EDFA58;\n\tTue, 29 Sep 2020 01:25:26 +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=\"YdCyJ7u8\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1601335526;\n\tbh=3guBN1eLo0DEjQx72swJ6wuonVN9/oAIZBiDqBbzcvA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=YdCyJ7u8wGpEL4ON1d8jol0VPV1kuGbc23z521c+JaWopu3bIa/sIBv4P8etdeHhv\n\trHRFfpWofsUCX8/1ddMy+oF5XxB+Iu4JVSMyUsKLssyKq3didthVTk8RO42bh+DCSf\n\t5Ekzcx2+xYV8pJ/VWnx2BxnpY6zfIOmCDAHStEEs=","Date":"Tue, 29 Sep 2020 02:24:51 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Message-ID":"<20200928232451.GD14614@pendragon.ideasonboard.com>","References":"<20200924145143.117733-1-jacopo@jmondi.org>\n\t<20200924145143.117733-4-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200924145143.117733-4-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH 3/3] libcamera: ipu3: Adjust comment\n\ton ImgU configuration","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":12867,"web_url":"https://patchwork.libcamera.org/comment/12867/","msgid":"<20200929053928.GG1052036@oden.dyn.berto.se>","date":"2020-09-29T05:39:28","subject":"Re: [libcamera-devel] [PATCH 3/3] libcamera: ipu3: Adjust comment\n\ton ImgU configuration","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Jacopo,\n\nThanks for your work.\n\nOn 2020-09-24 16:51:43 +0200, Jacopo Mondi wrote:\n> Be more precise in commenting why the ImgU shall not be configured\n> if only the RAW stream is requested.\n> \n> As an example, if the ImgU gets unecessary configured:\n> cam -srole=viewfinder -c2 -C -> WORKS\n> cam -srole=stillraw -c2 -C -> WORKS\n> cam -srole=viewfinder -c2 -C -> Failed to queue buffer 0: Invalid argument\n> \n> Since commit 1bd2e981a209 (\"libcamera: ipu3: Fix RAW+YUV capture\")\n\nUnless you are _very_ lucky the commit hash will not be the same when \nthis patch is merged :-)\n\n> the ImgU configuration procedure also correctly implements the\n> assumption that at least one of the YUV output is being operated. If\n> that's not the case, as in the RAW-only capture use case, the code\n> tries to access a non-existing configuration. One more reason to\n> exit early if no YUV stream is requested.\n> \n> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n\nWith Laurent's comment addressed and the commit hash removed (or proof \nof guaranteed hash collision ;-P)\n\nReviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n\n> ---\n>  src/libcamera/pipeline/ipu3/ipu3.cpp | 6 +++---\n>  1 file changed, 3 insertions(+), 3 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 477f85a45755..2f90ad2e2780 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -459,9 +459,9 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n>  \t\treturn ret;\n>  \n>  \t/*\n> -\t * If the ImgU gets configured with proper IF, BDS and GDC sizes, it\n> -\t * is then expected that frames are dequeued from its main output\n> -\t * otherwise the system stalls.\n> +\t * If the ImgU gets configured it is then expected that buffers are\n> +\t * queued to its outputs, otherwise the next capture session\n> +\t * that uses the ImgU fails at queueing buffers at its input.\n>  \t *\n>  \t * If no ImgU configuration has been computed, it means only a RAW\n>  \t * stream has been requested: return here to skip the ImgU configuration\n> -- \n> 2.28.0\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","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 09182C3B5C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 29 Sep 2020 05:39:32 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8AE6E620BC;\n\tTue, 29 Sep 2020 07:39:31 +0200 (CEST)","from mail-lf1-x144.google.com (mail-lf1-x144.google.com\n\t[IPv6:2a00:1450:4864:20::144])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E502D6035E\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 29 Sep 2020 07:39:29 +0200 (CEST)","by mail-lf1-x144.google.com with SMTP id y11so4037559lfl.5\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 28 Sep 2020 22:39:29 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\ti5sm3153719lfe.8.2020.09.28.22.39.28\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 28 Sep 2020 22:39:28 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"HhHWercN\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=kEZRVBAlQ5RH60J44udi3dyZLm254pzPI/q4HNACP7k=;\n\tb=HhHWercNkfMCosSVq0BzqhQroawILs2+VnV3hR7l/HBoKNhkpd3CVpt+jAp1ei2e8S\n\txmLmgiZ1l3UVaLe22PikfEBkHHk/2bDk8znzjuzS49v7C4Fs/cHbHQhzXuXnTXASKtO2\n\t/k7tO784eL5rWAZckqAwES1+RvQ1Hme4pfwc0vHQDKv6ZtBaEcqSsVMWSzdQcAge7GNV\n\t1GL85/XCBCeZEr2VcRQOXm12ni1l/kY8uqEXTK6iYAfq9l3FQI/UF2gaf3HtgJI8ZSxX\n\txPj192xxjze3AAFUjKF4pu06nv/8c2kMtk/hGn5bmdRmgx4IW4ha/oQrKi9tGt9Be1Y2\n\t5Puw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=kEZRVBAlQ5RH60J44udi3dyZLm254pzPI/q4HNACP7k=;\n\tb=GWkR26Uf0vPnaOiCwwM74SGSaCMCqFpbgzGv8cNznZ+4ZP512lUdi1VmZkeEK/fGtH\n\t4YBTTgl5aWxyEVhtQtpEALsZ/IYJwa66dOHvM3vhpcl6GTVvdnsEXSZXlGXYfk4DE0YK\n\t+jlS5NXqTDo/ofp3JrPluaKT2uaP4zZ7Hz02bhqB4E5gN6L25OKW9vH5sUkdqnkvCXtI\n\tRnC+4tKtHqeGozTYIdnpvOf4sXcCYB6spWCbTC8Fj49qAtX/hiCPO/UlPYfhg8ANbc9U\n\tYRlSm6qDLoco7vX239WDTme3WPR8pqqsdl3yTHnLKhZ33uymXdudPfHUZCFL2Lk3jP23\n\t5I4g==","X-Gm-Message-State":"AOAM530dsL1otKHTihyxLPbDU6lWJyQ7BUvlIcHfUPp2T9nAFjnFhTXV\n\tDAwyBELIsul+pLKurYTtRporsQ==","X-Google-Smtp-Source":"ABdhPJyCTN2hG0lf9wx1SBRxLm5Ea/S41PzgQiX16qUtMU2Jb2zZLjJHX2ny02r1b5VAAP7N1GMe8g==","X-Received":"by 2002:ac2:5458:: with SMTP id d24mr545217lfn.446.1601357969300;\n\tMon, 28 Sep 2020 22:39:29 -0700 (PDT)","Date":"Tue, 29 Sep 2020 07:39:28 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Jacopo Mondi <jacopo@jmondi.org>","Message-ID":"<20200929053928.GG1052036@oden.dyn.berto.se>","References":"<20200924145143.117733-1-jacopo@jmondi.org>\n\t<20200924145143.117733-4-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200924145143.117733-4-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH 3/3] libcamera: ipu3: Adjust comment\n\ton ImgU configuration","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=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":12877,"web_url":"https://patchwork.libcamera.org/comment/12877/","msgid":"<20200929145638.emfej5yxtricwcfn@uno.localdomain>","date":"2020-09-29T14:56:38","subject":"Re: [libcamera-devel] [PATCH 3/3] libcamera: ipu3: Adjust comment\n\ton ImgU configuration","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Niklas, Laurent,\n\nOn Tue, Sep 29, 2020 at 07:39:28AM +0200, Niklas Söderlund wrote:\n> Hi Jacopo,\n>\n> Thanks for your work.\n>\n> On 2020-09-24 16:51:43 +0200, Jacopo Mondi wrote:\n> > Be more precise in commenting why the ImgU shall not be configured\n> > if only the RAW stream is requested.\n> >\n> > As an example, if the ImgU gets unecessary configured:\n> > cam -srole=viewfinder -c2 -C -> WORKS\n> > cam -srole=stillraw -c2 -C -> WORKS\n> > cam -srole=viewfinder -c2 -C -> Failed to queue buffer 0: Invalid argument\n> >\n> > Since commit 1bd2e981a209 (\"libcamera: ipu3: Fix RAW+YUV capture\")\n>\n> Unless you are _very_ lucky the commit hash will not be the same when\n> this patch is merged :-)\n>\n\nYou're very right!\n\n> > the ImgU configuration procedure also correctly implements the\n> > assumption that at least one of the YUV output is being operated. If\n> > that's not the case, as in the RAW-only capture use case, the code\n> > tries to access a non-existing configuration. One more reason to\n> > exit early if no YUV stream is requested.\n> >\n> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n>\n> With Laurent's comment addressed and the commit hash removed (or proof\n> of guaranteed hash collision ;-P)\n>\n> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n>\n\nThanks, with this fixed I'll now push the series\n\nThanks\n  j\n\n> > ---\n> >  src/libcamera/pipeline/ipu3/ipu3.cpp | 6 +++---\n> >  1 file changed, 3 insertions(+), 3 deletions(-)\n> >\n> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > index 477f85a45755..2f90ad2e2780 100644\n> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > @@ -459,9 +459,9 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> >  \t\treturn ret;\n> >\n> >  \t/*\n> > -\t * If the ImgU gets configured with proper IF, BDS and GDC sizes, it\n> > -\t * is then expected that frames are dequeued from its main output\n> > -\t * otherwise the system stalls.\n> > +\t * If the ImgU gets configured it is then expected that buffers are\n> > +\t * queued to its outputs, otherwise the next capture session\n> > +\t * that uses the ImgU fails at queueing buffers at its input.\n> >  \t *\n> >  \t * If no ImgU configuration has been computed, it means only a RAW\n> >  \t * stream has been requested: return here to skip the ImgU configuration\n> > --\n> > 2.28.0\n> >\n> > _______________________________________________\n> > libcamera-devel mailing list\n> > libcamera-devel@lists.libcamera.org\n> > https://lists.libcamera.org/listinfo/libcamera-devel\n>\n> --\n> Regards,\n> Niklas Söderlund","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 7300EC3B5B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 29 Sep 2020 14:52:45 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EB131621C9;\n\tTue, 29 Sep 2020 16:52:44 +0200 (CEST)","from relay10.mail.gandi.net (relay10.mail.gandi.net\n\t[217.70.178.230])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E96E160365\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 29 Sep 2020 16:52:43 +0200 (CEST)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay10.mail.gandi.net (Postfix) with ESMTPSA id 39F8E240004;\n\tTue, 29 Sep 2020 14:52:42 +0000 (UTC)"],"Date":"Tue, 29 Sep 2020 16:56:38 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Message-ID":"<20200929145638.emfej5yxtricwcfn@uno.localdomain>","References":"<20200924145143.117733-1-jacopo@jmondi.org>\n\t<20200924145143.117733-4-jacopo@jmondi.org>\n\t<20200929053928.GG1052036@oden.dyn.berto.se>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200929053928.GG1052036@oden.dyn.berto.se>","Subject":"Re: [libcamera-devel] [PATCH 3/3] libcamera: ipu3: Adjust comment\n\ton ImgU configuration","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=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":12878,"web_url":"https://patchwork.libcamera.org/comment/12878/","msgid":"<20200929150417.GD3936@pendragon.ideasonboard.com>","date":"2020-09-29T15:04:17","subject":"Re: [libcamera-devel] [PATCH 3/3] libcamera: ipu3: Adjust comment\n\ton ImgU configuration","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Tue, Sep 29, 2020 at 04:56:38PM +0200, Jacopo Mondi wrote:\n> On Tue, Sep 29, 2020 at 07:39:28AM +0200, Niklas Söderlund wrote:\n> > On 2020-09-24 16:51:43 +0200, Jacopo Mondi wrote:\n> > > Be more precise in commenting why the ImgU shall not be configured\n> > > if only the RAW stream is requested.\n> > >\n> > > As an example, if the ImgU gets unecessary configured:\n> > > cam -srole=viewfinder -c2 -C -> WORKS\n> > > cam -srole=stillraw -c2 -C -> WORKS\n> > > cam -srole=viewfinder -c2 -C -> Failed to queue buffer 0: Invalid argument\n> > >\n> > > Since commit 1bd2e981a209 (\"libcamera: ipu3: Fix RAW+YUV capture\")\n> >\n> > Unless you are _very_ lucky the commit hash will not be the same when\n> > this patch is merged :-)\n> \n> You're very right!\n> \n> > > the ImgU configuration procedure also correctly implements the\n> > > assumption that at least one of the YUV output is being operated. If\n> > > that's not the case, as in the RAW-only capture use case, the code\n> > > tries to access a non-existing configuration. One more reason to\n> > > exit early if no YUV stream is requested.\n> > >\n> > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> >\n> > With Laurent's comment addressed and the commit hash removed (or proof\n> > of guaranteed hash collision ;-P)\n> >\n> > Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> \n> Thanks, with this fixed I'll now push the series\n\nIf you push the first patch first then you can update this one with the\nright commit ID before pushing it.\n\n> > > ---\n> > >  src/libcamera/pipeline/ipu3/ipu3.cpp | 6 +++---\n> > >  1 file changed, 3 insertions(+), 3 deletions(-)\n> > >\n> > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > index 477f85a45755..2f90ad2e2780 100644\n> > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > @@ -459,9 +459,9 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> > >  \t\treturn ret;\n> > >\n> > >  \t/*\n> > > -\t * If the ImgU gets configured with proper IF, BDS and GDC sizes, it\n> > > -\t * is then expected that frames are dequeued from its main output\n> > > -\t * otherwise the system stalls.\n> > > +\t * If the ImgU gets configured it is then expected that buffers are\n> > > +\t * queued to its outputs, otherwise the next capture session\n> > > +\t * that uses the ImgU fails at queueing buffers at its input.\n> > >  \t *\n> > >  \t * If no ImgU configuration has been computed, it means only a RAW\n> > >  \t * stream has been requested: return here to skip the ImgU configuration","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 A0EEDC3B5C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 29 Sep 2020 15:05:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1687E621C7;\n\tTue, 29 Sep 2020 17:05:10 +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 1D6F360365\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 29 Sep 2020 17:05:08 +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 AFC7D540;\n\tTue, 29 Sep 2020 17:04:52 +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=\"cyM5Cxnc\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1601391892;\n\tbh=CluUY1+ODNM1JxdjaF2sscAK9+BwUG0U63/VuCR7bDM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=cyM5CxncJERFuWWBb01JVyX4ZvtUMv6VqzleKWxU4Lc4V4pytVhRoRdBkJKDad7ti\n\tSGR0DYZZhURL1bp8EwbTahWf9wrbX3+ZRARQf9t6g8yWpGombcmJHHYwGobvHyWZKG\n\tzVTV/fh6/DHxWmNvo4NdmKS25rRl+9z958ZXirhk=","Date":"Tue, 29 Sep 2020 18:04:17 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Message-ID":"<20200929150417.GD3936@pendragon.ideasonboard.com>","References":"<20200924145143.117733-1-jacopo@jmondi.org>\n\t<20200924145143.117733-4-jacopo@jmondi.org>\n\t<20200929053928.GG1052036@oden.dyn.berto.se>\n\t<20200929145638.emfej5yxtricwcfn@uno.localdomain>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200929145638.emfej5yxtricwcfn@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH 3/3] libcamera: ipu3: Adjust comment\n\ton ImgU configuration","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=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]