| Message ID | 20190527001543.13593-15-niklas.soderlund@ragnatech.se |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
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@ragnatech.se> Reviewed-by: Laurent Pinchart <laurent.pinchart@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_); > } >
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_); }
Most of the camera configuration preparation which is done in the 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@ragnatech.se> --- src/cam/capture.cpp | 84 ++++----------------------------------------- src/cam/capture.h | 7 ++-- src/cam/main.cpp | 75 ++++++++++++++++++++++++++++++++++++++-- 3 files changed, 83 insertions(+), 83 deletions(-)