[{"id":3933,"web_url":"https://patchwork.libcamera.org/comment/3933/","msgid":"<20200304232836.GD28814@pendragon.ideasonboard.com>","date":"2020-03-04T23:28:36","subject":"Re: [libcamera-devel] [PATCH v3 4/7] libcamera: V4L2BufferCache:\n\tUse the entry reference","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nThank you for the patch.\n\nOn Thu, Mar 05, 2020 at 12:22:43AM +0100, Niklas Söderlund wrote:\n> Instead of looking up the index in the storage vector use the reference\n> to it created at the beginning of the loop.\n> \n> Signed-off-by: Niklas Söderlund \n\nThis could have been fixed in the next patches, when modifying the code.\nAre you trying to improve your patch count ? ;-)\n\nReviewed-by: Laurent Pinchart \n\n> ---\n> src/libcamera/v4l2_videodevice.cpp | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp\n> index d88cb0bd0771e545..268de60bc7965f58 100644\n> --- a/src/libcamera/v4l2_videodevice.cpp\n> +++ b/src/libcamera/v4l2_videodevice.cpp\n> @@ -216,7 +216,7 @@ int V4L2BufferCache::get(const FrameBuffer &buffer)\n> \t\t\tuse = index;\n> \n> \t\t/* Try to find a cache hit by comparing the planes. */\n> -\t\tif (cache_[index] == buffer) {\n> +\t\tif (entry == buffer) {\n> \t\t\thit = true;\n> \t\t\tuse = index;\n> \t\t\tbreak;","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id EB61762734\n\tfor ;\n\tThu, 5 Mar 2020 00:28:38 +0100 (CET)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7137533E;\n\tThu, 5 Mar 2020 00:28:38 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1583364518;\n\tbh=xlLBdkEMQ/3mMkxjeTSP+pE/gKEVkysqcjGalFb2mPA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=YRXAIM1b2uMupK3hMFw1dbmpwLvcwKnugTPEwrHRMoRjHslwvhhklbzPgYI3pGfR3\n\tliJapJTca7iGOStAFZZLrc3D8m5tq4u/prDj3gwKugw10cvG5UnmrceB542oRznEmY\n\tHnxmQn8FqwtqL2QRTQFIR6sEgAPN/Gm2BwBJWuyQ=","Date":"Thu, 5 Mar 2020 01:28:36 +0200","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200304232836.GD28814@pendragon.ideasonboard.com>","References":"<20200304232246.325384-1-niklas.soderlund@ragnatech.se>\n\t<20200304232246.325384-5-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20200304232246.325384-5-niklas.soderlund@ragnatech.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v3 4/7] libcamera: V4L2BufferCache:\n\tUse the entry reference","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","X-List-Received-Date":"Wed, 04 Mar 2020 23:28:39 -0000"}},{"id":3949,"web_url":"https://patchwork.libcamera.org/comment/3949/","msgid":"<20200305200820.GA2490871@oden.dyn.berto.se>","date":"2020-03-05T20:08:20","subject":"Re: [libcamera-devel] [PATCH v3 4/7] libcamera: V4L2BufferCache:\n\tUse the entry reference","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nThanks for your feedback.\n\nOn 2020-03-05 01:28:36 +0200, Laurent Pinchart wrote:\n> Hi Niklas,\n> \n> Thank you for the patch.\n> \n> On Thu, Mar 05, 2020 at 12:22:43AM +0100, Niklas Söderlund wrote:\n> > Instead of looking up the index in the storage vector use the reference\n> > to it created at the beginning of the loop.\n> > \n> > Signed-off-by: Niklas Söderlund \n> \n> This could have been fixed in the next patches, when modifying the code.\n> Are you trying to improve your patch count ? ;-)\n\nIf I wanted to inflate my patch count I know of more efficient ways to \ndo so then this ;-P\n\nI write patches as I would like them to read them if I where a reviewer.\n\nFixing things around the real change in the patch is a red flag for me \nwhen reviewing. I have to spend time reviewing such patches by \nquestioning myself as I cant see why the change is needed only later to \nfind out that that line is changed just because we are modifying things \nin the area anyhow. So whenever I find my self starting to describe more \nthen one change in the commit message I try break it out to it's own \npatch, even if it's trivial.\n\n> \n> Reviewed-by: Laurent Pinchart \n> \n> > ---\n> > src/libcamera/v4l2_videodevice.cpp | 2 +-\n> > 1 file changed, 1 insertion(+), 1 deletion(-)\n> > \n> > diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp\n> > index d88cb0bd0771e545..268de60bc7965f58 100644\n> > --- a/src/libcamera/v4l2_videodevice.cpp\n> > +++ b/src/libcamera/v4l2_videodevice.cpp\n> > @@ -216,7 +216,7 @@ int V4L2BufferCache::get(const FrameBuffer &buffer)\n> > \t\t\tuse = index;\n> > \n> > \t\t/* Try to find a cache hit by comparing the planes. */\n> > -\t\tif (cache_[index] == buffer) {\n> > +\t\tif (entry == buffer) {\n> > \t\t\thit = true;\n> > \t\t\tuse = index;\n> > \t\t\tbreak;\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"","Received":["from mail-lf1-x143.google.com (mail-lf1-x143.google.com\n\t[IPv6:2a00:1450:4864:20::143])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id AF3E360427\n\tfor ;\n\tThu, 5 Mar 2020 21:08:23 +0100 (CET)","by mail-lf1-x143.google.com with SMTP id j11so5706540lfg.4\n\tfor ;\n\tThu, 05 Mar 2020 12:08:23 -0800 (PST)","from localhost (h-200-138.A463.priv.bahnhof.se. [176.10.200.138])\n\tby smtp.gmail.com with ESMTPSA id\n\t142sm5621077ljf.101.2020.03.05.12.08.21\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tThu, 05 Mar 2020 12:08:21 -0800 (PST)"],"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=C/8HEWCTrlR3ayilCt2qJj15XVwm5UPppHDcJyW9H3Q=;\n\tb=WoTcO6O0lETDLvtmgrAIIT5rqipbfCgzIwySRoPSm/whNDWydq1RDtptIWcFMscY+K\n\tBOLDhcodQUEM9T1amT5m+Q6rMMCpV0KagKQnzHhp8CneimG8hFqBYGjJKk4tRUDcZKQ7\n\tWQDK/IRdt9bi4qgaowjd3XDmXsAsqEmF5l4FcSGdbatyviYji5CULfWVMGXCW9z2KDWy\n\tR3rRSuWFc6sMHuouwCQQ1InXJYS9R8M5qf3Me+0+Re3TSeotH7WEGK1N283UVdl+vum1\n\tzS1Z8T9hBZzKXIN1tmpze6mewg8IqwO/2vcOFJQgl6SatoN1ucNrs6Myb1RJCmmWpFgP\n\t67OQ==","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=C/8HEWCTrlR3ayilCt2qJj15XVwm5UPppHDcJyW9H3Q=;\n\tb=IxtI0YuomEVtqFGa4rFHH6eSURsIBmh2sTfQec+C8n3UAHPOeVTUy8ByfXd1BGpLNH\n\tbjItJYIfLruSEzy1WodGGJ6TfudEnAEgl4HM4ZdMI/IKqlb8OVUgxY0tEF4d4QMm3gXZ\n\t249FiiZoJ7vA/2huijpqVebtHL2Iziyajy8Y7odfEYoruL3cb60fm2szxAXbjqrwSF5s\n\tufcOWlg1T6fBo0AwfN+JfPf8O+kKK0B0MT5fNQC2ZLorms+thBbPFDKFAtl2uF4YR5xp\n\tBWpKBxINdVGnVK3RMBbBhk6IPWafOKjHvak5iz6K1+HsJFZC/G7U9PMUdh3a3hJ879cr\n\tHt5g==","X-Gm-Message-State":"ANhLgQ1Bb4D1Xvc3ai4DuOjQQ9EKJgosyy724ymQ4ekevnBg4SwbPSbC\n\tlLR5BvgWrCF1kUJqXYaHIXit6+7W1r8=","X-Google-Smtp-Source":"ADFU+vuGys3KSC5y3476KVX4d/q0TcEvlaGxzkF7u8FeBlPP4b1VztggEFqQnExJ+NjNVCVR3Nx4+A==","X-Received":"by 2002:a05:6512:68b:: with SMTP id\n\tt11mr159749lfe.214.1583438902826; \n\tThu, 05 Mar 2020 12:08:22 -0800 (PST)","Date":"Thu, 5 Mar 2020 21:08:20 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200305200820.GA2490871@oden.dyn.berto.se>","References":"<20200304232246.325384-1-niklas.soderlund@ragnatech.se>\n\t<20200304232246.325384-5-niklas.soderlund@ragnatech.se>\n\t<20200304232836.GD28814@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20200304232836.GD28814@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 4/7] libcamera: V4L2BufferCache:\n\tUse the entry reference","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","X-List-Received-Date":"Thu, 05 Mar 2020 20:08:23 -0000"}},{"id":3950,"web_url":"https://patchwork.libcamera.org/comment/3950/","msgid":"<20200305201403.GD4722@pendragon.ideasonboard.com>","date":"2020-03-05T20:14:03","subject":"Re: [libcamera-devel] [PATCH v3 4/7] libcamera: V4L2BufferCache:\n\tUse the entry reference","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nOn Thu, Mar 05, 2020 at 09:08:20PM +0100, Niklas Söderlund wrote:\n> On 2020-03-05 01:28:36 +0200, Laurent Pinchart wrote:\n> > On Thu, Mar 05, 2020 at 12:22:43AM +0100, Niklas Söderlund wrote:\n> > > Instead of looking up the index in the storage vector use the reference\n> > > to it created at the beginning of the loop.\n> > > \n> > > Signed-off-by: Niklas Söderlund \n> > \n> > This could have been fixed in the next patches, when modifying the code.\n> > Are you trying to improve your patch count ? ;-)\n> \n> If I wanted to inflate my patch count I know of more efficient ways to \n> do so then this ;-P\n> \n> I write patches as I would like them to read them if I where a reviewer.\n> \n> Fixing things around the real change in the patch is a red flag for me \n> when reviewing. I have to spend time reviewing such patches by \n> questioning myself as I cant see why the change is needed only later to \n> find out that that line is changed just because we are modifying things \n> in the area anyhow. So whenever I find my self starting to describe more \n> then one change in the commit message I try break it out to it's own \n> patch, even if it's trivial.\n\nHence the \"While at it, ...\" mentions in the commit message :-) Without\nthem I agree it can be confusing.\n\n> > Reviewed-by: Laurent Pinchart \n> > \n> > > ---\n> > > src/libcamera/v4l2_videodevice.cpp | 2 +-\n> > > 1 file changed, 1 insertion(+), 1 deletion(-)\n> > > \n> > > diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp\n> > > index d88cb0bd0771e545..268de60bc7965f58 100644\n> > > --- a/src/libcamera/v4l2_videodevice.cpp\n> > > +++ b/src/libcamera/v4l2_videodevice.cpp\n> > > @@ -216,7 +216,7 @@ int V4L2BufferCache::get(const FrameBuffer &buffer)\n> > > \t\t\tuse = index;\n> > > \n> > > \t\t/* Try to find a cache hit by comparing the planes. */\n> > > -\t\tif (cache_[index] == buffer) {\n> > > +\t\tif (entry == buffer) {\n> > > \t\t\thit = true;\n> > > \t\t\tuse = index;\n> > > \t\t\tbreak;","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 04FE160427\n\tfor ;\n\tThu, 5 Mar 2020 21:14:08 +0100 (CET)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 6979C312;\n\tThu, 5 Mar 2020 21:14:07 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1583439247;\n\tbh=XRP6golCgxVftOE7Qr3Ynmz1UdMoJQfkoR45CQBiGOE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=df5QtL2HqaFzzVSpauF6NsLHNdVa1s0Bw5oqYJZQi/4mTzEOmP7Twu2lUleS7x9l7\n\tBc7xvaNHutNvD5x9lIvqDBZmYp/pJAd9CPCCL+E/w5RCbnkEaPpG4oL60mGTV4iU3o\n\tNNM7bzUav5z8PEVklCl+vJcuofcxVvpbJdAjQNgY=","Date":"Thu, 5 Mar 2020 22:14:03 +0200","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200305201403.GD4722@pendragon.ideasonboard.com>","References":"<20200304232246.325384-1-niklas.soderlund@ragnatech.se>\n\t<20200304232246.325384-5-niklas.soderlund@ragnatech.se>\n\t<20200304232836.GD28814@pendragon.ideasonboard.com>\n\t<20200305200820.GA2490871@oden.dyn.berto.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20200305200820.GA2490871@oden.dyn.berto.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v3 4/7] libcamera: V4L2BufferCache:\n\tUse the entry reference","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","X-List-Received-Date":"Thu, 05 Mar 2020 20:14:08 -0000"}}]