[libcamera-devel] [PATCH 28/30] cam: Make camera-related options sub-options of OptCamera
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Mon Jul 12 20:50:37 CEST 2021
Hi Kieran,
On Mon, Jul 12, 2021 at 05:11:38PM +0100, Kieran Bingham wrote:
> On 07/07/2021 03:19, Laurent Pinchart wrote:
> > Use the new hierarchical options feature of the option parser to turn
> > camera-related option (--capture, --file, --stream, --strict-formats and
> > --metadata) into children of the --camera option. As an added bonus, we
> > don't need to check anymore if a camera has been specified when capture
> > is requested, as that's now enforced by the option parser.
>
> Aha that's what I was looking for earlier... so here it is ;-)
>
> Although - wouldn't this also mean we could handle printing camera
> information here too ? (--list-controls or such)
Yes, we could do so too, and I'm not sure what would be best. That's why
I haven't moved those arguments yet. Do we expect use cases where we
want to print different information for different cameras ?
> Anyway, this is still progress:
>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>
> > This change prepares for support of multiple cameras.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> > src/cam/camera_session.cpp | 22 ++++++++--------
> > src/cam/camera_session.h | 6 ++++-
> > src/cam/main.cpp | 53 +++++++++++++++++++++-----------------
> > 3 files changed, 45 insertions(+), 36 deletions(-)
> >
> > diff --git a/src/cam/camera_session.cpp b/src/cam/camera_session.cpp
> > index ceb2c3ab3a92..f2383567af3b 100644
> > --- a/src/cam/camera_session.cpp
> > +++ b/src/cam/camera_session.cpp
> > @@ -21,11 +21,11 @@
> > using namespace libcamera;
> >
> > CameraSession::CameraSession(CameraManager *cm,
> > + const std::string &cameraId,
> > const OptionsParser::Options &options)
> > - : last_(0), queueCount_(0), captureCount_(0),
> > + : options_(options), last_(0), queueCount_(0), captureCount_(0),
> > captureLimit_(0), printMetadata_(false)
> > {
> > - const std::string &cameraId = options[OptCamera];
> > char *endptr;
> > unsigned long index = strtoul(cameraId.c_str(), &endptr, 10);
> > if (*endptr == '\0' && index > 0 && index <= cm->cameras().size())
> > @@ -44,7 +44,7 @@ CameraSession::CameraSession(CameraManager *cm,
> > return;
> > }
> >
> > - StreamRoles roles = StreamKeyValueParser::roles(options[OptStream]);
> > + StreamRoles roles = StreamKeyValueParser::roles(options_[OptStream]);
> >
> > std::unique_ptr<CameraConfiguration> config =
> > camera_->generateConfiguration(roles);
> > @@ -56,12 +56,12 @@ CameraSession::CameraSession(CameraManager *cm,
> >
> > /* Apply configuration if explicitly requested. */
> > if (StreamKeyValueParser::updateConfiguration(config.get(),
> > - options[OptStream])) {
> > + options_[OptStream])) {
> > std::cerr << "Failed to update configuration" << std::endl;
> > return;
> > }
> >
> > - bool strictFormats = options.isSet(OptStrictFormats);
> > + bool strictFormats = options_.isSet(OptStrictFormats);
> >
> > switch (config->validate()) {
> > case CameraConfiguration::Valid:
> > @@ -134,14 +134,14 @@ void CameraSession::infoConfiguration() const
> > }
> > }
> >
> > -int CameraSession::start(const OptionsParser::Options &options)
> > +int CameraSession::start()
> > {
> > int ret;
> >
> > queueCount_ = 0;
> > captureCount_ = 0;
> > - captureLimit_ = options[OptCapture].toInteger();
> > - printMetadata_ = options.isSet(OptMetadata);
> > + captureLimit_ = options_[OptCapture].toInteger();
> > + printMetadata_ = options_.isSet(OptMetadata);
> >
> > ret = camera_->configure(config_.get());
> > if (ret < 0) {
> > @@ -157,9 +157,9 @@ int CameraSession::start(const OptionsParser::Options &options)
> >
> > camera_->requestCompleted.connect(this, &CameraSession::requestComplete);
> >
> > - if (options.isSet(OptFile)) {
> > - if (!options[OptFile].toString().empty())
> > - writer_ = std::make_unique<BufferWriter>(options[OptFile]);
> > + if (options_.isSet(OptFile)) {
> > + if (!options_[OptFile].toString().empty())
> > + writer_ = std::make_unique<BufferWriter>(options_[OptFile]);
> > else
> > writer_ = std::make_unique<BufferWriter>();
> > }
> > diff --git a/src/cam/camera_session.h b/src/cam/camera_session.h
> > index 6221aadadf90..f137279ab421 100644
> > --- a/src/cam/camera_session.h
> > +++ b/src/cam/camera_session.h
> > @@ -9,6 +9,7 @@
> >
> > #include <memory>
> > #include <stdint.h>
> > +#include <string>
> > #include <vector>
> >
> > #include <libcamera/base/signal.h>
> > @@ -27,10 +28,12 @@ class CameraSession
> > {
> > public:
> > CameraSession(libcamera::CameraManager *cm,
> > + const std::string &cameraId,
> > const OptionsParser::Options &options);
> > ~CameraSession();
> >
> > bool isValid() const { return config_ != nullptr; }
> > + const OptionsParser::Options &options() { return options_; };
> >
> > libcamera::Camera *camera() { return camera_.get(); }
> > libcamera::CameraConfiguration *config() { return config_.get(); }
> > @@ -39,7 +42,7 @@ public:
> > void listProperties() const;
> > void infoConfiguration() const;
> >
> > - int start(const OptionsParser::Options &options);
> > + int start();
> > void stop();
> >
> > libcamera::Signal<> captureDone;
> > @@ -51,6 +54,7 @@ private:
> > void requestComplete(libcamera::Request *request);
> > void processRequest(libcamera::Request *request);
> >
> > + const OptionsParser::Options &options_;
> > std::shared_ptr<libcamera::Camera> camera_;
> > std::unique_ptr<libcamera::CameraConfiguration> config_;
> >
> > diff --git a/src/cam/main.cpp b/src/cam/main.cpp
> > index 6d7d45e11499..7688fa5540ea 100644
> > --- a/src/cam/main.cpp
> > +++ b/src/cam/main.cpp
> > @@ -113,19 +113,6 @@ int CamApp::parseOptions(int argc, char *argv[])
> > parser.addOption(OptCamera, OptionString,
> > "Specify which camera to operate on, by id or by index", "camera",
> > ArgumentRequired, "camera");
> > - parser.addOption(OptCapture, OptionInteger,
> > - "Capture until interrupted by user or until <count> frames captured",
> > - "capture", ArgumentOptional, "count");
> > - parser.addOption(OptFile, OptionString,
> > - "Write captured frames to disk\n"
> > - "If the file name ends with a '/', it sets the directory in which\n"
> > - "to write files, using the default file name. Otherwise it sets the\n"
> > - "full file path and name. The first '#' character in the file name\n"
> > - "is expanded to the stream name and frame sequence number.\n"
> > - "The default file name is 'frame-#.bin'.",
> > - "file", ArgumentOptional, "filename");
> > - parser.addOption(OptStream, &streamKeyValue,
> > - "Set configuration of a camera stream", "stream", true);
> > parser.addOption(OptHelp, OptionNone, "Display this help message",
> > "help");
> > parser.addOption(OptInfo, OptionNone,
> > @@ -138,12 +125,32 @@ int CamApp::parseOptions(int argc, char *argv[])
> > parser.addOption(OptMonitor, OptionNone,
> > "Monitor for hotplug and unplug camera events",
> > "monitor");
> > +
> > + /* Sub-options of OptCamera: */
> > + parser.addOption(OptCapture, OptionInteger,
> > + "Capture until interrupted by user or until <count> frames captured",
> > + "capture", ArgumentOptional, "count", false,
> > + OptCamera);
> > + parser.addOption(OptFile, OptionString,
> > + "Write captured frames to disk\n"
> > + "If the file name ends with a '/', it sets the directory in which\n"
> > + "to write files, using the default file name. Otherwise it sets the\n"
> > + "full file path and name. The first '#' character in the file name\n"
> > + "is expanded to the stream name and frame sequence number.\n"
> > + "The default file name is 'frame-#.bin'.",
> > + "file", ArgumentOptional, "filename", false,
> > + OptCamera);
> > + parser.addOption(OptStream, &streamKeyValue,
> > + "Set configuration of a camera stream", "stream", true,
> > + OptCamera);
> > parser.addOption(OptStrictFormats, OptionNone,
> > "Do not allow requested stream format(s) to be adjusted",
> > - "strict-formats");
> > + "strict-formats", ArgumentNone, nullptr, false,
> > + OptCamera);
> > parser.addOption(OptMetadata, OptionNone,
> > "Print the metadata for completed requests",
> > - "metadata");
> > + "metadata", ArgumentNone, nullptr, false,
> > + OptCamera);
> >
> > options_ = parser.parse(argc, argv);
> > if (!options_.valid())
> > @@ -192,7 +199,10 @@ int CamApp::run()
> > std::unique_ptr<CameraSession> session;
> >
> > if (options_.isSet(OptCamera)) {
> > - session = std::make_unique<CameraSession>(cm_.get(), options_);
> > + const OptionValue &camera = options_[OptCamera];
> > + session = std::make_unique<CameraSession>(cm_.get(),
> > + camera.toString(),
> > + camera.children());
> > if (!session->isValid()) {
> > std::cout << "Failed to create camera session" << std::endl;
> > return -EINVAL;
> > @@ -223,13 +233,8 @@ int CamApp::run()
> > }
> >
> > /* 4. Start capture. */
> > - if (options_.isSet(OptCapture)) {
> > - if (!session) {
> > - std::cout << "Can't capture without a camera" << std::endl;
> > - return -ENODEV;
> > - }
> > -
> > - ret = session->start(options_);
> > + if (session && session->options().isSet(OptCapture)) {
> > + ret = session->start();
> > if (ret) {
> > std::cout << "Failed to start camera session" << std::endl;
> > return ret;
> > @@ -253,7 +258,7 @@ int CamApp::run()
> > loop_.exec();
> >
> > /* 6. Stop capture. */
> > - if (options_.isSet(OptCapture))
> > + if (session && session->options().isSet(OptCapture))
> > session->stop();
> >
> > return 0;
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list