From patchwork Fri May 17 23:06:10 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 1218 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 6E3AC60E4C for ; Sat, 18 May 2019 01:06:43 +0200 (CEST) Received: from pendragon.bb.dnainternet.fi (dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi [IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id E3E26336 for ; Sat, 18 May 2019 01:06:42 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1558134403; bh=OztG7646FvjlGJg9+Qv8ZGXpLDBaAmZjKkvDnogxvgo=; h=From:To:Subject:Date:In-Reply-To:References:From; b=Rf813KH/LqB2n0ipA218Myu161GPaQoUsiy5ORb1rnN+H5o14anx6n7u0+/LLFwom xYq1NPM9nf8Q7FdECycdbKWCnUiKo8kR2yXIZWYRTPIoVQdgUETKrd2Do5X7xsGZ2R 4pWqjU4in1xHqYwEnUpHhC9tZS1shQDzQJaG8y1E= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sat, 18 May 2019 02:06:10 +0300 Message-Id: <20190517230621.24668-2-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190517230621.24668-1-laurent.pinchart@ideasonboard.com> References: <20190517230621.24668-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH/RFC 01/12] libcamera: camera: Fix std::ostringstream initialisation 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, 17 May 2019 23:06:43 -0000 We use the std::ostringstream class to generate log messages in the Camera class. The stream is initialised with initial content, but is not opened without seeking to the end, which results in the content being overwritten immediately. Fix it by opening the stream with std::ios_base::ate. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- src/libcamera/camera.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index fbc66dedba51..1a21acac9899 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -560,7 +560,7 @@ Camera::streamConfiguration(const std::vector &usages) CameraConfiguration config = pipe_->streamConfiguration(this, usages); - std::ostringstream msg("streams configuration:"); + std::ostringstream msg("streams configuration:", std::ios_base::ate); unsigned int index = 0; for (Stream *stream : config) { @@ -614,7 +614,7 @@ int Camera::configureStreams(const CameraConfiguration &config) return -EINVAL; } - std::ostringstream msg("configuring streams:"); + std::ostringstream msg("configuring streams:", std::ios_base::ate); unsigned int index = 0; for (Stream *stream : config) { From patchwork Fri May 17 23:06:11 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 1219 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id A0DB060E68 for ; Sat, 18 May 2019 01:06:43 +0200 (CEST) Received: from pendragon.bb.dnainternet.fi (dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi [IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 350E9549 for ; Sat, 18 May 2019 01:06:43 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1558134403; bh=bfnHf3LhyQtvDcCGANggOZyULw2ojpxdZuAOlDbh1pU=; h=From:To:Subject:Date:In-Reply-To:References:From; b=KA3QmrFNK9sgjHA2DxDT+c0OTvgeBJ7Zu8w6yoso/Zg021Xm6JiH0iO30C7hvDB48 BgKcyzXr1Z3DIphXMEoorGlkZkGTdw5gL6jReOeE/msMBW5zVdHGlTo8UBrlYyDebF bdhaH6wV57ysWbl3fzW5wjjLXBqa+Ryn2Vd/jp5A= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sat, 18 May 2019 02:06:11 +0300 Message-Id: <20190517230621.24668-3-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190517230621.24668-1-laurent.pinchart@ideasonboard.com> References: <20190517230621.24668-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH/RFC 02/12] libcamera: camera: Rename configureStreams() and streamConfiguration() 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, 17 May 2019 23:06:44 -0000 Rename the configureStreams() and streamConfiguration() methods to configure() and generateConfiguration() respectively in order to clarify the API. Both methods deal with CameraConfiguration objects, and are thus not limited to streams, even if a CameraConfiguration currently contains streams only. While at it, remove the qcam MainWindow::configureStreams() method that is declared but never defined or used. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- include/libcamera/camera.h | 4 ++-- src/cam/main.cpp | 6 ++--- src/libcamera/camera.cpp | 28 ++++++++++++------------ src/libcamera/include/pipeline_handler.h | 4 ++-- src/libcamera/pipeline/ipu3/ipu3.cpp | 21 +++++++++--------- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 10 ++++----- src/libcamera/pipeline/uvcvideo.cpp | 16 +++++++------- src/libcamera/pipeline/vimc.cpp | 16 +++++++------- src/libcamera/pipeline_handler.cpp | 28 ++++++++++++------------ src/libcamera/stream.cpp | 5 ++--- src/qcam/main_window.cpp | 4 ++-- src/qcam/main_window.h | 1 - test/camera/capture.cpp | 4 ++-- test/camera/configuration_default.cpp | 4 ++-- test/camera/configuration_set.cpp | 10 ++++----- test/camera/statemachine.cpp | 16 +++++++------- 16 files changed, 87 insertions(+), 90 deletions(-) diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h index 777b7e4d48b8..306739b7014a 100644 --- a/include/libcamera/camera.h +++ b/include/libcamera/camera.h @@ -75,8 +75,8 @@ public: const std::set &streams() const; CameraConfiguration - streamConfiguration(const std::vector &usage); - int configureStreams(const CameraConfiguration &config); + generateConfiguration(const std::vector &usage); + int configure(const CameraConfiguration &config); int allocateBuffers(); int freeBuffers(); diff --git a/src/cam/main.cpp b/src/cam/main.cpp index f03c32b385a9..6a2508dd3bd9 100644 --- a/src/cam/main.cpp +++ b/src/cam/main.cpp @@ -93,7 +93,7 @@ static int prepareCameraConfig(CameraConfiguration *config) /* If no configuration is provided assume a single video stream. */ if (!options.isSet(OptStream)) { - *config = camera->streamConfiguration({ Stream::VideoRecording() }); + *config = camera->generateConfiguration({ Stream::VideoRecording() }); streamInfo[config->front()] = "stream0"; return 0; } @@ -121,7 +121,7 @@ static int prepareCameraConfig(CameraConfiguration *config) } } - *config = camera->streamConfiguration(roles); + *config = camera->generateConfiguration(roles); if (!config->isValid()) { std::cerr << "Failed to get default stream configuration" @@ -211,7 +211,7 @@ static int capture() return ret; } - ret = camera->configureStreams(config); + ret = camera->configure(config); if (ret < 0) { std::cout << "Failed to configure camera" << std::endl; return ret; diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index 1a21acac9899..359174a41823 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -275,10 +275,10 @@ const StreamConfiguration &CameraConfiguration::operator[](Stream *stream) const * Available -> Acquired [label = "acquire()"]; * * Acquired -> Available [label = "release()"]; - * Acquired -> Configured [label = "configureStreams()"]; + * Acquired -> Configured [label = "configure()"]; * * Configured -> Available [label = "release()"]; - * Configured -> Configured [label = "configureStreams()"]; + * Configured -> Configured [label = "configure()"]; * Configured -> Prepared [label = "allocateBuffers()"]; * * Prepared -> Configured [label = "freeBuffers()"]; @@ -542,23 +542,23 @@ const std::set &Camera::streams() const } /** - * \brief Retrieve a group of stream configurations according to stream usages + * \brief Generate a default camera configuration according to stream usages * \param[in] usages A list of stream usages * - * Retrieve configuration for a set of desired usages. The caller specifies a - * list of stream usages and the camera returns a map of suitable streams and - * their suggested default configurations. + * Generate a camera configuration for a set of desired usages. The caller + * specifies a list of stream usages and the camera returns a configuration + * containing suitable streams and their suggested default configurations. * * \return A valid CameraConfiguration if the requested usages can be satisfied, * or a invalid one otherwise */ CameraConfiguration -Camera::streamConfiguration(const std::vector &usages) +Camera::generateConfiguration(const std::vector &usages) { if (disconnected_ || !usages.size() || usages.size() > streams_.size()) return CameraConfiguration(); - CameraConfiguration config = pipe_->streamConfiguration(this, usages); + CameraConfiguration config = pipe_->generateConfiguration(this, usages); std::ostringstream msg("streams configuration:", std::ios_base::ate); unsigned int index = 0; @@ -575,7 +575,7 @@ Camera::streamConfiguration(const std::vector &usages) } /** - * \brief Configure the camera's streams prior to capture + * \brief Configure the camera prior to capture * \param[in] config The camera configurations to setup * * Prior to starting capture, the camera must be configured to select a @@ -584,9 +584,9 @@ Camera::streamConfiguration(const std::vector &usages) * by populating \a config. * * The easiest way to populate the array of config is to fetch an initial - * configuration from the camera with streamConfiguration() and then change the - * parameters to fit the caller's need and once all the streams parameters are - * configured hand that over to configureStreams() to actually setup the camera. + * configuration from the camera with generateConfiguration() and then change + * the parameters to fit the caller's need and once all the streams parameters + * are configured hand that over to configure() to actually setup the camera. * * Exclusive access to the camera shall be ensured by a call to acquire() prior * to calling this function, otherwise an -EACCES error will be returned. @@ -598,7 +598,7 @@ Camera::streamConfiguration(const std::vector &usages) * \retval -EACCES The camera is not in a state where it can be configured * \retval -EINVAL The configuration is not valid */ -int Camera::configureStreams(const CameraConfiguration &config) +int Camera::configure(const CameraConfiguration &config) { int ret; @@ -629,7 +629,7 @@ int Camera::configureStreams(const CameraConfiguration &config) LOG(Camera, Info) << msg.str(); - ret = pipe_->configureStreams(this, config); + ret = pipe_->configure(this, config); if (ret) return ret; diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h index 9f5fe3d673e2..9cc11a8e192e 100644 --- a/src/libcamera/include/pipeline_handler.h +++ b/src/libcamera/include/pipeline_handler.h @@ -61,8 +61,8 @@ public: void unlock(); virtual CameraConfiguration - streamConfiguration(Camera *camera, const std::vector &usages) = 0; - virtual int configureStreams(Camera *camera, const CameraConfiguration &config) = 0; + generateConfiguration(Camera *camera, const std::vector &usages) = 0; + virtual int configure(Camera *camera, const CameraConfiguration &config) = 0; virtual int allocateBuffers(Camera *camera, const std::set &streams) = 0; diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 75a70e66eacc..ba0c708f9e1e 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -151,10 +151,10 @@ public: PipelineHandlerIPU3(CameraManager *manager); CameraConfiguration - streamConfiguration(Camera *camera, - const std::vector &usages) override; - int configureStreams(Camera *camera, - const CameraConfiguration &config) override; + generateConfiguration(Camera *camera, + const std::vector &usages) override; + int configure(Camera *camera, + const CameraConfiguration &config) override; int allocateBuffers(Camera *camera, const std::set &streams) override; @@ -210,8 +210,8 @@ PipelineHandlerIPU3::PipelineHandlerIPU3(CameraManager *manager) } CameraConfiguration -PipelineHandlerIPU3::streamConfiguration(Camera *camera, - const std::vector &usages) +PipelineHandlerIPU3::generateConfiguration(Camera *camera, + const std::vector &usages) { IPU3CameraData *data = cameraData(camera); CameraConfiguration config = {}; @@ -309,8 +309,8 @@ PipelineHandlerIPU3::streamConfiguration(Camera *camera, return config; } -int PipelineHandlerIPU3::configureStreams(Camera *camera, - const CameraConfiguration &config) +int PipelineHandlerIPU3::configure(Camera *camera, + const CameraConfiguration &config) { IPU3CameraData *data = cameraData(camera); IPU3Stream *outStream = &data->outStream_; @@ -631,11 +631,10 @@ bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator) * 1) Link enable/disable cannot be done at start/stop time as video * devices needs to be linked first before format can be configured on * them. - * 2) As link enable has to be done at the least in configureStreams, + * 2) As link enable has to be done at the least in configure(), * before configuring formats, the only place where to disable links * would be 'stop()', but the Camera class state machine allows - * start()<->stop() sequences without any streamConfiguration() in - * between. + * start()<->stop() sequences without any configure() in between. * * As of now, disable all links in the media graph at 'match()' time, * to allow testing different cameras in different test applications diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index b395405c9ddb..caf0abe57c42 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -34,9 +34,9 @@ public: PipelineHandlerRkISP1(CameraManager *manager); ~PipelineHandlerRkISP1(); - CameraConfiguration streamConfiguration(Camera *camera, + CameraConfiguration generateConfiguration(Camera *camera, const std::vector &usages) override; - int configureStreams(Camera *camera, + int configure(Camera *camera, const CameraConfiguration &config) override; int allocateBuffers(Camera *camera, @@ -106,7 +106,7 @@ PipelineHandlerRkISP1::~PipelineHandlerRkISP1() * Pipeline Operations */ -CameraConfiguration PipelineHandlerRkISP1::streamConfiguration(Camera *camera, +CameraConfiguration PipelineHandlerRkISP1::generateConfiguration(Camera *camera, const std::vector &usages) { RkISP1CameraData *data = cameraData(camera); @@ -122,8 +122,8 @@ CameraConfiguration PipelineHandlerRkISP1::streamConfiguration(Camera *camera, return config; } -int PipelineHandlerRkISP1::configureStreams(Camera *camera, - const CameraConfiguration &config) +int PipelineHandlerRkISP1::configure(Camera *camera, + const CameraConfiguration &config) { RkISP1CameraData *data = cameraData(camera); const StreamConfiguration &cfg = config[&data->stream_]; diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index 351712cfdc69..118b97457d2a 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -26,10 +26,10 @@ public: PipelineHandlerUVC(CameraManager *manager); CameraConfiguration - streamConfiguration(Camera *camera, - const std::vector &usages) override; - int configureStreams(Camera *camera, - const CameraConfiguration &config) override; + generateConfiguration(Camera *camera, + const std::vector &usages) override; + int configure(Camera *camera, + const CameraConfiguration &config) override; int allocateBuffers(Camera *camera, const std::set &streams) override; @@ -76,8 +76,8 @@ PipelineHandlerUVC::PipelineHandlerUVC(CameraManager *manager) } CameraConfiguration -PipelineHandlerUVC::streamConfiguration(Camera *camera, - const std::vector &usages) +PipelineHandlerUVC::generateConfiguration(Camera *camera, + const std::vector &usages) { UVCCameraData *data = cameraData(camera); CameraConfiguration config; @@ -92,8 +92,8 @@ PipelineHandlerUVC::streamConfiguration(Camera *camera, return config; } -int PipelineHandlerUVC::configureStreams(Camera *camera, - const CameraConfiguration &config) +int PipelineHandlerUVC::configure(Camera *camera, + const CameraConfiguration &config) { UVCCameraData *data = cameraData(camera); const StreamConfiguration *cfg = &config[&data->stream_]; diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index 737d6df67def..74959581a7ef 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -26,10 +26,10 @@ public: PipelineHandlerVimc(CameraManager *manager); CameraConfiguration - streamConfiguration(Camera *camera, - const std::vector &usages) override; - int configureStreams(Camera *camera, - const CameraConfiguration &config) override; + generateConfiguration(Camera *camera, + const std::vector &usages) override; + int configure(Camera *camera, + const CameraConfiguration &config) override; int allocateBuffers(Camera *camera, const std::set &streams) override; @@ -76,8 +76,8 @@ PipelineHandlerVimc::PipelineHandlerVimc(CameraManager *manager) } CameraConfiguration -PipelineHandlerVimc::streamConfiguration(Camera *camera, - const std::vector &usages) +PipelineHandlerVimc::generateConfiguration(Camera *camera, + const std::vector &usages) { VimcCameraData *data = cameraData(camera); CameraConfiguration config; @@ -92,8 +92,8 @@ PipelineHandlerVimc::streamConfiguration(Camera *camera, return config; } -int PipelineHandlerVimc::configureStreams(Camera *camera, - const CameraConfiguration &config) +int PipelineHandlerVimc::configure(Camera *camera, + const CameraConfiguration &config) { VimcCameraData *data = cameraData(camera); const StreamConfiguration *cfg = &config[&data->stream_]; diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index 1eeaf4bb6dae..b9ac64328f1d 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -218,26 +218,26 @@ void PipelineHandler::unlock() } /** - * \fn PipelineHandler::streamConfiguration() - * \brief Retrieve a group of stream configurations for a specified camera - * \param[in] camera The camera to fetch default configuration from + * \fn PipelineHandler::generateConfiguration() + * \brief Generate a camera configuration for a specified camera + * \param[in] camera The camera to generate a default configuration for * \param[in] usages A list of stream usages * - * Retrieve the species camera's default configuration for a specified group of - * use-cases. The caller shall populate the \a usages array with the use-cases it - * wishes to fetch the default configuration for. The map of streams and - * configurations returned can then be examined by the caller to learn about - * the default parameters for the specified streams. + * Generate a default configuration for the \a camera for a specified group of + * use-cases. The caller shall populate the \a usages array with the use-cases + * it wishes to fetch the default configuration for. The returned configuration + * can then be examined by the caller to learn about the selected streams and + * their default parameters. * - * The intended companion to this is \a configureStreams() which can be used to - * change the group of streams parameters. + * The intended companion to this is \a configure() which can be used to change + * the group of streams parameters. * * \return A valid CameraConfiguration if the requested usages can be satisfied, * or a invalid configuration otherwise */ /** - * \fn PipelineHandler::configureStreams() + * \fn PipelineHandler::configure() * \brief Configure a group of streams for capture * \param[in] camera The camera to configure * \param[in] config The camera configurations to setup @@ -293,9 +293,9 @@ void PipelineHandler::unlock() * \param[in] camera The camera to start * * Start the group of streams that have been configured for capture by - * \a configureStreams(). The intended caller of this method is the Camera - * class which will in turn be called from the application to indicate that it - * has configured the streams and is ready to capture. + * \a configure(). The intended caller of this method is the Camera class which + * will in turn be called from the application to indicate that it has + * configured the streams and is ready to capture. * * \return 0 on success or a negative error code otherwise */ diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp index 4ff296e3a75b..af259510b19c 100644 --- a/src/libcamera/stream.cpp +++ b/src/libcamera/stream.cpp @@ -218,9 +218,8 @@ Stream::Stream() * \brief The stream configuration * * The configuration for the stream is set by any successful call to - * Camera::configureStreams() that includes the stream, and remains valid until - * the next call to Camera::configureStreams() regardless of if it includes the - * stream. + * Camera::configure() that includes the stream, and remains valid until the + * next call to Camera::configure() regardless of if it includes the stream. */ } /* namespace libcamera */ diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp index ee06d751672b..c91b82727ec6 100644 --- a/src/qcam/main_window.cpp +++ b/src/qcam/main_window.cpp @@ -97,9 +97,9 @@ int MainWindow::startCapture() { int ret; - config_ = camera_->streamConfiguration({ Stream::VideoRecording() }); + config_ = camera_->generateConfiguration({ Stream::VideoRecording() }); Stream *stream = config_.front(); - ret = camera_->configureStreams(config_); + ret = camera_->configure(config_); if (ret < 0) { std::cout << "Failed to configure camera" << std::endl; return ret; diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h index 9b31da2bf4b7..143b5b08a5a0 100644 --- a/src/qcam/main_window.h +++ b/src/qcam/main_window.h @@ -35,7 +35,6 @@ private: int openCamera(); int startCapture(); - int configureStreams(Camera *camera, std::set &streams); void stopCapture(); void requestComplete(Request *request, diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp index 0101cc94e665..bc3a4d6cb9f2 100644 --- a/test/camera/capture.cpp +++ b/test/camera/capture.cpp @@ -43,7 +43,7 @@ protected: int run() { CameraConfiguration config = - camera_->streamConfiguration({ Stream::VideoRecording() }); + camera_->generateConfiguration({ Stream::VideoRecording() }); Stream *stream = config.front(); StreamConfiguration *cfg = &config[stream]; @@ -57,7 +57,7 @@ protected: return TestFail; } - if (camera_->configureStreams(config)) { + if (camera_->configure(config)) { cout << "Failed to set default configuration" << endl; return TestFail; } diff --git a/test/camera/configuration_default.cpp b/test/camera/configuration_default.cpp index 2a10ea507a67..340b5f58f04c 100644 --- a/test/camera/configuration_default.cpp +++ b/test/camera/configuration_default.cpp @@ -21,7 +21,7 @@ protected: CameraConfiguration config; /* Test asking for configuration for a video stream. */ - config = camera_->streamConfiguration({ Stream::VideoRecording() }); + config = camera_->generateConfiguration({ Stream::VideoRecording() }); if (!config.isValid()) { cout << "Default configuration invalid" << endl; return TestFail; @@ -31,7 +31,7 @@ protected: * Test that asking for configuration for an empty array of * stream usages returns an empty list of configurations. */ - config = camera_->streamConfiguration({}); + config = camera_->generateConfiguration({}); if (config.isValid()) { cout << "Failed to retrieve configuration for empty usage list" << endl; diff --git a/test/camera/configuration_set.cpp b/test/camera/configuration_set.cpp index ca41ed689511..24d5ca6690b7 100644 --- a/test/camera/configuration_set.cpp +++ b/test/camera/configuration_set.cpp @@ -19,7 +19,7 @@ protected: int run() { CameraConfiguration config = - camera_->streamConfiguration({ Stream::VideoRecording() }); + camera_->generateConfiguration({ Stream::VideoRecording() }); StreamConfiguration *cfg = &config[config.front()]; if (!config.isValid()) { @@ -33,7 +33,7 @@ protected: } /* Test that setting the default configuration works. */ - if (camera_->configureStreams(config)) { + if (camera_->configure(config)) { cout << "Failed to set default configuration" << endl; return TestFail; } @@ -48,7 +48,7 @@ protected: return TestFail; } - if (!camera_->configureStreams(config)) { + if (!camera_->configure(config)) { cout << "Setting configuration on a camera not acquired succeeded when it should have failed" << endl; return TestFail; @@ -66,7 +66,7 @@ protected: */ cfg->size.width *= 2; cfg->size.height *= 2; - if (camera_->configureStreams(config)) { + if (camera_->configure(config)) { cout << "Failed to set modified configuration" << endl; return TestFail; } @@ -75,7 +75,7 @@ protected: * Test that setting an invalid configuration fails. */ cfg->size = { 0, 0 }; - if (!camera_->configureStreams(config)) { + if (!camera_->configure(config)) { cout << "Invalid configuration incorrectly accepted" << endl; return TestFail; } diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp index 8ae93bee7ca1..bd2e61ff2939 100644 --- a/test/camera/statemachine.cpp +++ b/test/camera/statemachine.cpp @@ -19,7 +19,7 @@ protected: int testAvailable() { /* Test operations which should fail. */ - if (camera_->configureStreams(defconf_) != -EACCES) + if (camera_->configure(defconf_) != -EACCES) return TestFail; if (camera_->allocateBuffers() != -EACCES) @@ -84,7 +84,7 @@ protected: if (camera_->acquire()) return TestFail; - if (camera_->configureStreams(defconf_)) + if (camera_->configure(defconf_)) return TestFail; return TestPass; @@ -113,7 +113,7 @@ protected: return TestFail; /* Test operations which should pass. */ - if (camera_->configureStreams(defconf_)) + if (camera_->configure(defconf_)) return TestFail; /* Test valid state transitions, end in Prepared state. */ @@ -123,7 +123,7 @@ protected: if (camera_->acquire()) return TestFail; - if (camera_->configureStreams(defconf_)) + if (camera_->configure(defconf_)) return TestFail; if (camera_->allocateBuffers()) @@ -141,7 +141,7 @@ protected: if (camera_->release() != -EBUSY) return TestFail; - if (camera_->configureStreams(defconf_) != -EACCES) + if (camera_->configure(defconf_) != -EACCES) return TestFail; if (camera_->allocateBuffers() != -EACCES) @@ -172,7 +172,7 @@ protected: if (camera_->acquire()) return TestFail; - if (camera_->configureStreams(defconf_)) + if (camera_->configure(defconf_)) return TestFail; if (camera_->allocateBuffers()) @@ -193,7 +193,7 @@ protected: if (camera_->release() != -EBUSY) return TestFail; - if (camera_->configureStreams(defconf_) != -EACCES) + if (camera_->configure(defconf_) != -EACCES) return TestFail; if (camera_->allocateBuffers() != -EACCES) @@ -235,7 +235,7 @@ protected: int run() { - defconf_ = camera_->streamConfiguration({ Stream::VideoRecording() }); + defconf_ = camera_->generateConfiguration({ Stream::VideoRecording() }); if (testAvailable() != TestPass) { cout << "State machine in Available state failed" << endl; From patchwork Fri May 17 23:06:12 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 1220 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id EF7C860E4C for ; Sat, 18 May 2019 01:06:43 +0200 (CEST) Received: from pendragon.bb.dnainternet.fi (dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi [IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 88A182F3 for ; Sat, 18 May 2019 01:06:43 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1558134403; bh=13UkFDvQt60PRiFHWjjpP6qO3T6gnLFQ7UBse6OrHfQ=; h=From:To:Subject:Date:In-Reply-To:References:From; b=EehRN1791rSVpCUVW5Jrg4jwwu5jCwYCupLxUpi4tybrNMmioE3R5jr2AtGNqN9xy 8i/6uL9CEiRgunYZmsHkzH0CdS2Cxi0ACkWuZeeNGjruczU9CBXStwOLCDcHNUmc1R y9JWumRn5CAkPNvPzZzqPan/rpuXUZEGhs/gmQKg= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sat, 18 May 2019 02:06:12 +0300 Message-Id: <20190517230621.24668-4-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190517230621.24668-1-laurent.pinchart@ideasonboard.com> References: <20190517230621.24668-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH/RFC 03/12] libcamera: Use stream roles directly instead of StreamUsage 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, 17 May 2019 23:06:44 -0000 In order to prepare for an API overhall of the camera configuration generation, remove the StreamUsage class and replace its uses by stream roles. The size hints can't be specified anymore, and will be replaced with an API on the StreamConfiguration to negotiate configuration parameters with cameras. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- include/libcamera/camera.h | 8 +-- include/libcamera/stream.h | 44 ++---------- src/cam/main.cpp | 13 ++-- src/libcamera/camera.cpp | 16 ++--- src/libcamera/include/pipeline_handler.h | 6 +- src/libcamera/pipeline/ipu3/ipu3.cpp | 28 ++++---- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 4 +- src/libcamera/pipeline/uvcvideo.cpp | 5 +- src/libcamera/pipeline/vimc.cpp | 5 +- src/libcamera/pipeline_handler.cpp | 10 +-- src/libcamera/stream.cpp | 89 ++++-------------------- src/qcam/main_window.cpp | 2 +- test/camera/capture.cpp | 2 +- test/camera/configuration_default.cpp | 6 +- test/camera/configuration_set.cpp | 2 +- test/camera/statemachine.cpp | 2 +- 16 files changed, 69 insertions(+), 173 deletions(-) diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h index 306739b7014a..42ba5201eabc 100644 --- a/include/libcamera/camera.h +++ b/include/libcamera/camera.h @@ -14,16 +14,13 @@ #include #include +#include namespace libcamera { class Buffer; class PipelineHandler; class Request; -class Stream; -class StreamUsage; - -struct StreamConfiguration; class CameraConfiguration { @@ -74,8 +71,7 @@ public: int release(); const std::set &streams() const; - CameraConfiguration - generateConfiguration(const std::vector &usage); + CameraConfiguration generateConfiguration(const StreamRoles &roles); int configure(const CameraConfiguration &config); int allocateBuffers(); diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h index 0c986310939b..59bdf217eb31 100644 --- a/include/libcamera/stream.h +++ b/include/libcamera/stream.h @@ -8,6 +8,7 @@ #define __LIBCAMERA_STREAM_H__ #include +#include #include #include @@ -25,48 +26,17 @@ struct StreamConfiguration { std::string toString() const; }; -class StreamUsage -{ -public: - enum Role { - StillCapture, - VideoRecording, - Viewfinder, - }; - - Role role() const { return role_; } - const Size &size() const { return size_; } - -protected: - explicit StreamUsage(Role role); - StreamUsage(Role role, const Size &size); - -private: - Role role_; - Size size_; +enum StreamRole { + StillCapture, + VideoRecording, + Viewfinder, }; +using StreamRoles = std::vector; + class Stream { public: - class StillCapture : public StreamUsage - { - public: - StillCapture(); - }; - - class VideoRecording : public StreamUsage - { - public: - VideoRecording(); - }; - - class Viewfinder : public StreamUsage - { - public: - Viewfinder(int width, int height); - }; - Stream(); BufferPool &bufferPool() { return bufferPool_; } const StreamConfiguration &configuration() const { return configuration_; } diff --git a/src/cam/main.cpp b/src/cam/main.cpp index 6a2508dd3bd9..d603228c0116 100644 --- a/src/cam/main.cpp +++ b/src/cam/main.cpp @@ -87,13 +87,13 @@ static int parseOptions(int argc, char *argv[]) static int prepareCameraConfig(CameraConfiguration *config) { - std::vector roles; + StreamRoles roles; streamInfo.clear(); /* If no configuration is provided assume a single video stream. */ if (!options.isSet(OptStream)) { - *config = camera->generateConfiguration({ Stream::VideoRecording() }); + *config = camera->generateConfiguration({ StreamRole::VideoRecording }); streamInfo[config->front()] = "stream0"; return 0; } @@ -106,14 +106,13 @@ static int prepareCameraConfig(CameraConfiguration *config) KeyValueParser::Options conf = value.toKeyValues(); if (!conf.isSet("role")) { - roles.push_back(Stream::VideoRecording()); + roles.push_back(StreamRole::VideoRecording); } else if (conf["role"].toString() == "viewfinder") { - roles.push_back(Stream::Viewfinder(conf["width"], - conf["height"])); + roles.push_back(StreamRole::Viewfinder); } else if (conf["role"].toString() == "video") { - roles.push_back(Stream::VideoRecording()); + roles.push_back(StreamRole::VideoRecording); } else if (conf["role"].toString() == "still") { - roles.push_back(Stream::StillCapture()); + roles.push_back(StreamRole::StillCapture); } else { std::cerr << "Unknown stream role " << conf["role"].toString() << std::endl; diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index 359174a41823..a3921f91f1c9 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -542,23 +542,23 @@ const std::set &Camera::streams() const } /** - * \brief Generate a default camera configuration according to stream usages - * \param[in] usages A list of stream usages + * \brief Generate a default camera configuration according to stream roles + * \param[in] roles A list of stream roles * - * Generate a camera configuration for a set of desired usages. The caller - * specifies a list of stream usages and the camera returns a configuration + * Generate a camera configuration for a set of desired stream roles. The caller + * specifies a list of stream roles and the camera returns a configuration * containing suitable streams and their suggested default configurations. * - * \return A valid CameraConfiguration if the requested usages can be satisfied, + * \return A valid CameraConfiguration if the requested roles can be satisfied, * or a invalid one otherwise */ CameraConfiguration -Camera::generateConfiguration(const std::vector &usages) +Camera::generateConfiguration(const StreamRoles &roles) { - if (disconnected_ || !usages.size() || usages.size() > streams_.size()) + if (disconnected_ || !roles.size() || roles.size() > streams_.size()) return CameraConfiguration(); - CameraConfiguration config = pipe_->generateConfiguration(this, usages); + CameraConfiguration config = pipe_->generateConfiguration(this, roles); std::ostringstream msg("streams configuration:", std::ios_base::ate); unsigned int index = 0; diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h index 9cc11a8e192e..3352cb0e5bc9 100644 --- a/src/libcamera/include/pipeline_handler.h +++ b/src/libcamera/include/pipeline_handler.h @@ -14,6 +14,8 @@ #include #include +#include + namespace libcamera { class Buffer; @@ -26,8 +28,6 @@ class DeviceMatch; class MediaDevice; class PipelineHandler; class Request; -class Stream; -class StreamUsage; class CameraData { @@ -61,7 +61,7 @@ public: void unlock(); virtual CameraConfiguration - generateConfiguration(Camera *camera, const std::vector &usages) = 0; + generateConfiguration(Camera *camera, const StreamRoles &roles) = 0; virtual int configure(Camera *camera, const CameraConfiguration &config) = 0; virtual int allocateBuffers(Camera *camera, diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index ba0c708f9e1e..d234a8ac5289 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -151,8 +151,7 @@ public: PipelineHandlerIPU3(CameraManager *manager); CameraConfiguration - generateConfiguration(Camera *camera, - const std::vector &usages) override; + generateConfiguration(Camera *camera, const StreamRoles &roles) override; int configure(Camera *camera, const CameraConfiguration &config) override; @@ -211,7 +210,7 @@ PipelineHandlerIPU3::PipelineHandlerIPU3(CameraManager *manager) CameraConfiguration PipelineHandlerIPU3::generateConfiguration(Camera *camera, - const std::vector &usages) + const StreamRoles &roles) { IPU3CameraData *data = cameraData(camera); CameraConfiguration config = {}; @@ -220,13 +219,12 @@ PipelineHandlerIPU3::generateConfiguration(Camera *camera, &data->vfStream_, }; - for (const StreamUsage &usage : usages) { + for (const StreamRole role : roles) { StreamConfiguration cfg = {}; - StreamUsage::Role role = usage.role(); IPU3Stream *stream = nullptr; switch (role) { - case StreamUsage::Role::StillCapture: + case StreamRole::StillCapture: /* * Pick the output stream by default as the Viewfinder * and VideoRecording roles are not allowed on @@ -256,11 +254,11 @@ PipelineHandlerIPU3::generateConfiguration(Camera *camera, break; - case StreamUsage::Role::Viewfinder: - case StreamUsage::Role::VideoRecording: { + case StreamRole::Viewfinder: + case StreamRole::VideoRecording: { /* * We can't use the 'output' stream for viewfinder or - * video capture usages. + * video capture roles. * * \todo This is an artificial limitation until we * figure out the exact capabilities of the hardware. @@ -275,15 +273,13 @@ PipelineHandlerIPU3::generateConfiguration(Camera *camera, stream = &data->vfStream_; /* - * Align the requested viewfinder size to the - * maximum available sensor resolution and to the - * IPU3 alignment constraints. + * Align the default 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); + unsigned int width = std::min(1280U, res.width); + unsigned int height = std::min(720U, res.height); cfg.size = { width & ~7, height & ~3 }; break; diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index caf0abe57c42..05dd517846d6 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -35,7 +35,7 @@ public: ~PipelineHandlerRkISP1(); CameraConfiguration generateConfiguration(Camera *camera, - const std::vector &usages) override; + const StreamRoles &roles) override; int configure(Camera *camera, const CameraConfiguration &config) override; @@ -107,7 +107,7 @@ PipelineHandlerRkISP1::~PipelineHandlerRkISP1() */ CameraConfiguration PipelineHandlerRkISP1::generateConfiguration(Camera *camera, - const std::vector &usages) + const StreamRoles &roles) { RkISP1CameraData *data = cameraData(camera); CameraConfiguration config; diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index 118b97457d2a..d2e1f7d4e5b2 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -26,8 +26,7 @@ public: PipelineHandlerUVC(CameraManager *manager); CameraConfiguration - generateConfiguration(Camera *camera, - const std::vector &usages) override; + generateConfiguration(Camera *camera, const StreamRoles &roles) override; int configure(Camera *camera, const CameraConfiguration &config) override; @@ -77,7 +76,7 @@ PipelineHandlerUVC::PipelineHandlerUVC(CameraManager *manager) CameraConfiguration PipelineHandlerUVC::generateConfiguration(Camera *camera, - const std::vector &usages) + const StreamRoles &roles) { UVCCameraData *data = cameraData(camera); CameraConfiguration config; diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index 74959581a7ef..17e2491e5c27 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -26,8 +26,7 @@ public: PipelineHandlerVimc(CameraManager *manager); CameraConfiguration - generateConfiguration(Camera *camera, - const std::vector &usages) override; + generateConfiguration(Camera *camera, const StreamRoles &roles) override; int configure(Camera *camera, const CameraConfiguration &config) override; @@ -77,7 +76,7 @@ PipelineHandlerVimc::PipelineHandlerVimc(CameraManager *manager) CameraConfiguration PipelineHandlerVimc::generateConfiguration(Camera *camera, - const std::vector &usages) + const StreamRoles &roles) { VimcCameraData *data = cameraData(camera); CameraConfiguration config; diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index b9ac64328f1d..81c11149c9fe 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -221,18 +221,18 @@ void PipelineHandler::unlock() * \fn PipelineHandler::generateConfiguration() * \brief Generate a camera configuration for a specified camera * \param[in] camera The camera to generate a default configuration for - * \param[in] usages A list of stream usages + * \param[in] roles A list of stream roles * - * Generate a default configuration for the \a camera for a specified group of - * use-cases. The caller shall populate the \a usages array with the use-cases - * it wishes to fetch the default configuration for. The returned configuration + * Generate a default configuration for the \a camera for a specified list of + * stream roles. The caller shall populate the \a roles with the use-cases it + * wishes to fetch the default configuration for. The returned configuration * can then be examined by the caller to learn about the selected streams and * their default parameters. * * The intended companion to this is \a configure() which can be used to change * the group of streams parameters. * - * \return A valid CameraConfiguration if the requested usages can be satisfied, + * \return A valid CameraConfiguration if the requested roles can be satisfied, * or a invalid configuration otherwise */ diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp index af259510b19c..fe4c4ecf4150 100644 --- a/src/libcamera/stream.cpp +++ b/src/libcamera/stream.cpp @@ -75,61 +75,31 @@ std::string StreamConfiguration::toString() const } /** - * \class StreamUsage - * \brief Stream usage information - * - * The StreamUsage class describes how an application intends to use a stream. - * Usages are specified by applications and passed to cameras, that then select - * the most appropriate streams and their default configurations. - */ - -/** - * \enum StreamUsage::Role + * \enum StreamRole * \brief Identify the role a stream is intended to play - * \var StreamUsage::StillCapture + * + * The StreamRole describes how an application intends to use a stream. Roles + * are specified by applications and passed to cameras, that then select the + * most appropriate streams and their default configurations. + * + * \var StillCapture * The stream is intended to capture high-resolution, high-quality still images * with low frame rate. The captured frames may be exposed with flash. - * \var StreamUsage::VideoRecording + * \var VideoRecording * The stream is intended to capture video for the purpose of recording or * streaming. The video stream may produce a high frame rate and may be * enhanced with video stabilization. - * \var StreamUsage::Viewfinder + * \var Viewfinder * The stream is intended to capture video for the purpose of display on the - * local screen. The StreamUsage includes the desired resolution. Trade-offs - * between quality and usage of system resources are acceptable. + * local screen. Trade-offs between quality and usage of system resources are + * acceptable. */ /** - * \fn StreamUsage::role() - * \brief Retrieve the stream role - * \return The stream role + * \typedef StreamRoles + * \brief A vector of StreamRole */ -/** - * \fn StreamUsage::size() - * \brief Retrieve desired size - * \return The desired size - */ - -/** - * \brief Create a stream usage - * \param[in] role Stream role - */ -StreamUsage::StreamUsage(Role role) - : role_(role) -{ -} - -/** - * \brief Create a stream usage with a desired size - * \param[in] role Stream role - * \param[in] size The desired size - */ -StreamUsage::StreamUsage(Role role, const Size &size) - : role_(role), size_(size) -{ -} - /** * \class Stream * \brief Video stream for a camera @@ -148,39 +118,6 @@ StreamUsage::StreamUsage(Role role, const Size &size) * optimal stream for the task. */ -/** - * \class Stream::StillCapture - * \brief Describe a still capture usage - */ -Stream::StillCapture::StillCapture() - : StreamUsage(Role::StillCapture) -{ -} - -/** - * \class Stream::VideoRecording - * \brief Describe a video recording usage - */ -Stream::VideoRecording::VideoRecording() - : StreamUsage(Role::VideoRecording) -{ -} - -/** - * \class Stream::Viewfinder - * \brief Describe a viewfinder usage - */ - -/** - * \brief Create a viewfinder usage with a desired dimension - * \param[in] width The desired viewfinder width - * \param[in] height The desired viewfinder height - */ -Stream::Viewfinder::Viewfinder(int width, int height) - : StreamUsage(Role::Viewfinder, Size(width, height)) -{ -} - /** * \brief Construct a stream with default parameters */ diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp index c91b82727ec6..a984aaca764f 100644 --- a/src/qcam/main_window.cpp +++ b/src/qcam/main_window.cpp @@ -97,7 +97,7 @@ int MainWindow::startCapture() { int ret; - config_ = camera_->generateConfiguration({ Stream::VideoRecording() }); + config_ = camera_->generateConfiguration({ StreamRole::VideoRecording }); Stream *stream = config_.front(); ret = camera_->configure(config_); if (ret < 0) { diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp index bc3a4d6cb9f2..e7e6438203b9 100644 --- a/test/camera/capture.cpp +++ b/test/camera/capture.cpp @@ -43,7 +43,7 @@ protected: int run() { CameraConfiguration config = - camera_->generateConfiguration({ Stream::VideoRecording() }); + camera_->generateConfiguration({ StreamRole::VideoRecording }); Stream *stream = config.front(); StreamConfiguration *cfg = &config[stream]; diff --git a/test/camera/configuration_default.cpp b/test/camera/configuration_default.cpp index 340b5f58f04c..8c4a03db498a 100644 --- a/test/camera/configuration_default.cpp +++ b/test/camera/configuration_default.cpp @@ -21,7 +21,7 @@ protected: CameraConfiguration config; /* Test asking for configuration for a video stream. */ - config = camera_->generateConfiguration({ Stream::VideoRecording() }); + config = camera_->generateConfiguration({ StreamRole::VideoRecording }); if (!config.isValid()) { cout << "Default configuration invalid" << endl; return TestFail; @@ -29,11 +29,11 @@ protected: /* * Test that asking for configuration for an empty array of - * stream usages returns an empty list of configurations. + * stream roles returns an empty list of configurations. */ config = camera_->generateConfiguration({}); if (config.isValid()) { - cout << "Failed to retrieve configuration for empty usage list" + cout << "Failed to retrieve configuration for empty roles list" << endl; return TestFail; } diff --git a/test/camera/configuration_set.cpp b/test/camera/configuration_set.cpp index 24d5ca6690b7..76d8bc3e40a4 100644 --- a/test/camera/configuration_set.cpp +++ b/test/camera/configuration_set.cpp @@ -19,7 +19,7 @@ protected: int run() { CameraConfiguration config = - camera_->generateConfiguration({ Stream::VideoRecording() }); + camera_->generateConfiguration({ StreamRole::VideoRecording }); StreamConfiguration *cfg = &config[config.front()]; if (!config.isValid()) { diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp index bd2e61ff2939..7a74cd85a37a 100644 --- a/test/camera/statemachine.cpp +++ b/test/camera/statemachine.cpp @@ -235,7 +235,7 @@ protected: int run() { - defconf_ = camera_->generateConfiguration({ Stream::VideoRecording() }); + defconf_ = camera_->generateConfiguration({ StreamRole::VideoRecording }); if (testAvailable() != TestPass) { cout << "State machine in Available state failed" << endl; From patchwork Fri May 17 23:06:13 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 1221 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 52E9860E4C for ; Sat, 18 May 2019 01:06:44 +0200 (CEST) Received: from pendragon.bb.dnainternet.fi (dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi [IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id DB9E5336 for ; Sat, 18 May 2019 01:06:43 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1558134404; bh=n+rBe2oFZrtdlzRe/k94cKvG9e0y4QFu4/tYQpJRPkE=; h=From:To:Subject:Date:In-Reply-To:References:From; b=eHx2jvDp2kw2tAZWp0DRYxAuXOcvS133hcCtl5LtDduaYRCAChpV8hNLi8tiU4/Wk 3kNHIwk2+3V2vUPlNtdWQv7B+6u+qj2DtmxXtESnTXVEsmOkHHs44cDNjAvRu7OfzF 3lceMWU9N+tr50SMR0TucKWJbLiFWsPUFaAVrdYE= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sat, 18 May 2019 02:06:13 +0300 Message-Id: <20190517230621.24668-5-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190517230621.24668-1-laurent.pinchart@ideasonboard.com> References: <20190517230621.24668-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH/RFC 04/12] libcamera: Refactor the camera configuration storage and API 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, 17 May 2019 23:06:44 -0000 Refactor the CameraConfiguration structure to not rely on Stream instances. This is a step towards making the camera configuration object more powerful with configuration validation using "try" semantics. As applications need access to the Stream instances associated with the configuration entries in order to associate buffers with streams when creating requests, expose the stream selected by the pipeline handler through a new StreamConfiguration::stream(). Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- include/libcamera/camera.h | 19 ++- include/libcamera/stream.h | 7 + src/cam/main.cpp | 35 ++--- src/libcamera/camera.cpp | 174 +++++++++-------------- src/libcamera/include/pipeline_handler.h | 2 +- src/libcamera/pipeline/ipu3/ipu3.cpp | 32 ++--- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 12 +- src/libcamera/pipeline/uvcvideo.cpp | 23 ++- src/libcamera/pipeline/vimc.cpp | 23 ++- src/libcamera/pipeline_handler.cpp | 4 + src/libcamera/stream.cpp | 22 +++ src/qcam/main_window.cpp | 4 +- test/camera/capture.cpp | 4 +- test/camera/configuration_set.cpp | 2 +- 14 files changed, 179 insertions(+), 184 deletions(-) diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h index 42ba5201eabc..e2759405f497 100644 --- a/include/libcamera/camera.h +++ b/include/libcamera/camera.h @@ -25,11 +25,13 @@ class Request; class CameraConfiguration { public: - using iterator = std::vector::iterator; - using const_iterator = std::vector::const_iterator; + using iterator = std::vector::iterator; + using const_iterator = std::vector::const_iterator; CameraConfiguration(); + void addConfiguration(const StreamConfiguration &cfg); + iterator begin(); iterator end(); const_iterator begin() const; @@ -39,16 +41,11 @@ public: bool isEmpty() const; std::size_t size() const; - Stream *front(); - const Stream *front() const; - - Stream *operator[](unsigned int index) const; - StreamConfiguration &operator[](Stream *stream); - const StreamConfiguration &operator[](Stream *stream) const; + StreamConfiguration &operator[](unsigned int index); + const StreamConfiguration &operator[](unsigned int index) const; private: - std::vector order_; - std::map config_; + std::vector config_; }; class Camera final @@ -72,7 +69,7 @@ public: const std::set &streams() const; CameraConfiguration generateConfiguration(const StreamRoles &roles); - int configure(const CameraConfiguration &config); + int configure(CameraConfiguration &config); int allocateBuffers(); int freeBuffers(); diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h index 59bdf217eb31..47c007ed52e2 100644 --- a/include/libcamera/stream.h +++ b/include/libcamera/stream.h @@ -16,6 +16,7 @@ namespace libcamera { class Camera; +class Stream; struct StreamConfiguration { unsigned int pixelFormat; @@ -23,7 +24,13 @@ struct StreamConfiguration { unsigned int bufferCount; + Stream *stream() const { return stream_; } + void setStream(Stream *stream) { stream_ = stream; } + std::string toString() const; + +private: + Stream *stream_; }; enum StreamRole { diff --git a/src/cam/main.cpp b/src/cam/main.cpp index d603228c0116..cd165bea34cd 100644 --- a/src/cam/main.cpp +++ b/src/cam/main.cpp @@ -89,12 +89,9 @@ static int prepareCameraConfig(CameraConfiguration *config) { StreamRoles roles; - streamInfo.clear(); - /* If no configuration is provided assume a single video stream. */ if (!options.isSet(OptStream)) { *config = camera->generateConfiguration({ StreamRole::VideoRecording }); - streamInfo[config->front()] = "stream0"; return 0; } @@ -129,27 +126,20 @@ static int prepareCameraConfig(CameraConfiguration *config) } /* Apply configuration explicitly requested. */ - CameraConfiguration::iterator it = config->begin(); + unsigned int i = 0; for (auto const &value : streamOptions) { KeyValueParser::Options conf = value.toKeyValues(); - Stream *stream = *it; - it++; + StreamConfiguration &cfg = (*config)[i++]; if (conf.isSet("width")) - (*config)[stream].size.width = conf["width"]; + cfg.size.width = conf["width"]; if (conf.isSet("height")) - (*config)[stream].size.height = conf["height"]; + cfg.size.height = conf["height"]; /* TODO: Translate 4CC string to ID. */ if (conf.isSet("pixelformat")) - (*config)[stream].pixelFormat = conf["pixelformat"]; - } - - unsigned int index = 0; - for (Stream *stream : *config) { - streamInfo[stream] = "stream" + std::to_string(index); - index++; + cfg.pixelFormat = conf["pixelformat"]; } return 0; @@ -216,6 +206,13 @@ static int capture() return ret; } + streamInfo.clear(); + + for (unsigned int index = 0; index < config.size(); ++index) { + StreamConfiguration &cfg = config[index]; + streamInfo[cfg.stream()] = "stream" + std::to_string(index); + } + ret = camera->allocateBuffers(); if (ret) { std::cerr << "Failed to allocate buffers" @@ -227,8 +224,10 @@ static int capture() /* Identify the stream with the least number of buffers. */ unsigned int nbuffers = UINT_MAX; - for (Stream *stream : config) + for (StreamConfiguration &cfg : config) { + Stream *stream = cfg.stream(); nbuffers = std::min(nbuffers, stream->bufferPool().count()); + } /* * TODO: make cam tool smarter to support still capture by for @@ -245,8 +244,10 @@ static int capture() } std::map map; - for (Stream *stream : config) + for (StreamConfiguration &cfg : config) { + Stream *stream = cfg.stream(); map[stream] = &stream->bufferPool().buffers()[i]; + } ret = request->setBuffers(map); if (ret < 0) { diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index a3921f91f1c9..f8a4946b4a6a 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -46,72 +46,81 @@ LOG_DECLARE_CATEGORY(Camera) * \class CameraConfiguration * \brief Hold configuration for streams of the camera - * The CameraConfiguration holds an ordered list of streams and their associated - * StreamConfiguration. From a data storage point of view, the class operates as - * a map of Stream pointers to StreamConfiguration, with entries accessed with - * operator[](Stream *). Accessing an entry for a Stream pointer not yet stored - * in the configuration inserts a new empty entry. - * - * The class also suppors iterators, and from that point of view operates as a - * vector of Stream pointers. The streams are iterated in insertion order, and - * the operator[](int) returns the Stream pointer based on its insertion index. - * Accessing a stream with an invalid index returns a null pointer. + * The CameraConfiguration holds an ordered list of stream configurations. It + * supports iterators and operates as a vector of StreamConfiguration instances. + * The stream configurations are inserted by addConfiguration(), and the + * operator[](int) returns a reference to the StreamConfiguration based on its + * insertion index. Accessing a stream configuration with an invalid index + * results in undefined behaviour. */ /** * \typedef CameraConfiguration::iterator - * \brief Iterator for the streams in the configuration + * \brief Iterator for the stream configurations in the camera configuration */ /** * \typedef CameraConfiguration::const_iterator - * \brief Const iterator for the streams in the configuration + * \brief Const iterator for the stream configuration in the camera + * configuration */ /** * \brief Create an empty camera configuration */ CameraConfiguration::CameraConfiguration() - : order_({}), config_({}) + : config_({}) { } /** - * \brief Retrieve an iterator to the first stream in the sequence - * \return An iterator to the first stream + * \brief Add a stream configuration to the camera configuration + * \param[in] cfg The stream configuration */ -std::vector::iterator CameraConfiguration::begin() +void CameraConfiguration::addConfiguration(const StreamConfiguration &cfg) { - return order_.begin(); + config_.push_back(cfg); } /** - * \brief Retrieve an iterator pointing to the past-the-end stream in the + * \brief Retrieve an iterator to the first stream configuration in the * sequence - * \return An iterator to the element following the last stream + * \return An iterator to the first stream configuration */ -std::vector::iterator CameraConfiguration::end() +CameraConfiguration::iterator CameraConfiguration::begin() { - return order_.end(); + return config_.begin(); } /** - * \brief Retrieve a const iterator to the first element of the streams - * \return A const iterator to the first stream + * \brief Retrieve an iterator pointing to the past-the-end stream + * configuration in the sequence + * \return An iterator to the element following the last stream configuration */ -std::vector::const_iterator CameraConfiguration::begin() const +CameraConfiguration::iterator CameraConfiguration::end() { - return order_.begin(); + return config_.end(); } /** - * \brief Retrieve a const iterator pointing to the past-the-end stream in the - * sequence + * \brief Retrieve a const iterator to the first element of the stream + * configurations + * \return A const iterator to the first stream configuration + */ +CameraConfiguration::const_iterator CameraConfiguration::begin() const +{ + return config_.begin(); +} + +/** + * \brief Retrieve a const iterator pointing to the past-the-end stream + * configuration in the sequence * \return A const iterator to the element following the last stream + * configuration */ -std::vector::const_iterator CameraConfiguration::end() const +CameraConfiguration::const_iterator CameraConfiguration::end() const { - return order_.end(); + return config_.end(); } /** @@ -128,9 +137,7 @@ bool CameraConfiguration::isValid() const if (isEmpty()) return false; - for (auto const &it : config_) { - const StreamConfiguration &cfg = it.second; - + for (const StreamConfiguration &cfg : config_) { if (cfg.size.width == 0 || cfg.size.height == 0 || cfg.pixelFormat == 0 || cfg.bufferCount == 0) return false; @@ -145,7 +152,7 @@ bool CameraConfiguration::isValid() const */ bool CameraConfiguration::isEmpty() const { - return order_.empty(); + return config_.empty(); } /** @@ -154,75 +161,37 @@ bool CameraConfiguration::isEmpty() const */ std::size_t CameraConfiguration::size() const { - return order_.size(); -} - -/** - * \brief Access the first stream in the configuration - * \return The first stream in the configuration - */ -Stream *CameraConfiguration::front() -{ - return order_.front(); -} - -/** - * \brief Access the first stream in the configuration - * \return The first const stream pointer in the configuration - */ -const Stream *CameraConfiguration::front() const -{ - return order_.front(); -} - -/** - * \brief Retrieve a stream pointer from index - * \param[in] index Numerical index - * - * The \a index represents the zero based insertion order of stream and stream - * configuration into the camera configuration. - * - * \return The stream pointer at index, or a nullptr if the index is out of - * bounds - */ -Stream *CameraConfiguration::operator[](unsigned int index) const -{ - if (index >= order_.size()) - return nullptr; - - return order_.at(index); + return config_.size(); } /** * \brief Retrieve a reference to a stream configuration - * \param[in] stream Stream to retrieve configuration for + * \param[in] index Numerical index * - * If the camera configuration does not yet contain a configuration for - * the requested stream, create and return an empty stream configuration. + * The \a index represents the zero based insertion order of stream + * configuration into the camera configuration with addConfiguration(). Calling + * this method with an invalid index results in undefined behaviour. * - * \return The configuration for the stream + * \return The stream configuration */ -StreamConfiguration &CameraConfiguration::operator[](Stream *stream) +StreamConfiguration &CameraConfiguration::operator[](unsigned int index) { - if (config_.find(stream) == config_.end()) - order_.push_back(stream); - - return config_[stream]; + return config_[index]; } /** * \brief Retrieve a const reference to a stream configuration - * \param[in] stream Stream to retrieve configuration for + * \param[in] index Numerical index * - * No new stream configuration is created if called with \a stream that is not - * already part of the camera configuration, doing so is an invalid operation - * and results in undefined behaviour. + * The \a index represents the zero based insertion order of stream + * configuration into the camera configuration with addConfiguration(). Calling + * this method with an invalid index results in undefined behaviour. * - * \return The configuration for the stream + * \return The stream configuration */ -const StreamConfiguration &CameraConfiguration::operator[](Stream *stream) const +const StreamConfiguration &CameraConfiguration::operator[](unsigned int index) const { - return config_.at(stream); + return config_[index]; } /** @@ -561,13 +530,9 @@ Camera::generateConfiguration(const StreamRoles &roles) CameraConfiguration config = pipe_->generateConfiguration(this, roles); std::ostringstream msg("streams configuration:", std::ios_base::ate); - unsigned int index = 0; - for (Stream *stream : config) { - const StreamConfiguration &cfg = config[stream]; - msg << " (" << index << ") " << cfg.toString(); - index++; - } + for (unsigned int index = 0; index < config.size(); ++index) + msg << " (" << index << ") " << config[index].toString(); LOG(Camera, Debug) << msg.str(); @@ -593,12 +558,15 @@ Camera::generateConfiguration(const StreamRoles &roles) * * This function affects the state of the camera, see \ref camera_operation. * + * Upon return the StreamConfiguration entries in \a config are associated with + * Stream instances which can be retrieved with StreamConfiguration::stream(). + * * \return 0 on success or a negative error code otherwise * \retval -ENODEV The camera has been disconnected from the system * \retval -EACCES The camera is not in a state where it can be configured * \retval -EINVAL The configuration is not valid */ -int Camera::configure(const CameraConfiguration &config) +int Camera::configure(CameraConfiguration &config) { int ret; @@ -615,16 +583,11 @@ int Camera::configure(const CameraConfiguration &config) } std::ostringstream msg("configuring streams:", std::ios_base::ate); - unsigned int index = 0; - for (Stream *stream : config) { - if (streams_.find(stream) == streams_.end()) - return -EINVAL; - - const StreamConfiguration &cfg = config[stream]; - msg << std::dec << " (" << index << ") " << cfg.toString(); - - index++; + for (unsigned int index = 0; index < config.size(); ++index) { + StreamConfiguration &cfg = config[index]; + cfg.setStream(nullptr); + msg << " (" << index << ") " << cfg.toString(); } LOG(Camera, Info) << msg.str(); @@ -634,8 +597,11 @@ int Camera::configure(const CameraConfiguration &config) return ret; activeStreams_.clear(); - for (Stream *stream : config) { - const StreamConfiguration &cfg = config[stream]; + for (const StreamConfiguration &cfg : config) { + Stream *stream = cfg.stream(); + if (!stream) + LOG(Camera, Fatal) + << "Pipeline handler failed to update stream configuration"; stream->configuration_ = cfg; activeStreams_.insert(stream); diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h index 3352cb0e5bc9..a025905ab68f 100644 --- a/src/libcamera/include/pipeline_handler.h +++ b/src/libcamera/include/pipeline_handler.h @@ -62,7 +62,7 @@ public: virtual CameraConfiguration generateConfiguration(Camera *camera, const StreamRoles &roles) = 0; - virtual int configure(Camera *camera, const CameraConfiguration &config) = 0; + virtual int configure(Camera *camera, CameraConfiguration &config) = 0; virtual int allocateBuffers(Camera *camera, const std::set &streams) = 0; diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index d234a8ac5289..ed0ef69de1d1 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -152,8 +152,7 @@ public: CameraConfiguration generateConfiguration(Camera *camera, const StreamRoles &roles) override; - int configure(Camera *camera, - const CameraConfiguration &config) override; + int configure(Camera *camera, CameraConfiguration &config) override; int allocateBuffers(Camera *camera, const std::set &streams) override; @@ -299,14 +298,13 @@ PipelineHandlerIPU3::generateConfiguration(Camera *camera, cfg.pixelFormat = V4L2_PIX_FMT_NV12; cfg.bufferCount = IPU3_BUFFER_COUNT; - config[stream] = cfg; + config.addConfiguration(cfg); } return config; } -int PipelineHandlerIPU3::configure(Camera *camera, - const CameraConfiguration &config) +int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration &config) { IPU3CameraData *data = cameraData(camera); IPU3Stream *outStream = &data->outStream_; @@ -318,9 +316,13 @@ int PipelineHandlerIPU3::configure(Camera *camera, outStream->active_ = false; vfStream->active_ = false; - for (Stream *s : config) { - IPU3Stream *stream = static_cast(s); - const StreamConfiguration &cfg = config[stream]; + for (StreamConfiguration &cfg : config) { + /* + * Pick a stream for the configuration entry. + * \todo: This is a naive temporary implementation that will be + * reworked when implementing camera configuration validation. + */ + IPU3Stream *stream = vfStream->active_ ? outStream : vfStream; /* * Verify that the requested size respects the IPU3 alignment @@ -355,6 +357,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, sensorSize.height = cfg.size.height; stream->active_ = true; + cfg.setStream(stream); } /* @@ -379,10 +382,9 @@ int PipelineHandlerIPU3::configure(Camera *camera, return ret; /* Apply the format to the configured streams output devices. */ - for (Stream *s : config) { - IPU3Stream *stream = static_cast(s); - - ret = imgu->configureOutput(stream->device_, config[stream]); + for (StreamConfiguration &cfg : config) { + IPU3Stream *stream = static_cast(cfg.stream()); + ret = imgu->configureOutput(stream->device_, cfg); if (ret) return ret; } @@ -393,15 +395,13 @@ int PipelineHandlerIPU3::configure(Camera *camera, * be at least one active stream in the configuration request). */ if (!outStream->active_) { - ret = imgu->configureOutput(outStream->device_, - config[vfStream]); + ret = imgu->configureOutput(outStream->device_, config[0]); if (ret) return ret; } if (!vfStream->active_) { - ret = imgu->configureOutput(vfStream->device_, - config[outStream]); + ret = imgu->configureOutput(vfStream->device_, config[0]); if (ret) return ret; } diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 05dd517846d6..6b35aa3fe7c3 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -36,8 +36,7 @@ public: CameraConfiguration generateConfiguration(Camera *camera, const StreamRoles &roles) override; - int configure(Camera *camera, - const CameraConfiguration &config) override; + int configure(Camera *camera, CameraConfiguration &config) override; int allocateBuffers(Camera *camera, const std::set &streams) override; @@ -117,16 +116,15 @@ CameraConfiguration PipelineHandlerRkISP1::generateConfiguration(Camera *camera, cfg.size = data->sensor_->resolution(); cfg.bufferCount = RKISP1_BUFFER_COUNT; - config[&data->stream_] = cfg; + config.addConfiguration(cfg); return config; } -int PipelineHandlerRkISP1::configure(Camera *camera, - const CameraConfiguration &config) +int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration &config) { RkISP1CameraData *data = cameraData(camera); - const StreamConfiguration &cfg = config[&data->stream_]; + StreamConfiguration &cfg = config[0]; CameraSensor *sensor = data->sensor_; int ret; @@ -220,6 +218,8 @@ int PipelineHandlerRkISP1::configure(Camera *camera, return -EINVAL; } + cfg.setStream(&data->stream_); + return 0; } diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index d2e1f7d4e5b2..5dcc868b2fc9 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -27,8 +27,7 @@ public: CameraConfiguration generateConfiguration(Camera *camera, const StreamRoles &roles) override; - int configure(Camera *camera, - const CameraConfiguration &config) override; + int configure(Camera *camera, CameraConfiguration &config) override; int allocateBuffers(Camera *camera, const std::set &streams) override; @@ -78,38 +77,38 @@ CameraConfiguration PipelineHandlerUVC::generateConfiguration(Camera *camera, const StreamRoles &roles) { - UVCCameraData *data = cameraData(camera); CameraConfiguration config; - StreamConfiguration cfg{}; + StreamConfiguration cfg; cfg.pixelFormat = V4L2_PIX_FMT_YUYV; cfg.size = { 640, 480 }; cfg.bufferCount = 4; - config[&data->stream_] = cfg; + config.addConfiguration(cfg); return config; } -int PipelineHandlerUVC::configure(Camera *camera, - const CameraConfiguration &config) +int PipelineHandlerUVC::configure(Camera *camera, CameraConfiguration &config) { UVCCameraData *data = cameraData(camera); - const StreamConfiguration *cfg = &config[&data->stream_]; + StreamConfiguration &cfg = config[0]; int ret; V4L2DeviceFormat format = {}; - format.fourcc = cfg->pixelFormat; - format.size = cfg->size; + format.fourcc = cfg.pixelFormat; + format.size = cfg.size; ret = data->video_->setFormat(&format); if (ret) return ret; - if (format.size != cfg->size || - format.fourcc != cfg->pixelFormat) + if (format.size != cfg.size || + format.fourcc != cfg.pixelFormat) return -EINVAL; + cfg.setStream(&data->stream_); + return 0; } diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index 17e2491e5c27..af6b6f21e3c5 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -27,8 +27,7 @@ public: CameraConfiguration generateConfiguration(Camera *camera, const StreamRoles &roles) override; - int configure(Camera *camera, - const CameraConfiguration &config) override; + int configure(Camera *camera, CameraConfiguration &config) override; int allocateBuffers(Camera *camera, const std::set &streams) override; @@ -78,38 +77,38 @@ CameraConfiguration PipelineHandlerVimc::generateConfiguration(Camera *camera, const StreamRoles &roles) { - VimcCameraData *data = cameraData(camera); CameraConfiguration config; - StreamConfiguration cfg{}; + StreamConfiguration cfg; cfg.pixelFormat = V4L2_PIX_FMT_RGB24; cfg.size = { 640, 480 }; cfg.bufferCount = 4; - config[&data->stream_] = cfg; + config.addConfiguration(cfg); return config; } -int PipelineHandlerVimc::configure(Camera *camera, - const CameraConfiguration &config) +int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration &config) { VimcCameraData *data = cameraData(camera); - const StreamConfiguration *cfg = &config[&data->stream_]; + StreamConfiguration &cfg = config[0]; int ret; V4L2DeviceFormat format = {}; - format.fourcc = cfg->pixelFormat; - format.size = cfg->size; + format.fourcc = cfg.pixelFormat; + format.size = cfg.size; ret = data->video_->setFormat(&format); if (ret) return ret; - if (format.size != cfg->size || - format.fourcc != cfg->pixelFormat) + if (format.size != cfg.size || + format.fourcc != cfg.pixelFormat) return -EINVAL; + cfg.setStream(&data->stream_); + return 0; } diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index 81c11149c9fe..4185e3557dcb 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -255,6 +255,10 @@ void PipelineHandler::unlock() * configuration of a subset of the streams can't be satisfied, the * whole configuration is considered invalid. * + * Once the configuration is validated and the camera configured, the pipeline + * handler shall associate a Stream instance to each StreamConfiguration entry + * in the CameraConfiguration with the StreamConfiguration::setStream() method. + * * \return 0 on success or a negative error code otherwise */ diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp index fe4c4ecf4150..0c59a31a3a05 100644 --- a/src/libcamera/stream.cpp +++ b/src/libcamera/stream.cpp @@ -58,6 +58,28 @@ namespace libcamera { * \brief Requested number of buffers to allocate for the stream */ +/** + * \fn StreamConfiguration::stream() + * \brief Retrieve the stream associated with the configuration + * + * When a camera is configured with Camera::configure() Stream instances are + * associated with each stream configuration entry. This method retrieves the + * associated Stream, which remains valid until the next call to + * Camera::configure() or Camera::release(). + * + * \return The stream associated with the configuration + */ + +/** + * \fn StreamConfiguration::setStream() + * \brief Associate a stream with a configuration + * + * This method is meant for the PipelineHandler::configure() method and shall + * not be called by applications. + * + * \param[in] stream The stream + */ + /** * \brief Assemble and return a string describing the configuration * diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp index a984aaca764f..06ae2985f80d 100644 --- a/src/qcam/main_window.cpp +++ b/src/qcam/main_window.cpp @@ -98,14 +98,14 @@ int MainWindow::startCapture() int ret; config_ = camera_->generateConfiguration({ StreamRole::VideoRecording }); - Stream *stream = config_.front(); ret = camera_->configure(config_); if (ret < 0) { std::cout << "Failed to configure camera" << std::endl; return ret; } - const StreamConfiguration &cfg = config_[stream]; + const StreamConfiguration &cfg = config_[0]; + Stream *stream = cfg.stream(); ret = viewfinder_->setFormat(cfg.pixelFormat, cfg.size.width, cfg.size.height); if (ret < 0) { diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp index e7e6438203b9..bfd11eefedcf 100644 --- a/test/camera/capture.cpp +++ b/test/camera/capture.cpp @@ -44,8 +44,7 @@ protected: { CameraConfiguration config = camera_->generateConfiguration({ StreamRole::VideoRecording }); - Stream *stream = config.front(); - StreamConfiguration *cfg = &config[stream]; + StreamConfiguration *cfg = &config[0]; if (!config.isValid()) { cout << "Failed to read default configuration" << endl; @@ -67,6 +66,7 @@ protected: return TestFail; } + Stream *stream = cfg->stream(); BufferPool &pool = stream->bufferPool(); std::vector requests; for (Buffer &buffer : pool.buffers()) { diff --git a/test/camera/configuration_set.cpp b/test/camera/configuration_set.cpp index 76d8bc3e40a4..25b5db67103a 100644 --- a/test/camera/configuration_set.cpp +++ b/test/camera/configuration_set.cpp @@ -20,7 +20,7 @@ protected: { CameraConfiguration config = camera_->generateConfiguration({ StreamRole::VideoRecording }); - StreamConfiguration *cfg = &config[config.front()]; + StreamConfiguration *cfg = &config[0]; if (!config.isValid()) { cout << "Failed to read default configuration" << endl; From patchwork Fri May 17 23:06:14 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 1222 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id A052761865 for ; Sat, 18 May 2019 01:06:44 +0200 (CEST) Received: from pendragon.bb.dnainternet.fi (dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi [IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 3FD7D2F3 for ; Sat, 18 May 2019 01:06:44 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1558134404; bh=Cqi5EoCt+AqmKzbNpEwUV4+oKrz/iEFc7eqLl8qb7UY=; h=From:To:Subject:Date:In-Reply-To:References:From; b=wSzLpTxEz4Sek1elDRMrpSWT3eXShdXsRzncXGvLgMLq+JILU9De8EMFWNu6jyuHU +seIsJJsktxP1UvhXIDEzJXe0hEdPYfL+WXGERdbnlas1YP/E5N/j1104f85VIyfFC 5D/7Q3vj/NjE7RVLkf3vkFfPJE3l437dsvypDC0A= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sat, 18 May 2019 02:06:14 +0300 Message-Id: <20190517230621.24668-6-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190517230621.24668-1-laurent.pinchart@ideasonboard.com> References: <20190517230621.24668-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH/RFC 05/12] libcamera: camera: Return a pointer from generateConfiguration() 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, 17 May 2019 23:06:44 -0000 To prepare for specialising the CameraConfiguration class in pipeline handlers, return a pointer to a camera configuration instead of a reference from Camera::generateConfiguration(). The camera configuration always needs to be allocated from the pipeline handler, and its ownership is passed to the application. For symmetry, change Camera::configure() to take a CameraConfiguration pointer instead of a reference. This aligns with our coding practice of passing parameters that are modified by the callee by pointer. Signed-off-by: Laurent Pinchart --- include/libcamera/camera.h | 4 +-- src/cam/main.cpp | 38 ++++++++++---------- src/libcamera/camera.cpp | 31 +++++++++-------- src/libcamera/include/pipeline_handler.h | 6 ++-- src/libcamera/pipeline/ipu3/ipu3.cpp | 31 +++++++++-------- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 25 ++++++++------ src/libcamera/pipeline/uvcvideo.cpp | 30 ++++++++-------- src/libcamera/pipeline/vimc.cpp | 30 ++++++++-------- src/libcamera/pipeline_handler.cpp | 3 +- src/qcam/main_window.cpp | 5 ++- src/qcam/main_window.h | 2 +- test/camera/capture.cpp | 38 +++++++++++++++----- test/camera/configuration_default.cpp | 14 +++++--- test/camera/configuration_set.cpp | 44 +++++++++++++++++------- test/camera/statemachine.cpp | 22 ++++++++++-- 15 files changed, 201 insertions(+), 122 deletions(-) diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h index e2759405f497..841e8fc505b9 100644 --- a/include/libcamera/camera.h +++ b/include/libcamera/camera.h @@ -68,8 +68,8 @@ public: int release(); const std::set &streams() const; - CameraConfiguration generateConfiguration(const StreamRoles &roles); - int configure(CameraConfiguration &config); + CameraConfiguration *generateConfiguration(const StreamRoles &roles); + int configure(CameraConfiguration *config); int allocateBuffers(); int freeBuffers(); diff --git a/src/cam/main.cpp b/src/cam/main.cpp index cd165bea34cd..7a1b332f68c7 100644 --- a/src/cam/main.cpp +++ b/src/cam/main.cpp @@ -85,15 +85,13 @@ static int parseOptions(int argc, char *argv[]) return 0; } -static int prepareCameraConfig(CameraConfiguration *config) +static CameraConfiguration *prepareCameraConfig() { StreamRoles roles; /* If no configuration is provided assume a single video stream. */ - if (!options.isSet(OptStream)) { - *config = camera->generateConfiguration({ StreamRole::VideoRecording }); - return 0; - } + if (!options.isSet(OptStream)) + return camera->generateConfiguration({ StreamRole::VideoRecording }); const std::vector &streamOptions = options[OptStream].toArray(); @@ -113,16 +111,16 @@ static int prepareCameraConfig(CameraConfiguration *config) } else { std::cerr << "Unknown stream role " << conf["role"].toString() << std::endl; - return -EINVAL; + return nullptr; } } - *config = camera->generateConfiguration(roles); - - if (!config->isValid()) { + CameraConfiguration *config = camera->generateConfiguration(roles); + if (!config || !config->isValid()) { std::cerr << "Failed to get default stream configuration" << std::endl; - return -EINVAL; + delete config; + return nullptr; } /* Apply configuration explicitly requested. */ @@ -142,7 +140,7 @@ static int prepareCameraConfig(CameraConfiguration *config) cfg.pixelFormat = conf["pixelformat"]; } - return 0; + return config; } static void requestComplete(Request *request, const std::map &buffers) @@ -191,25 +189,25 @@ static void requestComplete(Request *request, const std::map static int capture() { - CameraConfiguration config; int ret; - ret = prepareCameraConfig(&config); - if (ret) { + CameraConfiguration *config = prepareCameraConfig(); + if (!config) { std::cout << "Failed to prepare camera configuration" << std::endl; - return ret; + return -EINVAL; } ret = camera->configure(config); if (ret < 0) { std::cout << "Failed to configure camera" << std::endl; + delete config; return ret; } streamInfo.clear(); - for (unsigned int index = 0; index < config.size(); ++index) { - StreamConfiguration &cfg = config[index]; + for (unsigned int index = 0; index < config->size(); ++index) { + StreamConfiguration &cfg = (*config)[index]; streamInfo[cfg.stream()] = "stream" + std::to_string(index); } @@ -217,6 +215,7 @@ static int capture() if (ret) { std::cerr << "Failed to allocate buffers" << std::endl; + delete config; return ret; } @@ -224,7 +223,7 @@ static int capture() /* Identify the stream with the least number of buffers. */ unsigned int nbuffers = UINT_MAX; - for (StreamConfiguration &cfg : config) { + for (StreamConfiguration &cfg : *config) { Stream *stream = cfg.stream(); nbuffers = std::min(nbuffers, stream->bufferPool().count()); } @@ -244,7 +243,7 @@ static int capture() } std::map map; - for (StreamConfiguration &cfg : config) { + for (StreamConfiguration &cfg : *config) { Stream *stream = cfg.stream(); map[stream] = &stream->bufferPool().buffers()[i]; } @@ -280,6 +279,7 @@ static int capture() std::cout << "Failed to stop capture" << std::endl; out: camera->freeBuffers(); + delete config; return ret; } diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index f8a4946b4a6a..115cdb1c024b 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -518,21 +518,24 @@ const std::set &Camera::streams() const * specifies a list of stream roles and the camera returns a configuration * containing suitable streams and their suggested default configurations. * - * \return A valid CameraConfiguration if the requested roles can be satisfied, - * or a invalid one otherwise + * \return A CameraConfiguration if the requested roles can be satisfied, or a + * null pointer otherwise. The ownership of the returned configuration is + * passed to the caller. */ -CameraConfiguration -Camera::generateConfiguration(const StreamRoles &roles) +CameraConfiguration *Camera::generateConfiguration(const StreamRoles &roles) { - if (disconnected_ || !roles.size() || roles.size() > streams_.size()) - return CameraConfiguration(); + if (disconnected_ || roles.size() > streams_.size()) + return nullptr; - CameraConfiguration config = pipe_->generateConfiguration(this, roles); + CameraConfiguration *config = pipe_->generateConfiguration(this, roles); std::ostringstream msg("streams configuration:", std::ios_base::ate); - for (unsigned int index = 0; index < config.size(); ++index) - msg << " (" << index << ") " << config[index].toString(); + if (config->isEmpty()) + msg << " empty"; + + for (unsigned int index = 0; index < config->size(); ++index) + msg << " (" << index << ") " << (*config)[index].toString(); LOG(Camera, Debug) << msg.str(); @@ -566,7 +569,7 @@ Camera::generateConfiguration(const StreamRoles &roles) * \retval -EACCES The camera is not in a state where it can be configured * \retval -EINVAL The configuration is not valid */ -int Camera::configure(CameraConfiguration &config) +int Camera::configure(CameraConfiguration *config) { int ret; @@ -576,7 +579,7 @@ int Camera::configure(CameraConfiguration &config) if (!stateBetween(CameraAcquired, CameraConfigured)) return -EACCES; - if (!config.isValid()) { + if (!config->isValid()) { LOG(Camera, Error) << "Can't configure camera with invalid configuration"; return -EINVAL; @@ -584,8 +587,8 @@ int Camera::configure(CameraConfiguration &config) std::ostringstream msg("configuring streams:", std::ios_base::ate); - for (unsigned int index = 0; index < config.size(); ++index) { - StreamConfiguration &cfg = config[index]; + for (unsigned int index = 0; index < config->size(); ++index) { + StreamConfiguration &cfg = (*config)[index]; cfg.setStream(nullptr); msg << " (" << index << ") " << cfg.toString(); } @@ -597,7 +600,7 @@ int Camera::configure(CameraConfiguration &config) return ret; activeStreams_.clear(); - for (const StreamConfiguration &cfg : config) { + for (const StreamConfiguration &cfg : *config) { Stream *stream = cfg.stream(); if (!stream) LOG(Camera, Fatal) diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h index a025905ab68f..7da6df1ab2a0 100644 --- a/src/libcamera/include/pipeline_handler.h +++ b/src/libcamera/include/pipeline_handler.h @@ -60,9 +60,9 @@ public: bool lock(); void unlock(); - virtual CameraConfiguration - generateConfiguration(Camera *camera, const StreamRoles &roles) = 0; - virtual int configure(Camera *camera, CameraConfiguration &config) = 0; + virtual CameraConfiguration *generateConfiguration(Camera *camera, + const StreamRoles &roles) = 0; + virtual int configure(Camera *camera, CameraConfiguration *config) = 0; virtual int allocateBuffers(Camera *camera, const std::set &streams) = 0; diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index ed0ef69de1d1..7c6f2d4a23be 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -150,9 +150,9 @@ class PipelineHandlerIPU3 : public PipelineHandler public: PipelineHandlerIPU3(CameraManager *manager); - CameraConfiguration - generateConfiguration(Camera *camera, const StreamRoles &roles) override; - int configure(Camera *camera, CameraConfiguration &config) override; + CameraConfiguration *generateConfiguration(Camera *camera, + const StreamRoles &roles) override; + int configure(Camera *camera, CameraConfiguration *config) override; int allocateBuffers(Camera *camera, const std::set &streams) override; @@ -207,12 +207,11 @@ PipelineHandlerIPU3::PipelineHandlerIPU3(CameraManager *manager) { } -CameraConfiguration -PipelineHandlerIPU3::generateConfiguration(Camera *camera, - const StreamRoles &roles) +CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera, + const StreamRoles &roles) { IPU3CameraData *data = cameraData(camera); - CameraConfiguration config = {}; + CameraConfiguration *config = new CameraConfiguration(); std::set streams = { &data->outStream_, &data->vfStream_, @@ -290,21 +289,23 @@ PipelineHandlerIPU3::generateConfiguration(Camera *camera, break; } - if (!stream) - return CameraConfiguration{}; + if (!stream) { + delete config; + return nullptr; + } streams.erase(stream); cfg.pixelFormat = V4L2_PIX_FMT_NV12; cfg.bufferCount = IPU3_BUFFER_COUNT; - config.addConfiguration(cfg); + config->addConfiguration(cfg); } return config; } -int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration &config) +int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *config) { IPU3CameraData *data = cameraData(camera); IPU3Stream *outStream = &data->outStream_; @@ -316,7 +317,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration &config) outStream->active_ = false; vfStream->active_ = false; - for (StreamConfiguration &cfg : config) { + for (StreamConfiguration &cfg : *config) { /* * Pick a stream for the configuration entry. * \todo: This is a naive temporary implementation that will be @@ -382,7 +383,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration &config) return ret; /* Apply the format to the configured streams output devices. */ - for (StreamConfiguration &cfg : config) { + for (StreamConfiguration &cfg : *config) { IPU3Stream *stream = static_cast(cfg.stream()); ret = imgu->configureOutput(stream->device_, cfg); if (ret) @@ -395,13 +396,13 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration &config) * be at least one active stream in the configuration request). */ if (!outStream->active_) { - ret = imgu->configureOutput(outStream->device_, config[0]); + ret = imgu->configureOutput(outStream->device_, (*config)[0]); if (ret) return ret; } if (!vfStream->active_) { - ret = imgu->configureOutput(vfStream->device_, config[0]); + ret = imgu->configureOutput(vfStream->device_, (*config)[0]); if (ret) return ret; } diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 6b35aa3fe7c3..c3b3912c96f3 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -34,9 +34,9 @@ public: PipelineHandlerRkISP1(CameraManager *manager); ~PipelineHandlerRkISP1(); - CameraConfiguration generateConfiguration(Camera *camera, + CameraConfiguration *generateConfiguration(Camera *camera, const StreamRoles &roles) override; - int configure(Camera *camera, CameraConfiguration &config) override; + int configure(Camera *camera, CameraConfiguration *config) override; int allocateBuffers(Camera *camera, const std::set &streams) override; @@ -105,26 +105,29 @@ PipelineHandlerRkISP1::~PipelineHandlerRkISP1() * Pipeline Operations */ -CameraConfiguration PipelineHandlerRkISP1::generateConfiguration(Camera *camera, +CameraConfiguration *PipelineHandlerRkISP1::generateConfiguration(Camera *camera, const StreamRoles &roles) { RkISP1CameraData *data = cameraData(camera); - CameraConfiguration config; - StreamConfiguration cfg{}; + CameraConfiguration *config = new CameraConfiguration(); - cfg.pixelFormat = V4L2_PIX_FMT_NV12; - cfg.size = data->sensor_->resolution(); - cfg.bufferCount = RKISP1_BUFFER_COUNT; + if (!roles.empty()) { + StreamConfiguration cfg{}; - config.addConfiguration(cfg); + cfg.pixelFormat = V4L2_PIX_FMT_NV12; + cfg.size = data->sensor_->resolution(); + cfg.bufferCount = RKISP1_BUFFER_COUNT; + + config->addConfiguration(cfg); + } return config; } -int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration &config) +int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *config) { RkISP1CameraData *data = cameraData(camera); - StreamConfiguration &cfg = config[0]; + StreamConfiguration &cfg = (*config)[0]; CameraSensor *sensor = data->sensor_; int ret; diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index 5dcc868b2fc9..5c061ca61016 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -25,9 +25,9 @@ class PipelineHandlerUVC : public PipelineHandler public: PipelineHandlerUVC(CameraManager *manager); - CameraConfiguration - generateConfiguration(Camera *camera, const StreamRoles &roles) override; - int configure(Camera *camera, CameraConfiguration &config) override; + CameraConfiguration *generateConfiguration(Camera *camera, + const StreamRoles &roles) override; + int configure(Camera *camera, CameraConfiguration *config) override; int allocateBuffers(Camera *camera, const std::set &streams) override; @@ -73,26 +73,28 @@ PipelineHandlerUVC::PipelineHandlerUVC(CameraManager *manager) { } -CameraConfiguration -PipelineHandlerUVC::generateConfiguration(Camera *camera, - const StreamRoles &roles) +CameraConfiguration *PipelineHandlerUVC::generateConfiguration(Camera *camera, + const StreamRoles &roles) { - CameraConfiguration config; - StreamConfiguration cfg; + CameraConfiguration *config = new CameraConfiguration(); - cfg.pixelFormat = V4L2_PIX_FMT_YUYV; - cfg.size = { 640, 480 }; - cfg.bufferCount = 4; + if (!roles.empty()) { + StreamConfiguration cfg; - config.addConfiguration(cfg); + cfg.pixelFormat = V4L2_PIX_FMT_YUYV; + cfg.size = { 640, 480 }; + cfg.bufferCount = 4; + + config->addConfiguration(cfg); + } return config; } -int PipelineHandlerUVC::configure(Camera *camera, CameraConfiguration &config) +int PipelineHandlerUVC::configure(Camera *camera, CameraConfiguration *config) { UVCCameraData *data = cameraData(camera); - StreamConfiguration &cfg = config[0]; + StreamConfiguration &cfg = (*config)[0]; int ret; V4L2DeviceFormat format = {}; diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index af6b6f21e3c5..0ece97f09e7e 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -25,9 +25,9 @@ class PipelineHandlerVimc : public PipelineHandler public: PipelineHandlerVimc(CameraManager *manager); - CameraConfiguration - generateConfiguration(Camera *camera, const StreamRoles &roles) override; - int configure(Camera *camera, CameraConfiguration &config) override; + CameraConfiguration *generateConfiguration(Camera *camera, + const StreamRoles &roles) override; + int configure(Camera *camera, CameraConfiguration *config) override; int allocateBuffers(Camera *camera, const std::set &streams) override; @@ -73,26 +73,28 @@ PipelineHandlerVimc::PipelineHandlerVimc(CameraManager *manager) { } -CameraConfiguration -PipelineHandlerVimc::generateConfiguration(Camera *camera, - const StreamRoles &roles) +CameraConfiguration *PipelineHandlerVimc::generateConfiguration(Camera *camera, + const StreamRoles &roles) { - CameraConfiguration config; - StreamConfiguration cfg; + CameraConfiguration *config = new CameraConfiguration(); - cfg.pixelFormat = V4L2_PIX_FMT_RGB24; - cfg.size = { 640, 480 }; - cfg.bufferCount = 4; + if (!roles.empty()) { + StreamConfiguration cfg; - config.addConfiguration(cfg); + cfg.pixelFormat = V4L2_PIX_FMT_RGB24; + cfg.size = { 640, 480 }; + cfg.bufferCount = 4; + + config->addConfiguration(cfg); + } return config; } -int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration &config) +int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config) { VimcCameraData *data = cameraData(camera); - StreamConfiguration &cfg = config[0]; + StreamConfiguration &cfg = (*config)[0]; int ret; V4L2DeviceFormat format = {}; diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index 4185e3557dcb..de46e98880a2 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -233,7 +233,8 @@ void PipelineHandler::unlock() * the group of streams parameters. * * \return A valid CameraConfiguration if the requested roles can be satisfied, - * or a invalid configuration otherwise + * or a null pointer otherwise. The ownership of the returned configuration is + * passed to the caller. */ /** diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp index 06ae2985f80d..9f0454b7a5f3 100644 --- a/src/qcam/main_window.cpp +++ b/src/qcam/main_window.cpp @@ -104,7 +104,7 @@ int MainWindow::startCapture() return ret; } - const StreamConfiguration &cfg = config_[0]; + const StreamConfiguration &cfg = (*config_)[0]; Stream *stream = cfg.stream(); ret = viewfinder_->setFormat(cfg.pixelFormat, cfg.size.width, cfg.size.height); @@ -180,6 +180,9 @@ void MainWindow::stopCapture() camera_->freeBuffers(); isCapturing_ = false; + + delete config_; + config_ = nullptr; } void MainWindow::requestComplete(Request *request, diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h index 143b5b08a5a0..866866e93d23 100644 --- a/src/qcam/main_window.h +++ b/src/qcam/main_window.h @@ -45,7 +45,7 @@ private: std::shared_ptr camera_; bool isCapturing_; - CameraConfiguration config_; + CameraConfiguration *config_; ViewFinder *viewfinder_; }; diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp index bfd11eefedcf..f640e80fbaf3 100644 --- a/test/camera/capture.cpp +++ b/test/camera/capture.cpp @@ -40,13 +40,25 @@ protected: camera_->queueRequest(request); } - int run() + int init() override { - CameraConfiguration config = - camera_->generateConfiguration({ StreamRole::VideoRecording }); - StreamConfiguration *cfg = &config[0]; + CameraTest::init(); - if (!config.isValid()) { + config_ = camera_->generateConfiguration({ StreamRole::VideoRecording }); + if (!config_) { + cout << "Failed to generate default configuration" << endl; + CameraTest::cleanup(); + return TestFail; + } + + return TestPass; + } + + int run() override + { + StreamConfiguration &cfg = (*config_)[0]; + + if (!config_->isValid()) { cout << "Failed to read default configuration" << endl; return TestFail; } @@ -56,7 +68,7 @@ protected: return TestFail; } - if (camera_->configure(config)) { + if (camera_->configure(config_)) { cout << "Failed to set default configuration" << endl; return TestFail; } @@ -66,7 +78,7 @@ protected: return TestFail; } - Stream *stream = cfg->stream(); + Stream *stream = cfg.stream(); BufferPool &pool = stream->bufferPool(); std::vector requests; for (Buffer &buffer : pool.buffers()) { @@ -110,10 +122,10 @@ protected: while (timer.isRunning()) dispatcher->processEvents(); - if (completeRequestsCount_ <= cfg->bufferCount * 2) { + if (completeRequestsCount_ <= cfg.bufferCount * 2) { cout << "Failed to capture enough frames (got " << completeRequestsCount_ << " expected at least " - << cfg->bufferCount * 2 << ")" << endl; + << cfg.bufferCount * 2 << ")" << endl; return TestFail; } @@ -134,6 +146,14 @@ protected: return TestPass; } + + void cleanup() override + { + delete config_; + CameraTest::cleanup(); + } + + CameraConfiguration *config_; }; } /* namespace */ diff --git a/test/camera/configuration_default.cpp b/test/camera/configuration_default.cpp index 8c4a03db498a..d5cefc1127c9 100644 --- a/test/camera/configuration_default.cpp +++ b/test/camera/configuration_default.cpp @@ -18,26 +18,32 @@ class ConfigurationDefault : public CameraTest protected: int run() { - CameraConfiguration config; + CameraConfiguration *config; /* Test asking for configuration for a video stream. */ config = camera_->generateConfiguration({ StreamRole::VideoRecording }); - if (!config.isValid()) { + if (!config || !config->isValid()) { cout << "Default configuration invalid" << endl; + delete config; return TestFail; } + delete config; + /* * Test that asking for configuration for an empty array of - * stream roles returns an empty list of configurations. + * stream roles returns an empty camera configuration. */ config = camera_->generateConfiguration({}); - if (config.isValid()) { + if (!config || config->isValid()) { cout << "Failed to retrieve configuration for empty roles list" << endl; + delete config; return TestFail; } + delete config; + return TestPass; } }; diff --git a/test/camera/configuration_set.cpp b/test/camera/configuration_set.cpp index 25b5db67103a..ca911f459c32 100644 --- a/test/camera/configuration_set.cpp +++ b/test/camera/configuration_set.cpp @@ -16,13 +16,25 @@ namespace { class ConfigurationSet : public CameraTest { protected: - int run() + int init() override { - CameraConfiguration config = - camera_->generateConfiguration({ StreamRole::VideoRecording }); - StreamConfiguration *cfg = &config[0]; + CameraTest::init(); - if (!config.isValid()) { + config_ = camera_->generateConfiguration({ StreamRole::VideoRecording }); + if (!config_) { + cout << "Failed to generate default configuration" << endl; + CameraTest::cleanup(); + return TestFail; + } + + return TestPass; + } + + int run() override + { + StreamConfiguration &cfg = (*config_)[0]; + + if (!config_->isValid()) { cout << "Failed to read default configuration" << endl; return TestFail; } @@ -33,7 +45,7 @@ protected: } /* Test that setting the default configuration works. */ - if (camera_->configure(config)) { + if (camera_->configure(config_)) { cout << "Failed to set default configuration" << endl; return TestFail; } @@ -48,7 +60,7 @@ protected: return TestFail; } - if (!camera_->configure(config)) { + if (!camera_->configure(config_)) { cout << "Setting configuration on a camera not acquired succeeded when it should have failed" << endl; return TestFail; @@ -64,9 +76,9 @@ protected: * the default configuration of the VIMC camera is known to * work. */ - cfg->size.width *= 2; - cfg->size.height *= 2; - if (camera_->configure(config)) { + cfg.size.width *= 2; + cfg.size.height *= 2; + if (camera_->configure(config_)) { cout << "Failed to set modified configuration" << endl; return TestFail; } @@ -74,14 +86,22 @@ protected: /* * Test that setting an invalid configuration fails. */ - cfg->size = { 0, 0 }; - if (!camera_->configure(config)) { + cfg.size = { 0, 0 }; + if (!camera_->configure(config_)) { cout << "Invalid configuration incorrectly accepted" << endl; return TestFail; } return TestPass; } + + void cleanup() override + { + delete config_; + CameraTest::cleanup(); + } + + CameraConfiguration *config_; }; } /* namespace */ diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp index 7a74cd85a37a..a662e869fadf 100644 --- a/test/camera/statemachine.cpp +++ b/test/camera/statemachine.cpp @@ -233,10 +233,22 @@ protected: return TestPass; } - int run() + int init() override { + CameraTest::init(); + defconf_ = camera_->generateConfiguration({ StreamRole::VideoRecording }); + if (!defconf_) { + cout << "Failed to generate default configuration" << endl; + CameraTest::cleanup(); + return TestFail; + } + return TestPass; + } + + int run() override + { if (testAvailable() != TestPass) { cout << "State machine in Available state failed" << endl; return TestFail; @@ -265,7 +277,13 @@ protected: return TestPass; } - CameraConfiguration defconf_; + void cleanup() override + { + delete defconf_; + CameraTest::cleanup(); + } + + CameraConfiguration *defconf_; }; } /* namespace */ From patchwork Fri May 17 23:06:15 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 1223 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id D73A36186C for ; Sat, 18 May 2019 01:06:44 +0200 (CEST) Received: from pendragon.bb.dnainternet.fi (dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi [IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 8998A336 for ; Sat, 18 May 2019 01:06:44 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1558134404; bh=Dn8ngc1OpTIEPdi0ydRlz5E4vTDyLQgS9Bu8gVLUgXg=; h=From:To:Subject:Date:In-Reply-To:References:From; b=JcsMFwVM0MlmmVnwPuVKFF73AiVS/a1Jl9Wgtm4WmmQMOkLWlY9RNHRbWuZQZDtN8 ikKGpn/77Ad55z6PJh/neGcZ7035EJteHWwiBq/yZ7h/JH/cRPBOhrl3gMvORfJa0H e8jMOyAjW+29p+CEhESxHCS2T8YtF3B8+mNgRp2k= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sat, 18 May 2019 02:06:15 +0300 Message-Id: <20190517230621.24668-7-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190517230621.24668-1-laurent.pinchart@ideasonboard.com> References: <20190517230621.24668-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH/RFC 06/12] libcamera: pipeline: Move camera data classes to the top level scope 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, 17 May 2019 23:06:47 -0000 Move the pipeline handler camera data classes, defined in the scope of the respective pipeline handler class, to the top level of the libcamera namespace. This prepares for the introduction of other classes that will make use of them in the IPU3 and RkISP1 pipeline handlers. The UVC and VIMC pipeline handlers are updated as well for consistency. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- src/libcamera/pipeline/ipu3/ipu3.cpp | 44 ++++++++++++------------ src/libcamera/pipeline/rkisp1/rkisp1.cpp | 34 +++++++++--------- src/libcamera/pipeline/uvcvideo.cpp | 40 ++++++++++----------- src/libcamera/pipeline/vimc.cpp | 40 ++++++++++----------- 4 files changed, 79 insertions(+), 79 deletions(-) diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 7c6f2d4a23be..8430e0591a41 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -145,6 +145,25 @@ public: ImgUDevice::ImgUOutput *device_; }; +class IPU3CameraData : public CameraData +{ +public: + IPU3CameraData(PipelineHandler *pipe) + : CameraData(pipe) + { + } + + void imguOutputBufferReady(Buffer *buffer); + void imguInputBufferReady(Buffer *buffer); + void cio2BufferReady(Buffer *buffer); + + CIO2Device cio2_; + ImgUDevice *imgu_; + + IPU3Stream outStream_; + IPU3Stream vfStream_; +}; + class PipelineHandlerIPU3 : public PipelineHandler { public: @@ -167,25 +186,6 @@ public: bool match(DeviceEnumerator *enumerator) override; private: - class IPU3CameraData : public CameraData - { - public: - IPU3CameraData(PipelineHandler *pipe) - : CameraData(pipe) - { - } - - void imguOutputBufferReady(Buffer *buffer); - void imguInputBufferReady(Buffer *buffer); - void cio2BufferReady(Buffer *buffer); - - CIO2Device cio2_; - ImgUDevice *imgu_; - - IPU3Stream outStream_; - IPU3Stream vfStream_; - }; - static constexpr unsigned int IPU3_BUFFER_COUNT = 4; IPU3CameraData *cameraData(const Camera *camera) @@ -749,7 +749,7 @@ int PipelineHandlerIPU3::registerCameras() * Buffers completed from the ImgU input are immediately queued back to the * CIO2 unit to continue frame capture. */ -void PipelineHandlerIPU3::IPU3CameraData::imguInputBufferReady(Buffer *buffer) +void IPU3CameraData::imguInputBufferReady(Buffer *buffer) { cio2_.output_->queueBuffer(buffer); } @@ -760,7 +760,7 @@ void PipelineHandlerIPU3::IPU3CameraData::imguInputBufferReady(Buffer *buffer) * * Buffers completed from the ImgU output are directed to the application. */ -void PipelineHandlerIPU3::IPU3CameraData::imguOutputBufferReady(Buffer *buffer) +void IPU3CameraData::imguOutputBufferReady(Buffer *buffer) { Request *request = buffer->request(); @@ -785,7 +785,7 @@ void PipelineHandlerIPU3::IPU3CameraData::imguOutputBufferReady(Buffer *buffer) * Buffers completed from the CIO2 are immediately queued to the ImgU unit * for further processing. */ -void PipelineHandlerIPU3::IPU3CameraData::cio2BufferReady(Buffer *buffer) +void IPU3CameraData::cio2BufferReady(Buffer *buffer) { imgu_->input_->queueBuffer(buffer); } diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index c3b3912c96f3..ec590a382751 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -28,6 +28,23 @@ namespace libcamera { LOG_DEFINE_CATEGORY(RkISP1) +class RkISP1CameraData : public CameraData +{ +public: + RkISP1CameraData(PipelineHandler *pipe) + : CameraData(pipe), sensor_(nullptr) + { + } + + ~RkISP1CameraData() + { + delete sensor_; + } + + Stream stream_; + CameraSensor *sensor_; +}; + class PipelineHandlerRkISP1 : public PipelineHandler { public: @@ -51,23 +68,6 @@ public: bool match(DeviceEnumerator *enumerator) override; private: - class RkISP1CameraData : public CameraData - { - public: - RkISP1CameraData(PipelineHandler *pipe) - : CameraData(pipe), sensor_(nullptr) - { - } - - ~RkISP1CameraData() - { - delete sensor_; - } - - Stream stream_; - CameraSensor *sensor_; - }; - static constexpr unsigned int RKISP1_BUFFER_COUNT = 4; RkISP1CameraData *cameraData(const Camera *camera) diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index 5c061ca61016..c20467766ed0 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -20,6 +20,25 @@ namespace libcamera { LOG_DEFINE_CATEGORY(UVC) +class UVCCameraData : public CameraData +{ +public: + UVCCameraData(PipelineHandler *pipe) + : CameraData(pipe), video_(nullptr) + { + } + + ~UVCCameraData() + { + delete video_; + } + + void bufferReady(Buffer *buffer); + + V4L2Device *video_; + Stream stream_; +}; + class PipelineHandlerUVC : public PipelineHandler { public: @@ -42,25 +61,6 @@ public: bool match(DeviceEnumerator *enumerator) override; private: - class UVCCameraData : public CameraData - { - public: - UVCCameraData(PipelineHandler *pipe) - : CameraData(pipe), video_(nullptr) - { - } - - ~UVCCameraData() - { - delete video_; - } - - void bufferReady(Buffer *buffer); - - V4L2Device *video_; - Stream stream_; - }; - UVCCameraData *cameraData(const Camera *camera) { return static_cast( @@ -206,7 +206,7 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator) return true; } -void PipelineHandlerUVC::UVCCameraData::bufferReady(Buffer *buffer) +void UVCCameraData::bufferReady(Buffer *buffer) { Request *request = queuedRequests_.front(); diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index 0ece97f09e7e..5575880cdbdf 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -20,6 +20,25 @@ namespace libcamera { LOG_DEFINE_CATEGORY(VIMC) +class VimcCameraData : public CameraData +{ +public: + VimcCameraData(PipelineHandler *pipe) + : CameraData(pipe) + { + } + + ~VimcCameraData() + { + delete video_; + } + + void bufferReady(Buffer *buffer); + + V4L2Device *video_; + Stream stream_; +}; + class PipelineHandlerVimc : public PipelineHandler { public: @@ -42,25 +61,6 @@ public: bool match(DeviceEnumerator *enumerator) override; private: - class VimcCameraData : public CameraData - { - public: - VimcCameraData(PipelineHandler *pipe) - : CameraData(pipe) - { - } - - ~VimcCameraData() - { - delete video_; - } - - void bufferReady(Buffer *buffer); - - V4L2Device *video_; - Stream stream_; - }; - VimcCameraData *cameraData(const Camera *camera) { return static_cast( @@ -202,7 +202,7 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator) return true; } -void PipelineHandlerVimc::VimcCameraData::bufferReady(Buffer *buffer) +void VimcCameraData::bufferReady(Buffer *buffer) { Request *request = queuedRequests_.front(); From patchwork Fri May 17 23:06:16 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 1224 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 464CE6186D for ; Sat, 18 May 2019 01:06:45 +0200 (CEST) Received: from pendragon.bb.dnainternet.fi (dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi [IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id CC7822F3 for ; Sat, 18 May 2019 01:06:44 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1558134405; bh=w0uDalIRYBRZ9Dl5Lw6AyPyJDY6J+TvdKBtLOmtv/lw=; h=From:To:Subject:Date:In-Reply-To:References:From; b=Uq11hpSodDEoEgSw1ClmFH0P5K+k9Y5tn+/W8FwQxTLrqO3BsAD1i+zX1NrP34jjK cnoJPTtKd3s85SCIJEtf6WbIhhwKwEu2Ekw0zwfZCWn51EiQI7AHxZwviZXnadlh/S uWWFvf6BQfmcgtPgm93cWQJ99u/EQwGm347LMLYk= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sat, 18 May 2019 02:06:16 +0300 Message-Id: <20190517230621.24668-8-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190517230621.24668-1-laurent.pinchart@ideasonboard.com> References: <20190517230621.24668-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH/RFC 07/12] libcamera: camera: Add a validation API to the CameraConfiguration class 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, 17 May 2019 23:06:48 -0000 The CameraConfiguration class implements a simple storage of StreamConfiguration with internal validation limited to verifying that the stream configurations are not empty. Extend this mechanism by implementing a smart validate() method backed by pipeline handlers. This new mechanism changes the semantics of the camera configuration. The Camera::generateConfiguration() operation still generates a default configuration based on roles, but now also supports generating empty configurations to be filled by applications. Applications can inspect the configuration, optionally modify it, and validate it. The validation implements "try" semantics and adjusts invalid configurations instead of rejecting them completely. Applications then decide whether to accept the modified configuration, or try again with a different set of parameters. Once the configuration is valid, it is passed to Camera::configure(), and pipeline handlers are guaranteed that the configuration they receive is valid. A reference to the Camera may need to be stored in the CameraConfiguration derived classes in order to access it from their validate() implementation. This must be stored as a std::shared_ptr<> as the CameraConfiguration instances belong to applications. In order to make this possible, make the Camera class inherit from std::shared_from_this<>. Signed-off-by: Laurent Pinchart --- include/libcamera/camera.h | 16 +- src/cam/main.cpp | 2 +- src/libcamera/camera.cpp | 90 +++++--- src/libcamera/pipeline/ipu3/ipu3.cpp | 255 ++++++++++++++++++----- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 149 ++++++++++--- src/libcamera/pipeline/uvcvideo.cpp | 53 ++++- src/libcamera/pipeline/vimc.cpp | 67 +++++- src/libcamera/pipeline_handler.cpp | 17 +- test/camera/capture.cpp | 7 +- test/camera/configuration_default.cpp | 4 +- test/camera/configuration_set.cpp | 7 +- 11 files changed, 521 insertions(+), 146 deletions(-) diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h index 841e8fc505b9..276bf1fb1887 100644 --- a/include/libcamera/camera.h +++ b/include/libcamera/camera.h @@ -25,30 +25,38 @@ class Request; class CameraConfiguration { public: + enum Status { + Valid, + Adjusted, + Invalid, + }; + using iterator = std::vector::iterator; using const_iterator = std::vector::const_iterator; - CameraConfiguration(); + virtual ~CameraConfiguration(); void addConfiguration(const StreamConfiguration &cfg); + virtual Status validate() = 0; iterator begin(); iterator end(); const_iterator begin() const; const_iterator end() const; - bool isValid() const; bool isEmpty() const; std::size_t size() const; StreamConfiguration &operator[](unsigned int index); const StreamConfiguration &operator[](unsigned int index) const; -private: +protected: + CameraConfiguration(); + std::vector config_; }; -class Camera final +class Camera final : public std::enable_shared_from_this { public: static std::shared_ptr create(PipelineHandler *pipe, diff --git a/src/cam/main.cpp b/src/cam/main.cpp index 7a1b332f68c7..a962f94c8f59 100644 --- a/src/cam/main.cpp +++ b/src/cam/main.cpp @@ -116,7 +116,7 @@ static CameraConfiguration *prepareCameraConfig() } CameraConfiguration *config = camera->generateConfiguration(roles); - if (!config || !config->isValid()) { + if (!config || config->size() != roles.size()) { std::cerr << "Failed to get default stream configuration" << std::endl; delete config; diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index 115cdb1c024b..572939956a34 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -52,6 +52,28 @@ LOG_DECLARE_CATEGORY(Camera) * operator[](int) returns a reference to the StreamConfiguration based on its * insertion index. Accessing a stream configuration with an invalid index * results in undefined behaviour. + * + * CameraConfiguration instances are retrieved from the camera with + * Camera::generateConfiguration(). Applications may then inspect the + * configuration, modify it, and possibly add new stream configuration entries + * with addConfiguration(). Once the camera configuration satisfies the + * application, it shall be validated by a call to validate(). The validation + * implements "try" semantics: it adjusts invalid configurations to the closest + * achievable parameters instead of rejecting them completely. Applications + * then decide whether to accept the modified configuration, or try again with + * a different set of parameters. Once the configuration is valid, it is passed + * to Camera::configure(). + */ + +/** + * \enum CameraConfiguration::Status + * \brief Validity of a camera configuration + * \var CameraConfiguration::Valid + * The configuration is fully valid + * \var CameraConfiguration::Adjusted + * The configuration has been adjusted to a valid configuration + * \var CameraConfiguration::Invalid + * The configuration is invalid and can't be adjusted automatically */ /** @@ -73,6 +95,10 @@ CameraConfiguration::CameraConfiguration() { } +CameraConfiguration::~CameraConfiguration() +{ +} + /** * \brief Add a stream configuration to the camera configuration * \param[in] cfg The stream configuration @@ -82,6 +108,33 @@ void CameraConfiguration::addConfiguration(const StreamConfiguration &cfg) config_.push_back(cfg); } +/** + * \fn CameraConfiguration::validate() + * \brief Validate and possibly adjust the camera configuration + * + * This method adjusts the camera configuration to the closest valid + * configuration and returns the validation status. + * + * \todo: Define exactly when to return each status code. Should stream + * parameters set to 0 by the caller be adjusted without returning Adjusted ? + * This would potentially be useful for applications but would get in the way + * in Camera::configure(). Do we need an extra status code to signal this ? + * + * \todo: Handle validation of buffers count when refactoring the buffers API. + * + * \return A CameraConfiguration::Status value that describes the validation + * status. + * \retval CameraConfiguration::Invalid The configuration is invalid and can't + * be adjusted. This may only occur in extreme cases such as when the + * configuration is empty. + * \retval CameraConfigutation::Adjusted The configuration has been adjusted + * and is now valid. Parameters may have changed for any stream, and stream + * configurations may have been removed. The caller shall check the + * configuration carefully. + * \retval CameraConfiguration::Valid The configuration was already valid and + * hasn't been adjusted. + */ + /** * \brief Retrieve an iterator to the first stream configuration in the * sequence @@ -123,29 +176,6 @@ CameraConfiguration::const_iterator CameraConfiguration::end() const return config_.end(); } -/** - * \brief Check if the camera configuration is valid - * - * A camera configuration is deemed to be valid if it contains at least one - * stream configuration and all stream configurations contain valid information. - * Stream configurations are deemed to be valid if all fields are none zero. - * - * \return True if the configuration is valid - */ -bool CameraConfiguration::isValid() const -{ - if (isEmpty()) - return false; - - for (const StreamConfiguration &cfg : config_) { - if (cfg.size.width == 0 || cfg.size.height == 0 || - cfg.pixelFormat == 0 || cfg.bufferCount == 0) - return false; - } - - return true; -} - /** * \brief Check if the camera configuration is empty * \return True if the configuration is empty @@ -194,6 +224,11 @@ const StreamConfiguration &CameraConfiguration::operator[](unsigned int index) c return config_[index]; } +/** + * \var CameraConfiguration::config_ + * \brief The vector of stream configurations + */ + /** * \class Camera * \brief Camera device @@ -551,10 +586,9 @@ CameraConfiguration *Camera::generateConfiguration(const StreamRoles &roles) * The caller specifies which streams are to be involved and their configuration * by populating \a config. * - * The easiest way to populate the array of config is to fetch an initial - * configuration from the camera with generateConfiguration() and then change - * the parameters to fit the caller's need and once all the streams parameters - * are configured hand that over to configure() to actually setup the camera. + * The configuration is created by generateConfiguration(), and adjusted by the + * caller with CameraConfiguration::validate(). This method only accepts fully + * valid configurations and returns an error if \a config is not valid. * * Exclusive access to the camera shall be ensured by a call to acquire() prior * to calling this function, otherwise an -EACCES error will be returned. @@ -579,7 +613,7 @@ int Camera::configure(CameraConfiguration *config) if (!stateBetween(CameraAcquired, CameraConfigured)) return -EACCES; - if (!config->isValid()) { + if (config->validate() != CameraConfiguration::Valid) { LOG(Camera, Error) << "Can't configure camera with invalid configuration"; return -EINVAL; diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 8430e0591a41..a5e71832d3c2 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -164,6 +164,33 @@ public: IPU3Stream vfStream_; }; +class IPU3CameraConfiguration : public CameraConfiguration +{ +public: + IPU3CameraConfiguration(Camera *camera, IPU3CameraData *data); + + Status validate() override; + + const V4L2SubdeviceFormat &sensorFormat() { return sensorFormat_; } + const std::vector &streams() { return streams_; } + +private: + static constexpr unsigned int IPU3_BUFFER_COUNT = 4; + + void adjustStream(unsigned int index, bool scale); + + /* + * The IPU3CameraData instance is guaranteed to be valid as long as the + * corresponding Camera instance is valid. In order to borrow a + * reference to the camera data, store a new reference to the camera. + */ + std::shared_ptr camera_; + const IPU3CameraData *data_; + + V4L2SubdeviceFormat sensorFormat_; + std::vector streams_; +}; + class PipelineHandlerIPU3 : public PipelineHandler { public: @@ -186,8 +213,6 @@ public: bool match(DeviceEnumerator *enumerator) override; private: - static constexpr unsigned int IPU3_BUFFER_COUNT = 4; - IPU3CameraData *cameraData(const Camera *camera) { return static_cast( @@ -202,6 +227,153 @@ private: MediaDevice *imguMediaDev_; }; +IPU3CameraConfiguration::IPU3CameraConfiguration(Camera *camera, + IPU3CameraData *data) + : CameraConfiguration() +{ + camera_ = camera->shared_from_this(); + data_ = data; +} + +void IPU3CameraConfiguration::adjustStream(unsigned int index, bool scale) +{ + StreamConfiguration &cfg = config_[index]; + + /* The only pixel format the driver supports is NV12. */ + cfg.pixelFormat = V4L2_PIX_FMT_NV12; + + if (scale) { + /* + * Provide a suitable default that matches the sensor aspect + * ratio. + */ + if (!cfg.size.width || !cfg.size.height) { + cfg.size.width = 1280; + cfg.size.height = 1280 * sensorFormat_.size.height + / sensorFormat_.size.width; + } + + /* + * \todo: Clamp the size to the hardware bounds when we will + * figure them out. + * + * \todo: Handle the scaler (BDS) restrictions. The BDS can + * only scale with the same factor in both directions, and the + * scaling factor is limited to a multiple of 1/32. At the + * moment the ImgU driver hides these constraints by applying + * additional cropping, this should be fixed on the driver + * side, and cropping should be exposed to us. + */ + } else { + /* + * \todo: Properly support cropping when the ImgU driver + * interface will be cleaned up. + */ + cfg.size = sensorFormat_.size; + } + + /* + * Clamp the size to match the ImgU alignment constraints. The width + * shall be a multiple of 8 pixels and the height a multiple of 4 + * pixels. + */ + if (cfg.size.width % 8 || cfg.size.height % 4) { + cfg.size.width &= ~7; + cfg.size.height &= ~3; + } + + cfg.bufferCount = IPU3_BUFFER_COUNT; +} + +CameraConfiguration::Status IPU3CameraConfiguration::validate() +{ + const CameraSensor *sensor = data_->cio2_.sensor_; + Status status = Valid; + + if (config_.empty()) + return Invalid; + + /* Cap the number of entries to the available streams. */ + if (config_.size() > 2) { + config_.resize(2); + status = Adjusted; + } + + /* + * Select the sensor format by collecting the maximum width and height + * and picking the closest larger match, as the IPU3 can downscale + * only. If no resolution is requested for any stream, or if no sensor + * resolution is large enough, pick the largest one. + */ + Size size = {}; + + for (const StreamConfiguration &cfg : config_) { + if (cfg.size.width > size.width) + size.width = cfg.size.width; + if (cfg.size.height > size.height) + size.height = cfg.size.height; + } + + if (!size.width || !size.height) + size = sensor->resolution(); + + sensorFormat_ = sensor->getFormat({ MEDIA_BUS_FMT_SBGGR10_1X10, + MEDIA_BUS_FMT_SGBRG10_1X10, + MEDIA_BUS_FMT_SGRBG10_1X10, + MEDIA_BUS_FMT_SRGGB10_1X10 }, + size); + if (!sensorFormat_.size.width || !sensorFormat_.size.height) + sensorFormat_.size = sensor->resolution(); + + /* + * Verify and update all configuration entries, and assign a stream to + * each of them. The viewfinder stream can scale, while the output + * stream can crop only, so select the output stream when the requested + * resolution is equal to the sensor resolution, and the viewfinder + * stream otherwise. + */ + std::set availableStreams = { + &data_->outStream_, + &data_->vfStream_, + }; + + streams_.clear(); + streams_.reserve(config_.size()); + + for (unsigned int i = 0; i < config_.size(); ++i) { + StreamConfiguration &cfg = config_[i]; + const unsigned int pixelFormat = cfg.pixelFormat; + const Size size = cfg.size; + const IPU3Stream *stream; + + if (cfg.size == sensorFormat_.size) + stream = &data_->outStream_; + else + stream = &data_->vfStream_; + + if (availableStreams.find(stream) == availableStreams.end()) + stream = *availableStreams.begin(); + + LOG(IPU3, Debug) + << "Assigned '" << stream->name_ << "' to stream " << i; + + bool scale = stream == &data_->vfStream_; + adjustStream(i, scale); + + if (cfg.pixelFormat != pixelFormat || cfg.size != size) { + LOG(IPU3, Debug) + << "Stream " << i << " configuration adjusted to " + << cfg.toString(); + status = Adjusted; + } + + streams_.push_back(stream); + availableStreams.erase(stream); + } + + return status; +} + PipelineHandlerIPU3::PipelineHandlerIPU3(CameraManager *manager) : PipelineHandler(manager), cio2MediaDev_(nullptr), imguMediaDev_(nullptr) { @@ -211,12 +383,14 @@ CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera, const StreamRoles &roles) { IPU3CameraData *data = cameraData(camera); - CameraConfiguration *config = new CameraConfiguration(); + IPU3CameraConfiguration *config; std::set streams = { &data->outStream_, &data->vfStream_, }; + config = new IPU3CameraConfiguration(camera, data); + for (const StreamRole role : roles) { StreamConfiguration cfg = {}; IPU3Stream *stream = nullptr; @@ -296,71 +470,25 @@ CameraConfiguration *PipelineHandlerIPU3::generateConfiguration(Camera *camera, streams.erase(stream); - cfg.pixelFormat = V4L2_PIX_FMT_NV12; - cfg.bufferCount = IPU3_BUFFER_COUNT; - config->addConfiguration(cfg); } + config->validate(); + return config; } -int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *config) +int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c) { + IPU3CameraConfiguration *config = + static_cast(c); IPU3CameraData *data = cameraData(camera); IPU3Stream *outStream = &data->outStream_; IPU3Stream *vfStream = &data->vfStream_; CIO2Device *cio2 = &data->cio2_; ImgUDevice *imgu = data->imgu_; - Size sensorSize = {}; int ret; - outStream->active_ = false; - vfStream->active_ = false; - for (StreamConfiguration &cfg : *config) { - /* - * Pick a stream for the configuration entry. - * \todo: This is a naive temporary implementation that will be - * reworked when implementing camera configuration validation. - */ - IPU3Stream *stream = vfStream->active_ ? outStream : vfStream; - - /* - * Verify that the requested size respects the IPU3 alignment - * requirements (the image width shall be a multiple of 8 - * pixels and its height a multiple of 4 pixels) and the camera - * maximum sizes. - * - * \todo: Consider the BDS scaling factor requirements: "the - * downscaling factor must be an integer value multiple of 1/32" - */ - if (cfg.size.width % 8 || cfg.size.height % 4) { - LOG(IPU3, Error) - << "Invalid stream size: bad alignment"; - return -EINVAL; - } - - const Size &resolution = cio2->sensor_->resolution(); - if (cfg.size.width > resolution.width || - cfg.size.height > resolution.height) { - LOG(IPU3, Error) - << "Invalid stream size: larger than sensor resolution"; - return -EINVAL; - } - - /* - * Collect the maximum width and height: IPU3 can downscale - * only. - */ - if (cfg.size.width > sensorSize.width) - sensorSize.width = cfg.size.width; - if (cfg.size.height > sensorSize.height) - sensorSize.height = cfg.size.height; - - stream->active_ = true; - cfg.setStream(stream); - } - /* * \todo: Enable links selectively based on the requested streams. * As of now, enable all links unconditionally. @@ -373,6 +501,7 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *config) * Pass the requested stream size to the CIO2 unit and get back the * adjusted format to be propagated to the ImgU output devices. */ + const Size &sensorSize = config->sensorFormat().size; V4L2DeviceFormat cio2Format = {}; ret = cio2->configure(sensorSize, &cio2Format); if (ret) @@ -383,8 +512,22 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *config) return ret; /* Apply the format to the configured streams output devices. */ - for (StreamConfiguration &cfg : *config) { - IPU3Stream *stream = static_cast(cfg.stream()); + outStream->active_ = false; + vfStream->active_ = false; + + for (unsigned int i = 0; i < config->size(); ++i) { + /* + * Use a const_cast<> here instead of storing a mutable stream + * pointer in the configuration to let the compiler catch + * unwanted modifications of camera data in the configuration + * validate() implementation. + */ + IPU3Stream *stream = const_cast(config->streams()[i]); + StreamConfiguration &cfg = (*config)[i]; + + stream->active_ = true; + cfg.setStream(stream); + ret = imgu->configureOutput(stream->device_, cfg); if (ret) return ret; diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index ec590a382751..b257f6534602 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -5,6 +5,7 @@ * rkisp1.cpp - Pipeline handler for Rockchip ISP1 */ +#include #include #include #include @@ -45,6 +46,29 @@ public: CameraSensor *sensor_; }; +class RkISP1CameraConfiguration : public CameraConfiguration +{ +public: + RkISP1CameraConfiguration(Camera *camera, RkISP1CameraData *data); + + Status validate() override; + + const V4L2SubdeviceFormat &sensorFormat() { return sensorFormat_; } + +private: + static constexpr unsigned int RKISP1_BUFFER_COUNT = 4; + + /* + * The RkISP1CameraData instance is guaranteed to be valid as long as the + * corresponding Camera instance is valid. In order to borrow a + * reference to the camera data, store a new reference to the camera. + */ + std::shared_ptr camera_; + const RkISP1CameraData *data_; + + V4L2SubdeviceFormat sensorFormat_; +}; + class PipelineHandlerRkISP1 : public PipelineHandler { public: @@ -68,8 +92,6 @@ public: bool match(DeviceEnumerator *enumerator) override; private: - static constexpr unsigned int RKISP1_BUFFER_COUNT = 4; - RkISP1CameraData *cameraData(const Camera *camera) { return static_cast( @@ -88,6 +110,95 @@ private: Camera *activeCamera_; }; +RkISP1CameraConfiguration::RkISP1CameraConfiguration(Camera *camera, + RkISP1CameraData *data) + : CameraConfiguration() +{ + camera_ = camera->shared_from_this(); + data_ = data; +} + +CameraConfiguration::Status RkISP1CameraConfiguration::validate() +{ + static const std::array formats{ + V4L2_PIX_FMT_YUYV, + V4L2_PIX_FMT_YVYU, + V4L2_PIX_FMT_VYUY, + V4L2_PIX_FMT_NV16, + V4L2_PIX_FMT_NV61, + V4L2_PIX_FMT_NV21, + V4L2_PIX_FMT_NV12, + V4L2_PIX_FMT_GREY, + }; + + const CameraSensor *sensor = data_->sensor_; + Status status = Valid; + + if (config_.empty()) + return Invalid; + + /* Cap the number of entries to the available streams. */ + if (config_.size() > 1) { + config_.resize(1); + status = Adjusted; + } + + StreamConfiguration &cfg = config_[0]; + + /* Adjust the pixel format. */ + if (std::find(formats.begin(), formats.end(), cfg.pixelFormat) == + formats.end()) { + LOG(RkISP1, Debug) << "Adjusting format to NV12"; + cfg.pixelFormat = V4L2_PIX_FMT_NV12; + status = Adjusted; + } + + /* Select the sensor format. */ + sensorFormat_ = sensor->getFormat({ MEDIA_BUS_FMT_SBGGR12_1X12, + MEDIA_BUS_FMT_SGBRG12_1X12, + MEDIA_BUS_FMT_SGRBG12_1X12, + MEDIA_BUS_FMT_SRGGB12_1X12, + MEDIA_BUS_FMT_SBGGR10_1X10, + MEDIA_BUS_FMT_SGBRG10_1X10, + MEDIA_BUS_FMT_SGRBG10_1X10, + MEDIA_BUS_FMT_SRGGB10_1X10, + MEDIA_BUS_FMT_SBGGR8_1X8, + MEDIA_BUS_FMT_SGBRG8_1X8, + MEDIA_BUS_FMT_SGRBG8_1X8, + MEDIA_BUS_FMT_SRGGB8_1X8 }, + cfg.size); + if (!sensorFormat_.size.width || !sensorFormat_.size.height) + sensorFormat_.size = sensor->resolution(); + + /* + * Provide a suitable default that matches the sensor aspect + * ratio and clamp the size to the hardware bounds. + * + * \todo: Check the hardware alignment constraints. + */ + const Size size = cfg.size; + + if (!cfg.size.width || !cfg.size.height) { + cfg.size.width = 1280; + cfg.size.height = 1280 * sensorFormat_.size.height + / sensorFormat_.size.width; + } + + cfg.size.width = std::max(32U, std::min(4416U, cfg.size.width)); + cfg.size.height = std::max(16U, std::min(3312U, cfg.size.height)); + + if (cfg.size != size) { + LOG(RkISP1, Debug) + << "Adjusting size from " << size.toString() + << " to " << cfg.size.toString(); + status = Adjusted; + } + + cfg.bufferCount = RKISP1_BUFFER_COUNT; + + return status; +} + PipelineHandlerRkISP1::PipelineHandlerRkISP1(CameraManager *manager) : PipelineHandler(manager), dphy_(nullptr), isp_(nullptr), video_(nullptr) @@ -109,37 +220,31 @@ CameraConfiguration *PipelineHandlerRkISP1::generateConfiguration(Camera *camera const StreamRoles &roles) { RkISP1CameraData *data = cameraData(camera); - CameraConfiguration *config = new CameraConfiguration(); + CameraConfiguration *config = new RkISP1CameraConfiguration(camera, data); if (!roles.empty()) { StreamConfiguration cfg{}; cfg.pixelFormat = V4L2_PIX_FMT_NV12; cfg.size = data->sensor_->resolution(); - cfg.bufferCount = RKISP1_BUFFER_COUNT; config->addConfiguration(cfg); } + config->validate(); + return config; } -int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *config) +int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c) { + RkISP1CameraConfiguration *config = + static_cast(c); RkISP1CameraData *data = cameraData(camera); StreamConfiguration &cfg = (*config)[0]; CameraSensor *sensor = data->sensor_; int ret; - /* Verify the configuration. */ - const Size &resolution = sensor->resolution(); - if (cfg.size.width > resolution.width || - cfg.size.height > resolution.height) { - LOG(RkISP1, Error) - << "Invalid stream size: larger than sensor resolution"; - return -EINVAL; - } - /* * Configure the sensor links: enable the link corresponding to this * camera and disable all the other sensor links. @@ -170,21 +275,7 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *config * Configure the format on the sensor output and propagate it through * the pipeline. */ - V4L2SubdeviceFormat format; - format = sensor->getFormat({ MEDIA_BUS_FMT_SBGGR12_1X12, - MEDIA_BUS_FMT_SGBRG12_1X12, - MEDIA_BUS_FMT_SGRBG12_1X12, - MEDIA_BUS_FMT_SRGGB12_1X12, - MEDIA_BUS_FMT_SBGGR10_1X10, - MEDIA_BUS_FMT_SGBRG10_1X10, - MEDIA_BUS_FMT_SGRBG10_1X10, - MEDIA_BUS_FMT_SRGGB10_1X10, - MEDIA_BUS_FMT_SBGGR8_1X8, - MEDIA_BUS_FMT_SGBRG8_1X8, - MEDIA_BUS_FMT_SGRBG8_1X8, - MEDIA_BUS_FMT_SRGGB8_1X8 }, - cfg.size); - + V4L2SubdeviceFormat format = config->sensorFormat(); LOG(RkISP1, Debug) << "Configuring sensor with " << format.toString(); ret = sensor->setFormat(&format); diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index c20467766ed0..286d19b0af01 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -39,6 +39,14 @@ public: Stream stream_; }; +class UVCCameraConfiguration : public CameraConfiguration +{ +public: + UVCCameraConfiguration(); + + Status validate() override; +}; + class PipelineHandlerUVC : public PipelineHandler { public: @@ -68,6 +76,45 @@ private: } }; +UVCCameraConfiguration::UVCCameraConfiguration() + : CameraConfiguration() +{ +} + +CameraConfiguration::Status UVCCameraConfiguration::validate() +{ + Status status = Valid; + + if (config_.empty()) + return Invalid; + + /* Cap the number of entries to the available streams. */ + if (config_.size() > 1) { + config_.resize(1); + status = Adjusted; + } + + StreamConfiguration &cfg = config_[0]; + + /* \todo: Validate the configuration against the device capabilities. */ + const unsigned int pixelFormat = cfg.pixelFormat; + const Size size = cfg.size; + + cfg.pixelFormat = V4L2_PIX_FMT_YUYV; + cfg.size = { 640, 480 }; + + if (cfg.pixelFormat != pixelFormat || cfg.size != size) { + LOG(UVC, Debug) + << "Adjusting configuration from " << cfg.toString() + << " to " << cfg.size.toString() << "-YUYV"; + status = Adjusted; + } + + cfg.bufferCount = 4; + + return status; +} + PipelineHandlerUVC::PipelineHandlerUVC(CameraManager *manager) : PipelineHandler(manager) { @@ -76,10 +123,10 @@ PipelineHandlerUVC::PipelineHandlerUVC(CameraManager *manager) CameraConfiguration *PipelineHandlerUVC::generateConfiguration(Camera *camera, const StreamRoles &roles) { - CameraConfiguration *config = new CameraConfiguration(); + CameraConfiguration *config = new UVCCameraConfiguration(); if (!roles.empty()) { - StreamConfiguration cfg; + StreamConfiguration cfg{}; cfg.pixelFormat = V4L2_PIX_FMT_YUYV; cfg.size = { 640, 480 }; @@ -88,6 +135,8 @@ CameraConfiguration *PipelineHandlerUVC::generateConfiguration(Camera *camera, config->addConfiguration(cfg); } + config->validate(); + return config; } diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index 5575880cdbdf..fd48d9b4ab39 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -5,6 +5,8 @@ * vimc.cpp - Pipeline handler for the vimc device */ +#include + #include #include #include @@ -39,6 +41,14 @@ public: Stream stream_; }; +class VimcCameraConfiguration : public CameraConfiguration +{ +public: + VimcCameraConfiguration(); + + Status validate() override; +}; + class PipelineHandlerVimc : public PipelineHandler { public: @@ -68,6 +78,57 @@ private: } }; +VimcCameraConfiguration::VimcCameraConfiguration() + : CameraConfiguration() +{ +} + +CameraConfiguration::Status VimcCameraConfiguration::validate() +{ + static const std::array formats{ + V4L2_PIX_FMT_BGR24, + V4L2_PIX_FMT_RGB24, + V4L2_PIX_FMT_ARGB32, + }; + + Status status = Valid; + + if (config_.empty()) + return Invalid; + + /* Cap the number of entries to the available streams. */ + if (config_.size() > 1) { + config_.resize(1); + status = Adjusted; + } + + StreamConfiguration &cfg = config_[0]; + + /* Adjust the pixel format. */ + if (std::find(formats.begin(), formats.end(), cfg.pixelFormat) == + formats.end()) { + LOG(VIMC, Debug) << "Adjusting format to RGB24"; + cfg.pixelFormat = V4L2_PIX_FMT_RGB24; + status = Adjusted; + } + + /* Clamp the size based on the device limits. */ + const Size size = cfg.size; + + cfg.size.width = std::max(16U, std::min(4096U, cfg.size.width)); + cfg.size.height = std::max(16U, std::min(2160U, cfg.size.height)); + + if (cfg.size != size) { + LOG(VIMC, Debug) + << "Adjusting size to " << cfg.size.toString(); + status = Adjusted; + } + + cfg.bufferCount = 4; + + return status; +} + PipelineHandlerVimc::PipelineHandlerVimc(CameraManager *manager) : PipelineHandler(manager) { @@ -76,10 +137,10 @@ PipelineHandlerVimc::PipelineHandlerVimc(CameraManager *manager) CameraConfiguration *PipelineHandlerVimc::generateConfiguration(Camera *camera, const StreamRoles &roles) { - CameraConfiguration *config = new CameraConfiguration(); + CameraConfiguration *config = new VimcCameraConfiguration(); if (!roles.empty()) { - StreamConfiguration cfg; + StreamConfiguration cfg{}; cfg.pixelFormat = V4L2_PIX_FMT_RGB24; cfg.size = { 640, 480 }; @@ -88,6 +149,8 @@ CameraConfiguration *PipelineHandlerVimc::generateConfiguration(Camera *camera, config->addConfiguration(cfg); } + config->validate(); + return config; } diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index de46e98880a2..dd56907d817e 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -248,17 +248,14 @@ void PipelineHandler::unlock() * is the Camera class which will receive configuration to apply from the * application. * - * Each pipeline handler implementation is responsible for validating - * that the configuration requested in \a config can be achieved - * exactly. Any difference in pixel format, frame size or any other - * parameter shall result in the -EINVAL error being returned, and no - * change in configuration being applied to the pipeline. If - * configuration of a subset of the streams can't be satisfied, the - * whole configuration is considered invalid. + * The configuration is guaranteed to have been validated with + * CameraConfiguration::valid(). The pipeline handler implementation shall not + * perform further validation and may rely on any custom field stored in its + * custom CameraConfiguration derived class. * - * Once the configuration is validated and the camera configured, the pipeline - * handler shall associate a Stream instance to each StreamConfiguration entry - * in the CameraConfiguration with the StreamConfiguration::setStream() method. + * When configuring the camera the pipeline handler shall associate a Stream + * instance to each StreamConfiguration entry in the CameraConfiguration using + * the StreamConfiguration::setStream() method. * * \return 0 on success or a negative error code otherwise */ diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp index f640e80fbaf3..526b44e3a0fc 100644 --- a/test/camera/capture.cpp +++ b/test/camera/capture.cpp @@ -45,7 +45,7 @@ protected: CameraTest::init(); config_ = camera_->generateConfiguration({ StreamRole::VideoRecording }); - if (!config_) { + if (!config_ || config_->size() != 1) { cout << "Failed to generate default configuration" << endl; CameraTest::cleanup(); return TestFail; @@ -58,11 +58,6 @@ protected: { StreamConfiguration &cfg = (*config_)[0]; - if (!config_->isValid()) { - cout << "Failed to read default configuration" << endl; - return TestFail; - } - if (camera_->acquire()) { cout << "Failed to acquire the camera" << endl; return TestFail; diff --git a/test/camera/configuration_default.cpp b/test/camera/configuration_default.cpp index d5cefc1127c9..81055da1d513 100644 --- a/test/camera/configuration_default.cpp +++ b/test/camera/configuration_default.cpp @@ -22,7 +22,7 @@ protected: /* Test asking for configuration for a video stream. */ config = camera_->generateConfiguration({ StreamRole::VideoRecording }); - if (!config || !config->isValid()) { + if (!config || config->size() != 1) { cout << "Default configuration invalid" << endl; delete config; return TestFail; @@ -35,7 +35,7 @@ protected: * stream roles returns an empty camera configuration. */ config = camera_->generateConfiguration({}); - if (!config || config->isValid()) { + if (!config || config->size() != 0) { cout << "Failed to retrieve configuration for empty roles list" << endl; delete config; diff --git a/test/camera/configuration_set.cpp b/test/camera/configuration_set.cpp index ca911f459c32..398a154cec69 100644 --- a/test/camera/configuration_set.cpp +++ b/test/camera/configuration_set.cpp @@ -21,7 +21,7 @@ protected: CameraTest::init(); config_ = camera_->generateConfiguration({ StreamRole::VideoRecording }); - if (!config_) { + if (!config_ || config_->size() != 1) { cout << "Failed to generate default configuration" << endl; CameraTest::cleanup(); return TestFail; @@ -34,11 +34,6 @@ protected: { StreamConfiguration &cfg = (*config_)[0]; - if (!config_->isValid()) { - cout << "Failed to read default configuration" << endl; - return TestFail; - } - if (camera_->acquire()) { cout << "Failed to acquire the camera" << endl; return TestFail; From patchwork Fri May 17 23:06:17 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 1225 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 8DCF661866 for ; Sat, 18 May 2019 01:06:45 +0200 (CEST) Received: from pendragon.bb.dnainternet.fi (dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi [IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 323EC336 for ; Sat, 18 May 2019 01:06:45 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1558134405; bh=/+NingGLt7ZKKaetggvdpsCB4O4Cf1Wr4zBdDH4zi9M=; h=From:To:Subject:Date:In-Reply-To:References:From; b=BzZSmZSSU/1cJpUjZUG/cG9/Lb3s2kOfOvIlvKipG9cjq2DSlIleS7+8Ujnjv9k6+ zqs9pptjiRPUQGq7vHEu3fmF0KKXCLRPS5ds1r2rV3tTSJ7fvbqab55rJQAjDh/78Q BLiLK7h3Tp4fTsKBSQPuptaUq5bCAqQk0mzxB2hY= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sat, 18 May 2019 02:06:17 +0300 Message-Id: <20190517230621.24668-9-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190517230621.24668-1-laurent.pinchart@ideasonboard.com> References: <20190517230621.24668-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH/RFC 08/12] libcamera: stream: Add StreamFormats 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, 17 May 2019 23:06:48 -0000 From: Niklas Söderlund Add a StreamFormats which describes all the formats a stream can support. The object does not collect any formation itself but can simplify users interaction with formats as it's able to translate a stream format range into discrete steps. Signed-off-by: Niklas Söderlund --- include/libcamera/stream.h | 30 ++++++ src/libcamera/stream.cpp | 191 +++++++++++++++++++++++++++++++++++++ 2 files changed, 221 insertions(+) diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h index 47c007ed52e2..138ed649ba8c 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 @@ -39,6 +40,35 @@ enum StreamRole { Viewfinder, }; +class StreamFormats +{ +public: + struct Range { + Size min; + Size max; + unsigned int stepWidth; + unsigned int stepHeight; + }; + + StreamFormats() {} + + StreamFormats(const std::map &rangeSizes) + : rangeSizes_(rangeSizes) {} + + StreamFormats(const std::map> &discreteSizes) + : discreteSizes_(discreteSizes) {} + + std::vector formats() const; + std::vector sizes(unsigned int pixelformat) const; + + bool isRange() const { return !rangeSizes_.empty(); } + Range range(unsigned int pixelformat) const; + +private: + std::map> discreteSizes_; + std::map rangeSizes_; +}; + using StreamRoles = std::vector; class Stream diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp index 0c59a31a3a05..e82fa8143b8f 100644 --- a/src/libcamera/stream.cpp +++ b/src/libcamera/stream.cpp @@ -7,6 +7,7 @@ #include +#include #include #include @@ -122,6 +123,196 @@ std::string StreamConfiguration::toString() const * \brief A vector of StreamRole */ +/** + * \class StreamFormats + * \brief Hold information about supported stream formats + * + * The StreamFormats class holds information about pixel formats and frame + * sizes a stream supports. The class groups size information by the pixel + * format which can produce it. There are two types of size information which + * can be described in the StreamFormats object discrete and range sizes. + * + * The discrete sizes are a list of fixed sizes of the only resolutions the + * stream can produce. While the range description contains a max and min + * size together with a stepping. The range information can either be consumed + * raw which allows users to calculate a size which the stream could support + * or be accessed thru the sizes() helper which will compute a list of common + * discrete sizes which can be produced within the range. + */ + +/** + * \class StreamFormats::Range + * \brief Hold information about stream format range + */ + +/** + * \var StreamFormats::Range::min + * \brief Range minimum size + */ + +/** + * \var StreamFormats::Range::max + * \brief Range maximum size + */ + +/** + * \var StreamFormats::Range::stepWidth + * \brief Range width step length in pixels + */ + +/** + * \var StreamFormats::Range::stepHeight + * \brief Range height step length in pixels + */ + +/** + * \fn StreamFormats::StreamFormats(const std::map< unsigned int, Range > &rangeSizes) + * \brief Constrict a ranged based StreamFormats object + * \param[in] rangeSizes A map of pixel format to a ranged description + * \sa StreamFormats::Range + */ + +/** + * \fn StreamFormats::StreamFormats(const std::map< unsigned int, std::vector< Size >> &discreteSizes) + * \brief Constrict a discrete based StreamFormats object + * \param[in] discreteSizes A map of pixel format to a list of frame sizes + */ + +/** + * \brief Retrive a list of pixel formats supported by the stram + * \returns A list of pixel formats + */ +std::vector StreamFormats::formats() const +{ + std::vector formats; + + if (isRange()) { + for (auto const &it : rangeSizes_) + formats.push_back(it.first); + } else { + for (auto const &it : discreteSizes_) + formats.push_back(it.first); + } + + return formats; +} + +/** + * \brief Retrive a list of frame sizes + * \param[in] pixelformat Pixel format to retrive sizes for + * \returns A list of frame sizes + */ +std::vector StreamFormats::sizes(unsigned int pixelformat) const +{ + std::vector sizes; + + /* + * Sizes to try and extract from ranges. + * \todo Verify list of resolutions are good + */ + static const std::vector rangeDescreteSizes = { + Size(160, 120), + Size(240, 160), + Size(320, 240), + Size(400, 240), + Size(480, 320), + Size(640, 360), + Size(640, 480), + Size(720, 480), + Size(720, 576), + Size(768, 480), + Size(800, 600), + Size(854, 480), + Size(960, 540), + Size(960, 640), + Size(1024, 576), + Size(1024, 600), + Size(1024, 768), + Size(1152, 864), + Size(1280, 1024), + Size(1280, 1080), + Size(1280, 720), + Size(1280, 800), + Size(1360, 768), + Size(1366, 768), + Size(1400, 1050), + Size(1440, 900), + Size(1536, 864), + Size(1600, 1200), + Size(1600, 900), + Size(1680, 1050), + Size(1920, 1080), + Size(1920, 1200), + Size(2048, 1080), + Size(2048, 1152), + Size(2048, 1536), + Size(2160, 1080), + Size(2560, 1080), + Size(2560, 1440), + Size(2560, 1600), + Size(2560, 2048), + Size(2960, 1440), + Size(3200, 1800), + Size(3200, 2048), + Size(3200, 2400), + Size(3440, 1440), + Size(3840, 1080), + Size(3840, 1600), + Size(3840, 2160), + Size(3840, 2400), + Size(4096, 2160), + Size(5120, 2160), + Size(5120, 2880), + Size(7680, 4320), + }; + + if (!isRange()) { + auto const &it = discreteSizes_.find(pixelformat); + if (it == discreteSizes_.end()) + return {}; + sizes = it->second; + } else { + Range limit = range(pixelformat); + + for (const Size &size : rangeDescreteSizes) { + if (size.width < limit.min.width || + size.width > limit.max.width || + size.height < limit.min.height || + size.height > limit.max.height || + size.width % limit.stepWidth || + size.height % limit.stepHeight) + continue; + + sizes.push_back(size); + } + } + + std::sort(sizes.begin(), sizes.end()); + + return sizes; +} + +/** + * \fn StreamFormats::isRange() + * \brief Check if the StreamFormat is a range description + * \returns True if the description is a range, false otherwise + */ + +/** + * \brief Retrive the range description + * \param[in] pixelformat Pixel format to retrive description for + * \returns The range description + */ +StreamFormats::Range StreamFormats::range(unsigned int pixelformat) const +{ + auto const it = rangeSizes_.find(pixelformat); + + if (it == rangeSizes_.end()) + return {}; + + return it->second; +} + /** * \class Stream * \brief Video stream for a camera From patchwork Fri May 17 23:06:18 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 1226 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id D3CCD61871 for ; Sat, 18 May 2019 01:06:45 +0200 (CEST) Received: from pendragon.bb.dnainternet.fi (dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi [IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 791C72F3 for ; Sat, 18 May 2019 01:06:45 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1558134405; bh=HphyvmR2Cy2YpaC/bCTqAnP7Pvc/m4VmT/fUHHIOzl0=; h=From:To:Subject:Date:In-Reply-To:References:From; b=qk66SYVtHHYfJaFz3G6GIGWPQGBqYly5JlmEop0/YInamgt0SGOrogEkdNXG9KLHE mmh4+rWSJ/suUOMSJtkB8atg8fOYrpFA6SFHK/pWxPS3v21w9HefZrWtal2UmY6fun 79oj9Yla6Myk8YD6XKGkhE1v1j3mmuLsp5LV6PEU= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sat, 18 May 2019 02:06:18 +0300 Message-Id: <20190517230621.24668-10-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190517230621.24668-1-laurent.pinchart@ideasonboard.com> References: <20190517230621.24668-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH/RFC 09/12] test: stream: Add test for StreamFormat 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, 17 May 2019 23:06:48 -0000 From: Niklas Söderlund Test that both discrete and range based stream format descriptions result in good discrete frame sizes. The range based stream formats needs to be fitted with a table of resolutions inside libcamera so if that table is update this test might need to be updated. Signed-off-by: Niklas Söderlund --- test/meson.build | 1 + test/stream/meson.build | 10 +++ test/stream/stream_formats.cpp | 112 +++++++++++++++++++++++++++++++++ 3 files changed, 123 insertions(+) create mode 100644 test/stream/meson.build create mode 100644 test/stream/stream_formats.cpp diff --git a/test/meson.build b/test/meson.build index d501f2beaf96..a25e203bcfed 100644 --- a/test/meson.build +++ b/test/meson.build @@ -3,6 +3,7 @@ subdir('libtest') subdir('camera') subdir('media_device') subdir('pipeline') +subdir('stream') subdir('v4l2_device') subdir('v4l2_subdevice') diff --git a/test/stream/meson.build b/test/stream/meson.build new file mode 100644 index 000000000000..1d5a4a97aec4 --- /dev/null +++ b/test/stream/meson.build @@ -0,0 +1,10 @@ +stream_tests = [ + [ 'stream_formats', 'stream_formats.cpp' ], +] + +foreach t : stream_tests + exe = executable(t[0], t[1], + link_with : test_libraries, + include_directories : test_includes_internal) + test(t[0], exe, suite: 'stream', is_parallel: false) +endforeach diff --git a/test/stream/stream_formats.cpp b/test/stream/stream_formats.cpp new file mode 100644 index 000000000000..2ee02840d4d7 --- /dev/null +++ b/test/stream/stream_formats.cpp @@ -0,0 +1,112 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * Copyright (C) 2019, Google Inc. + * + * stream_formats.cpp - StreamFormats test + */ + +#include + +#include +#include + +#include "test.h" + +using namespace std; +using namespace libcamera; + +class StreamFormatsTest : public Test +{ +protected: + int init() + { + return 0; + } + + int testSizes(std::string name, std::vector test, std::vector valid) + { + bool pass = false; + + for (Size &size : test) { + pass = false; + + for (Size &validSize : valid) { + if (size == validSize) { + pass = true; + break; + } + } + + if (!pass) + break; + } + + if (!pass) { + cout << "Failed " << name << endl; + cout << "Sizes to test:" << endl; + for (Size &size : test) + cout << size.toString() << endl; + cout << "Valid sizes:" << endl; + for (Size &size : valid) + cout << size.toString() << endl; + + return TestFail; + } + + return TestPass; + } + + int run() + { + /* Test discrete sizes */ + StreamFormats discrete({ + { 1, { { Size(100, 100), Size(200, 200) } } }, + { 2, { { Size(300, 300), Size(400, 400) } } }, + }); + + if (testSizes("discrete 1", discrete.sizes(1), + { Size(100, 100), Size(200, 200) })) + return TestFail; + + if (testSizes("discrete 2", discrete.sizes(2), + { Size(300, 300), Size(400, 400) })) + return TestFail; + + /* Test range sizes */ + StreamFormats range({ + { 1, { Size(640, 480), Size(640, 480), 1, 1 } }, + { 2, { Size(640, 480), Size(800, 600), 8, 8 } }, + { 3, { Size(640, 480), Size(800, 600), 16, 16 } }, + { 4, { Size(1, 1), Size(4096, 4096), 128, 128 } }, + }); + + if (testSizes("range 1", range.sizes(1), { Size(640, 480) })) + return TestFail; + + if (testSizes("range 2", range.sizes(2), { + Size(640, 480), Size(720, 480), + Size(720, 576), Size(768, 480), + Size(800, 600) })) + return TestFail; + + + if (testSizes("range 3", range.sizes(3), { + Size(640, 480), Size(720, 480), + Size(720, 576), Size(768, 480) })) + return TestFail; + + if (testSizes("range 4", range.sizes(4), { + Size(1024, 768), Size(1280, 1024), + Size(2048, 1152), Size(2048, 1536), + Size(2560, 2048), Size(3200, 2048), })) + return TestFail; + + return TestPass; + } + + void cleanup() + { + } +}; + +TEST_REGISTER(StreamFormatsTest) From patchwork Fri May 17 23:06:19 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 1227 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 1A22D60E53 for ; Sat, 18 May 2019 01:06:46 +0200 (CEST) Received: from pendragon.bb.dnainternet.fi (dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi [IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id BE4D8336 for ; Sat, 18 May 2019 01:06:45 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1558134405; bh=zvXB96Id6V1CoVvFd/HvN+v+pntl7HD3pxKx2o01h7M=; h=From:To:Subject:Date:In-Reply-To:References:From; b=lMcJlXzP2Ton5tkI4LPogFyN8LSVEsCgbfwC+iS/wBWLJhV9dGbojbtc2xkLTk27q ml61tpYmnIf6oj1wRJROcZajeEe7hSWjp2EpE8g5XwVTW4GAGTq+/E6uekA0g6ut7o YwrnU4Zd6OPRxrFQfqiFkcHVkv78KtD2drJPwpiw= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sat, 18 May 2019 02:06:19 +0300 Message-Id: <20190517230621.24668-11-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190517230621.24668-1-laurent.pinchart@ideasonboard.com> References: <20190517230621.24668-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH/RFC 10/12] libcamera: v4l2_device: Add method to enumerate all discrete frame sizes 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, 17 May 2019 23:06:49 -0000 From: Niklas Söderlund Allow the video device to be interrogated about which discrete frame sizes it supports. Signed-off-by: Niklas Söderlund --- src/libcamera/include/v4l2_device.h | 4 +++ src/libcamera/v4l2_device.cpp | 51 +++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+) diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h index 2e7bd1e7f4cc..481d63d9cc4c 100644 --- a/src/libcamera/include/v4l2_device.h +++ b/src/libcamera/include/v4l2_device.h @@ -8,6 +8,8 @@ #define __LIBCAMERA_V4L2_DEVICE_H__ #include +#include +#include #include #include @@ -126,6 +128,8 @@ public: int getFormat(V4L2DeviceFormat *format); int setFormat(V4L2DeviceFormat *format); + std::map> + enumerateFrameSizes(std::set pixelformats); int exportBuffers(BufferPool *pool); int importBuffers(BufferPool *pool); diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp index 8366ffc4db55..d26a89f4a27d 100644 --- a/src/libcamera/v4l2_device.cpp +++ b/src/libcamera/v4l2_device.cpp @@ -564,6 +564,57 @@ int V4L2Device::setFormatMultiplane(V4L2DeviceFormat *format) return 0; } +/** + * \brief Enumerate all discrete frame sizes for a set of pixel formats + * \param[in] pixelformats A set of pixel formats to enumerate sizes for + * + * Enumerate all discrete frame sizes reported by the video device. + * + * \return A map of pixel format to frame sizes + */ +std::map> +V4L2Device::enumerateFrameSizes(std::set pixelformats) +{ + std::map> sizes; + int ret; + + for (unsigned int pixelformat : pixelformats) { + struct v4l2_frmsizeenum frameSize = {}; + unsigned int index = 0; + + frameSize.pixel_format = pixelformat; + + while (true) { + frameSize.index = index; + + ret = ioctl(fd_, VIDIOC_ENUM_FRAMESIZES, &frameSize); + if (ret) { + ret = -errno; + + if (ret == -EINVAL) + break; + + if (ret != -ENOTTY) + LOG(V4L2, Error) + << "Unable to enumerate frame size" + << strerror(-ret); + + return {}; + } + + if (frameSize.type != V4L2_FRMSIZE_TYPE_DISCRETE) + return {}; + + sizes[pixelformat].push_back(Size(frameSize.discrete.width, + frameSize.discrete.height)); + + index++; + } + } + + return sizes; +} + int V4L2Device::requestBuffers(unsigned int count) { struct v4l2_requestbuffers rb = {}; From patchwork Fri May 17 23:06:20 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 1228 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 6F5F361880 for ; Sat, 18 May 2019 01:06:46 +0200 (CEST) Received: from pendragon.bb.dnainternet.fi (dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi [IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 16985549 for ; Sat, 18 May 2019 01:06:46 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1558134406; bh=N5Z05dGtKGOdQk7W9NFxBi/RrT1pg9hi6HqixLgiwoM=; h=From:To:Subject:Date:In-Reply-To:References:From; b=Ga+t3KvXT89l3IO83wKePCvitgesEB7ikx8eYnPtzeud9zbFvaAkrTwzll7cKZtAF Q2aM28lwAJz89HShDA7rUPPKKnSz4buAFXh7cexIGaBzIsNwBRk5I9CO6qEsmcYdaW GSKwG6veSw6WzDYVYsB8VKYNDj7sBefTrg9KsLo8= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sat, 18 May 2019 02:06:20 +0300 Message-Id: <20190517230621.24668-12-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190517230621.24668-1-laurent.pinchart@ideasonboard.com> References: <20190517230621.24668-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH/RFC 11/12] libcamera: pipeline: uvcvideo: Validate format in UVCCameraConfiguration::validate() 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, 17 May 2019 23:06:49 -0000 From: Niklas Söderlund Validate and potentially adjust the requested format with a list of discrete formats retrieved from the video device. Signed-off-by: Niklas Söderlund --- src/libcamera/pipeline/uvcvideo.cpp | 44 ++++++++++++++++++++++++----- 1 file changed, 37 insertions(+), 7 deletions(-) diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index 286d19b0af01..e02d5b19e82a 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -37,6 +37,7 @@ public: V4L2Device *video_; Stream stream_; + StreamFormats formats_; }; class UVCCameraConfiguration : public CameraConfiguration @@ -45,6 +46,7 @@ public: UVCCameraConfiguration(); Status validate() override; + StreamFormats formats; }; class PipelineHandlerUVC : public PipelineHandler @@ -96,21 +98,42 @@ CameraConfiguration::Status UVCCameraConfiguration::validate() StreamConfiguration &cfg = config_[0]; - /* \todo: Validate the configuration against the device capabilities. */ const unsigned int pixelFormat = cfg.pixelFormat; const Size size = cfg.size; + const unsigned int bufferCount = cfg.bufferCount; cfg.pixelFormat = V4L2_PIX_FMT_YUYV; - cfg.size = { 640, 480 }; + if (cfg.pixelFormat != pixelFormat) { + LOG(UVC, Debug) + << "Adjusting pixel format from " << pixelFormat + << " to " << cfg.pixelFormat; + status = Adjusted; + } - if (cfg.pixelFormat != pixelFormat || cfg.size != size) { + const std::vector &formatSizes = formats.sizes(cfg.pixelFormat); + cfg.size = formatSizes.front(); + for (const Size &formatsSize : formatSizes) { + if (formatsSize <= size) + cfg.size = formatsSize; + + if (formatsSize == size) + break; + } + + if (cfg.size != size) { LOG(UVC, Debug) - << "Adjusting configuration from " << cfg.toString() - << " to " << cfg.size.toString() << "-YUYV"; + << "Adjusting size from " << size.toString() + << " to " << cfg.size.toString(); status = Adjusted; } - cfg.bufferCount = 4; + if (bufferCount < 4) { + cfg.bufferCount = 4; + LOG(UVC, Debug) + << "Adjusting buffer count from " << bufferCount + << " to " << cfg.bufferCount; + status = Adjusted; + } return status; } @@ -123,7 +146,10 @@ PipelineHandlerUVC::PipelineHandlerUVC(CameraManager *manager) CameraConfiguration *PipelineHandlerUVC::generateConfiguration(Camera *camera, const StreamRoles &roles) { - CameraConfiguration *config = new UVCCameraConfiguration(); + UVCCameraData *data = cameraData(camera); + UVCCameraConfiguration *config = new UVCCameraConfiguration(); + + config->formats = data->formats_; if (!roles.empty()) { StreamConfiguration cfg{}; @@ -242,6 +268,10 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator) if (data->video_->open()) return false; + data->formats_ = StreamFormats(data->video_->enumerateFrameSizes({ V4L2_PIX_FMT_YUYV })); + for (const Size &size : data->formats_.sizes(V4L2_PIX_FMT_YUYV)) + LOG(UVC, Info) << size.toString(); + data->video_->bufferReady.connect(data.get(), &UVCCameraData::bufferReady); /* Create and register the camera. */ From patchwork Fri May 17 23:06:21 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 1229 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id BBE1D6186B for ; Sat, 18 May 2019 01:06:46 +0200 (CEST) Received: from pendragon.bb.dnainternet.fi (dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi [IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 5A2E7336 for ; Sat, 18 May 2019 01:06:46 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1558134406; bh=rR4ehpcqEikhcJyaY1PU1JMA7xCXc9+aH8pRz9tJQa8=; h=From:To:Subject:Date:In-Reply-To:References:From; b=hJqjRK6kAL2YvsmGar9l0YU02QApQ0uEuIdNltTDyBROTo2AqY7sennSfqeVwPmw+ dW7rawG8riL46vfngEpIni3OU0QE+mG6/WS3UmsaHBB4W8smEMRvOTY5N3t9iLKhPz clQeuy8p3GF0lf7iiYa552Rhu0PHGrVEjGPu1W2c= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Sat, 18 May 2019 02:06:21 +0300 Message-Id: <20190517230621.24668-13-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190517230621.24668-1-laurent.pinchart@ideasonboard.com> References: <20190517230621.24668-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH/RFC 12/12] cam: Validate camera 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, 17 May 2019 23:06:49 -0000 From: Niklas Söderlund Use CameraConfiguration::validate() to validate and possibly update the stream formats before configuring a camera. Signed-off-by: Niklas Söderlund --- src/cam/main.cpp | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/cam/main.cpp b/src/cam/main.cpp index a962f94c8f59..91785b7fea31 100644 --- a/src/cam/main.cpp +++ b/src/cam/main.cpp @@ -140,6 +140,18 @@ static CameraConfiguration *prepareCameraConfig() cfg.pixelFormat = conf["pixelformat"]; } + switch (config->validate()) { + case CameraConfiguration::Valid: + break; + case CameraConfiguration::Adjusted: + std::cout << "Adjusted request format" << std::endl; + break; + case CameraConfiguration::Invalid: + delete config; + config = nullptr; + break; + } + return config; }