[libcamera-devel] [PATCH v4 01/12] libcamera: stream: Add stream hints to StreamConfiguration

[Kieran Bingham]

Quoting Kieran Bingham (2023-01-16 16:15:56)
> Quoting Naushir Patuck via libcamera-devel (2022-12-09 09:00:39)
> > Add a new hints flags field in the StreamConfiguration structure to allow the
> > application to specify certain intended behavior when driving libcamera.
> > Pipeline handlers are expected to look at these hint flags and may optimise
> > internal operations based on them.
> > 
> > Currently, only one flag is listed, MandatoryRequestBuffer, which the
> > application sets to guarantee that a buffer will be provided in the Request for
> > each configured stream.
> > 
> > Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> > ---
> >  include/libcamera/stream.h |  8 ++++++++
> >  src/libcamera/stream.cpp   | 24 ++++++++++++++++++++++++
> >  2 files changed, 32 insertions(+)
> > 
> > diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h
> > index 29235ddf0d8a..1c5273004297 100644
> > --- a/include/libcamera/stream.h
> > +++ b/include/libcamera/stream.h
> > @@ -13,6 +13,8 @@
> >  #include <string>
> >  #include <vector>
> >  
> > +#include <libcamera/base/flags.h>
> > +
> >  #include <libcamera/color_space.h>
> >  #include <libcamera/framebuffer.h>
> >  #include <libcamera/geometry.h>
> > @@ -51,6 +53,12 @@ struct StreamConfiguration {
> >  
> >         std::optional<ColorSpace> colorSpace;
> >  
> > +       enum class Hint {
> > +               None = 0,
> > +               MandatoryRequestBuffer = (1 << 0),
> 
> Might be bikeshedding, but reading later in the series I understand this
> to mean that this is a 'Mandatory Stream'.
> 
> That makes me think "MandatoryStream" is what we're hinting at ? But
> even that might not convey it fully, so I'm not opposed to
> MandatoryRequestBuffer.
> 
> I get the idea here, but I don't currently know how we might expose to
> applications if the hints are 'possible'/'usable'/'valid'?
> 
> Or maybe it doesn't matter for hints. Applications could just set them
> to say "I'm going to do this..." ? And it doesn't matter if the pipeline
> supports it or not...

And to throw in a spanner / go for discussions:

Trying to think about this, I would expect that the 'default' case of
most applications handling a stream would be such that on that single
stream - it's expected that the stream is a mandatory stream.

That means I'm tempted to suggest this is inverted...

Make the stream hint 'optional' ... and only if the stream is marked as
optional can the buffers not be added to a request?

That way - applications for single streams get the correct behaviour 'by
default' ... while more complex systems with multiple streams define
which stream is likely to be only provided optionally...

Thoughts anyone?

--
Kieran


> 
> 
> > +       };
> > +       Flags<Hint> hints;
> > +
> >         Stream *stream() const { return stream_; }
> >         void setStream(Stream *stream) { stream_ = stream; }
> >         const StreamFormats &formats() const { return formats_; }
> > diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp
> > index 67f308157fbf..504c6d86cd04 100644
> > --- a/src/libcamera/stream.cpp
> > +++ b/src/libcamera/stream.cpp
> > @@ -349,6 +349,30 @@ StreamConfiguration::StreamConfiguration(const StreamFormats &formats)
> >   * color spaces can be supported and in what combinations.
> >   */
> >  
> > +/**
> > + * \enum StreamConfiguration::Hint
> > + * \brief List of available hint flags provided by the application for a stream
> > + *
> > + * \var StreamConfiguration::Hint::None
> > + * No hints for this stream.
> > + * \var StreamConfiguration::Hint::MandatoryRequestBuffer
> > + * Informs the pipeline handler that the application guarantee to provide a
> 
> s/guarantee/guarantees/
> or ..
> will guarantee
> 
> > + * buffer for the configured stream in the Request. This may allow the pipeline
> 
> /in the Request/in every Request/
> 
> > + * handler to allocate fewer (or no) buffers for internal use.
> > + */
> > +
> > +/**
> > + * \var StreamConfiguration::hints
> > + * \brief Application provided StreamConfiguration::Hint flags for specific
> > + * stream behavior
> > + *
> > + * Provides hints from the application to the pipeline handlers on how it
> > + * intends on handling a given configured stream. These hints may alter the
> > + * behavior of the pipeline handlers, for example, by allocating fewer buffers
> > + * for internal use if an application guarantees to always provide a buffer in
> > + * the Request for a stream.
> > + */
> > +
> >  /**
> >   * \fn StreamConfiguration::stream()
> >   * \brief Retrieve the stream associated with the configuration
> > -- 
> > 2.25.1
> >