Re: [PATCH v2 00/11] Venus stateful Codec API

[Hans Verkuil <hverkuil@xs4all.nl>]

On 6/28/19 4:23 PM, Stanimir Varbanov wrote:
> Hi Hans,
> 
> On 6/28/19 4:37 PM, Hans Verkuil wrote:
>> On 6/28/19 2:59 PM, Stanimir Varbanov wrote:
>>> Hello,
>>>
>>> Here is v2 of the Venus transition to stateful codec API
>>> compliance. The v2 can be found at [1].
>>>
>>> Changes since v1:
>>>  * codec_state is now enum
>>>  * dropped IS_OUT and IS_CAP macros and use vb2_start_streaming_called()
>>>  * corrected g_fmt and reconfig logic
>>>  * s/vdec_dst_buffers_done/vdec_cancel_dst_buffers
>>>  * use v4l2_m2m_ioctl_try_decoder_cmd M2M helper
>>>  * various fixes to make v4l2-compliance pass the streaming test
>>>
>>> To test the streaming with --stream-from-hdr v4l2-compliance option I have
>>> to make the following hack (it is needed because the size of decoder input
>>> buffers (OUTPUT queue) is not enough for the h264 bitstream, i.e the driver
>>> default resolution is 64x64 but the h264 stream is 320x240):
>>>
>>> diff --git a/utils/v4l2-compliance/v4l2-test-buffers.cpp b/utils/v4l2-compliance/v4l2-test-buffers.cpp
>>> index c71dcf65b721..dc0fcf20d3e4 100644
>>> --- a/utils/v4l2-compliance/v4l2-test-buffers.cpp
>>> +++ b/utils/v4l2-compliance/v4l2-test-buffers.cpp
>>> @@ -1294,6 +1294,11 @@ int testMmap(struct node *node, unsigned frame_count, enum poll_mode pollmode)
>>>                                         fmt.s_sizeimage(fmt.g_sizeimage(p) * 2, p);
>>>                         }
>>>                         fail_on_test(q.create_bufs(node, 1, &fmt));
>>> +
>>> +                       for (unsigned p = 0; p < fmt.g_num_planes(); p++)
>>> +                               fmt.s_sizeimage(fmt.g_sizeimage(p) * 2, p);
>>> +                       node->s_fmt(fmt);
>>> +
>>>                         fail_on_test(q.reqbufs(node, 2));
>>>                 }
>>>                 if (v4l_type_is_output(type))
>>
>> Does the venus driver set sizeimage based on the given output resolution?
> 
> Yes.
> 
>>
>> E.g. if v4l2-compliance would first set the output resolution to 320x240,
>> is the returned sizeimage value OK in that case?
> 
> Yes.
> 
> Here are few options to me:
>  - set the correct resolution
>  - set 0x0 and sizeimage at some arbitrary value (1 or 2MB). Despite if
> the bitstream is 4K it will not be enough if the bitrate is huge.
>  - invent some mechanism to trigger reconfiguration on the OUTPUT queue
> as well (similar to the CAPTURE queue)
> 
>>
>> And this also means that the venus driver requires each buffer to have
>> a single compressed frame, right? I.e. it can't be spread over multiple
>> OUTPUT buffers.
> 
> I cannot say for sure but that is how all downstream cases uses it i.e.
> one compressed frame per input buffer. I wonder if you fill input
> decoder buffer with many compressed frames in one input decoder buffer
> how you pass the timestamp for every packet?
> 
>>
>> We really need to let userspace know about such restrictions.
>>
>> Stanimir, can you list the restrictions of the decoder for the various
>> codecs?
> 
> What you mean? Restrictions like "one compressed frame per input buffer"?
> 

Yes :-)

Regards,

	Hans