Message ID | 20240628104828.2928109-10-stefan.klug@ideasonboard.com |
---|---|
State | New |
Headers | show |
Series |
|
Related | show |
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@ideasonboard.com> > Reviewed-by: Paul Elder <paul.elder@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 >
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 + lux = None + + parts = fn.stem.split('_') + for part in parts: + if part == 'alsc': + lsc_only = True + continue + r = re.match(r'(\d+)[kK]', part) + if r: + color = int(r.group(1)) + continue + r = re.match(r'(\d+)[lLuU]', part) + 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') + + 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