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

[Naushir Patuck]

Hi Kieran,

On Fri, 20 Jan 2023 at 11:42, Kieran Bingham <
kieran.bingham at ideasonboard.com> wrote:

> Quoting Naushir Patuck (2023-01-20 11:34:30)
> > On Fri, 20 Jan 2023 at 11:27, Kieran Bingham <
> > kieran.bingham at ideasonboard.com> wrote:
> >
> > > Quoting Naushir Patuck (2023-01-20 10:53:38)
> > > > On Fri, 20 Jan 2023 at 10:42, Kieran Bingham <
> > > > kieran.bingham at ideasonboard.com> wrote:
> > > >
> > > > > Quoting Naushir Patuck (2023-01-18 11:16:40)
> > > > > > On Wed, 18 Jan 2023 at 10:09, Kieran Bingham <
> > > > > > kieran.bingham at ideasonboard.com> wrote:
> > > > > >
> > > > > > > Hi Naush,
> > > > > > >
> > > > > > > Quoting Naushir Patuck via libcamera-devel (2023-01-18
> 08:59:42)
> > > > > > > > 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, OptionalStream, which the
> > > > > > > application can
> > > > > > > > set to inform the pipeline handler that a buffer may not be
> > > provided
> > > > > on
> > > > > > > every
> > > > > > > > Request for a given stream.
> > > > > > >
> > > > > > > Sorry - Laurent had comments on this yesterday when I was
> > > discussing
> > > > > > > with him, so I don't know which way it will go yet...
> > > > > > >
> > > > > > > I still think this is the better way around, but we'll have to
> > > consider
> > > > > > > that this is an 'ABI' breakage, as now apps that were able to
> use
> > > two
> > > > > > > streams won't unless they explicitly provide all buffers or
> inform
> > > > > > > libcamera that they won't provide all buffers....
> > > > > > >
> > > > > >
> > > > > > In the earlier versions of this series, we didn't have hints but
> used
> > > > > config
> > > > > > params to indicate this.  Perhaps I should go back to this
> mechanism
> > > for
> > > > > the
> > > > > > time being to avoid any ABI breakages until there is an agreed
> path
> > > > > forward?
> > > > >
> > > > > That's just swapping one ABI breakage for another - because then
> the
> > > > > hint would change (or we'd have both a 'MandatoryStream' hint, and
> an
> > > > > 'OptionalStream' hint ?)
> > > > >
> > > >
> > > > If the "hint" were to move back to the config file, I would make it
> such
> > >
> > > Move back to ? Was there previous discussion that moved this out of a
> > > config file that I've missed?
> > >
> >
> > In earlier versions of this series, these hints were effectively flags in
> > the config files.
> > It was suggested to move these into the StreamConfig as hints since they
> > may be useful to other pipeline handlers, and enforce explicit
> application
> > behavior through the API.
> >
>
> Ok - in that case, that confirms that the checks/assertions in [9/12]
> should be in the core, not just against raspberry pi pipeline handler.
>

I did consider this at the start, but didn't exactly know where to put the
test.
I tried to put it in  PipelineHandler::doQueueRequest(), but we don't have
access to the advertised StreamConfig structures there, do we?  Any other
place you could suggest where we have access to the objects?

Naush


> This may also require updating any tests that could be affected, and
> lc-compliance.
>
> It may be helpful to split out this core API 'hint's part to it's own
> series as I don't think it should necessarily block the configuration
> file work here.
>
> --
> KB
>
>
> > > > that the value is true for applications that provide buffers for
> every
> > > > configured stream on every request.
> > > > This would be defaulted to false, and libcamera-apps/picamera2 would
> set
> > > a
> > > > config file with the flag to true.  This would preserve behavior for
> any
> > > > other existing applications that don't pass in a configuration file,
> > > though
> > > > it may be suboptimal with allocations.  No ABI breakages either...
> > >
> > > --
> > > Kieran
> > >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20230120/a292c516/attachment.htm>