[{"id":12584,"web_url":"https://patchwork.libcamera.org/comment/12584/","msgid":"<20200919114238.GQ1850958@oden.dyn.berto.se>","date":"2020-09-19T11:42:38","subject":"Re: [libcamera-devel] [PATCH 13/23] libcamera: IPAInterface: Remove\n\tall functions from IPAInterface","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Paul,\n\nThanks for your work.\n\nOn 2020-09-15 23:20:28 +0900, Paul Elder wrote:\n> Now that all the functions in the IPA interface are defined in the data\n> definition file and a specialized IPAInterface is generated per pipeline\n> handler, remove all the functions from the case IPAInterface.\n> \n> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>\n\nI'm sure a lot of documentation shall be removed as well as part of this \nchange. With that addressed,\n\nReviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n\n> ---\n>  include/libcamera/ipa/ipa_interface.h | 16 ----------------\n>  1 file changed, 16 deletions(-)\n> \n> diff --git a/include/libcamera/ipa/ipa_interface.h b/include/libcamera/ipa/ipa_interface.h\n> index 5016ec25..cbe325ea 100644\n> --- a/include/libcamera/ipa/ipa_interface.h\n> +++ b/include/libcamera/ipa/ipa_interface.h\n> @@ -151,22 +151,6 @@ class IPAInterface\n>  {\n>  public:\n>  \tvirtual ~IPAInterface() {}\n> -\n> -\tvirtual int init(const IPASettings &settings) = 0;\n> -\tvirtual int start() = 0;\n> -\tvirtual void stop() = 0;\n> -\n> -\tvirtual void configure(const CameraSensorInfo &sensorInfo,\n> -\t\t\t       const std::map<unsigned int, IPAStream> &streamConfig,\n> -\t\t\t       const std::map<unsigned int, const ControlInfoMap &> &entityControls,\n> -\t\t\t       const IPAOperationData &ipaConfig,\n> -\t\t\t       IPAOperationData *result) = 0;\n> -\n> -\tvirtual void mapBuffers(const std::vector<IPABuffer> &buffers) = 0;\n> -\tvirtual void unmapBuffers(const std::vector<unsigned int> &ids) = 0;\n> -\n> -\tvirtual void processEvent(const IPAOperationData &data) = 0;\n> -\tSignal<unsigned int, const IPAOperationData &> queueFrameAction;\n>  };\n>  \n>  } /* namespace libcamera */\n> -- \n> 2.27.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 6003ABF01C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSat, 19 Sep 2020 11:42:41 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2ECE362FBC;\n\tSat, 19 Sep 2020 13:42:41 +0200 (CEST)","from mail-lf1-x141.google.com (mail-lf1-x141.google.com\n\t[IPv6:2a00:1450:4864:20::141])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0C66660367\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 19 Sep 2020 13:42:40 +0200 (CEST)","by mail-lf1-x141.google.com with SMTP id y11so8955056lfl.5\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 19 Sep 2020 04:42:40 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\t68sm1222486ljj.135.2020.09.19.04.42.38\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tSat, 19 Sep 2020 04:42:39 -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=\"WzSN2N3H\"; 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=fop4/XHIeVzJAyYUlgTcatRmzSA6fgKFrfVZwvDPmfs=;\n\tb=WzSN2N3HdPfvVBps+Q8GjbBp9L19LKY4P0Hg1Cisap0bOhf3wGLi9OrjTM5fLnyTdl\n\tpeiFJIN1SbsJSYTPtw1J5h3VPAjGTUZWRhaQGxbc+zWvEM9sZvvtyDFAQgA5bn5lD6Va\n\tCghln6xcYgu5qeKVKvadx436B5J4MCpjJhGHmQKY+P0pmnVYg9Rg9B9kH54rn4VOAhfr\n\t4oe9wfpBgNiIEPNw0a74I7lqb3rDIbJCZuWr4aFQDXoVOKrR67PTpWM8yO06FBw4oGeL\n\tN8EH8Y0sXym3GNEWI++50mVV0eBV50BdDuVfBZzvOBqn/km2YbLKEKGWXQAhOCIA2tG8\n\tRtsQ==","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=fop4/XHIeVzJAyYUlgTcatRmzSA6fgKFrfVZwvDPmfs=;\n\tb=fVfjS72pCyFSeXpM/cBdww3I1H2WofHATVoxcW/VFqg7wms0gDKJUXIIBdG3ZB/4/e\n\t1FbTCpCdpPnX+64UyKW5QTY3kUuVS9XVm4ROtikdfTQhTdvDBDpixDEl4DYLq3ODB6kv\n\ttAVWl6GTII78KnhWRuxgyLFrbqeyzsa2dSem59gG8VXqvHyYJ+Y9Yf7bCvwF/1LTH0sK\n\ti0/6uvOb9TH22ygLo1NP8bNzQCd5sgW6YJ5MMqm/rNjnMmSUPN0j6wWOzvpKfsdh4hEa\n\ta43ObzgoCrSRH7GOtlATULcMCVNFYks/kmR7pP7kyq0wuRjwNqePmXx1k+yTEK+m0PUK\n\tVUyA==","X-Gm-Message-State":"AOAM5310W+7/Ayb5/0nQdAcQG7Jalh6qaJSlpUCyeYPCbMPNt5gGF0UD\n\tMyfOehCZdEMhxLvLVx2RVPo1Cw==","X-Google-Smtp-Source":"ABdhPJzp0Mh+flos6hx+UoJYiAEDRCWikGdIYBCA0v/f7SERBP7q12kzdAQ9Ic6XOtQVPeE84ot19Q==","X-Received":"by 2002:a19:8286:: with SMTP id\n\te128mr11584732lfd.307.1600515759504; \n\tSat, 19 Sep 2020 04:42:39 -0700 (PDT)","Date":"Sat, 19 Sep 2020 13:42:38 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Paul Elder <paul.elder@ideasonboard.com>","Message-ID":"<20200919114238.GQ1850958@oden.dyn.berto.se>","References":"<20200915142038.28757-1-paul.elder@ideasonboard.com>\n\t<20200915142038.28757-14-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200915142038.28757-14-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 13/23] libcamera: IPAInterface: Remove\n\tall functions from IPAInterface","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":12599,"web_url":"https://patchwork.libcamera.org/comment/12599/","msgid":"<20200921033220.GC45948@pyrite.rasen.tech>","date":"2020-09-21T03:32:20","subject":"Re: [libcamera-devel] [PATCH 13/23] libcamera: IPAInterface: Remove\n\tall functions from IPAInterface","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi Niklas,\n\nThank you for the review.\n\nOn Sat, Sep 19, 2020 at 01:42:38PM +0200, Niklas Söderlund wrote:\n> Hi Paul,\n> \n> Thanks for your work.\n> \n> On 2020-09-15 23:20:28 +0900, Paul Elder wrote:\n> > Now that all the functions in the IPA interface are defined in the data\n> > definition file and a specialized IPAInterface is generated per pipeline\n> > handler, remove all the functions from the case IPAInterface.\n> > \n> > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>\n> \n> I'm sure a lot of documentation shall be removed as well as part of this \n> change. With that addressed,\n\nYes, it will. I was, however, still wondering where to move it to. I\nwant to keep init(), start(), and stop() as required IPA functions, but\nthere doesn't seem to be a place to mandate that technically, since mojo\n(the language) doesn't support interface inheritance. I suppose I could\nspit out an error at the code generation stage if these functions don't\nexist. (Technically correctly we should intervene at the parsing stage,\nbut we don't want to touch mojo code since that would ruin ease of\nupdating)\n\nThe other functions will become pipeline-specific, so I would move the\ndocumentation to their mojom interface definition files... which means\nif a lot of pipelines use the same interface (as they do now), they'd\nall duplicate the interface documentation.\n\n\nPaul\n\n> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> \n> > ---\n> >  include/libcamera/ipa/ipa_interface.h | 16 ----------------\n> >  1 file changed, 16 deletions(-)\n> > \n> > diff --git a/include/libcamera/ipa/ipa_interface.h b/include/libcamera/ipa/ipa_interface.h\n> > index 5016ec25..cbe325ea 100644\n> > --- a/include/libcamera/ipa/ipa_interface.h\n> > +++ b/include/libcamera/ipa/ipa_interface.h\n> > @@ -151,22 +151,6 @@ class IPAInterface\n> >  {\n> >  public:\n> >  \tvirtual ~IPAInterface() {}\n> > -\n> > -\tvirtual int init(const IPASettings &settings) = 0;\n> > -\tvirtual int start() = 0;\n> > -\tvirtual void stop() = 0;\n> > -\n> > -\tvirtual void configure(const CameraSensorInfo &sensorInfo,\n> > -\t\t\t       const std::map<unsigned int, IPAStream> &streamConfig,\n> > -\t\t\t       const std::map<unsigned int, const ControlInfoMap &> &entityControls,\n> > -\t\t\t       const IPAOperationData &ipaConfig,\n> > -\t\t\t       IPAOperationData *result) = 0;\n> > -\n> > -\tvirtual void mapBuffers(const std::vector<IPABuffer> &buffers) = 0;\n> > -\tvirtual void unmapBuffers(const std::vector<unsigned int> &ids) = 0;\n> > -\n> > -\tvirtual void processEvent(const IPAOperationData &data) = 0;\n> > -\tSignal<unsigned int, const IPAOperationData &> queueFrameAction;\n> >  };\n> >  \n> >  } /* namespace libcamera */\n> > -- \n> > 2.27.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 776D3C3B5B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 21 Sep 2020 03:32:31 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id F35F862FBF;\n\tMon, 21 Sep 2020 05:32:30 +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 59D9060367\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 21 Sep 2020 05:32:29 +0200 (CEST)","from pyrite.rasen.tech (unknown\n\t[IPv6:2400:4051:61:600:2c71:1b79:d06d:5032])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 9B72927B;\n\tMon, 21 Sep 2020 05:32:27 +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=\"eD8y5bRl\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1600659148;\n\tbh=lMYPChwBBxRhe9+8QhX1fFyYo5wMIQTzFPV5kA8+Exo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=eD8y5bRl++DlUeQouX9Ft/RiZgiq1+mGtCijX4z5QXKKMjvYNpw77l0mCqBYh+i6n\n\tDkvDkZsnkr3peSTqoSPa047JvnKBGLbWeWhivPhf+cuM6PXCouyfv2LE48VDuSn+4F\n\tZ3s+f8QgWYJQEVWknTazsv+ySwBUucCON5BApw10=","Date":"Mon, 21 Sep 2020 12:32:20 +0900","From":"paul.elder@ideasonboard.com","To":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","Message-ID":"<20200921033220.GC45948@pyrite.rasen.tech>","References":"<20200915142038.28757-1-paul.elder@ideasonboard.com>\n\t<20200915142038.28757-14-paul.elder@ideasonboard.com>\n\t<20200919114238.GQ1850958@oden.dyn.berto.se>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200919114238.GQ1850958@oden.dyn.berto.se>","Subject":"Re: [libcamera-devel] [PATCH 13/23] libcamera: IPAInterface: Remove\n\tall functions from IPAInterface","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>"}}]