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

Message ID 20190527223540.21855-9-paul.elder@ideasonboard.com
State Superseded
Headers show
Series
  • Add IPAManager and IPAInterface
Related show

Commit Message

Paul Elder May 27, 2019, 10:35 p.m. UTC
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@ideasonboard.com>
---
 src/libcamera/pipeline/vimc.cpp | 10 ++++++++++
 1 file changed, 10 insertions(+)

Comments

Laurent Pinchart May 28, 2019, 4:32 p.m. UTC | #1
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@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. */

Patch

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");
+	std::unique_ptr<IPAInterface> ipa = ipaManager->createIPA(this);
+	if (ipa == nullptr)
+		LOG(VIMC, Error) << "no matching IPA found";
+	else
+		ipa->init();
+
 	std::unique_ptr<VimcCameraData> data = utils::make_unique<VimcCameraData>(this);
 
 	/* Locate and open the capture video node. */