[libcamera-devel] [IPU3-IPA PATCH] ipu3: Use ChromeOS tuning file paths

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Fri, Aug 06, 2021 at 11:22:05AM +0100, Kieran Bingham wrote:
> During development, the IPA relied upon dupliated tuning file

Did development run into a shortage of 'C's as well ? :-)

> information to load as the aiqb.
> 
> Use the ChromeOS files directly, and remove the libcamera dupliations.

Same here.

The commit message could be improved a bit, it's not clear what's
duplicated and why it should change.

> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
> 
> A separate patch also deletes the duplicated tuning files from this
> repostiory, but as it's simply a remove of the unused files, and renders
> as a git-binary patch, I will not post it to the list, but simply apply
> it and remove the unused files if/when this patch is integrated.

+1 for not carrying copies of the tuning files.

>  data/meson.build | 12 ------------
>  ipu3.cpp         | 13 ++++++++-----
>  meson.build      |  1 -
>  3 files changed, 8 insertions(+), 18 deletions(-)
>  delete mode 100644 data/meson.build
> 
> diff --git a/data/meson.build b/data/meson.build
> deleted file mode 100644
> index 3d6e5f56502a..000000000000
> --- a/data/meson.build
> +++ /dev/null
> @@ -1,12 +0,0 @@
> -# SPDX-License-Identifier: CC0-1.0
> -
> -ipa_data_dir = get_option('datadir') / 'libcamera' / 'ipa'
> -
> -ipu3_aiqb_data = files([
> -    '00ov13858.aiqb',
> -    '01ov5670.aiqb',
> -])
> -
> -install_data(ipu3_aiqb_data,
> -             install_dir : ipa_data_dir / 'ipu3')
> -
> diff --git a/ipu3.cpp b/ipu3.cpp
> index 4fce64785e3f..a9923fe7daf4 100644
> --- a/ipu3.cpp
> +++ b/ipu3.cpp
> @@ -94,9 +94,9 @@ int IPAIPU3::init(const IPASettings &settings)
>  	* or through the configuration interfaces perhaps.
>  	*/
>  	std::map<std::string, std::string> aiqb_paths = {
> -		{ "ov13858", "/usr/share/libcamera/ipa/ipu3/00ov13858.aiqb" },
> -		{ "ov5670", "/usr/share/libcamera/ipa/ipu3/01ov5670.aiqb" },
> -		{ "imx258", "/etc/camera/ipu3/00imx258.aiqb" },
> +		{ "ov13858", "00ov13858.aiqb" },
> +		{ "ov5670", "01ov5670.aiqb" },
> +		{ "imx258", "00imx258.aiqb" },
>  	};
>  
>  	LOG(IPAIPU3, Info) << "Initialising IPA IPU3 for "
> @@ -108,8 +108,11 @@ int IPAIPU3::init(const IPASettings &settings)
>  		return -EINVAL;
>  	}
>  
> -	LOG(IPAIPU3, Info) << "Using tuning file: " << it->second;
> -	ret = aiqb_.load(it->second.c_str());
> +	std::string tuningPath = "/etc/camera/ipu3/";
> +	std::string tuningFile = tuningPath + it->second;

A comment to explain that the patch is specific to Chrome OS would be
nice, in case someone wants to use this on a different platform in the
future.

> +
> +	LOG(IPAIPU3, Info) << "Using tuning file: " << tuningFile;

s/://

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> +	ret = aiqb_.load(tuningFile.c_str());
>  	if (ret) {
>  		LOG(IPAIPU3, Error) << "Failed to load AIQB";
>  		return -ENODATA;
> diff --git a/meson.build b/meson.build
> index e7d6c6b45208..6ea2793d92b8 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -96,7 +96,6 @@ ipu3_ipa_deps = [
>  
>  subdir('aic')
>  subdir('aiq')
> -subdir('data')
>  subdir('src')
>  subdir('stats')
>  

-- 
Regards,

Laurent Pinchart