| Message ID | 20190220235939.25147-2-niklas.soderlund@ragnatech.se |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
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@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. > */ >
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@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
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 + * configuration requested in \a config is exactly the same that is actually + * programmed and accepted by the hardware itself. If the format programmed + * differers from the one requested the pipeline handler shall return the + * 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. + * * \return 0 on success or a negative error code on error. */
Extend the documentation to explicitly state that the pipeline handler implementations are responsible to validate the requested configuration can be exactly satisfied by the hardware. Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> --- src/libcamera/pipeline_handler.cpp | 9 +++++++++ 1 file changed, 9 insertions(+)