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

[Kieran Bingham]

Quoting Kieran Bingham (2023-01-16 17:41:38)
> 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...

This way around, we can also add a check when the request is queued. If
any stream is not marked as optional - and doesn't have a buffer, -
fail the request ?

--
Kieran


> 
> 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
> > >