[libcamera-devel] [PATCH 1/4] libcamera: pipeline_handler: extend documentation for configureStreams()

[Niklas Söderlund]

Hi Laurent,

Thanks for your feedback, as always I'm very happy for comments on 
documentation patches as I hope they improve my skill in writing them.  
Something I really wish to gain more skill in.

I have taken all your suggestions as is for v2, thanks!

On 2019-02-22 12:41:38 +0200, Laurent Pinchart wrote:
> Hi Niklas,
> 
> Thank you for the patch.
> 
> On Thu, Feb 21, 2019 at 12:59:36AM +0100, Niklas Söderlund wrote:
> > Extend the documentation to explicitly state that the pipeline handler
> > implementations are responsible to validate the requested configuration
> 
> s/to validate/for validating that/
> 
> > can be exactly satisfied by the hardware.
> > 
> > Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> > ---
> >  src/libcamera/pipeline_handler.cpp | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> > index 616838fed702fec7..991674b34286ab16 100644
> > --- a/src/libcamera/pipeline_handler.cpp
> > +++ b/src/libcamera/pipeline_handler.cpp
> > @@ -136,6 +136,15 @@ PipelineHandler::~PipelineHandler()
> >   * is the Camera class which will receive configuration to apply from the
> >   * application.
> >   *
> > + * Each pipeline handler implementation is responsible to validate that the
> 
> s/to validate/for validating/
> 
> > + * configuration requested in \a config is exactly the same that is actually
> > + * programmed and accepted by the hardware itself. If the format programmed
> 
> Maybe s/is exactly.*by the hardware itself/can be achieved exactly/ ?
> It's not just the hardware, other components may be involved, and what
> matters for applications is not how the configuration is achieved
> exactly, but that it is achieved exactly. Similarly I wouldn't mention
> "format programmed" for the same reason.
> 
> > + * differers from the one requested the pipeline handler shall return the
> 
> s/differers+differs/
> 
> > + * error -EINVAL. If one streams configuration can't be satisfied by the
> > + * hardware the pipeline handler does not need to try and program any other
> > + * streams in \a config and the whole operation shall be consider to have
> > + * failed.
> 
> The result could be
> 
>  * Each pipeline handler implementation is responsible for validating
>  * that the configuration requested in \a config can be achieved
>  * exactly. Any difference in pixel format, frame size or any other
>  * parameter shall result in the -EINVAL error being returned, and no
>  * change in configuration being applied to the pipeline. If
>  * configuration of a subset of the streams can't be satisfied, the
>  * whole configuration is considered invalid.
> 
> >   * \return 0 on success or a negative error code on error.
> >   */
> >  
> 
> -- 
> Regards,
> 
> Laurent Pinchart

-- 
Regards,
Niklas Söderlund