[libcamera-devel] [PATCH v2] tests: v4l2_compat: Add test for v4l2_compat

[Laurent Pinchart]

Hi Paul,

Thank you for the patch.

On Fri, Jun 26, 2020 at 05:22:08PM +0900, Paul Elder wrote:
> Test the V4L2 compatibility layer by running v4l2-compliance -s on every
> /dev/video* device.
> 
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> 
> ---
> Note that as of v2, the tests will fail if the tester has a camera
> supported by libcamera that has unsupported formats, since they will
> cause a floating point exception. So I don't think this should be merged
> until that is fixed, otherwise we might get nasty test failures in
> bisection.

I wonder if we shouldn't temporarily disable testing with UVC. There's
the additional issue that meson has a fixed timeout for tests, and
depending on the number of UVC devices plugged in the system, the
timeout could be exceeded.

Or maybe it would make sense to restrict tests to a single video node
for each driver ? We could add an argument to the test to override that
behaviour and test all devices when run manually (or the other way
around, test all devices by default, but restrict to a smaller number of
devices when run from meson test).

> Changes in v2:
> - change all strings to single-quote
> - simplify meson.build
> - get path to v4l2-compat.so from cli arg, rather than running custom find()
>   - remove find_file()
> - extend test timeout to 60 seconds (from default of 30)
> - don't run v4l2-compliance subprocesses in shell
> - check if v4l2-compliance runs are killed by signal (eg. the known
>   SIGABRT due to floating point exception)
> - move the check to see if libcamera supports the camera to v4l2-ctl
>   instead of v4l2-compliance (in v1 we only checked if the pipeline was
>   supported with v4l2-ctl)
> ---
>  test/meson.build                     |   1 +
>  test/v4l2_compat/meson.build         |  10 +++
>  test/v4l2_compat/v4l2_compat_test.py | 127 +++++++++++++++++++++++++++
>  3 files changed, 138 insertions(+)
>  create mode 100644 test/v4l2_compat/meson.build
>  create mode 100755 test/v4l2_compat/v4l2_compat_test.py
> 
> diff --git a/test/meson.build b/test/meson.build
> index 7808a26..f41d6e7 100644
> --- a/test/meson.build
> +++ b/test/meson.build
> @@ -12,6 +12,7 @@ subdir('pipeline')
>  subdir('process')
>  subdir('serialization')
>  subdir('stream')
> +subdir('v4l2_compat')
>  subdir('v4l2_subdevice')
>  subdir('v4l2_videodevice')
>  
> diff --git a/test/v4l2_compat/meson.build b/test/v4l2_compat/meson.build
> new file mode 100644
> index 0000000..5b29de7
> --- /dev/null
> +++ b/test/v4l2_compat/meson.build
> @@ -0,0 +1,10 @@
> +# SPDX-License-Identifier: CC0-1.0
> +
> +if is_variable('v4l2_compat')
> +    v4l2_compat_test = files('v4l2_compat_test.py')
> +
> +    test('v4l2_compat_test', v4l2_compat_test,
> +         args : v4l2_compat,
> +         suite : 'v4l2_compat',
> +         timeout : 60)
> +endif
> diff --git a/test/v4l2_compat/v4l2_compat_test.py b/test/v4l2_compat/v4l2_compat_test.py
> new file mode 100755
> index 0000000..6c028e7
> --- /dev/null
> +++ b/test/v4l2_compat/v4l2_compat_test.py
> @@ -0,0 +1,127 @@
> +#!/usr/bin/env python3
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +# Copyright (C) 2020, Google Inc.
> +#
> +# Author: Paul Elder <paul.elder at ideasonboard.com>
> +#
> +# v4l2_compat_test.py - Test the V4L2 compatibility layer
> +
> +import glob
> +import os
> +import re
> +import shutil
> +import signal
> +import subprocess
> +import sys
> +
> +TestPass = 0
> +TestFail = -1
> +TestSkip = 77
> +
> +
> +supported_pipelines = [
> +    'uvcvideo',
> +    'vimc',
> +]
> +
> +
> +def grep(exp, arr):
> +    return [s for s in arr if re.search(exp, s)]
> +
> +
> +def run_with_stdout(*args, env={}):
> +    try:
> +        with open(os.devnull, 'w') as devnull:
> +            output = subprocess.check_output(args, env=env, stderr=devnull)
> +        ret = 0
> +    except subprocess.CalledProcessError as err:
> +        output = err.output
> +        ret = err.returncode
> +    return ret, output.decode('utf-8').split('\n')
> +
> +
> +def extract_result(result):
> +    res = result.split(', ')
> +    ret = {}
> +    ret['total']     = int(res[0].split(': ')[-1])
> +    ret['succeeded'] = int(res[1].split(': ')[-1])
> +    ret['failed']    = int(res[2].split(': ')[-1])
> +    ret['warnings']  = int(res[3].split(': ')[-1])
> +    ret['device']    = res[0].split()[4].strip(':')
> +    ret['driver']    = res[0].split()[2]
> +    return ret
> +
> +
> +def print_output_arr(output_arr):
> +    print('\n'.join(output_arr))
> +
> +
> +def test_v4l2_compliance(v4l2_compliance, v4l2_compat, device, base_driver):
> +    ret, output = run_with_stdout(v4l2_compliance, '-s', '-d', device, env={'LD_PRELOAD': v4l2_compat})
> +    # TODO fix format so that we don't die from floating point exceptions
> +    if ret < 0:
> +        print_output_arr(output)
> +        print(f'Test for {device} terminated due to signal {signal.Signals(-ret).name}')
> +        return TestFail
> +
> +    result = extract_result(output[-2])
> +    if result['failed'] == 0:
> +        return TestPass
> +
> +    # vimc will fail s_fmt because it only supports framesizes that are
> +    # multiples of 3
> +    if base_driver == 'vimc' and result['failed'] <= 1:

I was trying to figure out if this code would work correctly when
v4l2-compliance doesn't report any error, and didn't notice the == 0
early return initially. Maybe == 1 here to make it clearer that you
don't handle the == 0 case ?

> +        failures = grep('fail', output)
> +        if re.search('S_FMT cannot handle an invalid format', failures[0]) is None:
> +            print_output_arr(output)
> +            return TestFail
> +        return TestPass
> +
> +    print_output_arr(output)
> +    return TestFail
> +
> +
> +def main(v4l2_compat):
> +    v4l2_compliance = shutil.which('v4l2-compliance')
> +    if v4l2_compliance is None:
> +        print('v4l2-compliance is not available')
> +        return TestSkip
> +
> +    v4l2_ctl = shutil.which('v4l2-ctl')
> +    if v4l2_ctl is None:
> +        print('v4l2-ctl is not available')
> +        return TestSkip
> +
> +    dev_nodes = glob.glob('/dev/video*')
> +    if len(dev_nodes) == 0:
> +        print('no video nodes available to test with')
> +        return TestSkip
> +
> +    failed = []
> +    for device in dev_nodes:
> +        _, out = run_with_stdout(v4l2_ctl, '-D', '-d', device, env={'LD_PRELOAD': v4l2_compat})
> +        driver = grep('Driver name', out)[0].split(':')[-1].strip()
> +        if driver != "libcamera":
> +            continue
> +
> +        _, out = run_with_stdout(v4l2_ctl, '-D', '-d', device)
> +        driver = grep('Driver name', out)[0].split(':')[-1].strip()
> +        if driver not in supported_pipelines:
> +            continue
> +
> +        ret = test_v4l2_compliance(v4l2_compliance, v4l2_compat, device, driver)
> +        if ret == TestFail:
> +            failed.append(device)
> +
> +    if len(failed) > 0:
> +        print(f'Failed {len(failed)} tests:')
> +        for device in failed:
> +            print(f'- {device}')

I didn't know about format strings in Python before reading v1 of this
patch. It's both scary and exciting.

> +
> +    return TestPass if not failed else TestFail
> +
> +
> +if __name__ == '__main__':
> +    if len(sys.argv) < 2:
> +        sys.exit(TestSkip)
> +    sys.exit(main(sys.argv[1]))

I tend to write this

if __name__ == '__main__':
    sys.exit(main(sys.argv))

to mimick C and have all arguments passed to the main function.

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

-- 
Regards,

Laurent Pinchart