[{"id":1168,"web_url":"https://patchwork.libcamera.org/comment/1168/","msgid":"<20190402073810.jl6yacxkvt5g2qvb@uno.localdomain>","date":"2019-04-02T07:38:10","subject":"Re: [libcamera-devel] [RFC 1/5] cam: Rework how streams\n\tconfiguration is prepared","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Niklas,\n\nOn Tue, Apr 02, 2019 at 02:53:28AM +0200, Niklas Söderlund wrote:\n> In preparation of reworking how a default configuration is retrieved\n> from a camera separate preparation of stream configuration and applying\n> of it into to different functions. Reason for this is that preparation\n> of camera configuration will become more complex.\n>\n> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> ---\n>  src/cam/main.cpp | 35 ++++++++++++++++++++---------------\n>  1 file changed, 20 insertions(+), 15 deletions(-)\n>\n> diff --git a/src/cam/main.cpp b/src/cam/main.cpp\n> index e7490c32f99a01ad..cc5302ca4e72ea6f 100644\n> --- a/src/cam/main.cpp\n> +++ b/src/cam/main.cpp\n> @@ -78,27 +78,27 @@ static int parseOptions(int argc, char *argv[])\n>  \treturn 0;\n>  }\n>\n> -static int configureStreams(Camera *camera, std::set<Stream *> &streams)\n> +static int prepare_camera_config(std::map<Stream *, StreamConfiguration> *config)\n\nWhy have you here gone for name_with_underscores while the rest of the\ncode uses camelCaseNames ?\n\n>  {\n> -\tKeyValueParser::Options format = options[OptFormat];\n> -\tStream *id = *streams.begin();\n> -\n> -\tstd::map<Stream *, StreamConfiguration> config =\n> -\t\tcamera->streamConfiguration(streams);\n> +\tstd::set<Stream *> streams = camera->streams();\n> +\t*config = camera->streamConfiguration(streams);\n> +\tStream *stream = config->begin()->first;\n>\n>  \tif (options.isSet(OptFormat)) {\n> +\t\tKeyValueParser::Options format = options[OptFormat];\n> +\n>  \t\tif (format.isSet(\"width\"))\n> -\t\t\tconfig[id].width = format[\"width\"];\n> +\t\t\t(*config)[stream].width = format[\"width\"];\n>\n>  \t\tif (format.isSet(\"height\"))\n> -\t\t\tconfig[id].height = format[\"height\"];\n> +\t\t\t(*config)[stream].height = format[\"height\"];\n>\n>  \t\t/* TODO: Translate 4CC string to ID. */\n>  \t\tif (format.isSet(\"pixelformat\"))\n> -\t\t\tconfig[id].pixelFormat = format[\"pixelformat\"];\n> +\t\t\t(*config)[stream].pixelFormat = format[\"pixelformat\"];\n>  \t}\n>\n> -\treturn camera->configureStreams(config);\n> +\treturn 0;\n>  }\n>\n>  static void requestComplete(Request *request, const std::map<Stream *, Buffer *> &buffers)\n> @@ -136,18 +136,23 @@ static void requestComplete(Request *request, const std::map<Stream *, Buffer *>\n>\n>  static int capture()\n>  {\n> -\tint ret;\n> -\n> -\tstd::set<Stream *> streams = camera->streams();\n> +\tstd::map<Stream *, StreamConfiguration> config;\n>  \tstd::vector<Request *> requests;\n> +\tint ret;\n>\n> -\tret = configureStreams(camera.get(), streams);\n> +\tret = prepare_camera_config(&config);\n> +\tif (ret) {\n> +\t\tstd::cout << \"Failed to prepare camera configuration\" << std::endl;\n> +\t\treturn ret;\n> +\t}\n> +\n> +\tret = camera->configureStreams(config);\n>  \tif (ret < 0) {\n>  \t\tstd::cout << \"Failed to configure camera\" << std::endl;\n>  \t\treturn ret;\n>  \t}\n>\n> -\tStream *stream = *streams.begin();\n> +\tStream *stream = config.begin()->first;\n>\n>  \tret = camera->allocateBuffers();\n>  \tif (ret) {\n> --\n> 2.21.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":"<jacopo@jmondi.org>","Received":["from relay10.mail.gandi.net (relay10.mail.gandi.net\n\t[217.70.178.230])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 78F3E60DB3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  2 Apr 2019 09:37:25 +0200 (CEST)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay10.mail.gandi.net (Postfix) with ESMTPSA id 006FF240006;\n\tTue,  2 Apr 2019 07:37:24 +0000 (UTC)"],"Date":"Tue, 2 Apr 2019 09:38:10 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190402073810.jl6yacxkvt5g2qvb@uno.localdomain>","References":"<20190402005332.25018-1-niklas.soderlund@ragnatech.se>\n\t<20190402005332.25018-2-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"optjxonvs47ysehl\"","Content-Disposition":"inline","In-Reply-To":"<20190402005332.25018-2-niklas.soderlund@ragnatech.se>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [RFC 1/5] cam: Rework how streams\n\tconfiguration is prepared","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","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>","X-List-Received-Date":"Tue, 02 Apr 2019 07:37:25 -0000"}},{"id":1201,"web_url":"https://patchwork.libcamera.org/comment/1201/","msgid":"<20190402114618.GU23466@bigcity.dyn.berto.se>","date":"2019-04-02T11:46:18","subject":"Re: [libcamera-devel] [RFC 1/5] cam: Rework how streams\n\tconfiguration is prepared","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 feedback.\n\nOn 2019-04-02 09:38:10 +0200, Jacopo Mondi wrote:\n> Hi Niklas,\n> \n> On Tue, Apr 02, 2019 at 02:53:28AM +0200, Niklas Söderlund wrote:\n> > In preparation of reworking how a default configuration is retrieved\n> > from a camera separate preparation of stream configuration and applying\n> > of it into to different functions. Reason for this is that preparation\n> > of camera configuration will become more complex.\n> >\n> > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> > ---\n> >  src/cam/main.cpp | 35 ++++++++++++++++++++---------------\n> >  1 file changed, 20 insertions(+), 15 deletions(-)\n> >\n> > diff --git a/src/cam/main.cpp b/src/cam/main.cpp\n> > index e7490c32f99a01ad..cc5302ca4e72ea6f 100644\n> > --- a/src/cam/main.cpp\n> > +++ b/src/cam/main.cpp\n> > @@ -78,27 +78,27 @@ static int parseOptions(int argc, char *argv[])\n> >  \treturn 0;\n> >  }\n> >\n> > -static int configureStreams(Camera *camera, std::set<Stream *> &streams)\n> > +static int prepare_camera_config(std::map<Stream *, StreamConfiguration> *config)\n> \n> Why have you here gone for name_with_underscores while the rest of the\n> code uses camelCaseNames ?\n\nBecause camel_case is unnatural :-) Will remedy this in next version \nthanks for pointing it out.\n\n> \n> >  {\n> > -\tKeyValueParser::Options format = options[OptFormat];\n> > -\tStream *id = *streams.begin();\n> > -\n> > -\tstd::map<Stream *, StreamConfiguration> config =\n> > -\t\tcamera->streamConfiguration(streams);\n> > +\tstd::set<Stream *> streams = camera->streams();\n> > +\t*config = camera->streamConfiguration(streams);\n> > +\tStream *stream = config->begin()->first;\n> >\n> >  \tif (options.isSet(OptFormat)) {\n> > +\t\tKeyValueParser::Options format = options[OptFormat];\n> > +\n> >  \t\tif (format.isSet(\"width\"))\n> > -\t\t\tconfig[id].width = format[\"width\"];\n> > +\t\t\t(*config)[stream].width = format[\"width\"];\n> >\n> >  \t\tif (format.isSet(\"height\"))\n> > -\t\t\tconfig[id].height = format[\"height\"];\n> > +\t\t\t(*config)[stream].height = format[\"height\"];\n> >\n> >  \t\t/* TODO: Translate 4CC string to ID. */\n> >  \t\tif (format.isSet(\"pixelformat\"))\n> > -\t\t\tconfig[id].pixelFormat = format[\"pixelformat\"];\n> > +\t\t\t(*config)[stream].pixelFormat = format[\"pixelformat\"];\n> >  \t}\n> >\n> > -\treturn camera->configureStreams(config);\n> > +\treturn 0;\n> >  }\n> >\n> >  static void requestComplete(Request *request, const std::map<Stream *, Buffer *> &buffers)\n> > @@ -136,18 +136,23 @@ static void requestComplete(Request *request, const std::map<Stream *, Buffer *>\n> >\n> >  static int capture()\n> >  {\n> > -\tint ret;\n> > -\n> > -\tstd::set<Stream *> streams = camera->streams();\n> > +\tstd::map<Stream *, StreamConfiguration> config;\n> >  \tstd::vector<Request *> requests;\n> > +\tint ret;\n> >\n> > -\tret = configureStreams(camera.get(), streams);\n> > +\tret = prepare_camera_config(&config);\n> > +\tif (ret) {\n> > +\t\tstd::cout << \"Failed to prepare camera configuration\" << std::endl;\n> > +\t\treturn ret;\n> > +\t}\n> > +\n> > +\tret = camera->configureStreams(config);\n> >  \tif (ret < 0) {\n> >  \t\tstd::cout << \"Failed to configure camera\" << std::endl;\n> >  \t\treturn ret;\n> >  \t}\n> >\n> > -\tStream *stream = *streams.begin();\n> > +\tStream *stream = config.begin()->first;\n> >\n> >  \tret = camera->allocateBuffers();\n> >  \tif (ret) {\n> > --\n> > 2.21.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":"<niklas.soderlund@ragnatech.se>","Received":["from mail-lj1-x22d.google.com (mail-lj1-x22d.google.com\n\t[IPv6:2a00:1450:4864:20::22d])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B50CC60DB3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  2 Apr 2019 13:46:19 +0200 (CEST)","by mail-lj1-x22d.google.com with SMTP id l7so11267471ljg.6\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 02 Apr 2019 04:46:19 -0700 (PDT)","from localhost (89-233-230-99.cust.bredband2.com. [89.233.230.99])\n\tby smtp.gmail.com with ESMTPSA id\n\tx3sm2400009lfn.64.2019.04.02.04.46.18\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tTue, 02 Apr 2019 04:46:18 -0700 (PDT)"],"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\t:user-agent; bh=GPAMNzIxjD0rEUBrTIk2DKV0yPVZTmSN2aTZtFUm2Zg=;\n\tb=suIGx7oF/k85QE6xbHRn0P7m2qrb5TKt7A2JcWuFBZNiiOzg3rqPgZyLyF2mRES0M1\n\tOgzxdgc6kUk9EkWR5RFdxGGZkP5cEFOUsDjJZKDMkX0qp9ZreOW1+SIGZBXEbjU/nRUM\n\tSKeR9tDyDSz+mAxIbJpDv/Ls6tzwHbL1Z174bXcsOLVhqWO37UvdVp1C3KdmqOPpVu5x\n\tkT5cOmMXU5ncNqzNpwBCZ/stHFbPTxq8dfxV6wNsO0x7b/OBEN6+4BwLv6ChOS5pGZXS\n\tFGU7DOlgWPb0Hj5iisBtlEzZ1Hw54hcQRt/IQsum9b/flFfFVdpgYiDHyj4bLEx5o0el\n\t9Zcw==","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:user-agent;\n\tbh=GPAMNzIxjD0rEUBrTIk2DKV0yPVZTmSN2aTZtFUm2Zg=;\n\tb=l09DK6BxHVO6P1LEKLcZPkzSOioc9aZ7kwErmXuVnCOuNu+hU6dO7RzJlsLwM7rWuT\n\tyAid65d+UwVmha71mfcbNWQSSLEB8juyCjerRezD8usqAVhF480zi/JGzi03iHKqNaj0\n\tCAAD+LcRAAy+qw5Iaz0v5SgZsSt87uf3WD8E2FIkarJiiNbJ7Toyz727rqwG9E3te7/c\n\t5fI/2Wbpyjy2MXYjiq1vyg+h35SpzTGqtGXx9FfXG1PMzrsYB0nKoyMgbyilZ5atxNPK\n\tnpToB+O6bM+fthHAOmfTV03/NhcWOI10pp0vD9Kx/wzTeorUbGNPoc43amRVmMZdTIfH\n\tKnSQ==","X-Gm-Message-State":"APjAAAWXT0VSgEZWu+Tmf+qEcvHpI2im0dQSD2ABnn5H5aTNAY1AUkKT\n\tG7jZMwCDGmdycSwIzmggFhGyHw==","X-Google-Smtp-Source":"APXvYqzQ1QMqsPILIj+IYlwJulB7YWgiLMlrRxVhfXHjcBpDbbbntEHe0CjmGMVDfgHhxJevSf6LGQ==","X-Received":"by 2002:a2e:895a:: with SMTP id\n\tb26mr36864373ljk.89.1554205579238; \n\tTue, 02 Apr 2019 04:46:19 -0700 (PDT)","Date":"Tue, 2 Apr 2019 13:46:18 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190402114618.GU23466@bigcity.dyn.berto.se>","References":"<20190402005332.25018-1-niklas.soderlund@ragnatech.se>\n\t<20190402005332.25018-2-niklas.soderlund@ragnatech.se>\n\t<20190402073810.jl6yacxkvt5g2qvb@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190402073810.jl6yacxkvt5g2qvb@uno.localdomain>","User-Agent":"Mutt/1.11.3 (2019-02-01)","Subject":"Re: [libcamera-devel] [RFC 1/5] cam: Rework how streams\n\tconfiguration is prepared","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","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>","X-List-Received-Date":"Tue, 02 Apr 2019 11:46:20 -0000"}},{"id":1205,"web_url":"https://patchwork.libcamera.org/comment/1205/","msgid":"<20190402141623.GZ4805@pendragon.ideasonboard.com>","date":"2019-04-02T14:16:23","subject":"Re: [libcamera-devel] [RFC 1/5] cam: Rework how streams\n\tconfiguration is prepared","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 Tue, Apr 02, 2019 at 02:53:28AM +0200, Niklas Söderlund wrote:\n> In preparation of reworking how a default configuration is retrieved\n> from a camera separate preparation of stream configuration and applying\n> of it into to different functions. Reason for this is that preparation\n\ns/into to/into two/\n\n\"applying of it\" could also be replaced by \"application\".\n\n> of camera configuration will become more complex.\n> \n> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> ---\n>  src/cam/main.cpp | 35 ++++++++++++++++++++---------------\n>  1 file changed, 20 insertions(+), 15 deletions(-)\n> \n> diff --git a/src/cam/main.cpp b/src/cam/main.cpp\n> index e7490c32f99a01ad..cc5302ca4e72ea6f 100644\n> --- a/src/cam/main.cpp\n> +++ b/src/cam/main.cpp\n> @@ -78,27 +78,27 @@ static int parseOptions(int argc, char *argv[])\n>  \treturn 0;\n>  }\n>  \n> -static int configureStreams(Camera *camera, std::set<Stream *> &streams)\n> +static int prepare_camera_config(std::map<Stream *, StreamConfiguration> *config)\n>  {\n> -\tKeyValueParser::Options format = options[OptFormat];\n> -\tStream *id = *streams.begin();\n> -\n> -\tstd::map<Stream *, StreamConfiguration> config =\n> -\t\tcamera->streamConfiguration(streams);\n> +\tstd::set<Stream *> streams = camera->streams();\n> +\t*config = camera->streamConfiguration(streams);\n> +\tStream *stream = config->begin()->first;\n>  \n>  \tif (options.isSet(OptFormat)) {\n> +\t\tKeyValueParser::Options format = options[OptFormat];\n> +\n>  \t\tif (format.isSet(\"width\"))\n> -\t\t\tconfig[id].width = format[\"width\"];\n> +\t\t\t(*config)[stream].width = format[\"width\"];\n>  \n>  \t\tif (format.isSet(\"height\"))\n> -\t\t\tconfig[id].height = format[\"height\"];\n> +\t\t\t(*config)[stream].height = format[\"height\"];\n>  \n>  \t\t/* TODO: Translate 4CC string to ID. */\n>  \t\tif (format.isSet(\"pixelformat\"))\n> -\t\t\tconfig[id].pixelFormat = format[\"pixelformat\"];\n> +\t\t\t(*config)[stream].pixelFormat = format[\"pixelformat\"];\n>  \t}\n>  \n> -\treturn camera->configureStreams(config);\n> +\treturn 0;\n>  }\n>  \n>  static void requestComplete(Request *request, const std::map<Stream *, Buffer *> &buffers)\n> @@ -136,18 +136,23 @@ static void requestComplete(Request *request, const std::map<Stream *, Buffer *>\n>  \n>  static int capture()\n>  {\n> -\tint ret;\n> -\n> -\tstd::set<Stream *> streams = camera->streams();\n> +\tstd::map<Stream *, StreamConfiguration> config;\n>  \tstd::vector<Request *> requests;\n> +\tint ret;\n>  \n> -\tret = configureStreams(camera.get(), streams);\n> +\tret = prepare_camera_config(&config);\n\nWould it make sense to return the map by value instead of passing a\npointer to it ? Apart from that,\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\n> +\tif (ret) {\n> +\t\tstd::cout << \"Failed to prepare camera configuration\" << std::endl;\n> +\t\treturn ret;\n> +\t}\n> +\n> +\tret = camera->configureStreams(config);\n>  \tif (ret < 0) {\n>  \t\tstd::cout << \"Failed to configure camera\" << std::endl;\n>  \t\treturn ret;\n>  \t}\n>  \n> -\tStream *stream = *streams.begin();\n> +\tStream *stream = config.begin()->first;\n>  \n>  \tret = camera->allocateBuffers();\n>  \tif (ret) {","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["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 C55DB60DB3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  2 Apr 2019 16:16:35 +0200 (CEST)","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 262B32F9;\n\tTue,  2 Apr 2019 16:16:35 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1554214595;\n\tbh=WwFrXUgD2SZoqwHAqrXK85mykSDRC2KyS0V5R075xY4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=DFPd3NmM3q2yEMtD7Oi4dQ22VS7PoDLcg+ykoRPh/vGTC5fuouSxyZ77iUq0Zpl9F\n\tRxnjOcTc1kr3Cv6zIeLkRjy3KsLCq7P0kpfs5ojDvqpXF6axVw31i0Utfw4skzhrCz\n\tqDwhznieS+ZKHeB8EdR6VstDBVF16ozJJAkUdw8Q=","Date":"Tue, 2 Apr 2019 17:16:23 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190402141623.GZ4805@pendragon.ideasonboard.com>","References":"<20190402005332.25018-1-niklas.soderlund@ragnatech.se>\n\t<20190402005332.25018-2-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":"<20190402005332.25018-2-niklas.soderlund@ragnatech.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [RFC 1/5] cam: Rework how streams\n\tconfiguration is prepared","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","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>","X-List-Received-Date":"Tue, 02 Apr 2019 14:16:36 -0000"}},{"id":1231,"web_url":"https://patchwork.libcamera.org/comment/1231/","msgid":"<20190402184918.GG23466@bigcity.dyn.berto.se>","date":"2019-04-02T18:49:18","subject":"Re: [libcamera-devel] [RFC 1/5] cam: Rework how streams\n\tconfiguration is prepared","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 2019-04-02 17:16:23 +0300, Laurent Pinchart wrote:\n> Hi Niklas,\n> \n> Thank you for the patch.\n> \n> On Tue, Apr 02, 2019 at 02:53:28AM +0200, Niklas Söderlund wrote:\n> > In preparation of reworking how a default configuration is retrieved\n> > from a camera separate preparation of stream configuration and applying\n> > of it into to different functions. Reason for this is that preparation\n> \n> s/into to/into two/\n> \n> \"applying of it\" could also be replaced by \"application\".\n> \n> > of camera configuration will become more complex.\n> > \n> > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> > ---\n> >  src/cam/main.cpp | 35 ++++++++++++++++++++---------------\n> >  1 file changed, 20 insertions(+), 15 deletions(-)\n> > \n> > diff --git a/src/cam/main.cpp b/src/cam/main.cpp\n> > index e7490c32f99a01ad..cc5302ca4e72ea6f 100644\n> > --- a/src/cam/main.cpp\n> > +++ b/src/cam/main.cpp\n> > @@ -78,27 +78,27 @@ static int parseOptions(int argc, char *argv[])\n> >  \treturn 0;\n> >  }\n> >  \n> > -static int configureStreams(Camera *camera, std::set<Stream *> &streams)\n> > +static int prepare_camera_config(std::map<Stream *, StreamConfiguration> *config)\n> >  {\n> > -\tKeyValueParser::Options format = options[OptFormat];\n> > -\tStream *id = *streams.begin();\n> > -\n> > -\tstd::map<Stream *, StreamConfiguration> config =\n> > -\t\tcamera->streamConfiguration(streams);\n> > +\tstd::set<Stream *> streams = camera->streams();\n> > +\t*config = camera->streamConfiguration(streams);\n> > +\tStream *stream = config->begin()->first;\n> >  \n> >  \tif (options.isSet(OptFormat)) {\n> > +\t\tKeyValueParser::Options format = options[OptFormat];\n> > +\n> >  \t\tif (format.isSet(\"width\"))\n> > -\t\t\tconfig[id].width = format[\"width\"];\n> > +\t\t\t(*config)[stream].width = format[\"width\"];\n> >  \n> >  \t\tif (format.isSet(\"height\"))\n> > -\t\t\tconfig[id].height = format[\"height\"];\n> > +\t\t\t(*config)[stream].height = format[\"height\"];\n> >  \n> >  \t\t/* TODO: Translate 4CC string to ID. */\n> >  \t\tif (format.isSet(\"pixelformat\"))\n> > -\t\t\tconfig[id].pixelFormat = format[\"pixelformat\"];\n> > +\t\t\t(*config)[stream].pixelFormat = format[\"pixelformat\"];\n> >  \t}\n> >  \n> > -\treturn camera->configureStreams(config);\n> > +\treturn 0;\n> >  }\n> >  \n> >  static void requestComplete(Request *request, const std::map<Stream *, Buffer *> &buffers)\n> > @@ -136,18 +136,23 @@ static void requestComplete(Request *request, const std::map<Stream *, Buffer *>\n> >  \n> >  static int capture()\n> >  {\n> > -\tint ret;\n> > -\n> > -\tstd::set<Stream *> streams = camera->streams();\n> > +\tstd::map<Stream *, StreamConfiguration> config;\n> >  \tstd::vector<Request *> requests;\n> > +\tint ret;\n> >  \n> > -\tret = configureStreams(camera.get(), streams);\n> > +\tret = prepare_camera_config(&config);\n> \n> Would it make sense to return the map by value instead of passing a\n> pointer to it ? Apart from that,\n\nI have done so in the past in other places but I'm starting to dislike \nreturning an empty map to indicate an error. I will keep it as is for v1 \nbut I'm open to change this.\n\n> \n> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> \n> > +\tif (ret) {\n> > +\t\tstd::cout << \"Failed to prepare camera configuration\" << std::endl;\n> > +\t\treturn ret;\n> > +\t}\n> > +\n> > +\tret = camera->configureStreams(config);\n> >  \tif (ret < 0) {\n> >  \t\tstd::cout << \"Failed to configure camera\" << std::endl;\n> >  \t\treturn ret;\n> >  \t}\n> >  \n> > -\tStream *stream = *streams.begin();\n> > +\tStream *stream = config.begin()->first;\n> >  \n> >  \tret = camera->allocateBuffers();\n> >  \tif (ret) {\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"<niklas.soderlund@ragnatech.se>","Received":["from mail-lj1-x234.google.com (mail-lj1-x234.google.com\n\t[IPv6:2a00:1450:4864:20::234])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4A150610B6\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  2 Apr 2019 20:49:21 +0200 (CEST)","by mail-lj1-x234.google.com with SMTP id p14so12558010ljg.5\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 02 Apr 2019 11:49:21 -0700 (PDT)","from localhost (89-233-230-99.cust.bredband2.com. [89.233.230.99])\n\tby smtp.gmail.com with ESMTPSA id\n\tq5sm2560503lfn.85.2019.04.02.11.49.19\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tTue, 02 Apr 2019 11:49:19 -0700 (PDT)"],"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\t:user-agent; bh=FrU/sLTMwcKpgwpXBkEvmjosijK+c82zFv82SLsiK6A=;\n\tb=UO/lh9YZjiQX8a8M6lpn8xB+EsHbZ+aROfaz3adDd9o2mVb4krQFtCjzOXkXKqurc0\n\tDFwbxWAcyAv3rwMH516hSsaLpdQwZ8L2hnc8s3JDgOLmaDgg5X9p1rfbCDJ/U1VuICaD\n\tYoGCKw/q4YY6bi61b3AUYBqYqNbKX7zZA8YTfX3e7+mwNTjpgZSIDtynKKWoOyeljS2o\n\t9pPo6RFkmAXDtwk2SEkjpcUgYOaJWIIGvQQehamrQYOkHPVlo93/RjT73cGn4cGwaEPL\n\twddxccbrkXCO9of19DeUsaNeaapT+IoLf3vjFUP9fkARIZDsMcED5Ij0ZnLicTXl4g97\n\th10Q==","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:user-agent;\n\tbh=FrU/sLTMwcKpgwpXBkEvmjosijK+c82zFv82SLsiK6A=;\n\tb=qTcYoJQUOozkhKiZp5iKVQ9TOMYBQzfutqnTBxaqFxgB0Cx6Ay3XovZkoKjZuB9OZE\n\tB0ZDQA/DUc1zkz/dd0hKgE/d/MRpYQVwewsTbhlVbtegv4zjUOVAzTq5LUZIaTVpyyIf\n\tMzaMkKsQlQq88NiE4L2azHSvD51Nl1pNHTJwYZ52s2vmF4VAbRWXhj/GrjzLhzHzPMYt\n\tIP3DNr+9SCOyNDvlimn3gcUiatG9ceJtDyHMzkpaX9K9KJM7cNTWnO6ItA+iguoCqBZj\n\t3nbRYlBAPXVidhUfHKsML9mNZid44NUqHLHgWS2K9Mg3pJRrxxqGc7c1g5ywMrs7gtai\n\t4uLw==","X-Gm-Message-State":"APjAAAWfHU0T99wes+8RW2YA2qldM05t4QOCMl//iOk1ZLkv+s3vNoPI\n\tbgyNJAMxzUxZrw6dU4Dlwqk7ckf6MsA=","X-Google-Smtp-Source":"APXvYqxPvlFde8VZTTieRBQUmL6ixam3EpSDb43+ovD/wh6nJz2yiN8pEPm9LJl9p3wtC8h/X/WSzw==","X-Received":"by 2002:a2e:5056:: with SMTP id\n\tv22mr39998063ljd.153.1554230960696; \n\tTue, 02 Apr 2019 11:49:20 -0700 (PDT)","Date":"Tue, 2 Apr 2019 20:49:18 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190402184918.GG23466@bigcity.dyn.berto.se>","References":"<20190402005332.25018-1-niklas.soderlund@ragnatech.se>\n\t<20190402005332.25018-2-niklas.soderlund@ragnatech.se>\n\t<20190402141623.GZ4805@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":"<20190402141623.GZ4805@pendragon.ideasonboard.com>","User-Agent":"Mutt/1.11.3 (2019-02-01)","Subject":"Re: [libcamera-devel] [RFC 1/5] cam: Rework how streams\n\tconfiguration is prepared","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","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>","X-List-Received-Date":"Tue, 02 Apr 2019 18:49:21 -0000"}}]