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

Laurent Pinchart laurent.pinchart at ideasonboard.com
Thu Jun 25 00:26:41 CEST 2020


Hi Niklas and Paul,

On Wed, Jun 24, 2020 at 07:38:04PM +0200, Niklas Söderlund wrote:
> Hi Paul,
> 
> Thanks for your work.
> 
> On 2020-06-24 23:07:52 +0900, Paul Elder wrote:
> > Test the V4L2 compatibility layer by running v4l2-compliance -s on every
> > /dev/video* device.
> 
> I wonder if this is not a tad to aggressive. Imagine the test being run 
> on a device where one video node is not covered by a libcamera pipeline 
> and also does not pass v4l2-compliance. Would this not lead to the 
> libcamera test suite to fail?
> 
> Would it be possible you think to check the driver name of the video 
> device and check it against an whitelist (vimc, vivid, ?) before running 
> the v4l2-compliance suite?
> 
> You can get the driver name from
> 
>     v4l2-ctl -D -d /dev/videoX

Isn't this exactly what the code below is doing ? :-)

> > Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> > ---
> >  test/meson.build                     |   1 +
> >  test/v4l2_compat/meson.build         |  10 +++
> >  test/v4l2_compat/v4l2_compat_test.py | 125 +++++++++++++++++++++++++++
> >  3 files changed, 136 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 a868813..591920f 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..a67aac4
> > --- /dev/null
> > +++ b/test/v4l2_compat/meson.build
> > @@ -0,0 +1,10 @@
> > +# SPDX-License-Identifier: CC0-1.0
> > +
> > +pymod = import('python')
> > +py3 = pymod.find_installation('python3')
> > +
> > +v4l2_compat_test = files('v4l2_compat_test.py')
> > +
> > +test('v4l2_compat_test', py3,
> > +     args : v4l2_compat_test,
> > +     suite : 'v4l2_compat')

Would this be simpler ?

test('v4l2_compat_test', v4l2_compat_test,
     suite : 'v4l2_compat')

> > diff --git a/test/v4l2_compat/v4l2_compat_test.py b/test/v4l2_compat/v4l2_compat_test.py
> > new file mode 100755
> > index 0000000..f56db4e
> > --- /dev/null
> > +++ b/test/v4l2_compat/v4l2_compat_test.py
> > @@ -0,0 +1,125 @@
> > +#!/usr/bin/env python3
> > +# SPDX-License-Identifier: GPL-2.0-or-later
> > +# Copyright (C) 2019, Google Inc.

2020 ?

> > +#
> > +# Author: Paul Elder <paul.elder at ideasonboard.com>
> > +#
> > +# v4l2_compat_test.py - Test the V4L2 compatibility layer
> > +
> > +import os
> > +import re
> > +from shutil import which

I'd just import shutil and use shutil.which, up to you.

> > +import subprocess
> > +import sys
> > +
> > +TestPass = 0
> > +TestFail = -1
> > +TestSkip = 77
> > +
> > +
> > +supported_pipelines = [
> > +    "uvcvideo",

Our python scripts usually use single quotes for strings.

> > +    "vimc",
> > +]
> > +
> > +
> > +def find_file(name, path):
> > +    for root, dirs, files in os.walk(path):
> > +        if name in files:
> > +            return os.path.join(root, name)

That's a bit of heavy artillery. How about passing the path to
v4l2-compat.so to the test ? Something along the lines of

diff --git a/test/v4l2_compat/meson.build b/test/v4l2_compat/meson.build
index a67aac41b273..e24d10bd327c 100644
--- a/test/v4l2_compat/meson.build
+++ b/test/v4l2_compat/meson.build
@@ -1,10 +1,9 @@
 # SPDX-License-Identifier: CC0-1.0

-pymod = import('python')
-py3 = pymod.find_installation('python3')
+if is_variable('v4l2_compat')
+    v4l2_compat_test = files('v4l2_compat_test.py')

-v4l2_compat_test = files('v4l2_compat_test.py')
-
-test('v4l2_compat_test', py3,
-     args : v4l2_compat_test,
-     suite : 'v4l2_compat')
+    test('v4l2_compat_test', v4l2_compat_test,
+         args : v4l2_compat,
+         suite : 'v4l2_compat')
+endif

> > +
> > +
> > +def grep(exp, arr):
> > +    return [s for s in arr if re.search(exp, s)]
> > +
> > +
> > +def run_with_stdout(cmd, args="", env={}):
> > +    try:
> > +        with open(os.devnull, 'w') as devnull:
> > +            output = subprocess.check_output(f"{cmd} {args}", shell=True, env=env, stderr=devnull)

Do you need to run this through a shell ? On my system, one of the video
nodes related to my integrated UVC webcam isn't handled by libcamera.
When v4l2-compliance is run on it with LD_PRELOAD=v4l2-compat.so, it
hangs if run through a shell, and doesn't otherwise. I suspect it's the
shell hanging, but if we don't need shell=True, then we may skip the
investigation :-)

You will need to turn the first argument into an array. I've tried the
following, it seems to work.

diff --git a/test/v4l2_compat/v4l2_compat_test.py b/test/v4l2_compat/v4l2_compat_test.py
index f56db4e91890..d05b3235a78c 100755
--- a/test/v4l2_compat/v4l2_compat_test.py
+++ b/test/v4l2_compat/v4l2_compat_test.py
@@ -33,10 +33,10 @@ def grep(exp, arr):
     return [s for s in arr if re.search(exp, s)]
 
 
-def run_with_stdout(cmd, args="", env={}):
+def run_with_stdout(*args, env={}):
     try:
         with open(os.devnull, 'w') as devnull:
-            output = subprocess.check_output(f"{cmd} {args}", shell=True, env=env, stderr=devnull)
+            output = subprocess.check_output(args, env=env, stderr=devnull)
     except subprocess.CalledProcessError as err:
         output = err.output
     return output.decode("utf-8").split("\n")
@@ -59,7 +59,7 @@ def print_output_arr(output_arr):
 
 
 def test_v4l2_compliance(v4l2_compliance, v4l2_compat, device, base_driver):
-    output = run_with_stdout(v4l2_compliance, f"-s -d {device}", {"LD_PRELOAD": v4l2_compat})
+    output = run_with_stdout(v4l2_compliance, "-s", "-d", device, env={"LD_PRELOAD": v4l2_compat})
     result = extract_result(output[-2])
     if result["driver"] != "libcamera":
         return TestSkip
@@ -104,7 +104,7 @@ def main():
 
     failed = []
     for device in dev_nodes:
-        out = run_with_stdout(v4l2_ctl, f"-D -d {device}")
+        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

And after further testing, it seems the test stills hangs from time to
time :-S I'll let you investigate if you can reproduce the issue.

I'm also getting a floating point exception when I run v4l2-compliance
with LD_PRELOAD on my UVC webcam. Not too surprising as v4l2-compat
reports

WARN V4L2Compat v4l2_camera_proxy.cpp:515 sizeimage of at least one format is zero. Streaming this format will cause a floating point exception.

The camera produces formats::R8, which isn't listed in pixelFormatInfo.
I think that will be fixed when moving pixelFormatInfo out of the
v4l2-compat layer, like we've discussed offline.

> > +    except subprocess.CalledProcessError as err:
> > +        output = err.output
> > +    return 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):
> > +    output = run_with_stdout(v4l2_compliance, f"-s -d {device}", {"LD_PRELOAD": v4l2_compat})
> > +    result = extract_result(output[-2])
> > +    if result["driver"] != "libcamera":
> > +        return TestSkip
> > +
> > +    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:
> > +        failures = grep("fail", output)
> > +        if re.search("S_FMT cannot handle an invalid format", failures[0]) is None:

Is there a reason not to use your grep function ?

> > +            print_output_arr(output)
> > +            return TestFail
> > +        return TestPass
> > +
> > +    print_output_arr(output)
> > +    return TestFail
> > +
> > +
> > +def main():
> > +    v4l2_compliance = which("v4l2-compliance")
> > +    if v4l2_compliance is None:
> > +        print("v4l2-compliance is not available")
> > +        return TestSkip
> > +
> > +    v4l2_ctl = which("v4l2-ctl")
> > +    if v4l2_ctl is None:
> > +        print("v4l2-ctl is not available")
> > +        return TestSkip
> > +
> > +    v4l2_compat = find_file("v4l2-compat.so", ".")
> > +    if v4l2_compat is None:
> > +        print("v4l2-compat.so is not built")
> > +        return TestSkip
> > +
> > +    dev_nodes = grep("video", os.listdir("/dev"))

import glob

    dev_nodes = glob.glob("/dev/video*")

> > +    dev_nodes = list(map(lambda s: f"/dev/{s}", dev_nodes))

Or the more Pythonic syntax:

    dev_nodes = [f"/dev/{s]" for s in dev_nodes]

> > +    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, f"-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}")
> > +
> > +    return TestPass if not failed else TestFail
> > +
> > +
> > +if __name__ == '__main__':
> > +    sys.exit(main())

-- 
Regards,

Laurent Pinchart


More information about the libcamera-devel mailing list