[libcamera-devel] [PATCH 14/17] cam: Move camera configuration preparation to CamApp
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Mon Jun 10 08:59:49 CEST 2019
Hi Niklas,
Thank you for the patch.
On Mon, May 27, 2019 at 02:15:40AM +0200, Niklas Söderlund wrote:
> Most of the camera configuration preparation which is done in the
s/which/that/ or s/which is //
> Capture module is not specific to capturing and could be useful for
> other modules. Extract the generic parts to CamApp and do basic
> preparation of the configuration before passing it to modules.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
> src/cam/capture.cpp | 84 ++++-----------------------------------------
> src/cam/capture.h | 7 ++--
> src/cam/main.cpp | 75 ++++++++++++++++++++++++++++++++++++++--
> 3 files changed, 83 insertions(+), 83 deletions(-)
>
> diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp
> index a4aa44af25828f23..e455612b36238157 100644
> --- a/src/cam/capture.cpp
> +++ b/src/cam/capture.cpp
> @@ -15,8 +15,8 @@
>
> using namespace libcamera;
>
> -Capture::Capture(Camera *camera)
> - : camera_(camera), writer_(nullptr), last_(0)
> +Capture::Capture(Camera *camera, CameraConfiguration *config)
> + : camera_(camera), config_(config), writer_(nullptr), last_(0)
> {
> }
>
> @@ -29,13 +29,13 @@ int Capture::run(EventLoop *loop, const OptionsParser::Options &options)
> return -ENODEV;
> }
>
> - ret = prepareConfig(options);
> - if (ret) {
> - std::cout << "Failed to prepare camera configuration" << std::endl;
> - return -EINVAL;
> + streamName_.clear();
> + for (unsigned int index = 0; index < config_->size(); ++index) {
> + StreamConfiguration &cfg = config_->at(index);
> + streamName_[cfg.stream()] = "stream" + std::to_string(index);
> }
>
> - ret = camera_->configure(config_.get());
> + ret = camera_->configure(config_);
> if (ret < 0) {
> std::cout << "Failed to configure camera" << std::endl;
> return ret;
> @@ -64,80 +64,10 @@ int Capture::run(EventLoop *loop, const OptionsParser::Options &options)
> }
>
> camera_->freeBuffers();
> - config_.reset();
>
> return ret;
> }
>
> -int Capture::prepareConfig(const OptionsParser::Options &options)
> -{
> - StreamRoles roles;
> -
> - if (options.isSet(OptStream)) {
> - const std::vector<OptionValue> &streamOptions =
> - options[OptStream].toArray();
> -
> - /* Use roles and get a default configuration. */
> - for (auto const &value : streamOptions) {
> - KeyValueParser::Options opt = value.toKeyValues();
> -
> - if (!opt.isSet("role")) {
> - roles.push_back(StreamRole::VideoRecording);
> - } else if (opt["role"].toString() == "viewfinder") {
> - roles.push_back(StreamRole::Viewfinder);
> - } else if (opt["role"].toString() == "video") {
> - roles.push_back(StreamRole::VideoRecording);
> - } else if (opt["role"].toString() == "still") {
> - roles.push_back(StreamRole::StillCapture);
> - } else {
> - std::cerr << "Unknown stream role "
> - << opt["role"].toString() << std::endl;
> - return -EINVAL;
> - }
> - }
> - } else {
> - /* If no configuration is provided assume a single video stream. */
> - roles.push_back(StreamRole::VideoRecording);
> - }
> -
> - config_ = camera_->generateConfiguration(roles);
> - if (!config_ || config_->size() != roles.size()) {
> - std::cerr << "Failed to get default stream configuration"
> - << std::endl;
> - return -EINVAL;
> - }
> -
> - /* Apply configuration if explicitly requested. */
> - if (options.isSet(OptStream)) {
> - const std::vector<OptionValue> &streamOptions =
> - options[OptStream].toArray();
> -
> - unsigned int i = 0;
> - for (auto const &value : streamOptions) {
> - KeyValueParser::Options opt = value.toKeyValues();
> - StreamConfiguration &cfg = config_->at(i++);
> -
> - if (opt.isSet("width"))
> - cfg.size.width = opt["width"];
> -
> - if (opt.isSet("height"))
> - cfg.size.height = opt["height"];
> -
> - /* TODO: Translate 4CC string to ID. */
> - if (opt.isSet("pixelformat"))
> - cfg.pixelFormat = opt["pixelformat"];
> - }
> - }
> -
> - streamName_.clear();
> - for (unsigned int index = 0; index < config_->size(); ++index) {
> - StreamConfiguration &cfg = config_->at(index);
> - streamName_[cfg.stream()] = "stream" + std::to_string(index);
> - }
> -
> - return 0;
> -}
> -
> int Capture::capture(EventLoop *loop)
> {
> int ret;
> diff --git a/src/cam/capture.h b/src/cam/capture.h
> index a97d1f44d229c214..1d4a25a84a51403b 100644
> --- a/src/cam/capture.h
> +++ b/src/cam/capture.h
> @@ -20,19 +20,18 @@
> class Capture
> {
> public:
> - Capture(libcamera::Camera *camera);
> + Capture(libcamera::Camera *camera,
> + libcamera::CameraConfiguration *config);
>
> int run(EventLoop *loop, const OptionsParser::Options &options);
> private:
> - int prepareConfig(const OptionsParser::Options &options);
> -
> int capture(EventLoop *loop);
>
> void requestComplete(libcamera::Request *request,
> const std::map<libcamera::Stream *, libcamera::Buffer *> &buffers);
>
> libcamera::Camera *camera_;
> - std::unique_ptr<libcamera::CameraConfiguration> config_;
> + libcamera::CameraConfiguration *config_;
>
> std::map<libcamera::Stream *, std::string> streamName_;
> BufferWriter *writer_;
> diff --git a/src/cam/main.cpp b/src/cam/main.cpp
> index dbf04917bcc5aa38..338740d1512c7189 100644
> --- a/src/cam/main.cpp
> +++ b/src/cam/main.cpp
> @@ -33,19 +33,21 @@ public:
>
> private:
> int parseOptions(int argc, char *argv[]);
> + int prepareConfig();
> int run();
>
> static CamApp *app_;
> OptionsParser::Options options_;
> CameraManager *cm_;
> std::shared_ptr<Camera> camera_;
> + std::unique_ptr<libcamera::CameraConfiguration> config_;
> EventLoop *loop_;
> };
>
> CamApp *CamApp::app_ = nullptr;
>
> CamApp::CamApp()
> - : cm_(nullptr), camera_(nullptr), loop_(nullptr)
> + : cm_(nullptr), camera_(nullptr), config_(nullptr), loop_(nullptr)
> {
> CamApp::app_ = this;
> }
> @@ -90,6 +92,10 @@ int CamApp::init(int argc, char **argv)
> }
>
> std::cout << "Using camera " << camera_->name() << std::endl;
> +
> + ret = prepareConfig();
> + if (ret)
> + return ret;
> }
>
> loop_ = new EventLoop(cm_->eventDispatcher());
> @@ -107,6 +113,8 @@ void CamApp::cleanup()
> camera_.reset();
> }
>
> + config_.reset();
> +
> cm_->stop();
> }
>
> @@ -168,6 +176,69 @@ int CamApp::parseOptions(int argc, char *argv[])
> return 0;
> }
>
> +int CamApp::prepareConfig()
> +{
> + StreamRoles roles;
> +
> + if (options_.isSet(OptStream)) {
> + const std::vector<OptionValue> &streamOptions =
> + options_[OptStream].toArray();
> +
> + /* Use roles and get a default configuration. */
> + for (auto const &value : streamOptions) {
> + KeyValueParser::Options opt = value.toKeyValues();
> +
> + if (!opt.isSet("role")) {
> + roles.push_back(StreamRole::VideoRecording);
> + } else if (opt["role"].toString() == "viewfinder") {
> + roles.push_back(StreamRole::Viewfinder);
> + } else if (opt["role"].toString() == "video") {
> + roles.push_back(StreamRole::VideoRecording);
> + } else if (opt["role"].toString() == "still") {
> + roles.push_back(StreamRole::StillCapture);
> + } else {
> + std::cerr << "Unknown stream role "
> + << opt["role"].toString() << std::endl;
> + return -EINVAL;
> + }
> + }
> + } else {
> + /* If no configuration is provided assume a single video stream. */
> + roles.push_back(StreamRole::VideoRecording);
> + }
> +
> + config_ = camera_->generateConfiguration(roles);
> + if (!config_ || config_->size() != roles.size()) {
> + std::cerr << "Failed to get default stream configuration"
> + << std::endl;
> + return -EINVAL;
> + }
> +
> + /* Apply configuration if explicitly requested. */
> + if (options_.isSet(OptStream)) {
> + const std::vector<OptionValue> &streamOptions =
> + options_[OptStream].toArray();
> +
> + unsigned int i = 0;
> + for (auto const &value : streamOptions) {
> + KeyValueParser::Options opt = value.toKeyValues();
> + StreamConfiguration &cfg = config_->at(i++);
> +
> + if (opt.isSet("width"))
> + cfg.size.width = opt["width"];
> +
> + if (opt.isSet("height"))
> + cfg.size.height = opt["height"];
> +
> + /* TODO: Translate 4CC string to ID. */
> + if (opt.isSet("pixelformat"))
> + cfg.pixelFormat = opt["pixelformat"];
> + }
> + }
> +
> + return 0;
> +}
> +
> int CamApp::run()
> {
> if (options_.isSet(OptList)) {
> @@ -177,7 +248,7 @@ int CamApp::run()
> }
>
> if (options_.isSet(OptCapture)) {
> - Capture capture(camera_.get());
> + Capture capture(camera_.get(), config_.get());
> return capture.run(loop_, options_);
> }
>
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list