[libcamera-devel] [PATCH v2 3/5] cam: Add helper class to parse stream configuration
Niklas Söderlund
niklas.soderlund at ragnatech.se
Wed Apr 29 02:07:48 CEST 2020
Hi Andrey,
Thanks for your feedback.
On 2020-04-29 03:02:23 +0300, Andrey Konovalov wrote:
> Hi Niklas,
>
> Thank you for the patch.
>
> On 28.04.2020 01:05, Niklas Söderlund wrote:
> > Create a new helper class StreamKeyValueParser to parse command line
> > options describing stream configurations. The goal is to share this new
> > class between cam and qcam.
> >
> > Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> > ---
> > src/cam/meson.build | 1 +
> > src/cam/stream_options.cpp | 133 +++++++++++++++++++++++++++++++++++++
> > src/cam/stream_options.h | 32 +++++++++
> > 3 files changed, 166 insertions(+)
> > create mode 100644 src/cam/stream_options.cpp
> > create mode 100644 src/cam/stream_options.h
> >
> > diff --git a/src/cam/meson.build b/src/cam/meson.build
> > index 2419d648bc17e02b..162d6333f94e4851 100644
> > --- a/src/cam/meson.build
> > +++ b/src/cam/meson.build
> > @@ -4,6 +4,7 @@ cam_sources = files([
> > 'event_loop.cpp',
> > 'main.cpp',
> > 'options.cpp',
> > + 'stream_options.cpp',
> > ])
> > cam = executable('cam', cam_sources,
> > diff --git a/src/cam/stream_options.cpp b/src/cam/stream_options.cpp
> > new file mode 100644
> > index 0000000000000000..9f726a7e70628648
> > --- /dev/null
> > +++ b/src/cam/stream_options.cpp
> > @@ -0,0 +1,133 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*
> > + * Copyright (C) 2020, Raspberry Pi (Trading) Ltd.
> > + *
> > + * stream_options.cpp - Helper to parse options for streams
> > + */
> > +#include "stream_options.h"
> > +
> > +#include <iostream>
> > +
> > +using namespace libcamera;
> > +
> > +StreamKeyValueParser::StreamKeyValueParser()
> > +{
> > + addOption("role", OptionString,
> > + "Role for the stream (viewfinder, video, still, stillraw)",
> > + ArgumentRequired);
> > + addOption("width", OptionInteger, "Width in pixels",
> > + ArgumentRequired);
> > + addOption("height", OptionInteger, "Height in pixels",
> > + ArgumentRequired);
> > + addOption("pixelformat", OptionInteger, "Pixel format",
> > + ArgumentRequired);
> > +}
> > +
> > +KeyValueParser::Options StreamKeyValueParser::parse(const char *arguments)
> > +{
> > + KeyValueParser::Options options = KeyValueParser::parse(arguments);
> > + StreamRole role;
> > +
> > + if (options.valid() && options.isSet("role") &&
> > + parseRole(&role, options)) {
> > + std::cerr << "Unknown stream role "
> > + << options["role"].toString() << std::endl;
> > + options.invalidate();
> > + }
> > +
> > + return options;
> > +}
> > +
> > +StreamRoles StreamKeyValueParser::roles(const OptionValue &values)
> > +{
> > + const std::vector<OptionValue> &streamParameters = values.toArray();
> > +
> > + /* If no configuration values to examine default to viewfinder. */
> > + if (!streamParameters.size())
> > + return { StreamRole::Viewfinder };
> > +
> > + StreamRoles roles;
> > + for (auto const &value : streamParameters) {
> > + KeyValueParser::Options opts = value.toKeyValues();
> > + StreamRole role;
> > +
> > + /* If role is invalid or not set default to viewfinder. */
> > + if (parseRole(&role, value))
> > + role = StreamRole::Viewfinder;
> > +
> > + roles.push_back(role);
> > + }
> > +
> > + return roles;
> > +}
> > +
> > +int StreamKeyValueParser::updateConfiguration(CameraConfiguration *config,
> > + const OptionValue &values)
> > +{
> > + const std::vector<OptionValue> &streamParameters = values.toArray();
> > +
> > + if (!config) {
> > + std::cerr << "No configuration provided" << std::endl;
> > + return -EINVAL;
> > + }
> > +
> > + /* If no configuration values nothing to do. */
> > + if (!streamParameters.size())
> > + return 0;
> > +
> > + if (config->size() != streamParameters.size()) {
> > + std::cerr
> > + << "Number of streams in configuration "
> > + << config->size()
> > + << " does not match number of streams parsed "
> > + << streamParameters.size()
> > + << std::endl;
> > + return -EINVAL;
> > + }
> > +
> > + unsigned int i = 0;
> > + for (auto const &value : streamParameters) {
> > + KeyValueParser::Options opts = value.toKeyValues();
> > + StreamConfiguration &cfg = config->at(i++);
> > +
> > + if (opts.isSet("width") && opts.isSet("height")) {
> > + cfg.size.width = opts["width"];
> > + cfg.size.height = opts["height"];
> > + }
> > +
> > + /* TODO: Translate 4CC string to ID. */
> > + if (opts.isSet("pixelformat"))
> > + cfg.pixelFormat = PixelFormat(opts["pixelformat"]);
>
> This is not quite related to this patch set, as it doesn't change the current behavior.
>
> But with the current implementation this is not possible to specify a PixelFormat with
> (nonzero) modifier from the command line.
> Does it worth adding the "TODO:"?
That is a good question. I'm not yet sure on how we shall create a
string representation of a FourCC + modifier in a neat way. But
extending the TODO to also mention we need to deal with modifiers is a
good idea.
>
> Thanks,
> Andrey
>
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +int StreamKeyValueParser::parseRole(StreamRole *role,
> > + const KeyValueParser::Options &options)
> > +{
> > + bool found = false;
> > +
> > + if (options.isSet("role")) {
> > + std::string name = options["role"].toString();
> > +
> > + if (name == "viewfinder") {
> > + *role = StreamRole::Viewfinder;
> > + found = true;
> > + }
> > + if (name == "video") {
> > + *role = StreamRole::VideoRecording;
> > + found = true;
> > + }
> > + if (name == "still") {
> > + *role = StreamRole::StillCapture;
> > + found = true;
> > + }
> > + if (name == "stillraw") {
> > + *role = StreamRole::StillCaptureRaw;
> > + found = true;
> > + }
> > + }
> > +
> > + return found ? 0 : -EINVAL;
> > +}
> > diff --git a/src/cam/stream_options.h b/src/cam/stream_options.h
> > new file mode 100644
> > index 0000000000000000..c90d8c4dc595814d
> > --- /dev/null
> > +++ b/src/cam/stream_options.h
> > @@ -0,0 +1,32 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/*
> > + * Copyright (C) 2020, Raspberry Pi (Trading) Ltd.
> > + *
> > + * stream_options.h - Helper to parse options for streams
> > + */
> > +#ifndef __CAM_STREAM_OPTIONS_H__
> > +#define __CAM_STREAM_OPTIONS_H__
> > +
> > +#include <libcamera/camera.h>
> > +
> > +#include "options.h"
> > +
> > +using namespace libcamera;
> > +
> > +class StreamKeyValueParser : public KeyValueParser
> > +{
> > +public:
> > + StreamKeyValueParser();
> > +
> > + KeyValueParser::Options parse(const char *arguments) override;
> > +
> > + static StreamRoles roles(const OptionValue &values);
> > + static int updateConfiguration(CameraConfiguration *config,
> > + const OptionValue &values);
> > +
> > +private:
> > + static int parseRole(StreamRole *role,
> > + const KeyValueParser::Options &options);
> > +};
> > +
> > +#endif /* __CAM_STREAM_OPTIONS_H__ */
> >
--
Regards,
Niklas Söderlund
More information about the libcamera-devel
mailing list