[libcamera-devel,2/2] test: v4l2_compat_test: Fix v4l2-compliance version parsing
diff mbox series

Message ID 20210827062521.2541170-2-paul.elder@ideasonboard.com
State Superseded
Headers show
Series
  • [libcamera-devel,1/2] v4l2: v4l2_compat_manager: Don't print "camera not found" on openat()
Related show

Commit Message

Paul Elder Aug. 27, 2021, 6:25 a.m. UTC 
v4l2-compliance changed their version string:

v4l2-compliance 1.21.0-4618
v4l2-compliance SHA: cc211b76476aca2c072ffa83a9b003957d5f3909, 64 bits, 64-bit time_t

v4l2-compliance 1.21.0-4838, 64 bits, 64-bit time_t

The current parsing takes the last result of split, which works for the
former, but not the latter. Take the second result of split instead, and
strip away any commas.

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>

---
Kieran, can you please check if this works?
---
 test/v4l2_compat/v4l2_compat_test.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Kieran Bingham Aug. 27, 2021, 8:46 a.m. UTC | #1 
Hi Paul,

On 27/08/2021 07:25, Paul Elder wrote:
> v4l2-compliance changed their version string:
> 
> v4l2-compliance 1.21.0-4618
> v4l2-compliance SHA: cc211b76476aca2c072ffa83a9b003957d5f3909, 64 bits, 64-bit time_t
> 
> v4l2-compliance 1.21.0-4838, 64 bits, 64-bit time_t
> 
> The current parsing takes the last result of split, which works for the
> former, but not the latter. Take the second result of split instead, and
> strip away any commas.
> 
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> 
> ---
> Kieran, can you please check if this works?


Great, this patch works and does what it needs.

But now I have a failing test :-(

Testing /dev/video2 with vimc driver... failed

Format ioctls (Input 0):
                fail:
../../../utils/v4l2-compliance/v4l2-test-formats.cpp(263):
fmtdesc.description mismatch: was 'Video Format Description', expected
'24-bit RGB 8-8-8'
        test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: FAIL
        test VIDIOC_G/S_PARM: OK (Not Supported)
        test VIDIOC_G_FBUF: OK (Not Supported)
                fail:
../../../utils/v4l2-compliance/v4l2-test-formats.cpp(460): pixelformat
33424752 (RGB3) for buftype 1 not reported by ENUM_FMT
        test VIDIOC_G_FMT: FAIL
                fail:
../../../utils/v4l2-compliance/v4l2-test-formats.cpp(460): pixelformat
33424752 (RGB3) for buftype 1 not reported by ENUM_FMT
        test VIDIOC_TRY_FMT: FAIL
                fail:
../../../utils/v4l2-compliance/v4l2-test-formats.cpp(460): pixelformat
33424752 (RGB3) for buftype 1 not reported by ENUM_FMT
        test VIDIOC_S_FMT: FAIL
        test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
        test Cropping: OK (Not Supported)
        test Composing: OK (Not Supported)
        test Scaling: OK

Total for libcamera device /dev/video2: 54, Succeeded: 50, Failed: 4,
Warnings: 1

Failed 1 tests:
- /dev/video2


I wonder if that's a kernel bug more than a bug from us though ?



> ---
>  test/v4l2_compat/v4l2_compat_test.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/test/v4l2_compat/v4l2_compat_test.py b/test/v4l2_compat/v4l2_compat_test.py
> index 51bfa952..ae892ced 100755
> --- a/test/v4l2_compat/v4l2_compat_test.py
> +++ b/test/v4l2_compat/v4l2_compat_test.py
> @@ -94,7 +94,7 @@ def main(argv):
>          return TestSkip
>  
>      ret, out = run_with_stdout(v4l2_compliance, '--version')
> -    if ret != 0 or version.parse(out[0].split()[-1]) < MIN_V4L_UTILS_VERSION:
> +    if ret != 0 or version.parse(out[0].split()[1].replace(',', '')) < MIN_V4L_UTILS_VERSION:
>          print('v4l2-compliance version >= 1.21.0 required')
>          return TestSkip
>  
>
Kieran Bingham Aug. 27, 2021, 8:47 a.m. UTC | #2 
On 27/08/2021 07:25, Paul Elder wrote:
> v4l2-compliance changed their version string:
> 
> v4l2-compliance 1.21.0-4618
> v4l2-compliance SHA: cc211b76476aca2c072ffa83a9b003957d5f3909, 64 bits, 64-bit time_t
> 
> v4l2-compliance 1.21.0-4838, 64 bits, 64-bit time_t
> 
> The current parsing takes the last result of split, which works for the
> former, but not the latter. Take the second result of split instead, and
> strip away any commas.
> 
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> 
> ---
> Kieran, can you please check if this works?
> ---
>  test/v4l2_compat/v4l2_compat_test.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/test/v4l2_compat/v4l2_compat_test.py b/test/v4l2_compat/v4l2_compat_test.py
> index 51bfa952..ae892ced 100755
> --- a/test/v4l2_compat/v4l2_compat_test.py
> +++ b/test/v4l2_compat/v4l2_compat_test.py
> @@ -94,7 +94,7 @@ def main(argv):
>          return TestSkip
>  
>      ret, out = run_with_stdout(v4l2_compliance, '--version')
> -    if ret != 0 or version.parse(out[0].split()[-1]) < MIN_V4L_UTILS_VERSION:
> +    if ret != 0 or version.parse(out[0].split()[1].replace(',', '')) < MIN_V4L_UTILS_VERSION:

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>


>          print('v4l2-compliance version >= 1.21.0 required')
>          return TestSkip
>  
>
Laurent Pinchart Aug. 27, 2021, 11:58 a.m. UTC | #3 
Hi Paul,

Thank you for the patch.

On Fri, Aug 27, 2021 at 03:25:21PM +0900, Paul Elder wrote:
> v4l2-compliance changed their version string:
> 
> v4l2-compliance 1.21.0-4618
> v4l2-compliance SHA: cc211b76476aca2c072ffa83a9b003957d5f3909, 64 bits, 64-bit time_t
> 
> v4l2-compliance 1.21.0-4838, 64 bits, 64-bit time_t
> 
> The current parsing takes the last result of split, which works for the
> former, but not the latter. Take the second result of split instead, and
> strip away any commas.
> 
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>

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

However, the unit test now fails for me:

stderr:
Traceback (most recent call last):
  File "/home/laurent/src/iob/oss/libcamera/libcamera/test/v4l2_compat/v4l2_compat_test.py", line 159, in <module>
    sys.exit(main(sys.argv))
  File "/home/laurent/src/iob/oss/libcamera/libcamera/test/v4l2_compat/v4l2_compat_test.py", line 124, in main
    driver = grep('Driver name', out)[0].split(':')[-1].strip()
IndexError: list index out of range

This should be fixed before merging this patch. I'll have a quick look.

> ---
> Kieran, can you please check if this works?
> ---
>  test/v4l2_compat/v4l2_compat_test.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/test/v4l2_compat/v4l2_compat_test.py b/test/v4l2_compat/v4l2_compat_test.py
> index 51bfa952..ae892ced 100755
> --- a/test/v4l2_compat/v4l2_compat_test.py
> +++ b/test/v4l2_compat/v4l2_compat_test.py
> @@ -94,7 +94,7 @@ def main(argv):
>          return TestSkip
>  
>      ret, out = run_with_stdout(v4l2_compliance, '--version')
> -    if ret != 0 or version.parse(out[0].split()[-1]) < MIN_V4L_UTILS_VERSION:
> +    if ret != 0 or version.parse(out[0].split()[1].replace(',', '')) < MIN_V4L_UTILS_VERSION:
>          print('v4l2-compliance version >= 1.21.0 required')
>          return TestSkip
>
Laurent Pinchart Aug. 27, 2021, 8:58 p.m. UTC | #4 
On Fri, Aug 27, 2021 at 02:58:15PM +0300, Laurent Pinchart wrote:
> On Fri, Aug 27, 2021 at 03:25:21PM +0900, Paul Elder wrote:
> > v4l2-compliance changed their version string:
> > 
> > v4l2-compliance 1.21.0-4618
> > v4l2-compliance SHA: cc211b76476aca2c072ffa83a9b003957d5f3909, 64 bits, 64-bit time_t
> > 
> > v4l2-compliance 1.21.0-4838, 64 bits, 64-bit time_t
> > 
> > The current parsing takes the last result of split, which works for the
> > former, but not the latter. Take the second result of split instead, and
> > strip away any commas.
> > 
> > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Tested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> However, the unit test now fails for me:
> 
> stderr:
> Traceback (most recent call last):
>   File "/home/laurent/src/iob/oss/libcamera/libcamera/test/v4l2_compat/v4l2_compat_test.py", line 159, in <module>
>     sys.exit(main(sys.argv))
>   File "/home/laurent/src/iob/oss/libcamera/libcamera/test/v4l2_compat/v4l2_compat_test.py", line 124, in main
>     driver = grep('Driver name', out)[0].split(':')[-1].strip()
> IndexError: list index out of range
> 
> This should be fixed before merging this patch. I'll have a quick look.

This was caused by ASan. "[PATCH] test: v4l2_compat: Disable test when
ASan is enabled" works around it. If anyone can find a way to LD_PRELOAD
libasan instead, that would be nice.

Now I can reproduce the v4l2-compliance failure itself :-) Here are the
failures for my UVC camera:

Format ioctls (Input 0):
                fail: v4l2-test-formats.cpp(263): fmtdesc.description mismatch: was 'Video Format Description', expected 'Motion-JPEG'
        test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: FAIL
        test VIDIOC_G/S_PARM: OK (Not Supported)
        test VIDIOC_G_FBUF: OK (Not Supported)
                fail: v4l2-test-formats.cpp(460): pixelformat 47504a4d (MJPG) for buftype 1 not reported by ENUM_FMT
        test VIDIOC_G_FMT: FAIL
                fail: v4l2-test-formats.cpp(460): pixelformat 47504a4d (MJPG) for buftype 1 not reported by ENUM_FMT
        test VIDIOC_TRY_FMT: FAIL
                fail: v4l2-test-formats.cpp(460): pixelformat 47504a4d (MJPG) for buftype 1 not reported by ENUM_FMT
        test VIDIOC_S_FMT: FAIL

And for vimc:

Format ioctls (Input 0):
                fail: v4l2-test-formats.cpp(263): fmtdesc.description mismatch: was 'Video Format Description', expected '24-bit RGB 8-8-8'
        test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: FAIL
        test VIDIOC_G/S_PARM: OK (Not Supported)
        test VIDIOC_G_FBUF: OK (Not Supported)
                fail: v4l2-test-formats.cpp(460): pixelformat 33424752 (RGB3) for buftype 1 not reported by ENUM_FMT
        test VIDIOC_G_FMT: FAIL
                fail: v4l2-test-formats.cpp(460): pixelformat 33424752 (RGB3) for buftype 1 not reported by ENUM_FMT
        test VIDIOC_TRY_FMT: FAIL
                fail: v4l2-test-formats.cpp(460): pixelformat 33424752 (RGB3) for buftype 1 not reported by ENUM_FMT
        test VIDIOC_S_FMT: FAIL

> > ---
> > Kieran, can you please check if this works?
> > ---
> >  test/v4l2_compat/v4l2_compat_test.py | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/test/v4l2_compat/v4l2_compat_test.py b/test/v4l2_compat/v4l2_compat_test.py
> > index 51bfa952..ae892ced 100755
> > --- a/test/v4l2_compat/v4l2_compat_test.py
> > +++ b/test/v4l2_compat/v4l2_compat_test.py
> > @@ -94,7 +94,7 @@ def main(argv):
> >          return TestSkip
> >  
> >      ret, out = run_with_stdout(v4l2_compliance, '--version')
> > -    if ret != 0 or version.parse(out[0].split()[-1]) < MIN_V4L_UTILS_VERSION:
> > +    if ret != 0 or version.parse(out[0].split()[1].replace(',', '')) < MIN_V4L_UTILS_VERSION:
> >          print('v4l2-compliance version >= 1.21.0 required')
> >          return TestSkip
> >

Patch
diff mbox series 

diff --git a/test/v4l2_compat/v4l2_compat_test.py b/test/v4l2_compat/v4l2_compat_test.py
index 51bfa952..ae892ced 100755
--- a/test/v4l2_compat/v4l2_compat_test.py
+++ b/test/v4l2_compat/v4l2_compat_test.py
@@ -94,7 +94,7 @@  def main(argv):
         return TestSkip
 
     ret, out = run_with_stdout(v4l2_compliance, '--version')
-    if ret != 0 or version.parse(out[0].split()[-1]) < MIN_V4L_UTILS_VERSION:
+    if ret != 0 or version.parse(out[0].split()[1].replace(',', '')) < MIN_V4L_UTILS_VERSION:
         print('v4l2-compliance version >= 1.21.0 required')
         return TestSkip