[libcamera-devel] [PATCH 8/8] libcamera: pipeline: vimc: add dummy IPA

[Laurent Pinchart]

Hi Paul,

Thank you for the patch.

On Mon, May 27, 2019 at 06:35:40PM -0400, Paul Elder wrote:
> Make the vimc pipeline handler get the dummy IPA, to show how an IPA can
> be acquired by a pipeline handler.
> 
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> ---
>  src/libcamera/pipeline/vimc.cpp | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
> index d3ff527..1df5e5a 100644
> --- a/src/libcamera/pipeline/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc.cpp
> @@ -9,10 +9,12 @@
>  #include <array>
>  
>  #include <libcamera/camera.h>
> +#include <libcamera/ipa/ipa_interface.h>
>  #include <libcamera/request.h>
>  #include <libcamera/stream.h>
>  
>  #include "device_enumerator.h"
> +#include "ipa_manager.h"
>  #include "log.h"
>  #include "media_device.h"
>  #include "pipeline_handler.h"
> @@ -252,6 +254,14 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)
>  	if (!media)
>  		return false;
>  
> +	IPAManager *ipaManager = IPAManager::instance();
> +	ipaManager->addDir("src/ipa");

As explained before, adding directories should not be the responsibility
of pipeline handlers. Please also make sure to test the IPAManager after
installing libcamera, to make sure directories are right.

This will create an issue when running libcamera from the build
directory. I'm not sure how we could fix that. One option would be to
addDir() the build directory in addition to the installation directory,
but that's not very nice. Maybe we could override (or extend) the system
plugin directory with an environment variable ?

> +	std::unique_ptr<IPAInterface> ipa = ipaManager->createIPA(this);
> +	if (ipa == nullptr)
> +		LOG(VIMC, Error) << "no matching IPA found";

Should this be a fatal error ?

You should store the ipa in the pipeline data.

> +	else
> +		ipa->init();
> +
>  	std::unique_ptr<VimcCameraData> data = utils::make_unique<VimcCameraData>(this);
>  
>  	/* Locate and open the capture video node. */

-- 
Regards,

Laurent Pinchart