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

[Laurent Pinchart]

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