From patchwork Fri Apr 19 17:15:38 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jacopo Mondi X-Patchwork-Id: 1095 X-Patchwork-Delegate: jacopo@jmondi.org Return-Path: Received: from relay12.mail.gandi.net (relay12.mail.gandi.net [217.70.178.232]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id A2C4E60004 for ; Fri, 19 Apr 2019 19:14:51 +0200 (CEST) Received: from uno.lan (2-224-242-101.ip172.fastwebnet.it [2.224.242.101]) (Authenticated sender: jacopo@jmondi.org) by relay12.mail.gandi.net (Postfix) with ESMTPSA id 30DF9200009; Fri, 19 Apr 2019 17:14:51 +0000 (UTC) From: Jacopo Mondi To: libcamera-devel@lists.libcamera.org Date: Fri, 19 Apr 2019 19:15:38 +0200 Message-Id: <20190419171538.27675-1-jacopo@jmondi.org> X-Mailer: git-send-email 2.21.0 MIME-Version: 1.0 Subject: [libcamera-devel] [RFC] libcamera: Use properties for Stream Configuration X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 19 Apr 2019 17:14:51 -0000 Use the stream's property to perform stream configuration. --- Hello, this patch is clearly an RFC and its only intention is to get the discussion started on how to use stream's properties to configure streams. This patch introduces a naive implementation of a StreamProperties class, which gets associated to a stream partly at stream creation time (i.e. stream name) and partly at stream configuration time, where dependencies between streams should be evalutated and the list of supported stream sizes and format created (currently, for IPU3, all streams are created equal, with the viewfinder one supporting a smaller size). Application receives back a map of Stream * to StreamProperties and cycle on them to find out which stream has been associated to which role and use the stream properties to create a CameraConfiguration to be then used for the 'configureStreams()' method. I'm mostly interested in knowing your opinion on the pipeline handler and application API, it this is the direction we should take and provide a real storage class for StreamProperties. Thanks j ---- --- include/libcamera/camera.h | 3 +- include/libcamera/stream.h | 12 +++++ src/cam/main.cpp | 62 +++++++++++++----------- src/libcamera/camera.cpp | 4 +- src/libcamera/include/pipeline_handler.h | 3 +- src/libcamera/pipeline/ipu3/ipu3.cpp | 62 +++++++++--------------- src/libcamera/pipeline/meson.build | 10 ++-- src/meson.build | 2 +- test/meson.build | 2 +- 9 files changed, 84 insertions(+), 76 deletions(-) diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h index b2dafda342fe..c01bf86da92e 100644 --- a/include/libcamera/camera.h +++ b/include/libcamera/camera.h @@ -22,6 +22,7 @@ class PipelineHandler; class Request; class Stream; class StreamConfiguration; +class StreamProperties; class StreamUsage; class CameraConfiguration @@ -73,7 +74,7 @@ public: int release(); const std::set &streams() const; - CameraConfiguration + std::map streamConfiguration(const std::vector &usage); int configureStreams(const CameraConfiguration &config); diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h index 3caaefc9f8e9..d22289e07708 100644 --- a/include/libcamera/stream.h +++ b/include/libcamera/stream.h @@ -7,6 +7,7 @@ #ifndef __LIBCAMERA_STREAM_H__ #define __LIBCAMERA_STREAM_H__ +#include #include #include @@ -47,6 +48,15 @@ private: Size size_; }; +class StreamProperties +{ +public: + std::string name_; + StreamUsage::Role role_; + std::vector sizes_; + std::vector formats_; +}; + class Stream { public: @@ -72,6 +82,8 @@ public: BufferPool &bufferPool() { return bufferPool_; } const StreamConfiguration &configuration() const { return configuration_; } + StreamProperties props_; + protected: friend class Camera; diff --git a/src/cam/main.cpp b/src/cam/main.cpp index 46aba728393c..c57a72047b85 100644 --- a/src/cam/main.cpp +++ b/src/cam/main.cpp @@ -87,14 +87,12 @@ static int parseOptions(int argc, char *argv[]) static int prepareCameraConfig(CameraConfiguration *config) { + std::map props; std::vector roles; - streamInfo.clear(); - /* If no configuration is provided assume a single video stream. */ if (!options.isSet(OptStream)) { - *config = camera->streamConfiguration({ Stream::VideoRecording() }); - streamInfo[config->front()] = "stream0"; + props = camera->streamConfiguration({ Stream::VideoRecording() }); return 0; } @@ -121,36 +119,46 @@ static int prepareCameraConfig(CameraConfiguration *config) } } - *config = camera->streamConfiguration(roles); - - if (!config->isValid()) { - std::cerr << "Failed to get default stream configuration" - << std::endl; + props = camera->streamConfiguration(roles); + if (props.empty()) { + std::cerr << "Failed to get stream properties" << std::endl; return -EINVAL; } - /* Apply configuration explicitly requested. */ - CameraConfiguration::iterator it = config->begin(); - for (auto const &value : streamOptions) { - KeyValueParser::Options conf = value.toKeyValues(); - Stream *stream = *it; - it++; + /* Save the streams names and apply the requested configuration. */ + for (auto it : props) { + Stream *stream = it.first; + StreamProperties &props = it.second; - if (conf.isSet("width")) - (*config)[stream].width = conf["width"]; + streamInfo[stream] = props.name_; - if (conf.isSet("height")) - (*config)[stream].height = conf["height"]; + for (auto const &value : streamOptions) { + KeyValueParser::Options conf = value.toKeyValues(); - /* TODO: Translate 4CC string to ID. */ - if (conf.isSet("pixelformat")) - (*config)[stream].pixelFormat = conf["pixelformat"]; - } + if (!conf.isSet("role") || + conf["role"] != props.role_) + continue; + + /* Get the stream max sizes */ + Size maxSize = props.sizes_.back(); - unsigned int index = 0; - for (Stream *stream : *config) { - streamInfo[stream] = "stream" + std::to_string(index); - index++; + if (conf.isSet("width")) + (*config)[stream].width = conf["width"]; + else + (*config)[stream].width = maxSize.width; + + if (conf.isSet("height")) + (*config)[stream].height = conf["height"]; + else + (*config)[stream].height = maxSize.height; + + /* TODO: Translate 4CC string to ID. */ + unsigned int format = *props.formats_.begin(); + if (conf.isSet("pixelformat")) + (*config)[stream].pixelFormat = conf["pixelformat"]; + else + (*config)[stream].pixelFormat = format; + } } return 0; diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index 655996f26224..b750307e33bd 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -540,11 +540,11 @@ const std::set &Camera::streams() const * \return A valid CameraConfiguration if the requested usages can be satisfied, * or a invalid one otherwise */ -CameraConfiguration +std::map Camera::streamConfiguration(const std::vector &usages) { if (disconnected_ || !usages.size() || usages.size() > streams_.size()) - return CameraConfiguration(); + return std::map{}; return pipe_->streamConfiguration(this, usages); } diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h index c3f7d4c29205..99d9a34efeb0 100644 --- a/src/libcamera/include/pipeline_handler.h +++ b/src/libcamera/include/pipeline_handler.h @@ -27,6 +27,7 @@ class PipelineHandler; class Request; class Stream; class StreamConfiguration; +class StreamProperties; class StreamUsage; class CameraData @@ -55,7 +56,7 @@ public: virtual bool match(DeviceEnumerator *enumerator) = 0; - virtual CameraConfiguration + virtual std::map streamConfiguration(Camera *camera, const std::vector &usages) = 0; virtual int configureStreams(Camera *camera, const CameraConfiguration &config) = 0; diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 2c2929207d35..553673a99953 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -141,7 +141,6 @@ public: } bool active_; - std::string name_; ImgUDevice::ImgUOutput *device_; }; @@ -151,7 +150,7 @@ public: PipelineHandlerIPU3(CameraManager *manager); ~PipelineHandlerIPU3(); - CameraConfiguration + std::map streamConfiguration(Camera *camera, const std::vector &usages) override; int configureStreams(Camera *camera, @@ -219,19 +218,19 @@ PipelineHandlerIPU3::~PipelineHandlerIPU3() imguMediaDev_->release(); } -CameraConfiguration +std::map PipelineHandlerIPU3::streamConfiguration(Camera *camera, const std::vector &usages) { IPU3CameraData *data = cameraData(camera); - CameraConfiguration cameraConfig = {}; + std::map propertiesMap; std::set streams = { &data->outStream_, &data->vfStream_, }; for (const StreamUsage &usage : usages) { - StreamConfiguration streamConfig = {}; + StreamProperties streamProperties = {}; StreamUsage::Role role = usage.role(); IPU3Stream *stream = nullptr; @@ -253,17 +252,13 @@ PipelineHandlerIPU3::streamConfiguration(Camera *camera, break; } - /* - * FIXME: Soraka: the maximum resolution reported by - * both sensors (2592x1944 for ov5670 and 4224x3136 for - * ov13858) are returned as default configurations but - * they're not correctly processed by the ImgU. - * Resolutions up tp 2560x1920 have been validated. - * - * \todo Clarify ImgU alignment requirements. - */ - streamConfig.width = 2560; - streamConfig.height = 1920; + streamProperties = stream->props_; + streamProperties.role_ = role; + streamProperties.formats_ = { V4L2_PIX_FMT_NV12 }; + streamProperties.sizes_ = { + { 320, 240 }, { 640, 480 }, { 1024, 768 }, + { 1920, 1080 }, { 2560, 1920 }, + }; break; @@ -285,18 +280,13 @@ PipelineHandlerIPU3::streamConfiguration(Camera *camera, stream = &data->vfStream_; - /* - * Align the requested viewfinder size to the - * maximum available sensor resolution and to the - * IPU3 alignment constraints. - */ - const Size &res = data->cio2_.sensor_->resolution(); - unsigned int width = std::min(usage.size().width, - res.width); - unsigned int height = std::min(usage.size().height, - res.height); - streamConfig.width = width & ~7; - streamConfig.height = height & ~3; + streamProperties.role_ = role; + streamProperties = stream->props_; + streamProperties.formats_ = { V4L2_PIX_FMT_NV12 }; + streamProperties.sizes_ = { + { 320, 240 }, { 640, 480 }, { 1024, 768 }, + { 1920, 1080 }, { 2560, 1920 }, + }; break; } @@ -308,21 +298,17 @@ PipelineHandlerIPU3::streamConfiguration(Camera *camera, } if (!stream) - return CameraConfiguration{}; + return std::map{}; streams.erase(stream); - streamConfig.pixelFormat = V4L2_PIX_FMT_NV12; - streamConfig.bufferCount = IPU3_BUFFER_COUNT; - - cameraConfig[stream] = streamConfig; + propertiesMap[stream] = streamProperties; LOG(IPU3, Debug) - << "Stream '" << stream->name_ << "' format set to " - << streamConfig.toString(); + << "Stream '" << stream->props_.name_ << "'"; } - return cameraConfig; + return propertiesMap; } int PipelineHandlerIPU3::configureStreams(Camera *camera, @@ -743,9 +729,9 @@ int PipelineHandlerIPU3::registerCameras() */ data->imgu_ = numCameras ? &imgu1_ : &imgu0_; data->outStream_.device_ = &data->imgu_->output_; - data->outStream_.name_ = "output"; + data->outStream_.props_.name_ = "output"; data->vfStream_.device_ = &data->imgu_->viewfinder_; - data->vfStream_.name_ = "viewfinder"; + data->outStream_.props_.name_ = "viewfinder"; /* * Connect video devices' 'bufferReady' signals to their diff --git a/src/libcamera/pipeline/meson.build b/src/libcamera/pipeline/meson.build index 0d466225a72e..d7766edf96d8 100644 --- a/src/libcamera/pipeline/meson.build +++ b/src/libcamera/pipeline/meson.build @@ -1,7 +1,7 @@ -libcamera_sources += files([ - 'uvcvideo.cpp', - 'vimc.cpp', -]) +#libcamera_sources += files([ +# 'uvcvideo.cpp', +# 'vimc.cpp', +#]) subdir('ipu3') -subdir('rkisp1') +#subdir('rkisp1') diff --git a/src/meson.build b/src/meson.build index 4e41fd3e7887..018e9d75c0ac 100644 --- a/src/meson.build +++ b/src/meson.build @@ -1,3 +1,3 @@ subdir('libcamera') subdir('cam') -subdir('qcam') +#subdir('qcam') diff --git a/test/meson.build b/test/meson.build index d501f2beaf96..edbcc090f524 100644 --- a/test/meson.build +++ b/test/meson.build @@ -1,6 +1,6 @@ subdir('libtest') -subdir('camera') +#subdir('camera') subdir('media_device') subdir('pipeline') subdir('v4l2_device')