[PATCH v2 09/25] libtuning: Improve filename parsing

[Laurent Pinchart]

Hi Stefan,

Thank you for the patch.

On Fri, Jun 28, 2024 at 12:47:02PM +0200, Stefan Klug wrote:
> In the tuning datasets, the files had names like
> 'imx335_1600l_3000k_1.dng'. That failed on the old filename parsing
> function. As there is no need to dictate the order of the tags, split
> the big regex into chunks and parse them one by one. This also makes
> the code easier to digest.

Is there a risk we'll later add tags we can't differentiate without
considering their position ? I suppose we can decide how to format them,
so it should be fine ?

> Signed-off-by: Stefan Klug <stefan.klug at ideasonboard.com>
> Reviewed-by: Paul Elder <paul.elder at ideasonboard.com>
> ---
>  utils/tuning/libtuning/utils.py | 30 ++++++++++++++++++++++--------
>  1 file changed, 22 insertions(+), 8 deletions(-)
> 
> diff --git a/utils/tuning/libtuning/utils.py b/utils/tuning/libtuning/utils.py
> index 178c6957c581..00cf5a57512f 100644
> --- a/utils/tuning/libtuning/utils.py
> +++ b/utils/tuning/libtuning/utils.py
> @@ -43,14 +43,28 @@ def _list_image_files(directory):
>  
>  
>  def _parse_image_filename(fn: Path):
> -    result = re.search(r'^(alsc_)?(\d+)[kK]_(\d+)?[lLuU]?.\w{3,4}$', fn.name)
> -    if result is None:
> -        logger.error(f'The file name of {fn.name} is incorrectly formatted')
> -        return None, None, None
> -
> -    color = int(result.group(2))
> -    lsc_only = result.group(1) is not None
> -    lux = None if lsc_only else int(result.group(3))
> +    lsc_only = False
> +    color = None

color_temperature ?

> +    lux = None
> +
> +    parts = fn.stem.split('_')
> +    for part in parts:
> +        if part == 'alsc':

Should we rename this to 'lsc' at some point ? The *adaptive* LSC is
specific to Raspberry Pi.

> +            lsc_only = True
> +            continue

Maybe a blank line here and after the next continue to let the code
breathe a bit more.

> +        r = re.match(r'(\d+)[kK]', part)

Would

        r = re.match(r'([0-9]+)[kK]', part)

be more readable ?

Do we need to be case-insensitive here, or could we standardize on one
option and stick to it ?

> +        if r:
> +            color = int(r.group(1))
> +            continue
> +        r = re.match(r'(\d+)[lLuU]', part)

Same here, and additionally, do we need to support both L and U ? Is
there a difference ?

> +        if r:
> +            lux = int(r.group(1))
> +
> +    if color is None:
> +        logger.error(f'The file name of {fn.name} does not contain a color temperature')

        logger.error(f'The file name "{fn.name}" does not contain a color temperature')

Same below.

> +
> +    if lux is None and lsc_only is False:
> +        logger.error(f'The file name of {fn.name} must either contain alsc or a lux level')
>  
>      return color, lux, lsc_only
>  

-- 
Regards,

Laurent Pinchart