Re: [PATCH v3 2/4] media: vicodec: use v4l2-mem2mem draining, stopped and next-buf-is-last states handling

From: Neil Armstrong <narmstrong@baylibre.com>
To: Hans Verkuil <hverkuil@xs4all.nl>,
	mchehab@kernel.org, hans.verkuil@cisco.com
Cc: linux-media@vger.kernel.org, linux-amlogic@lists.infradead.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org,
	Maxime Jourdan <mjourdan@baylibre.com>
Subject: Re: [PATCH v3 2/4] media: vicodec: use v4l2-mem2mem draining, stopped and next-buf-is-last states handling
Date: Fri, 13 Dec 2019 14:25:58 +0100	[thread overview]
Message-ID: <f7e91a9c-266c-0282-414d-5d0333c97d91@baylibre.com> (raw)
In-Reply-To: <0eb52de8-97a9-40cf-a926-262b1ecdc3e9@xs4all.nl>

On 13/12/2019 14:21, Hans Verkuil wrote:
> On 12/9/19 1:20 PM, Neil Armstrong wrote:
>> Use the previously introduced v4l2-mem2mem core APIs to handle the drainig,
>> stopped and next-buf-is-last states.
>>
>> With these changes, the v4l2-compliance still passes with the following
>> commands :
>> # v4l2-ctl --stream-mmap --stream-out-mmap --stream-to-hdr out.comp --stream-from in.yuv
>>>>>> <><><><><><><><><><><><><><><><>< 15.53 fps
>>  15.53 fps
>>> <><><><><><><><><><><><>< 13.99 fps
>>  13.99 fps
>>> <><><><><><><><><><><>< 13.52 fps
>>  13.52 fps
>>> <><><><><><><><><><><><>< 13.41 fps
>>  13.41 fps
>>> <><><><><><><><><><><><>< 13.21 fps
>>  13.21 fps
>>> <><><><><><><><><><><>< 13.09 fps
>>  13.09 fps
>>> <><><><><><><
>> STOP ENCODER
>> <<<
>> EOS EVENT
>>
>> # v4l2-compliance --stream-from in.yuv -s
>> v4l2-compliance SHA: 7ead0e1856b89f2e19369af452bb03fd0cd16793, 64 bits
>> [...]
>> Total for vicodec device /dev/video0: 50, Succeeded: 50, Failed: 0, Warnings: 0
>>
>> The full output is available at [1]
>>
>> # v4l2-compliance -d1 --stream-from-hdr out.comp -s
>> v4l2-compliance SHA: 7ead0e1856b89f2e19369af452bb03fd0cd16793, 64 bits
>> [...]
>> Total for vicodec device /dev/video1: 50, Succeeded: 50, Failed: 0, Warnings: 0
>>
>> The full output is available at [2]
>>
>> No functional changes should be noticed.
> 
> Ah, unfortunately there *are* functional changes.
> 
> There is a (much) more extensive test that is done in the test-media script.
> 
> In v4l-utils, go to contrib/test. Now run (as root): test-media vicodec
> 
> This test now fails on some tests for the stateful decoder:
> 
> Streaming ioctls:
>         test read/write: OK (Not Supported)
>         test blocking wait: OK
>                 fail: v4l2-test-buffers.cpp(943): ret == 0
>                 fail: v4l2-test-buffers.cpp(1353): captureBufs(node, node_m2m_cap, q, m2m_q, frame_count, pollmode, capture_count)
>         test MMAP (select): FAIL
>                 fail: v4l2-test-buffers.cpp(951): ret == 0
>                 fail: v4l2-test-buffers.cpp(1353): captureBufs(node, node_m2m_cap, q, m2m_q, frame_count, pollmode, capture_count)
>         test MMAP (epoll): FAIL
>                 fail: v4l2-test-buffers.cpp(943): ret == 0
>                 fail: v4l2-test-buffers.cpp(1607): captureBufs(node, node_m2m_cap, q, m2m_q, frame_count, pollmode, capture_count)
>         test USERPTR (select): FAIL
>                 fail: v4l2-test-buffers.cpp(943): ret == 0
>                 fail: v4l2-test-buffers.cpp(1761): captureBufs(node, node_m2m_cap, q, m2m_q, frame_count, pollmode, capture_count)
>         test DMABUF (select): FAIL
> 
> I also see this:
> 
> cmp: EOF on /tmp/tmp.7KAXKAIkVZ/raw.yu12.1280.24 after byte 23500800, in line 1
> 
> which shouldn't be there either.
> 
> I can recommend the test-media script: it can test all the virtual drivers and it is
> part of the daily build to check for regressions.

Thx for the pointer, I'll analyze the failures.
> 
> Regards,
> 
> 	Hans
>