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

[Naushir Patuck]

On Fri, 20 Jan 2023 at 12:29, Naushir Patuck <naush at raspberrypi.com> wrote:

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

Actually, I did not look hard enough.  Request::camera->stream() will give
me access
to the StreamConfiguration objects I think.  I can then do the validation in
PipelineHandler::doQueueRequest() as I originally intended.  I'll make that
change.

The question of what sense to use for the hints is still outstanding
though....

Regards,
Naush


> 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/e8e865a8/attachment.htm>