[{"id":1268,"web_url":"https://patchwork.libcamera.org/comment/1268/","msgid":"<20190405082648.vwghsikdaaqw3gor@uno.localdomain>","date":"2019-04-05T08:26:48","subject":"Re: [libcamera-devel] [PATCH v2 5/8] libcamera: stream: Add basic\n\tstream usages","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Niklas,\n   thank you for the patch\n\nOn Fri, Apr 05, 2019 at 04:02:53AM +0200, Niklas Söderlund wrote:\n> In preparation of reworking how a default configuration is retrieved\n> from a camera add stream usages. The usages will be used by applications\n> to describe how they intend to use a camera and replace the Stream IDs\n> when retrieving default configuration from the camera using\n> streamConfiguration().\n>\n> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> ---\n>  include/libcamera/stream.h | 40 +++++++++++++++++\n>  src/libcamera/stream.cpp   | 92 ++++++++++++++++++++++++++++++++++++++\n>  2 files changed, 132 insertions(+)\n>\n> diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h\n> index 970c479627fab064..1dea696f2e5ca95d 100644\n> --- a/include/libcamera/stream.h\n> +++ b/include/libcamera/stream.h\n> @@ -8,6 +8,7 @@\n>  #define __LIBCAMERA_STREAM_H__\n>\n>  #include <libcamera/buffer.h>\n> +#include <libcamera/geometry.h>\n>\n>  namespace libcamera {\n>\n> @@ -21,9 +22,48 @@ struct StreamConfiguration {\n>  \tunsigned int bufferCount;\n>  };\n>\n> +class StreamUsage\n> +{\n> +public:\n> +\tenum Role {\n> +\t\tStillCapture,\n> +\t\tVideoRecording,\n> +\t\tViewfinder,\n> +\t};\n> +\n> +\tRole role() const { return role_; }\n> +\tSize size() const { return size_; }\n\nMaybe not that relevant as Size is just two integers, but\n        Size &size() const\nor maybe even\n        const Size &size() const\n\nAlso\n        const Role role() const\n> +\n> +protected:\n> +\tStreamUsage(Role role);\n\nexplicit\n\n> +\tStreamUsage(Role role, int width, int height);\n> +\n> +private:\n> +\tRole role_;\n> +\tSize size_;\n> +};\n> +\n>  class Stream final\n>  {\n>  public:\n> +\tclass StillCapture : public StreamUsage\n> +\t{\n> +\tpublic:\n> +\t\tStillCapture();\n> +\t};\n> +\n> +\tclass VideoRecording : public StreamUsage\n> +\t{\n> +\tpublic:\n> +\t\tVideoRecording();\n> +\t};\n> +\n> +\tclass Viewfinder : public StreamUsage\n> +\t{\n> +\tpublic:\n> +\t\texplicit Viewfinder(int width, int height);\n\nNo need for explicit, it takes two arguments.\n\n> +\t};\n> +\n>  \tStream();\n>  \tBufferPool &bufferPool() { return bufferPool_; }\n>  \tconst StreamConfiguration &configuration() const { return configuration_; }\n> diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp\n> index c4943c91b2e6ce13..816dff2d12acd923 100644\n> --- a/src/libcamera/stream.cpp\n> +++ b/src/libcamera/stream.cpp\n> @@ -60,6 +60,65 @@ namespace libcamera {\n>   * \\brief Requested number of buffers to allocate for the stream\n>   */\n>\n> +/**\n> + * \\class StreamUsage\n> + * \\brief Stream usage information\n> + *\n> + * The StreamUsage class describes how an application intends to use a stream.\n> + * Usages are specified by applications and passed to cameras, that then select\n> + * the most appropriate streams and their default configurations.\n> + */\n> +\n> +/**\n> + * \\enum StreamUsage::Role\n> + * \\brief Identify the role a stream is intended to play\n> + * \\var StreamUsage::StillCapture\n> + * The stream is intended to capture high-resolution, high-quality still images\n> + * with low frame rate. The captured frames may be exposed with flash.\n> + * \\var StreamUsage::VideoRecording\n> + * The stream is intended to capture video for the purpose of recording or\n> + * streaming. The video stream may produce a high frame rate and may be\n> + * enhanced with video stabilization.\n> + * \\var StreamUsage::Viewfinder\n> + * The stream is intended to capture video for the purpose of display on the\n> + * local screen. The StreamUsage includes the desired resolution. Trade-offs\n> + * between quality and usage of system resources are acceptable.\n> + */\n> +\n> +/**\n> + * \\fn StreamUsage::role()\n> + * \\brief Retrieve the stream role\n> + *\n> + * \\return The stream role\n> + */\n> +\n> +/**\n> + * \\fn StreamUsage::size()\n> + * \\brief Retrieve desired size\n> + *\n> + * \\return The desired size\n> + */\n> +\n> +/**\n> + * \\brief Create a stream usage\n\nshould you somehow mention \"a stream usage with sizes = 0\" ?\n\n> + * \\param[in] role Stream role\n> + */\n> +StreamUsage::StreamUsage(Role role)\n> +\t: role_(role), size_(Size(0, 0))\n\nSize default contructor initialize both fields to 0, so you can drop\nthe assignement.\n\n> +{\n> +}\n> +\n> +/**\n> + * \\brief Create a stream usage with size hint\n> + * \\param[in] role Stream role\n> + * \\param[in] width Desired width\n> + * \\param[in] height Desired height\n\n\"The desired\" ?\n\n> + */\n> +StreamUsage::StreamUsage(Role role, int width, int height)\n> +\t: role_(role), size_(Size(width, height))\n> +{\n> +}\n> +\n>  /**\n>   * \\class Stream\n>   * \\brief Video stream for a camera\n> @@ -78,6 +137,39 @@ namespace libcamera {\n>   * optimal stream for the task.\n>   */\n>\n> +/**\n> + * \\class Stream::StillCapture\n> + * \\brief Describe a still capture usage\n> + */\n> +Stream::StillCapture::StillCapture()\n> +\t: StreamUsage(Role::StillCapture)\n> +{\n> +}\n> +\n> +/**\n> + * \\class Stream::VideoRecording\n> + * \\brief Describe a video recording usage\n> + */\n> +Stream::VideoRecording::VideoRecording()\n> +\t: StreamUsage(Role::VideoRecording)\n> +{\n> +}\n> +\n> +/**\n> + * \\class Stream::Viewfinder\n> + * \\brief Describe a viewfinder usage\n> + */\n> +\n> +/**\n> + * \\brief Create a viewfinder usage with dimension hints\n\nNit: you have used \"size\" everywhere in place of dimension if I'm not\nwrong. I like dimension too though.\n\nThanks\n   j\n\n> + * \\param[in] width Desired viewfinder width\n> + * \\param[in] height Desired viewfinder height\n> + */\n> +Stream::Viewfinder::Viewfinder(int width, int height)\n> +\t: StreamUsage(Role::Viewfinder, width, height)\n> +{\n> +}\n> +\n>  /**\n>   * \\brief Construct a stream with default parameters\n>   */\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 relay6-d.mail.gandi.net (relay6-d.mail.gandi.net\n\t[217.70.183.198])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5607660DB2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri,  5 Apr 2019 10:26:03 +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 relay6-d.mail.gandi.net (Postfix) with ESMTPSA id D0CF2C0007;\n\tFri,  5 Apr 2019 08:26:01 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Fri, 5 Apr 2019 10:26:48 +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":"<20190405082648.vwghsikdaaqw3gor@uno.localdomain>","References":"<20190405020256.22520-1-niklas.soderlund@ragnatech.se>\n\t<20190405020256.22520-6-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"2dlc334f34solk6n\"","Content-Disposition":"inline","In-Reply-To":"<20190405020256.22520-6-niklas.soderlund@ragnatech.se>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH v2 5/8] libcamera: stream: Add basic\n\tstream usages","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":"Fri, 05 Apr 2019 08:26:03 -0000"}},{"id":1269,"web_url":"https://patchwork.libcamera.org/comment/1269/","msgid":"<20190405094320.GR23466@bigcity.dyn.berto.se>","date":"2019-04-05T09:43:20","subject":"Re: [libcamera-devel] [PATCH v2 5/8] libcamera: stream: Add basic\n\tstream usages","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-05 10:26:48 +0200, Jacopo Mondi wrote:\n> Hi Niklas,\n>    thank you for the patch\n> \n> On Fri, Apr 05, 2019 at 04:02:53AM +0200, Niklas Söderlund wrote:\n> > In preparation of reworking how a default configuration is retrieved\n> > from a camera add stream usages. The usages will be used by applications\n> > to describe how they intend to use a camera and replace the Stream IDs\n> > when retrieving default configuration from the camera using\n> > streamConfiguration().\n> >\n> > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> > ---\n> >  include/libcamera/stream.h | 40 +++++++++++++++++\n> >  src/libcamera/stream.cpp   | 92 ++++++++++++++++++++++++++++++++++++++\n> >  2 files changed, 132 insertions(+)\n> >\n> > diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h\n> > index 970c479627fab064..1dea696f2e5ca95d 100644\n> > --- a/include/libcamera/stream.h\n> > +++ b/include/libcamera/stream.h\n> > @@ -8,6 +8,7 @@\n> >  #define __LIBCAMERA_STREAM_H__\n> >\n> >  #include <libcamera/buffer.h>\n> > +#include <libcamera/geometry.h>\n> >\n> >  namespace libcamera {\n> >\n> > @@ -21,9 +22,48 @@ struct StreamConfiguration {\n> >  \tunsigned int bufferCount;\n> >  };\n> >\n> > +class StreamUsage\n> > +{\n> > +public:\n> > +\tenum Role {\n> > +\t\tStillCapture,\n> > +\t\tVideoRecording,\n> > +\t\tViewfinder,\n> > +\t};\n> > +\n> > +\tRole role() const { return role_; }\n> > +\tSize size() const { return size_; }\n> \n> Maybe not that relevant as Size is just two integers, but\n>         Size &size() const\n> or maybe even\n>         const Size &size() const\n\nI have no strong opinion, I will make it so in next version.\n\n> \n> Also\n>         const Role role() const\n\nAs this returns a value and not a reference this can not be const :-)\n\n> > +\n> > +protected:\n> > +\tStreamUsage(Role role);\n> \n> explicit\n> \n> > +\tStreamUsage(Role role, int width, int height);\n> > +\n> > +private:\n> > +\tRole role_;\n> > +\tSize size_;\n> > +};\n> > +\n> >  class Stream final\n> >  {\n> >  public:\n> > +\tclass StillCapture : public StreamUsage\n> > +\t{\n> > +\tpublic:\n> > +\t\tStillCapture();\n> > +\t};\n> > +\n> > +\tclass VideoRecording : public StreamUsage\n> > +\t{\n> > +\tpublic:\n> > +\t\tVideoRecording();\n> > +\t};\n> > +\n> > +\tclass Viewfinder : public StreamUsage\n> > +\t{\n> > +\tpublic:\n> > +\t\texplicit Viewfinder(int width, int height);\n> \n> No need for explicit, it takes two arguments.\n\nWops, my intention was for this to be on StreamRole, somthing went very \nwrong :-) Thanks for spotting it!\n\n> \n> > +\t};\n> > +\n> >  \tStream();\n> >  \tBufferPool &bufferPool() { return bufferPool_; }\n> >  \tconst StreamConfiguration &configuration() const { return configuration_; }\n> > diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp\n> > index c4943c91b2e6ce13..816dff2d12acd923 100644\n> > --- a/src/libcamera/stream.cpp\n> > +++ b/src/libcamera/stream.cpp\n> > @@ -60,6 +60,65 @@ namespace libcamera {\n> >   * \\brief Requested number of buffers to allocate for the stream\n> >   */\n> >\n> > +/**\n> > + * \\class StreamUsage\n> > + * \\brief Stream usage information\n> > + *\n> > + * The StreamUsage class describes how an application intends to use a stream.\n> > + * Usages are specified by applications and passed to cameras, that then select\n> > + * the most appropriate streams and their default configurations.\n> > + */\n> > +\n> > +/**\n> > + * \\enum StreamUsage::Role\n> > + * \\brief Identify the role a stream is intended to play\n> > + * \\var StreamUsage::StillCapture\n> > + * The stream is intended to capture high-resolution, high-quality still images\n> > + * with low frame rate. The captured frames may be exposed with flash.\n> > + * \\var StreamUsage::VideoRecording\n> > + * The stream is intended to capture video for the purpose of recording or\n> > + * streaming. The video stream may produce a high frame rate and may be\n> > + * enhanced with video stabilization.\n> > + * \\var StreamUsage::Viewfinder\n> > + * The stream is intended to capture video for the purpose of display on the\n> > + * local screen. The StreamUsage includes the desired resolution. Trade-offs\n> > + * between quality and usage of system resources are acceptable.\n> > + */\n> > +\n> > +/**\n> > + * \\fn StreamUsage::role()\n> > + * \\brief Retrieve the stream role\n> > + *\n> > + * \\return The stream role\n> > + */\n> > +\n> > +/**\n> > + * \\fn StreamUsage::size()\n> > + * \\brief Retrieve desired size\n> > + *\n> > + * \\return The desired size\n> > + */\n> > +\n> > +/**\n> > + * \\brief Create a stream usage\n> \n> should you somehow mention \"a stream usage with sizes = 0\" ?\n\nNo, this should IMHO be explained in the documentation for Size. As \nsimilar behavior would be true for all instances of Size.\n\n> \n> > + * \\param[in] role Stream role\n> > + */\n> > +StreamUsage::StreamUsage(Role role)\n> > +\t: role_(role), size_(Size(0, 0))\n> \n> Size default contructor initialize both fields to 0, so you can drop\n> the assignement.\n\nCool then I will drop it for v3.\n\n> \n> > +{\n> > +}\n> > +\n> > +/**\n> > + * \\brief Create a stream usage with size hint\n> > + * \\param[in] role Stream role\n> > + * \\param[in] width Desired width\n> > + * \\param[in] height Desired height\n> \n> \"The desired\" ?\n\nYou are correct this seems to be the convention in other places, will do \nso for v3.\n\n> \n> > + */\n> > +StreamUsage::StreamUsage(Role role, int width, int height)\n> > +\t: role_(role), size_(Size(width, height))\n> > +{\n> > +}\n> > +\n> >  /**\n> >   * \\class Stream\n> >   * \\brief Video stream for a camera\n> > @@ -78,6 +137,39 @@ namespace libcamera {\n> >   * optimal stream for the task.\n> >   */\n> >\n> > +/**\n> > + * \\class Stream::StillCapture\n> > + * \\brief Describe a still capture usage\n> > + */\n> > +Stream::StillCapture::StillCapture()\n> > +\t: StreamUsage(Role::StillCapture)\n> > +{\n> > +}\n> > +\n> > +/**\n> > + * \\class Stream::VideoRecording\n> > + * \\brief Describe a video recording usage\n> > + */\n> > +Stream::VideoRecording::VideoRecording()\n> > +\t: StreamUsage(Role::VideoRecording)\n> > +{\n> > +}\n> > +\n> > +/**\n> > + * \\class Stream::Viewfinder\n> > + * \\brief Describe a viewfinder usage\n> > + */\n> > +\n> > +/**\n> > + * \\brief Create a viewfinder usage with dimension hints\n> \n> Nit: you have used \"size\" everywhere in place of dimension if I'm not\n> wrong. I like dimension too though.\n\nI like dimension, no strong feeling will bend to popular opinion else \nkeep it as is.\n\n> \n> Thanks\n>    j\n> \n> > + * \\param[in] width Desired viewfinder width\n> > + * \\param[in] height Desired viewfinder height\n> > + */\n> > +Stream::Viewfinder::Viewfinder(int width, int height)\n> > +\t: StreamUsage(Role::Viewfinder, width, height)\n> > +{\n> > +}\n> > +\n> >  /**\n> >   * \\brief Construct a stream with default parameters\n> >   */\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-lf1-x131.google.com (mail-lf1-x131.google.com\n\t[IPv6:2a00:1450:4864:20::131])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id ECA0860DB2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri,  5 Apr 2019 11:43:29 +0200 (CEST)","by mail-lf1-x131.google.com with SMTP id d18so3956065lfn.3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 05 Apr 2019 02:43:29 -0700 (PDT)","from localhost (89-233-230-99.cust.bredband2.com. [89.233.230.99])\n\tby smtp.gmail.com with ESMTPSA id\n\tb194sm4208142lfg.39.2019.04.05.02.43.21\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tFri, 05 Apr 2019 02:43:21 -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=3ueaEt30hhq76L5kyVTjONAlYOB/NNnH/8Hst9p0428=;\n\tb=ctMNoMfZiuD8YE/sU3WsIUrQt3hMo1lLr11AzhL54+hINLMB7tgEqG0yZ6ZUejHMef\n\tuS0GtJMAFiHinoYU0os4ihNGp43Az7QR8TENS5fip5Dw3L/y2L//3pZhkoviSp0AQ05S\n\tvmKuA0i3hU6uuM7BwQbTKsCUn57fmyYWR9Kz0C8BlS3IYa7LXwWfa1nBiges1T+jCVP1\n\txBEEjS8dGBTOC0UzyKMsYA38eUYWUlqnEJSYRtp+3NtS2k/2rR7pKD4qtrskSfRkWirD\n\tMQ9eQBoEY48eWhd1yG3KrOTWlS4C2Fji6ksMxL5KjB3KWQRluIFHT2J2PpC/iwxGtk+A\n\tl89g==","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=3ueaEt30hhq76L5kyVTjONAlYOB/NNnH/8Hst9p0428=;\n\tb=spwEbxxy5CQXh/heqlg3E5TlMc0Hx0+rCk5Yc9Y/nCEzGlVmrbExDwfRPucAPRgDSl\n\thBgFkweyRQllny3z+eJTEdjhrM3vAqQQWRQBmfux4OMjuIVDCNKjVWpT65JXLIBtzjNa\n\trOSlxCTczR8EAdExd80/JbbOrOBFcoTx4z+67AUEPxk+CgV8Eg6XZ8bHJju0XU+Bbq25\n\t2d2JGazVRIEJH0PecB4HdRR/RPBP0OSoEtmiyDDwyGuitUl5XsZqSa2yks+WwWCtQa29\n\tmOn7kqO/ubh5efh7u+qzaLdUqVoyiJ2jkCKE8OjX8ae7xkeo/D/v5ENMPxNHqb/X5wg4\n\tvRJg==","X-Gm-Message-State":"APjAAAUVLsjFwygKpOM5VkwYdJ1mDoT23b8rZVs2e8AxLuQIOyZ0Cc46\n\t0AL3RjC43rgSjNCVjYcyExWzhZ7bQbs=","X-Google-Smtp-Source":"APXvYqxtkjbXW/2MZniB6Dufg+yw3/kResBkZeFv9cx3PQCvNYYMNmEm/3stBVWaZiKAwKSiyKT+dQ==","X-Received":"by 2002:ac2:4152:: with SMTP id c18mr6011698lfi.84.1554457402174;\n\tFri, 05 Apr 2019 02:43:22 -0700 (PDT)","Date":"Fri, 5 Apr 2019 11:43:20 +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":"<20190405094320.GR23466@bigcity.dyn.berto.se>","References":"<20190405020256.22520-1-niklas.soderlund@ragnatech.se>\n\t<20190405020256.22520-6-niklas.soderlund@ragnatech.se>\n\t<20190405082648.vwghsikdaaqw3gor@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190405082648.vwghsikdaaqw3gor@uno.localdomain>","User-Agent":"Mutt/1.11.3 (2019-02-01)","Subject":"Re: [libcamera-devel] [PATCH v2 5/8] libcamera: stream: Add basic\n\tstream usages","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":"Fri, 05 Apr 2019 09:43:30 -0000"}},{"id":1271,"web_url":"https://patchwork.libcamera.org/comment/1271/","msgid":"<20190405095950.47apg5wwosqsybp4@uno.localdomain>","date":"2019-04-05T09:59:50","subject":"Re: [libcamera-devel] [PATCH v2 5/8] libcamera: stream: Add basic\n\tstream usages","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Niklas,\n\nOn Fri, Apr 05, 2019 at 11:43:20AM +0200, Niklas Söderlund wrote:\n> Hi Jacopo,\n>\n> Thanks for your feedback.\n>\n> On 2019-04-05 10:26:48 +0200, Jacopo Mondi wrote:\n> > Hi Niklas,\n> >    thank you for the patch\n> >\n> > On Fri, Apr 05, 2019 at 04:02:53AM +0200, Niklas Söderlund wrote:\n> > > In preparation of reworking how a default configuration is retrieved\n> > > from a camera add stream usages. The usages will be used by applications\n> > > to describe how they intend to use a camera and replace the Stream IDs\n> > > when retrieving default configuration from the camera using\n> > > streamConfiguration().\n> > >\n> > > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> > > ---\n> > >  include/libcamera/stream.h | 40 +++++++++++++++++\n> > >  src/libcamera/stream.cpp   | 92 ++++++++++++++++++++++++++++++++++++++\n> > >  2 files changed, 132 insertions(+)\n> > >\n> > > diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h\n> > > index 970c479627fab064..1dea696f2e5ca95d 100644\n> > > --- a/include/libcamera/stream.h\n> > > +++ b/include/libcamera/stream.h\n> > > @@ -8,6 +8,7 @@\n> > >  #define __LIBCAMERA_STREAM_H__\n> > >\n> > >  #include <libcamera/buffer.h>\n> > > +#include <libcamera/geometry.h>\n> > >\n> > >  namespace libcamera {\n> > >\n> > > @@ -21,9 +22,48 @@ struct StreamConfiguration {\n> > >  \tunsigned int bufferCount;\n> > >  };\n> > >\n> > > +class StreamUsage\n> > > +{\n> > > +public:\n> > > +\tenum Role {\n> > > +\t\tStillCapture,\n> > > +\t\tVideoRecording,\n> > > +\t\tViewfinder,\n> > > +\t};\n> > > +\n> > > +\tRole role() const { return role_; }\n> > > +\tSize size() const { return size_; }\n> >\n> > Maybe not that relevant as Size is just two integers, but\n> >         Size &size() const\n> > or maybe even\n> >         const Size &size() const\n>\n> I have no strong opinion, I will make it so in next version.\n>\n> >\n> > Also\n> >         const Role role() const\n>\n> As this returns a value and not a reference this can not be const :-)\n\nIndeed, sorry for the stupid comment.\n\nThanks\n  j\n\n>\n> > > +\n> > > +protected:\n> > > +\tStreamUsage(Role role);\n> >\n> > explicit\n> >\n> > > +\tStreamUsage(Role role, int width, int height);\n> > > +\n> > > +private:\n> > > +\tRole role_;\n> > > +\tSize size_;\n> > > +};\n> > > +\n> > >  class Stream final\n> > >  {\n> > >  public:\n> > > +\tclass StillCapture : public StreamUsage\n> > > +\t{\n> > > +\tpublic:\n> > > +\t\tStillCapture();\n> > > +\t};\n> > > +\n> > > +\tclass VideoRecording : public StreamUsage\n> > > +\t{\n> > > +\tpublic:\n> > > +\t\tVideoRecording();\n> > > +\t};\n> > > +\n> > > +\tclass Viewfinder : public StreamUsage\n> > > +\t{\n> > > +\tpublic:\n> > > +\t\texplicit Viewfinder(int width, int height);\n> >\n> > No need for explicit, it takes two arguments.\n>\n> Wops, my intention was for this to be on StreamRole, somthing went very\n> wrong :-) Thanks for spotting it!\n>\n> >\n> > > +\t};\n> > > +\n> > >  \tStream();\n> > >  \tBufferPool &bufferPool() { return bufferPool_; }\n> > >  \tconst StreamConfiguration &configuration() const { return configuration_; }\n> > > diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp\n> > > index c4943c91b2e6ce13..816dff2d12acd923 100644\n> > > --- a/src/libcamera/stream.cpp\n> > > +++ b/src/libcamera/stream.cpp\n> > > @@ -60,6 +60,65 @@ namespace libcamera {\n> > >   * \\brief Requested number of buffers to allocate for the stream\n> > >   */\n> > >\n> > > +/**\n> > > + * \\class StreamUsage\n> > > + * \\brief Stream usage information\n> > > + *\n> > > + * The StreamUsage class describes how an application intends to use a stream.\n> > > + * Usages are specified by applications and passed to cameras, that then select\n> > > + * the most appropriate streams and their default configurations.\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\enum StreamUsage::Role\n> > > + * \\brief Identify the role a stream is intended to play\n> > > + * \\var StreamUsage::StillCapture\n> > > + * The stream is intended to capture high-resolution, high-quality still images\n> > > + * with low frame rate. The captured frames may be exposed with flash.\n> > > + * \\var StreamUsage::VideoRecording\n> > > + * The stream is intended to capture video for the purpose of recording or\n> > > + * streaming. The video stream may produce a high frame rate and may be\n> > > + * enhanced with video stabilization.\n> > > + * \\var StreamUsage::Viewfinder\n> > > + * The stream is intended to capture video for the purpose of display on the\n> > > + * local screen. The StreamUsage includes the desired resolution. Trade-offs\n> > > + * between quality and usage of system resources are acceptable.\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\fn StreamUsage::role()\n> > > + * \\brief Retrieve the stream role\n> > > + *\n> > > + * \\return The stream role\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\fn StreamUsage::size()\n> > > + * \\brief Retrieve desired size\n> > > + *\n> > > + * \\return The desired size\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\brief Create a stream usage\n> >\n> > should you somehow mention \"a stream usage with sizes = 0\" ?\n>\n> No, this should IMHO be explained in the documentation for Size. As\n> similar behavior would be true for all instances of Size.\n>\n> >\n> > > + * \\param[in] role Stream role\n> > > + */\n> > > +StreamUsage::StreamUsage(Role role)\n> > > +\t: role_(role), size_(Size(0, 0))\n> >\n> > Size default contructor initialize both fields to 0, so you can drop\n> > the assignement.\n>\n> Cool then I will drop it for v3.\n>\n> >\n> > > +{\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\brief Create a stream usage with size hint\n> > > + * \\param[in] role Stream role\n> > > + * \\param[in] width Desired width\n> > > + * \\param[in] height Desired height\n> >\n> > \"The desired\" ?\n>\n> You are correct this seems to be the convention in other places, will do\n> so for v3.\n>\n> >\n> > > + */\n> > > +StreamUsage::StreamUsage(Role role, int width, int height)\n> > > +\t: role_(role), size_(Size(width, height))\n> > > +{\n> > > +}\n> > > +\n> > >  /**\n> > >   * \\class Stream\n> > >   * \\brief Video stream for a camera\n> > > @@ -78,6 +137,39 @@ namespace libcamera {\n> > >   * optimal stream for the task.\n> > >   */\n> > >\n> > > +/**\n> > > + * \\class Stream::StillCapture\n> > > + * \\brief Describe a still capture usage\n> > > + */\n> > > +Stream::StillCapture::StillCapture()\n> > > +\t: StreamUsage(Role::StillCapture)\n> > > +{\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\class Stream::VideoRecording\n> > > + * \\brief Describe a video recording usage\n> > > + */\n> > > +Stream::VideoRecording::VideoRecording()\n> > > +\t: StreamUsage(Role::VideoRecording)\n> > > +{\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\class Stream::Viewfinder\n> > > + * \\brief Describe a viewfinder usage\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\brief Create a viewfinder usage with dimension hints\n> >\n> > Nit: you have used \"size\" everywhere in place of dimension if I'm not\n> > wrong. I like dimension too though.\n>\n> I like dimension, no strong feeling will bend to popular opinion else\n> keep it as is.\n>\n> >\n> > Thanks\n> >    j\n> >\n> > > + * \\param[in] width Desired viewfinder width\n> > > + * \\param[in] height Desired viewfinder height\n> > > + */\n> > > +Stream::Viewfinder::Viewfinder(int width, int height)\n> > > +\t: StreamUsage(Role::Viewfinder, width, height)\n> > > +{\n> > > +}\n> > > +\n> > >  /**\n> > >   * \\brief Construct a stream with default parameters\n> > >   */\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\n>\n>\n>\n> --\n> Regards,\n> Niklas Söderlund","headers":{"Return-Path":"<jacopo@jmondi.org>","Received":["from relay8-d.mail.gandi.net (relay8-d.mail.gandi.net\n\t[217.70.183.201])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E8B8360DB2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri,  5 Apr 2019 11:59:09 +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 relay8-d.mail.gandi.net (Postfix) with ESMTPSA id 73AD61BF203;\n\tFri,  5 Apr 2019 09:59:08 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Fri, 5 Apr 2019 11:59:50 +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":"<20190405095950.47apg5wwosqsybp4@uno.localdomain>","References":"<20190405020256.22520-1-niklas.soderlund@ragnatech.se>\n\t<20190405020256.22520-6-niklas.soderlund@ragnatech.se>\n\t<20190405082648.vwghsikdaaqw3gor@uno.localdomain>\n\t<20190405094320.GR23466@bigcity.dyn.berto.se>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"jw2r3jrb4g3437up\"","Content-Disposition":"inline","In-Reply-To":"<20190405094320.GR23466@bigcity.dyn.berto.se>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH v2 5/8] libcamera: stream: Add basic\n\tstream usages","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":"Fri, 05 Apr 2019 09:59:10 -0000"}},{"id":1291,"web_url":"https://patchwork.libcamera.org/comment/1291/","msgid":"<20190405154539.GD5184@pendragon.ideasonboard.com>","date":"2019-04-05T15:45:39","subject":"Re: [libcamera-devel] [PATCH v2 5/8] libcamera: stream: Add basic\n\tstream usages","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 Fri, Apr 05, 2019 at 04:02:53AM +0200, Niklas Söderlund wrote:\n> In preparation of reworking how a default configuration is retrieved\n> from a camera add stream usages. The usages will be used by applications\n> to describe how they intend to use a camera and replace the Stream IDs\n> when retrieving default configuration from the camera using\n> streamConfiguration().\n> \n> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> ---\n>  include/libcamera/stream.h | 40 +++++++++++++++++\n>  src/libcamera/stream.cpp   | 92 ++++++++++++++++++++++++++++++++++++++\n>  2 files changed, 132 insertions(+)\n> \n> diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h\n> index 970c479627fab064..1dea696f2e5ca95d 100644\n> --- a/include/libcamera/stream.h\n> +++ b/include/libcamera/stream.h\n> @@ -8,6 +8,7 @@\n>  #define __LIBCAMERA_STREAM_H__\n>  \n>  #include <libcamera/buffer.h>\n> +#include <libcamera/geometry.h>\n>  \n>  namespace libcamera {\n>  \n> @@ -21,9 +22,48 @@ struct StreamConfiguration {\n>  \tunsigned int bufferCount;\n>  };\n>  \n> +class StreamUsage\n> +{\n> +public:\n> +\tenum Role {\n> +\t\tStillCapture,\n> +\t\tVideoRecording,\n> +\t\tViewfinder,\n> +\t};\n> +\n> +\tRole role() const { return role_; }\n> +\tSize size() const { return size_; }\n> +\n> +protected:\n> +\tStreamUsage(Role role);\n> +\tStreamUsage(Role role, int width, int height);\n> +\n> +private:\n> +\tRole role_;\n> +\tSize size_;\n> +};\n> +\n>  class Stream final\n>  {\n>  public:\n> +\tclass StillCapture : public StreamUsage\n> +\t{\n> +\tpublic:\n> +\t\tStillCapture();\n> +\t};\n> +\n> +\tclass VideoRecording : public StreamUsage\n> +\t{\n> +\tpublic:\n> +\t\tVideoRecording();\n> +\t};\n> +\n> +\tclass Viewfinder : public StreamUsage\n> +\t{\n> +\tpublic:\n> +\t\texplicit Viewfinder(int width, int height);\n> +\t};\n> +\n>  \tStream();\n>  \tBufferPool &bufferPool() { return bufferPool_; }\n>  \tconst StreamConfiguration &configuration() const { return configuration_; }\n> diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp\n> index c4943c91b2e6ce13..816dff2d12acd923 100644\n> --- a/src/libcamera/stream.cpp\n> +++ b/src/libcamera/stream.cpp\n> @@ -60,6 +60,65 @@ namespace libcamera {\n>   * \\brief Requested number of buffers to allocate for the stream\n>   */\n>  \n> +/**\n> + * \\class StreamUsage\n> + * \\brief Stream usage information\n> + *\n> + * The StreamUsage class describes how an application intends to use a stream.\n> + * Usages are specified by applications and passed to cameras, that then select\n> + * the most appropriate streams and their default configurations.\n> + */\n> +\n> +/**\n> + * \\enum StreamUsage::Role\n> + * \\brief Identify the role a stream is intended to play\n> + * \\var StreamUsage::StillCapture\n> + * The stream is intended to capture high-resolution, high-quality still images\n> + * with low frame rate. The captured frames may be exposed with flash.\n> + * \\var StreamUsage::VideoRecording\n> + * The stream is intended to capture video for the purpose of recording or\n> + * streaming. The video stream may produce a high frame rate and may be\n> + * enhanced with video stabilization.\n> + * \\var StreamUsage::Viewfinder\n> + * The stream is intended to capture video for the purpose of display on the\n> + * local screen. The StreamUsage includes the desired resolution. Trade-offs\n> + * between quality and usage of system resources are acceptable.\n> + */\n> +\n> +/**\n> + * \\fn StreamUsage::role()\n> + * \\brief Retrieve the stream role\n> + *\n> + * \\return The stream role\n> + */\n> +\n> +/**\n> + * \\fn StreamUsage::size()\n> + * \\brief Retrieve desired size\n> + *\n> + * \\return The desired size\n> + */\n> +\n> +/**\n> + * \\brief Create a stream usage\n> + * \\param[in] role Stream role\n> + */\n> +StreamUsage::StreamUsage(Role role)\n> +\t: role_(role), size_(Size(0, 0))\n> +{\n> +}\n> +\n> +/**\n> + * \\brief Create a stream usage with size hint\n\ns/with size hint/with a desired size/ ?\n\n> + * \\param[in] role Stream role\n> + * \\param[in] width Desired width\n> + * \\param[in] height Desired height\n> + */\n> +StreamUsage::StreamUsage(Role role, int width, int height)\n> +\t: role_(role), size_(Size(width, height))\n> +{\n> +}\n> +\n>  /**\n>   * \\class Stream\n>   * \\brief Video stream for a camera\n> @@ -78,6 +137,39 @@ namespace libcamera {\n>   * optimal stream for the task.\n>   */\n>  \n> +/**\n> + * \\class Stream::StillCapture\n> + * \\brief Describe a still capture usage\n> + */\n> +Stream::StillCapture::StillCapture()\n> +\t: StreamUsage(Role::StillCapture)\n> +{\n> +}\n> +\n> +/**\n> + * \\class Stream::VideoRecording\n> + * \\brief Describe a video recording usage\n> + */\n> +Stream::VideoRecording::VideoRecording()\n> +\t: StreamUsage(Role::VideoRecording)\n> +{\n> +}\n> +\n> +/**\n> + * \\class Stream::Viewfinder\n> + * \\brief Describe a viewfinder usage\n> + */\n> +\n> +/**\n> + * \\brief Create a viewfinder usage with dimension hints\n\ns/dimension hints/a desired dimension/ ?\n\nApart from this, and with the small issues reported by Jacopo fixed,\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\n> + * \\param[in] width Desired viewfinder width\n> + * \\param[in] height Desired viewfinder height\n> + */\n> +Stream::Viewfinder::Viewfinder(int width, int height)\n> +\t: StreamUsage(Role::Viewfinder, width, height)\n> +{\n> +}\n> +\n>  /**\n>   * \\brief Construct a stream with default parameters\n>   */","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 CA7F160DB3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri,  5 Apr 2019 17:45:50 +0200 (CEST)","from pendragon.ideasonboard.com (unknown [109.140.214.47])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1B841E2;\n\tFri,  5 Apr 2019 17:45:50 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1554479150;\n\tbh=aaU9f0bnXovSuzO8GPpsU3uysxRZ9b+DwSFZ/LsNyzs=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Kit5mbvxBkINVKGRH4izYnhZIL2qQkPz4FPMAHylpKJNm4geBFJdvtZa7hZDzsqTE\n\tGvPjBNjqSGOUrCYpOHgD24bh42nMCdEMTOS8UtV60SrKzKyo2YILna+ZQO0ecN6tk4\n\tXb1X+03utBXdRMB7qnnPqxNBEZTX2SvqeVttYnSM=","Date":"Fri, 5 Apr 2019 18:45:39 +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":"<20190405154539.GD5184@pendragon.ideasonboard.com>","References":"<20190405020256.22520-1-niklas.soderlund@ragnatech.se>\n\t<20190405020256.22520-6-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":"<20190405020256.22520-6-niklas.soderlund@ragnatech.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v2 5/8] libcamera: stream: Add basic\n\tstream usages","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":"Fri, 05 Apr 2019 15:45:51 -0000"}}]