[{"id":644,"web_url":"https://patchwork.libcamera.org/comment/644/","msgid":"<20190127221940.GA5154@pendragon.ideasonboard.com>","date":"2019-01-27T22:19:40","subject":"Re: [libcamera-devel] [PATCH v3 1/6] libcamera: stream: add initial\n\tStream class","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 Sun, Jan 27, 2019 at 01:22:03AM +0100, Niklas Söderlund wrote:\n> Add an initial Stream implementation. The idea is that once capability\n> support is added to the library each stream will describe its\n> capabilities using this class. An application will then select one or\n> more streams based on these capabilities and use them to configure the\n> camera and capture.\n> \n> At this stage all the Stream class provides is a way for a Camera object\n> to communicate which streams it provides.\n> \n> Signed-off-by: Niklas Söderlund \n> ---\n> include/libcamera/libcamera.h | 1 +\n> include/libcamera/meson.build | 1 +\n> include/libcamera/stream.h | 25 +++++++++++++\n> src/libcamera/meson.build | 1 +\n> src/libcamera/stream.cpp | 67 +++++++++++++++++++++++++++++++++++\n> 5 files changed, 95 insertions(+)\n> create mode 100644 include/libcamera/stream.h\n> create mode 100644 src/libcamera/stream.cpp\n> \n> diff --git a/include/libcamera/libcamera.h b/include/libcamera/libcamera.h\n> index c0511cf6d662b63f..272dfd5e4a67d5de 100644\n> --- a/include/libcamera/libcamera.h\n> +++ b/include/libcamera/libcamera.h\n> @@ -12,6 +12,7 @@\n> #include \n> #include \n> #include \n> +#include \n> #include \n> \n> #endif /* __LIBCAMERA_LIBCAMERA_H__ */\n> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build\n> index d7cb55ba4a49e1e8..54a680787e5c17aa 100644\n> --- a/include/libcamera/meson.build\n> +++ b/include/libcamera/meson.build\n> @@ -5,6 +5,7 @@ libcamera_api = files([\n> 'event_notifier.h',\n> 'libcamera.h',\n> 'signal.h',\n> + 'stream.h',\n> 'timer.h',\n> ])\n> \n> diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h\n> new file mode 100644\n> index 0000000000000000..580f2cc8d3a3ad59\n> --- /dev/null\n> +++ b/include/libcamera/stream.h\n> @@ -0,0 +1,25 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2019, Google Inc.\n> + *\n> + * stream.h - Video stream for a Camera\n> + */\n> +#ifndef __LIBCAMERA_STREAM_H__\n> +#define __LIBCAMERA_STREAM_H__\n> +\n> +namespace libcamera {\n> +\n> +class Stream final\n> +{\n> +public:\n> +\tStream(unsigned int id);\n> +\n> +\tunsigned int id() const { return id_; };\n> +\n> +private:\n> +\tunsigned int id_;\n> +};\n> +\n> +} /* namespace libcamera */\n> +\n> +#endif /* __LIBCAMERA_STREAM_H__ */\n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index f9f25c0ecf1564cc..9f6ff99eebe2f5bc 100644\n> --- a/src/libcamera/meson.build\n> +++ b/src/libcamera/meson.build\n> @@ -10,6 +10,7 @@ libcamera_sources = files([\n> 'media_object.cpp',\n> 'pipeline_handler.cpp',\n> 'signal.cpp',\n> + 'stream.cpp',\n> 'timer.cpp',\n> 'v4l2_device.cpp',\n> ])\n> diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp\n> new file mode 100644\n> index 0000000000000000..307de3710d0ac6b1\n> --- /dev/null\n> +++ b/src/libcamera/stream.cpp\n> @@ -0,0 +1,67 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2019, Google Inc.\n> + *\n> + * stream.cpp - Video stream for a Camera\n> + */\n> +\n> +#include \n> +\n> +/**\n> + * \\file stream.h\n> + * \\brief Video stream for a Camera\n> + *\n> + * A camera device can provide frames in different resolutions and formats\n> + * concurrently from a single image source. The Stream class represents\n> + * one of the multiple concurrent streams.\n> + *\n> + * All streams exposed by a camera device share the same image source and are\n> + * thus not fully independent. Parameters related to the image source, such as\n> + * the exposure time or flash control, are common to all streams. Other\n> + * parameters, such as format or resolution, may be specified per-stream,\n> + * depending on the capabilities of the camera device.\n> + *\n> + * Camera devices expose at least one stream, and may expose additional streams\n> + * based on the device capabilities. This can be used, for instance, to\n> + * implement concurrent viewfinder and video capture, or concurrent viewfinder,\n> + * video capture and still image capture.\n> + */\n> +\n> +namespace libcamera {\n> +\n> +/**\n> + * \\class Stream\n> + * \\brief Video stream for a camera\n> + *\n> + * The Stream class models of all static information which are associated\n\ns/of all/all/ ?\n\n> + * with a single video stream. Stream are exposed by the Camera object they\n> + * belong to.\n> + *\n> + * Some cameras are capable of supplying more then one stream from the same\n> + * video source. In such cases an application can inspect all available streams\n> + * and select the ones that best fit its use case.\n> + *\n> + * \\todo Add capabilities to the stream API. Without this the Stream class only\n> + * serves to reveal how many streams of unknown capabilities a camera supports.\n> + * This in itself is productive as it allows applications to configure and\n> + * capture from one or more streams even if they won't be able to select the\n> + * optimal stream for the task.\n> + */\n> +\n> +/**\n> + * \\brief Create a new stream with a ID\n\ns/a ID/an ID/\n\n> + * \\param[in] id Numerical ID which should be unique for the camera device the\n> + * stream belongs to\n\nI think we should document the constraints on stream IDs (for instance\ncontiguous IDs starting at 0, but that might not be the best option).\nThis can be done in a subsequent patch as the Stream API will still\nevolve a lot.\n\nReviewed-by: Laurent Pinchart \n\n> + */\n> +Stream::Stream(unsigned int id)\n> +\t: id_(id)\n> +{\n> +}\n> +\n> +/**\n> + * \\fn Stream::id()\n> + * \\brief Retrieve the streams ID\n> + * \\return The stream ID\n> + */\n> +\n> +} /* namespace libcamera */","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 A003260C78\n\tfor ;\n\tSun, 27 Jan 2019 23:19:47 +0100 (CET)","from pendragon.ideasonboard.com (85-76-73-155-nat.elisa-mobile.fi\n\t[85.76.73.155])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D5D7F85;\n\tSun, 27 Jan 2019 23:19:45 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1548627587;\n\tbh=9KKl+U2tKzHE/V0xDBVblovPWHd6RXXJeW3evX5eODE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=eXniOSNyQtgqxLLIYHOecKRwkPL2Z0JfePv0oZW2A0YASjUoa8qTLN3zUIfEYMez2\n\tmGBzYehHPQw8i39j9sbhPhrs32UObMh8ZJF4tEk7NrAFo5w3vkYmrrsBtgwgTSFZKa\n\tPbt4Zh6TADLXg+iwaoMF/NxGTd4E1K9jZeB+99v8=","Date":"Mon, 28 Jan 2019 00:19:40 +0200","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190127221940.GA5154@pendragon.ideasonboard.com>","References":"<20190127002208.18913-1-niklas.soderlund@ragnatech.se>\n\t<20190127002208.18913-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":"<20190127002208.18913-2-niklas.soderlund@ragnatech.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v3 1/6] libcamera: stream: add initial\n\tStream class","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Sun, 27 Jan 2019 22:19:47 -0000"}},{"id":646,"web_url":"https://patchwork.libcamera.org/comment/646/","msgid":"<20190127223527.GC27958@bigcity.dyn.berto.se>","date":"2019-01-27T22:35:27","subject":"Re: [libcamera-devel] [PATCH v3 1/6] libcamera: stream: add initial\n\tStream class","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-01-28 00:19:40 +0200, Laurent Pinchart wrote:\n> Hi Niklas,\n> \n> Thank you for the patch.\n> \n> On Sun, Jan 27, 2019 at 01:22:03AM +0100, Niklas Söderlund wrote:\n> > Add an initial Stream implementation. The idea is that once capability\n> > support is added to the library each stream will describe its\n> > capabilities using this class. An application will then select one or\n> > more streams based on these capabilities and use them to configure the\n> > camera and capture.\n> > \n> > At this stage all the Stream class provides is a way for a Camera object\n> > to communicate which streams it provides.\n> > \n> > Signed-off-by: Niklas Söderlund \n> > ---\n> > include/libcamera/libcamera.h | 1 +\n> > include/libcamera/meson.build | 1 +\n> > include/libcamera/stream.h | 25 +++++++++++++\n> > src/libcamera/meson.build | 1 +\n> > src/libcamera/stream.cpp | 67 +++++++++++++++++++++++++++++++++++\n> > 5 files changed, 95 insertions(+)\n> > create mode 100644 include/libcamera/stream.h\n> > create mode 100644 src/libcamera/stream.cpp\n> > \n> > diff --git a/include/libcamera/libcamera.h b/include/libcamera/libcamera.h\n> > index c0511cf6d662b63f..272dfd5e4a67d5de 100644\n> > --- a/include/libcamera/libcamera.h\n> > +++ b/include/libcamera/libcamera.h\n> > @@ -12,6 +12,7 @@\n> > #include \n> > #include \n> > #include \n> > +#include \n> > #include \n> > \n> > #endif /* __LIBCAMERA_LIBCAMERA_H__ */\n> > diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build\n> > index d7cb55ba4a49e1e8..54a680787e5c17aa 100644\n> > --- a/include/libcamera/meson.build\n> > +++ b/include/libcamera/meson.build\n> > @@ -5,6 +5,7 @@ libcamera_api = files([\n> > 'event_notifier.h',\n> > 'libcamera.h',\n> > 'signal.h',\n> > + 'stream.h',\n> > 'timer.h',\n> > ])\n> > \n> > diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h\n> > new file mode 100644\n> > index 0000000000000000..580f2cc8d3a3ad59\n> > --- /dev/null\n> > +++ b/include/libcamera/stream.h\n> > @@ -0,0 +1,25 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2019, Google Inc.\n> > + *\n> > + * stream.h - Video stream for a Camera\n> > + */\n> > +#ifndef __LIBCAMERA_STREAM_H__\n> > +#define __LIBCAMERA_STREAM_H__\n> > +\n> > +namespace libcamera {\n> > +\n> > +class Stream final\n> > +{\n> > +public:\n> > +\tStream(unsigned int id);\n> > +\n> > +\tunsigned int id() const { return id_; };\n> > +\n> > +private:\n> > +\tunsigned int id_;\n> > +};\n> > +\n> > +} /* namespace libcamera */\n> > +\n> > +#endif /* __LIBCAMERA_STREAM_H__ */\n> > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> > index f9f25c0ecf1564cc..9f6ff99eebe2f5bc 100644\n> > --- a/src/libcamera/meson.build\n> > +++ b/src/libcamera/meson.build\n> > @@ -10,6 +10,7 @@ libcamera_sources = files([\n> > 'media_object.cpp',\n> > 'pipeline_handler.cpp',\n> > 'signal.cpp',\n> > + 'stream.cpp',\n> > 'timer.cpp',\n> > 'v4l2_device.cpp',\n> > ])\n> > diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp\n> > new file mode 100644\n> > index 0000000000000000..307de3710d0ac6b1\n> > --- /dev/null\n> > +++ b/src/libcamera/stream.cpp\n> > @@ -0,0 +1,67 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2019, Google Inc.\n> > + *\n> > + * stream.cpp - Video stream for a Camera\n> > + */\n> > +\n> > +#include \n> > +\n> > +/**\n> > + * \\file stream.h\n> > + * \\brief Video stream for a Camera\n> > + *\n> > + * A camera device can provide frames in different resolutions and formats\n> > + * concurrently from a single image source. The Stream class represents\n> > + * one of the multiple concurrent streams.\n> > + *\n> > + * All streams exposed by a camera device share the same image source and are\n> > + * thus not fully independent. Parameters related to the image source, such as\n> > + * the exposure time or flash control, are common to all streams. Other\n> > + * parameters, such as format or resolution, may be specified per-stream,\n> > + * depending on the capabilities of the camera device.\n> > + *\n> > + * Camera devices expose at least one stream, and may expose additional streams\n> > + * based on the device capabilities. This can be used, for instance, to\n> > + * implement concurrent viewfinder and video capture, or concurrent viewfinder,\n> > + * video capture and still image capture.\n> > + */\n> > +\n> > +namespace libcamera {\n> > +\n> > +/**\n> > + * \\class Stream\n> > + * \\brief Video stream for a camera\n> > + *\n> > + * The Stream class models of all static information which are associated\n> \n> s/of all/all/ ?\n\nYes, thanks.\n\n> \n> > + * with a single video stream. Stream are exposed by the Camera object they\n> > + * belong to.\n> > + *\n> > + * Some cameras are capable of supplying more then one stream from the same\n> > + * video source. In such cases an application can inspect all available streams\n> > + * and select the ones that best fit its use case.\n> > + *\n> > + * \\todo Add capabilities to the stream API. Without this the Stream class only\n> > + * serves to reveal how many streams of unknown capabilities a camera supports.\n> > + * This in itself is productive as it allows applications to configure and\n> > + * capture from one or more streams even if they won't be able to select the\n> > + * optimal stream for the task.\n> > + */\n> > +\n> > +/**\n> > + * \\brief Create a new stream with a ID\n> \n> s/a ID/an ID/\n\nI thought I found all of them after your good grammar lesson in v2 :-)\n\n> \n> > + * \\param[in] id Numerical ID which should be unique for the camera device the\n> > + * stream belongs to\n> \n> I think we should document the constraints on stream IDs (for instance\n> contiguous IDs starting at 0, but that might not be the best option).\n> This can be done in a subsequent patch as the Stream API will still\n> evolve a lot.\n\nSo far the only constraint is that they need to be unique within a \ncamera. I agree it might be a good idea that they should be contiguous \nstarting from 0, but any integer works with the current implementation. \nLets discuss this as the stream API evolves.\n\n> \n> Reviewed-by: Laurent Pinchart \n> \n> > + */\n> > +Stream::Stream(unsigned int id)\n> > +\t: id_(id)\n> > +{\n> > +}\n> > +\n> > +/**\n> > + * \\fn Stream::id()\n> > + * \\brief Retrieve the streams ID\n> > + * \\return The stream ID\n> > + */\n> > +\n> > +} /* namespace libcamera */\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"","Received":["from mail-lf1-x144.google.com (mail-lf1-x144.google.com\n\t[IPv6:2a00:1450:4864:20::144])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B87ED60C78\n\tfor ;\n\tSun, 27 Jan 2019 23:35:29 +0100 (CET)","by mail-lf1-x144.google.com with SMTP id n18so10474123lfh.6\n\tfor ;\n\tSun, 27 Jan 2019 14:35:29 -0800 (PST)","from localhost (89-233-230-99.cust.bredband2.com. [89.233.230.99])\n\tby smtp.gmail.com with ESMTPSA id\n\tg15sm2611466lfb.1.2019.01.27.14.35.27\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tSun, 27 Jan 2019 14:35:27 -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\t:user-agent; bh=Z/KzxH7+sjOHbIyWPdMk3cRDIah4IrCaJIwe0U/W+AA=;\n\tb=b7bJ4BK9Upz8SWDGtO1+WQtqM7OOJHIL13bUGi0KwWqveJoMSHriLLVEpcgbWH/Yla\n\tm4avd6Kxl1gSKNH7tayJzkndTYVxp1wCJc8kzgNRF53OGgZZkSFN/rmhhjMwV/2ulS45\n\thKnqJIoUhP4kLz8ID4aplhDrO+WbY7gbBnLDSMheO41jAKc4UABOnR/nBBSeah3vdyDN\n\tjuE0NysmrUbU5ibYk+o63dRezDYQ8wQyWqsIqZ1flH7amnc8YsTSZgSzLaNoj4lJJSii\n\tjLvdLJBXhncvB0kpFbHfFPX34cwBUs9Hu0tMqV0OimYhkksDRUHWRVwBKPF8DlZbh0E3\n\tFMdg==","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=Z/KzxH7+sjOHbIyWPdMk3cRDIah4IrCaJIwe0U/W+AA=;\n\tb=P/m1OLXDJ/DIZoO0Hfy4sKylriVc1qZK3G9J+e1CVhePdayagBZhF590qEypB37ViR\n\t9rL+B0Um5/+u2jH0V/a6GpZ/iohY6Z4D+/3u0JSFlwzP16WaxE/oJof9ELQRQbS6hLKW\n\tlt9/sMVGf7weqnf4K82JwqiTs2nAuy5ezvyiT8Z9n7xlgFzJ7EniDH3R2Eg4LEYTee3e\n\t3zNfkToQ7DaV6lsrOjVEIG2VNdueYkFjq46AsyTvohFauJFMWB5LoT69ZeRfXJYYOlDU\n\tNYH1rr1Rl671pP1HwNx7yVQAP12oGtTJ8tNdxpWor0peV2by+Tzm8LMFNqhPq+kWs/qY\n\tOebw==","X-Gm-Message-State":"AJcUukeCBe6gNxcZ3U49+UIv4gxChIrJTe7xpc9PmAjxpUgygSwUxed3\n\t1vDRaifeYH/TRVdOuF2epixPsg==","X-Google-Smtp-Source":"ALg8bN7OW1JZcVGfzuCJKa79+Qq05zOwcYmARrRY13oD5Tng3pcimfPQQ5crJffq2h1lXgMs3PNEJQ==","X-Received":"by 2002:ac2:53b1:: with SMTP id\n\tj17mr14163065lfh.167.1548628528659; \n\tSun, 27 Jan 2019 14:35:28 -0800 (PST)","Date":"Sun, 27 Jan 2019 23:35:27 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190127223527.GC27958@bigcity.dyn.berto.se>","References":"<20190127002208.18913-1-niklas.soderlund@ragnatech.se>\n\t<20190127002208.18913-2-niklas.soderlund@ragnatech.se>\n\t<20190127221940.GA5154@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":"<20190127221940.GA5154@pendragon.ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v3 1/6] libcamera: stream: add initial\n\tStream class","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Sun, 27 Jan 2019 22:35:30 -0000"}}]