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

Message ID 20200624140752.46066-1-paul.elder@ideasonboard.com
State Superseded
Headers show
Series
  • [libcamera-devel,RFC] tests: v4l2_compat: Add test for v4l2_compat
Related show

Commit Message

Paul Elder June 24, 2020, 2:07 p.m. UTC
Test the V4L2 compatibility layer by running v4l2-compliance -s on every
/dev/video* device.

Signed-off-by: Paul Elder <paul.elder@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

Comments

Niklas Söderlund June 24, 2020, 5:38 p.m. UTC | #1
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

> 
> Signed-off-by: Paul Elder <paul.elder@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')
> 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.
> +#
> +# Author: Paul Elder <paul.elder@ideasonboard.com>
> +#
> +# v4l2_compat_test.py - Test the V4L2 compatibility layer
> +
> +import os
> +import re
> +from shutil import which
> +import subprocess
> +import sys
> +
> +TestPass = 0
> +TestFail = -1
> +TestSkip = 77
> +
> +
> +supported_pipelines = [
> +    "uvcvideo",
> +    "vimc",
> +]
> +
> +
> +def find_file(name, path):
> +    for root, dirs, files in os.walk(path):
> +        if name in files:
> +            return os.path.join(root, name)
> +
> +
> +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)
> +    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:
> +            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"))
> +    dev_nodes = list(map(lambda s: f"/dev/{s}", 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())
> -- 
> 2.27.0
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Laurent Pinchart June 24, 2020, 10:26 p.m. UTC | #2
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@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@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())
Niklas Söderlund June 24, 2020, 10:36 p.m. UTC | #3
On 2020-06-25 01:26:41 +0300, Laurent Pinchart wrote:
> 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 ? :-)

You are correct, sorry for the noise.

Do you expect me to read the code as well as the commit message? :-P

> 
> > > Signed-off-by: Paul Elder <paul.elder@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@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
Paul Elder June 26, 2020, 7:51 a.m. UTC | #4
Hi Niklas,

Thank you for the comments.

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

Yes, as Laurent pointed out, I do do this already :)

There was one part though, that I ran v4l2-compliance -s and then
checked if libcamera actually intercepted it... in v2 I've moved this
over to v4l2-ctl.


Thanks,

Paul

> > 
> > Signed-off-by: Paul Elder <paul.elder@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')
> > 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.
> > +#
> > +# Author: Paul Elder <paul.elder@ideasonboard.com>
> > +#
> > +# v4l2_compat_test.py - Test the V4L2 compatibility layer
> > +
> > +import os
> > +import re
> > +from shutil import which
> > +import subprocess
> > +import sys
> > +
> > +TestPass = 0
> > +TestFail = -1
> > +TestSkip = 77
> > +
> > +
> > +supported_pipelines = [
> > +    "uvcvideo",
> > +    "vimc",
> > +]
> > +
> > +
> > +def find_file(name, path):
> > +    for root, dirs, files in os.walk(path):
> > +        if name in files:
> > +            return os.path.join(root, name)
> > +
> > +
> > +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)
> > +    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:
> > +            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"))
> > +    dev_nodes = list(map(lambda s: f"/dev/{s}", 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())
> > -- 
> > 2.27.0
> > 
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel@lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel
> 
> -- 
> Regards,
> Niklas Söderlund
Paul Elder June 26, 2020, 7:57 a.m. UTC | #5
Hi Laurent,

Thank you for the review.

On Thu, Jun 25, 2020 at 01:26:41AM +0300, Laurent Pinchart wrote:
> 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@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')

Ah yes, that is much better. I wasn't sure how tests with python worked
in meson.

> > > 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 ?

Yes :)

> > > +#
> > > +# Author: Paul Elder <paul.elder@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

Ah yes, much better.

> 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 :-)

It looks like we don't need to run it through as shell, so I've dropped
it.

> 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.

Yeah, this has to be fixed.

> > > +    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 ?

My grep function greps for a substring within a list of strings.

re.search("S_FMT cannot handle an invalid format", failures[0])
vs
grep("S_FMT cannot handle an invalid format", [failures[0]])

I think re.search is good enough for this.

> > > +            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]

With glob, this is no longer necessary :)

> > > +    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())

I've also figured out how to increase the test timeout in meson.
v2 coming up!


Thanks,

Paul

Patch

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')
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.
+#
+# Author: Paul Elder <paul.elder@ideasonboard.com>
+#
+# v4l2_compat_test.py - Test the V4L2 compatibility layer
+
+import os
+import re
+from shutil import which
+import subprocess
+import sys
+
+TestPass = 0
+TestFail = -1
+TestSkip = 77
+
+
+supported_pipelines = [
+    "uvcvideo",
+    "vimc",
+]
+
+
+def find_file(name, path):
+    for root, dirs, files in os.walk(path):
+        if name in files:
+            return os.path.join(root, name)
+
+
+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)
+    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:
+            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"))
+    dev_nodes = list(map(lambda s: f"/dev/{s}", 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())