[libcamera-devel] [PATCH v2 1/3] cam: Add a parser for capture scripts

Jacopo Mondi jacopo at jmondi.org
Sat May 21 18:24:49 CEST 2022


Hi Laurent,

On Fri, May 20, 2022 at 07:52:32PM +0300, Laurent Pinchart wrote:
> From: Jacopo Mondi <jacopo at jmondi.org>
>
> Add a parser class to the cam test application to control the capture
> operations through a yaml script.
>
> The capture script currently allow to specify a list of controls and
> their associated values to be applied per-frame.
>
> Also add a trivial capture script example to showcase the intended
> script structure.
>
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>

You sob is missing here.
You could almost change the authorship too :)

> ---
> Changes since v1:
>
> - Add SPDX license identifier to capture-script.yaml
> - Drop gcc 7 support as libcamera requires gcc 8 or newer
> - Make scriptPath_ a local variable in CaptureScript constructor
> - Drop file check as we want to accept symlinks too
> - Pass const reference to fileName to CaptureScript constructor
> - Avoid manual deletes of tokens
> - Simplify and improve error message handling
> - Print line and column number in error message
> - Simplify handling of key/value parsing
> - Simplify parser deletion
> - Don't consume last tokens as that's not needed.
> - Simplify CaptureScript::unpackFailure()
> - Switch to the simpler event API
> - Typo fixes
> ---
>  src/cam/capture-script.yaml |  46 +++++
>  src/cam/capture_script.cpp  | 334 ++++++++++++++++++++++++++++++++++++
>  src/cam/capture_script.h    |  60 +++++++
>  src/cam/meson.build         |   2 +
>  4 files changed, 442 insertions(+)
>  create mode 100644 src/cam/capture-script.yaml
>  create mode 100644 src/cam/capture_script.cpp
>  create mode 100644 src/cam/capture_script.h
>
> diff --git a/src/cam/capture-script.yaml b/src/cam/capture-script.yaml
> new file mode 100644
> index 000000000000..6a749bc60cf7
> --- /dev/null
> +++ b/src/cam/capture-script.yaml
> @@ -0,0 +1,46 @@
> +# SPDX-License-Identifier: CC0-1.0
> +
> +# Capture script example
> +#
> +# A capture script allows to associate a list of controls and their values
> +# to frame numbers.
> +
> +# \todo Formally define the capture script structure with a schema
> +
> +# Notes:
> +# - Controls have to be specified by name, as defined in the
> +#   libcamera::controls:: enumeration
> +# - Controls not supported by the camera currently operated are ignored
> +# - Frame numbers shall be monotonically incrementing, gaps are allowed
> +
> +# Example:
> +frames:
> +  - 1:
> +      Brightness: 0.0
> +
> +  - 40:
> +      Brightness: 0.2
> +
> +  - 80:
> +      Brightness: 0.4
> +
> +  - 120:
> +      Brightness: 0.8
> +
> +  - 160:
> +      Brightness: 0.4
> +
> +  - 200:
> +      Brightness: 0.2
> +
> +  - 240:
> +      Brightness: 0.0
> +
> +  - 280:
> +      Brightness: -0.2
> +
> +  - 300:
> +      Brightness: -0.4
> +
> +  - 340:
> +      Brightness: -0.8
> diff --git a/src/cam/capture_script.cpp b/src/cam/capture_script.cpp
> new file mode 100644
> index 000000000000..8e9120321a25
> --- /dev/null
> +++ b/src/cam/capture_script.cpp
> @@ -0,0 +1,334 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * capture_script.cpp - Capture session configuration script

Missing copyright. Can you add IoB one ?

> + */
> +
> +#include "capture_script.h"
> +
> +#include <iostream>
> +#include <stdio.h>
> +#include <stdlib.h>
> +
> +using namespace libcamera;
> +
> +CaptureScript::CaptureScript(std::shared_ptr<Camera> camera,
> +			     const std::string &fileName)
> +	: camera_(camera), valid_(false)
> +{
> +	FILE *fh = fopen(fileName.c_str(), "r");
> +	if (!fh) {
> +		int ret = -errno;
> +		std::cerr << "Failed to open capture script " << fileName
> +			  << ": " << strerror(-ret) << std::endl;
> +		return;
> +	}
> +
> +	/*
> +	 * Map the camera's controls to their name so that they can be
> +	 * easily identified when parsing the script file.
> +	 */
> +	for (const auto &[control, info] : camera_->controls())
> +		controls_[control->name()] = control;
> +
> +	int ret = parseScript(fh);
> +	fclose(fh);
> +	if (ret)
> +		return;
> +
> +	valid_ = true;
> +}
> +
> +/* Retrieve the control list associated with a frame number. */
> +const ControlList &CaptureScript::frameControls(unsigned int frame)
> +{
> +	static ControlList controls{};
> +
> +	auto it = frameControls_.find(frame);
> +	if (it == frameControls_.end())
> +		return controls;
> +
> +	return it->second;
> +}
> +
> +CaptureScript::EventPtr CaptureScript::nextEvent(yaml_event_type_t expectedType)
> +{
> +	EventPtr event(new yaml_event_t);
> +
> +	if (!yaml_parser_parse(&parser_, event.get()))
> +		return nullptr;
> +
> +	if (expectedType != YAML_NO_EVENT && !checkEvent(event, expectedType))

Could you move the expectedType != YAML_NO_EVENT check in checkEvent() ?

It's probably nicer to read, at the expense of an unconditional
function call


> +		return nullptr;
> +
> +	return event;
> +}
> +
> +bool CaptureScript::checkEvent(const EventPtr &event, yaml_event_type_t expectedType) const
> +{
> +	if (event->type != expectedType) {
> +		std::cerr << "Capture script error on line " << event->start_mark.line

Tried to insert a random error in the config file and I get line == 0,
column == 0

Apart from these two minors, the automatic deletion and usage of
event-based parsing is much nicer! Thanks

Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>

Thanks
  j

> +			  << " column " << event->start_mark.column << ": "
> +			  << "Expected " << eventTypeName(expectedType)
> +			  << " event, got " << eventTypeName(event->type)
> +			  << std::endl;
> +		return false;
> +	}
> +
> +	return true;
> +}
> +
> +std::string CaptureScript::eventScalarValue(const EventPtr &event)
> +{
> +	return std::string(reinterpret_cast<char *>(event->data.scalar.value),
> +			   event->data.scalar.length);
> +}
> +
> +std::string CaptureScript::eventTypeName(yaml_event_type_t type)
> +{
> +	static const std::map<yaml_event_type_t, std::string> typeNames = {
> +		{ YAML_STREAM_START_EVENT, "stream-start" },
> +		{ YAML_STREAM_END_EVENT, "stream-end" },
> +		{ YAML_DOCUMENT_START_EVENT, "document-start" },
> +		{ YAML_DOCUMENT_END_EVENT, "document-end" },
> +		{ YAML_ALIAS_EVENT, "alias" },
> +		{ YAML_SCALAR_EVENT, "scalar" },
> +		{ YAML_SEQUENCE_START_EVENT, "sequence-start" },
> +		{ YAML_SEQUENCE_END_EVENT, "sequence-end" },
> +		{ YAML_MAPPING_START_EVENT, "mapping-start" },
> +		{ YAML_MAPPING_END_EVENT, "mapping-end" },
> +	};
> +
> +	auto it = typeNames.find(type);
> +	if (it == typeNames.end())
> +		return "[type " + std::to_string(type) + "]";
> +
> +	return it->second;
> +}
> +
> +int CaptureScript::parseScript(FILE *script)
> +{
> +	int ret = yaml_parser_initialize(&parser_);
> +	if (!ret) {
> +		std::cerr << "Failed to initialize yaml parser" << std::endl;
> +		return ret;
> +	}
> +
> +	/* Delete the parser upon function exit. */
> +	struct ParserDeleter {
> +		ParserDeleter(yaml_parser_t *parser) : parser_(parser) { }
> +		~ParserDeleter() { yaml_parser_delete(parser_); }
> +		yaml_parser_t *parser_;
> +	} deleter(&parser_);
> +
> +	yaml_parser_set_input_file(&parser_, script);
> +
> +	EventPtr event = nextEvent(YAML_STREAM_START_EVENT);
> +	if (!event)
> +		return -EINVAL;
> +
> +	event = nextEvent(YAML_DOCUMENT_START_EVENT);
> +	if (!event)
> +		return -EINVAL;
> +
> +	event = nextEvent(YAML_MAPPING_START_EVENT);
> +	if (!event)
> +		return -EINVAL;
> +
> +	while (1) {
> +		event = nextEvent();
> +		if (!event)
> +			return -EINVAL;
> +
> +		if (event->type == YAML_MAPPING_END_EVENT)
> +			return 0;
> +
> +		if (!checkEvent(event, YAML_SCALAR_EVENT))
> +			return -EINVAL;
> +
> +		std::string section = eventScalarValue(event);
> +
> +		if (section == "frames") {
> +			parseFrames();
> +		} else {
> +			std::cerr << "Unsupported section '" << section << "'"
> +				  << std::endl;
> +			return -EINVAL;
> +		}
> +	}
3> +}
> +
> +int CaptureScript::parseFrames()
> +{
> +	EventPtr event = nextEvent(YAML_SEQUENCE_START_EVENT);
> +	if (!event)
> +		return -EINVAL;
> +
> +	while (1) {
> +		event = nextEvent();
> +		if (!event)
> +			return -EINVAL;
> +
> +		if (event->type == YAML_SEQUENCE_END_EVENT)
> +			return 0;
> +
> +		int ret = parseFrame(std::move(event));
> +		if (ret)
> +			return ret;
> +	}
> +}
> +
> +int CaptureScript::parseFrame(EventPtr event)
> +{
> +	if (!checkEvent(event, YAML_MAPPING_START_EVENT))
> +		return -EINVAL;
> +
> +	std::string key = parseScalar();
> +	if (key.empty())
> +		return -EINVAL;
> +
> +	unsigned int frameId = atoi(key.c_str());
> +
> +	event = nextEvent(YAML_MAPPING_START_EVENT);
> +	if (!event)
> +		return -EINVAL;
> +
> +	ControlList controls{};
> +
> +	while (1) {
> +		event = nextEvent();
> +		if (!event)
> +			return -EINVAL;
> +
> +		if (event->type == YAML_MAPPING_END_EVENT)
> +			break;
> +
> +		int ret = parseControl(std::move(event), controls);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	frameControls_[frameId] = std::move(controls);
> +
> +	event = nextEvent(YAML_MAPPING_END_EVENT);
> +	if (!event)
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
> +int CaptureScript::parseControl(EventPtr event, ControlList &controls)
> +{
> +	/* We expect a value after a key. */
> +	std::string name = eventScalarValue(event);
> +	if (name.empty())
> +		return -EINVAL;
> +
> +	/* If the camera does not support the control just ignore it. */
> +	auto it = controls_.find(name);
> +	if (it == controls_.end()) {
> +		std::cerr << "Unsupported control '" << name << "'" << std::endl;
> +		return -EINVAL;
> +	}
> +
> +	std::string value = parseScalar();
> +	if (value.empty())
> +		return -EINVAL;
> +
> +	const ControlId *controlId = it->second;
> +	ControlValue val = unpackControl(controlId, value);
> +	controls.set(controlId->id(), val);
> +
> +	return 0;
> +}
> +
> +std::string CaptureScript::parseScalar()
> +{
> +	EventPtr event = nextEvent(YAML_SCALAR_EVENT);
> +	if (!event)
> +		return "";
> +
> +	return eventScalarValue(event);
> +}
> +
> +void CaptureScript::unpackFailure(const ControlId *id, const std::string &repr)
> +{
> +	static const std::map<unsigned int, const char *> typeNames = {
> +		{ ControlTypeNone, "none" },
> +		{ ControlTypeBool, "bool" },
> +		{ ControlTypeByte, "byte" },
> +		{ ControlTypeInteger32, "int32" },
> +		{ ControlTypeInteger64, "int64" },
> +		{ ControlTypeFloat, "float" },
> +		{ ControlTypeString, "string" },
> +		{ ControlTypeRectangle, "Rectangle" },
> +		{ ControlTypeSize, "Size" },
> +	};
> +
> +	const char *typeName;
> +	auto it = typeNames.find(id->type());
> +	if (it != typeNames.end())
> +		typeName = it->second;
> +	else
> +		typeName = "unknown";
> +
> +	std::cerr << "Unsupported control '" << repr << "' for "
> +		  << typeName << " control " << id->name() << std::endl;
> +}
> +
> +ControlValue CaptureScript::unpackControl(const ControlId *id,
> +					  const std::string &repr)
> +{
> +	ControlValue value{};
> +
> +	switch (id->type()) {
> +	case ControlTypeNone:
> +		break;
> +	case ControlTypeBool: {
> +		bool val;
> +
> +		if (repr == "true") {
> +			val = true;
> +		} else if (repr == "false") {
> +			val = false;
> +		} else {
> +			unpackFailure(id, repr);
> +			return value;
> +		}
> +
> +		value.set<bool>(val);
> +		break;
> +	}
> +	case ControlTypeByte: {
> +		uint8_t val = strtol(repr.c_str(), NULL, 10);
> +		value.set<uint8_t>(val);
> +		break;
> +	}
> +	case ControlTypeInteger32: {
> +		int32_t val = strtol(repr.c_str(), NULL, 10);
> +		value.set<int32_t>(val);
> +		break;
> +	}
> +	case ControlTypeInteger64: {
> +		int64_t val = strtoll(repr.c_str(), NULL, 10);
> +		value.set<int64_t>(val);
> +		break;
> +	}
> +	case ControlTypeFloat: {
> +		float val = strtof(repr.c_str(), NULL);
> +		value.set<float>(val);
> +		break;
> +	}
> +	case ControlTypeString: {
> +		value.set<std::string>(repr);
> +		break;
> +	}
> +	case ControlTypeRectangle:
> +		/* \todo Parse rectangles. */
> +		break;
> +	case ControlTypeSize:
> +		/* \todo Parse Sizes. */
> +		break;
> +	}
> +
> +	return value;
> +}
> diff --git a/src/cam/capture_script.h b/src/cam/capture_script.h
> new file mode 100644
> index 000000000000..44368f63f34c
> --- /dev/null
> +++ b/src/cam/capture_script.h
> @@ -0,0 +1,60 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * capture_script.h - Capture session configuration script
> + */
> +
> +#pragma once
> +
> +#include <map>
> +#include <memory>
> +#include <string>
> +
> +#include <libcamera/camera.h>
> +#include <libcamera/controls.h>
> +
> +#include <yaml.h>
> +
> +class CaptureScript
> +{
> +public:
> +	CaptureScript(std::shared_ptr<libcamera::Camera> camera,
> +		      const std::string &fileName);
> +
> +	bool valid() const { return valid_; }
> +
> +	const libcamera::ControlList &frameControls(unsigned int frame);
> +
> +private:
> +	struct EventDeleter {
> +		void operator()(yaml_event_t *event) const
> +		{
> +			yaml_event_delete(event);
> +			delete event;
> +		}
> +	};
> +	using EventPtr = std::unique_ptr<yaml_event_t, EventDeleter>;
> +
> +	std::map<std::string, const libcamera::ControlId *> controls_;
> +	std::map<unsigned int, libcamera::ControlList> frameControls_;
> +	std::shared_ptr<libcamera::Camera> camera_;
> +	yaml_parser_t parser_;
> +	bool valid_;
> +
> +	EventPtr nextEvent(yaml_event_type_t expectedType = YAML_NO_EVENT);
> +	bool checkEvent(const EventPtr &event, yaml_event_type_t expectedType) const;
> +	static std::string eventScalarValue(const EventPtr &event);
> +	static std::string eventTypeName(yaml_event_type_t type);
> +
> +	int parseScript(FILE *script);
> +
> +	int parseFrames();
> +	int parseFrame(EventPtr event);
> +	int parseControl(EventPtr event, libcamera::ControlList &controls);
> +
> +	std::string parseScalar();
> +
> +	void unpackFailure(const libcamera::ControlId *id,
> +			   const std::string &repr);
> +	libcamera::ControlValue unpackControl(const libcamera::ControlId *id,
> +					      const std::string &repr);
> +};
> diff --git a/src/cam/meson.build b/src/cam/meson.build
> index 5bab8c9e331b..c47a85592478 100644
> --- a/src/cam/meson.build
> +++ b/src/cam/meson.build
> @@ -11,6 +11,7 @@ cam_enabled = true
>
>  cam_sources = files([
>      'camera_session.cpp',
> +    'capture_script.cpp',
>      'event_loop.cpp',
>      'file_sink.cpp',
>      'frame_sink.cpp',
> @@ -38,6 +39,7 @@ cam  = executable('cam', cam_sources,
>                        libcamera_public,
>                        libdrm,
>                        libevent,
> +                      libyaml,
>                    ],
>                    cpp_args : cam_cpp_args,
>                    install : true)
> --
> Regards,
>
> Laurent Pinchart
>


More information about the libcamera-devel mailing list