<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, 20 Jan 2023 at 12:29, Naushir Patuck <<a href="mailto:naush@raspberrypi.com">naush@raspberrypi.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr">Hi Kieran,<div><br></div></div><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, 20 Jan 2023 at 11:42, Kieran Bingham <<a href="mailto:kieran.bingham@ideasonboard.com" target="_blank">kieran.bingham@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Quoting Naushir Patuck (2023-01-20 11:34:30)<br>
> On Fri, 20 Jan 2023 at 11:27, Kieran Bingham <<br>
> <a href="mailto:kieran.bingham@ideasonboard.com" target="_blank">kieran.bingham@ideasonboard.com</a>> wrote:<br>
> <br>
> > Quoting Naushir Patuck (2023-01-20 10:53:38)<br>
> > > On Fri, 20 Jan 2023 at 10:42, Kieran Bingham <<br>
> > > <a href="mailto:kieran.bingham@ideasonboard.com" target="_blank">kieran.bingham@ideasonboard.com</a>> wrote:<br>
> > ><br>
> > > > Quoting Naushir Patuck (2023-01-18 11:16:40)<br>
> > > > > On Wed, 18 Jan 2023 at 10:09, Kieran Bingham <<br>
> > > > > <a href="mailto:kieran.bingham@ideasonboard.com" target="_blank">kieran.bingham@ideasonboard.com</a>> wrote:<br>
> > > > ><br>
> > > > > > Hi Naush,<br>
> > > > > ><br>
> > > > > > Quoting Naushir Patuck via libcamera-devel (2023-01-18 08:59:42)<br>
> > > > > > > Add a new hints flags field in the StreamConfiguration structure<br>
> > to<br>
> > > > > > allow the<br>
> > > > > > > application to specify certain intended behavior when driving<br>
> > > > libcamera.<br>
> > > > > > > Pipeline handlers are expected to look at these hint flags and<br>
> > may<br>
> > > > > > optimise<br>
> > > > > > > internal operations based on them.<br>
> > > > > > ><br>
> > > > > > > Currently, only one flag is listed, OptionalStream, which the<br>
> > > > > > application can<br>
> > > > > > > set to inform the pipeline handler that a buffer may not be<br>
> > provided<br>
> > > > on<br>
> > > > > > every<br>
> > > > > > > Request for a given stream.<br>
> > > > > ><br>
> > > > > > Sorry - Laurent had comments on this yesterday when I was<br>
> > discussing<br>
> > > > > > with him, so I don't know which way it will go yet...<br>
> > > > > ><br>
> > > > > > I still think this is the better way around, but we'll have to<br>
> > consider<br>
> > > > > > that this is an 'ABI' breakage, as now apps that were able to use<br>
> > two<br>
> > > > > > streams won't unless they explicitly provide all buffers or inform<br>
> > > > > > libcamera that they won't provide all buffers....<br>
> > > > > ><br>
> > > > ><br>
> > > > > In the earlier versions of this series, we didn't have hints but used<br>
> > > > config<br>
> > > > > params to indicate this.  Perhaps I should go back to this mechanism<br>
> > for<br>
> > > > the<br>
> > > > > time being to avoid any ABI breakages until there is an agreed path<br>
> > > > forward?<br>
> > > ><br>
> > > > That's just swapping one ABI breakage for another - because then the<br>
> > > > hint would change (or we'd have both a 'MandatoryStream' hint, and an<br>
> > > > 'OptionalStream' hint ?)<br>
> > > ><br>
> > ><br>
> > > If the "hint" were to move back to the config file, I would make it such<br>
> ><br>
> > Move back to ? Was there previous discussion that moved this out of a<br>
> > config file that I've missed?<br>
> ><br>
> <br>
> In earlier versions of this series, these hints were effectively flags in<br>
> the config files.<br>
> It was suggested to move these into the StreamConfig as hints since they<br>
> may be useful to other pipeline handlers, and enforce explicit application<br>
> behavior through the API.<br>
> <br>
<br>
Ok - in that case, that confirms that the checks/assertions in [9/12]<br>
should be in the core, not just against raspberry pi pipeline handler.<br></blockquote><div><br></div><div>I did consider this at the start, but didn't exactly know where to put the test.</div><div>I tried to put it in  PipelineHandler::doQueueRequest(), but we don't have <br></div><div>access to the advertised StreamConfig structures there, do we?  Any other</div><div>place you could suggest where we have access to the objects?</div></div></div></blockquote><div><br></div><div>Actually, I did not look hard enough.  Request::camera->stream() will give me access</div><div>to the StreamConfiguration objects I think.  I can then do the validation in</div><div>PipelineHandler::doQueueRequest() as I originally intended.  I'll make that change.</div><div><br></div><div>The question of what sense to use for the hints is still outstanding though....</div><div><br></div><div>Regards,</div><div>Naush</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div><br></div><div>Naush</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
This may also require updating any tests that could be affected, and<br>
lc-compliance.<br>
<br>
It may be helpful to split out this core API 'hint's part to it's own<br>
series as I don't think it should necessarily block the configuration<br>
file work here.<br>
<br>
--<br>
KB<br>
<br>
<br>
> > > that the value is true for applications that provide buffers for every<br>
> > > configured stream on every request.<br>
> > > This would be defaulted to false, and libcamera-apps/picamera2 would set<br>
> > a<br>
> > > config file with the flag to true.  This would preserve behavior for any<br>
> > > other existing applications that don't pass in a configuration file,<br>
> > though<br>
> > > it may be suboptimal with allocations.  No ABI breakages either...<br>
> ><br>
> > --<br>
> > Kieran<br>
> ><br>
</blockquote></div></div>
</blockquote></div></div>