linux-kernel - Re: [PATCH v5 00/39] i.MX Media Driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Sun, 19 Mar 2017 10:54:22 -0700
From:   Steve Longerbeam <slongerbeam@...il.com>
To:     Russell King - ARM Linux <linux@...linux.org.uk>,
        Steve Longerbeam <steve_longerbeam@...tor.com>
Cc:     robh+dt@...nel.org, mark.rutland@....com, shawnguo@...nel.org,
        kernel@...gutronix.de, fabio.estevam@....com, mchehab@...nel.org,
        hverkuil@...all.nl, nick@...anahar.org, markus.heiser@...marIT.de,
        p.zabel@...gutronix.de, laurent.pinchart+renesas@...asonboard.com,
        bparrot@...com, geert@...ux-m68k.org, arnd@...db.de,
        sudipm.mukherjee@...il.com, minghsiu.tsai@...iatek.com,
        tiffany.lin@...iatek.com, jean-christophe.trotin@...com,
        horms+renesas@...ge.net.au, niklas.soderlund+renesas@...natech.se,
        robert.jarzmik@...e.fr, songjun.wu@...rochip.com,
        andrew-ct.chen@...iatek.com, gregkh@...uxfoundation.org,
        shuah@...nel.org, sakari.ailus@...ux.intel.com, pavel@....cz,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, linux-media@...r.kernel.org,
        devel@...verdev.osuosl.org
Subject: Re: [PATCH v5 00/39] i.MX Media Driver



On 03/19/2017 03:38 AM, Russell King - ARM Linux wrote:
> On Sat, Mar 18, 2017 at 12:58:27PM -0700, Steve Longerbeam wrote:
>> Right, imx-media-capture.c (the "standard" v4l2 user interface module)
>> is not implementing VIDIOC_ENUM_FRAMESIZES. It should, but it can only
>> return the single frame size that the pipeline has configured (the mbus
>> format of the attached source pad).
> I now have a set of patches that enumerate the frame sizes and intervals
> from the source pad of the first subdev (since you're setting the formats
> etc there from the capture device, it seems sensible to return what it
> can support.)  This means my patch set doesn't add to non-CSI subdevs.
>
>> Can you share your gstreamer pipeline? For now, until
>> VIDIOC_ENUM_FRAMESIZES is implemented, try a pipeline that
>> does not attempt to specify a frame rate. I use the attached
>> script for testing, which works for me.
> Note that I'm not specifying a frame rate on gstreamer - I'm setting
> the pipeline up for 60fps, but gstreamer in its wisdom is unable to
> enumerate the frame sizes, and therefore is unable to enumerate the
> frame intervals (frame intervals depend on frame sizes), so it
> falls back to the "tvnorms" which are basically 25/1 and 30000/1001.
>
> It sees 60fps via G_PARM, and then decides to set 30000/1001 via S_PARM.
> So, we end up with most of the pipeline operating at 60fps, with CSI
> doing frame skipping to reduce the frame rate to 30fps.
>
> gstreamer doesn't complain, doesn't issue any warnings, the only way
> you can spot this is to enable debugging and look through the copious
> debug log, or use -v and check the pad capabilities.
>
> Testing using gstreamer, and only using "does it produce video" is a
> good simple test, but it's just that - it's a simple test.  It doesn't
> tell you that what you're seeing is what you intended to see (such as
> video at the frame rate you expected) without more work.
>
>> Thanks, I've fixed most of v4l2-compliance issues, but this is not
>> done yet. Is that something you can help with?
> What did you do with:
>
> ioctl(3, VIDIOC_REQBUFS, {count=0, type=0 /* V4L2_BUF_TYPE_??? */, memory=0 /* V4L2_MEMORY_??? */}) = -1 EINVAL (Invalid argument)
>                  test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
> ioctl(3, VIDIOC_EXPBUF, 0xbef405bc)     = -1 EINVAL (Invalid argument)
>                  fail: v4l2-test-buffers.cpp(571): q.has_expbuf(node)
>                  test VIDIOC_EXPBUF: FAIL
>
> To me, this looks like a bug in v4l2-compliance (I'm using 1.10.0).
> I'm not sure what buffer VIDIOC_EXPBUF is expected to export, since
> afaics no buffers have been allocated, so of course it's going to fail.
> Either that, or the v4l2 core vb2 code is non-compliant with v4l2's
> interface requirements.
>
> In any case, it doesn't look like the buffer management is being
> tested at all by v4l2-compliance - we know that gstreamer works, so
> buffers _can_ be allocated, and I've also used dmabufs with gstreamer,
> so I also know that VIDIOC_EXPBUF works there.
>

I wouldn't be surprised if you hit on a bug in v4l2-compliance. I 
stopped with v4l2-compliance
at a different test failure that also didn't make sense to me:

Streaming ioctls:
     test read/write: OK (Not Supported)
         Video Capture:
         Buffer: 0 Sequence: 0 Field: Any Timestamp: 41.664259s
         fail: 
.../v4l-utils-1.6.2/utils/v4l2-compliance/v4l2-test-buffers.cpp(281): 
!(g_flags() & (V4L2_BUF_FLAG_DONE | V4L2_BUF_FLAG_ERROR))
         fail: 
.../v4l-utils-1.6.2/utils/v4l2-compliance/v4l2-test-buffers.cpp(610): 
buf.check(q, last_seq)
         fail: 
.../v4l-utils-1.6.2/utils/v4l2-compliance/v4l2-test-buffers.cpp(883): 
captureBufs(node, q, m2m_q, frame_count, false)
     test MMAP: FAIL
     test USERPTR: OK (Not Supported)
     test DMABUF: Cannot test, specify --expbuf-device

Total: 42, Succeeded: 38, Failed: 4, Warnings: 0


In this case the driver completed and returned only one buffer, and it set
VB2_BUF_STATE_DONE, so these test failures didn't make sense to me. I
was using version 1.6.2 at the time.

Steve