[libcamera-devel] [PATCH] cam: capture_script: Introduce 'loop' property
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Sat Sep 3 02:44:01 CEST 2022
On Fri, Sep 02, 2022 at 08:19:01PM -0400, Paul Elder via libcamera-devel wrote:
> On Fri, Sep 02, 2022 at 05:00:31PM +0200, Jacopo Mondi via libcamera-devel wrote:
> > Add to the capture script support for properties that control the script
"Add support to the capture script for properties" would be clearer.
> > execution. Script properties are specified in a 'properties' section
> > before the actual list of controls specified in the 'frames' section.
> >
> > Define a first 'loop' property that allows to repeat the frame list
>
> s/to repeat/repeating/
>
> > periodically. All the frame ids in the 'frames' section shall be smaller
> > than the loop control.
> >
> > Modify the capture script example to show usage of the 'loop' property
> > and better document the frames list while at it.
> >
> > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > ---
> > src/cam/capture-script.yaml | 50 +++++++++++------------
> > src/cam/capture_script.cpp | 79 +++++++++++++++++++++++++++++++++++--
> > src/cam/capture_script.h | 3 ++
> > 3 files changed, 102 insertions(+), 30 deletions(-)
> >
> > diff --git a/src/cam/capture-script.yaml b/src/cam/capture-script.yaml
> > index 6a749bc60cf7..dbea4a9f01a7 100644
> > --- a/src/cam/capture-script.yaml
> > +++ b/src/cam/capture-script.yaml
> > @@ -5,6 +5,20 @@
> > # A capture script allows to associate a list of controls and their values
> > # to frame numbers.
> >
> > +# The script allows to define a list of frames associated with controls and
>
> s/to define/defining/
>
> > +# an optional list of properties that controls the script behaviour
s/controls/control/
> > +#
> > +# properties:
> > +# - loop: idx
> > +# Repeat the controls every 'idx' frames.
This is confusing, it looks like the comment is part of the YAML file.
I'd write
# properties:
# # Repeat the controls every 'idx' frames.
# - loop: idx
> > +#
> > +# frames:
> > +# - frameid:
I'd write "frame-number" instead of "frameid" to emphasize it's a frame
number, not just any identifier.
> > +# Control1: value1
> > +# Control2: value2
> > +#
> > +# List of frame ids with associated a list of controls to be applied
Same here.
> > +#
> > # \todo Formally define the capture script structure with a schema
> >
> > # Notes:
> > @@ -12,35 +26,17 @@
> > # libcamera::controls:: enumeration
> > # - Controls not supported by the camera currently operated are ignored
> > # - Frame numbers shall be monotonically incrementing, gaps are allowed
> > +# - If a loop limit is specified, frame numbers in the 'frames' list shall be
> > +# strictly minor than the loop control
>
> s/minor/less than/
Or "smaller than" ?
> Reviewed-by: Paul Elder <paul.elder at ideasonboard.com>
>
> >
> > -# Example:
> > -frames:
> > - - 1:
> > - Brightness: 0.0
> > +# Example: Turn brightness up and down every 50 frames
> >
> > - - 40:
> > - Brightness: 0.2
> > +properties:
> > + - loop: 50
> >
> > - - 80:
> > - Brightness: 0.4
> > -
> > - - 120:
> > - Brightness: 0.8
> > -
> > - - 160:
> > - Brightness: 0.4
> > -
> > - - 200:
> > - Brightness: 0.2
> > -
> > - - 240:
> > +frames:
> > + - 0:
I like starting at 0 instead of 1.
> > Brightness: 0.0
> >
> > - - 280:
> > - Brightness: -0.2
> > -
> > - - 300:
> > - Brightness: -0.4
> > -
> > - - 340:
> > - Brightness: -0.8
> > + - 25:
> > + Brightness: 0.8
It's nice to have more than just two frames in the example script (I
regularly use it for testing :-)). Could you instead extend it to
continue with
- 380:
Brightness: -0.4
- 380:
Brightness: -0.2
and loop at 420 ?
> > diff --git a/src/cam/capture_script.cpp b/src/cam/capture_script.cpp
> > index 5e85b3ca604c..52bf19961c17 100644
> > --- a/src/cam/capture_script.cpp
> > +++ b/src/cam/capture_script.cpp
> > @@ -15,7 +15,7 @@ using namespace libcamera;
> >
> > CaptureScript::CaptureScript(std::shared_ptr<Camera> camera,
> > const std::string &fileName)
> > - : camera_(camera), valid_(false)
> > + : camera_(camera), loop_(0), valid_(false)
> > {
> > FILE *fh = fopen(fileName.c_str(), "r");
> > if (!fh) {
> > @@ -44,8 +44,13 @@ CaptureScript::CaptureScript(std::shared_ptr<Camera> camera,
> > const ControlList &CaptureScript::frameControls(unsigned int frame)
> > {
> > static ControlList controls{};
> > + unsigned int idx = frame;
> >
> > - auto it = frameControls_.find(frame);
> > + /* If we loop, repeat the controls every 'loop_' frames. */
> > + if (loop_)
> > + idx = frame % loop_;
> > +
> > + auto it = frameControls_.find(idx);
> > if (it == frameControls_.end())
> > return controls;
> >
> > @@ -149,7 +154,11 @@ int CaptureScript::parseScript(FILE *script)
> >
> > std::string section = eventScalarValue(event);
> >
> > - if (section == "frames") {
> > + if (section == "properties") {
> > + ret = parseProperties();
> > + if (ret)
> > + return ret;
> > + } else if (section == "frames") {
> > ret = parseFrames();
> > if (ret)
> > return ret;
> > @@ -161,6 +170,64 @@ int CaptureScript::parseScript(FILE *script)
> > }
> > }
> >
> > +int CaptureScript::parseProperty()
> > +{
> > + EventPtr event = nextEvent(YAML_MAPPING_START_EVENT);
> > + if (!event)
> > + return -EINVAL;
> > +
> > + std::string prop = parseScalar();
> > + if (prop.empty())
> > + return -EINVAL;
> > +
> > + if (prop == "loop") {
> > + event = nextEvent();
> > + if (!event)
> > + return -EINVAL;
> > +
> > + std::string value = eventScalarValue(event);
> > + if (value.empty())
> > + return -EINVAL;
> > +
> > + loop_ = atoi(value.c_str());
> > + if (!loop_) {
> > + std::cerr << "Invalid loop limit: " << loop_ << std::endl;
> > + return -EINVAL;
> > + }
> > + } else {
> > + std::cerr << "Unsupported property: " << prop << std::endl;
std::cerr << "Unsupported property `" << prop << "'" << std::endl;
Use a colon in a message if you introduce a sentence that explains or
complements the previous one, not when printing the value of a variable.
Compare the following two messages, assuming that someone would use a
YAML file that contains
- properties:
- "file a bug report on bugs.libcamera.org": 0
First message;
Unsupported property: file a bug report on bugs.libcamera.org
Second message:
Unsuported property `file a bug report on bugs.libcamera.org'
Same for the loop limit.
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > + return -EINVAL;
> > + }
> > +
> > + event = nextEvent(YAML_MAPPING_END_EVENT);
> > + if (!event)
> > + return -EINVAL;
> > +
> > + return 0;
> > +}
> > +
> > +int CaptureScript::parseProperties()
> > +{
> > + EventPtr event = nextEvent(YAML_SEQUENCE_START_EVENT);
> > + if (!event)
> > + return -EINVAL;
> > +
> > + while (1) {
> > + if (event->type == YAML_SEQUENCE_END_EVENT)
> > + return 0;
> > +
> > + int ret = parseProperty();
> > + if (ret)
> > + return ret;
> > +
> > + event = nextEvent();
> > + if (!event)
> > + return -EINVAL;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > int CaptureScript::parseFrames()
> > {
> > EventPtr event = nextEvent(YAML_SEQUENCE_START_EVENT);
> > @@ -191,6 +258,12 @@ int CaptureScript::parseFrame(EventPtr event)
> > return -EINVAL;
> >
> > unsigned int frameId = atoi(key.c_str());
> > + if (loop_ && frameId >= loop_) {
> > + std::cerr
> > + << "Frame id (" << frameId << ") shall be smaller than"
> > + << "loop limit (" << loop_ << ")" << std::endl;
> > + return -EINVAL;
> > + }
> >
> > event = nextEvent(YAML_MAPPING_START_EVENT);
> > if (!event)
> > diff --git a/src/cam/capture_script.h b/src/cam/capture_script.h
> > index fffe67e5a3df..7a0ddebb00b5 100644
> > --- a/src/cam/capture_script.h
> > +++ b/src/cam/capture_script.h
> > @@ -40,6 +40,7 @@ private:
> > std::map<unsigned int, libcamera::ControlList> frameControls_;
> > std::shared_ptr<libcamera::Camera> camera_;
> > yaml_parser_t parser_;
> > + unsigned int loop_;
> > bool valid_;
> >
> > EventPtr nextEvent(yaml_event_type_t expectedType = YAML_NO_EVENT);
> > @@ -49,6 +50,8 @@ private:
> >
> > int parseScript(FILE *script);
> >
> > + int parseProperties();
> > + int parseProperty();
> > int parseFrames();
> > int parseFrame(EventPtr event);
> > int parseControl(EventPtr event, libcamera::ControlList &controls);
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list