On 10/01/2018 01:57 PM, Maxime Jourdan wrote:
> Le lun. 1 oct. 2018 à 12:26, Hans Verkuil <hverkuil@xxxxxxxxx> a écrit :
>>
>> On 09/28/2018 04:28 PM, Maxime Jourdan wrote:
>>> + }
>>> +
>>> + return 0;
>>> + }
>>> +
>>> + switch (q->type) {
>>> + case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
>>> + sizes[0] = amvdec_get_output_size(sess);
>>> + *num_planes = 1;
>>> + break;
>>> + case V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE:
>>> + switch (sess->pixfmt_cap) {
>>> + case V4L2_PIX_FMT_NV12M:
>>> + sizes[0] = output_size;
>>> + sizes[1] = output_size / 2;
>>> + *num_planes = 2;
>>> + break;
>>> + case V4L2_PIX_FMT_YUV420M:
>>> + sizes[0] = output_size;
>>> + sizes[1] = output_size / 4;
>>> + sizes[2] = output_size / 4;
>>> + *num_planes = 3;
>>> + break;
>>> + default:
>>> + return -EINVAL;
>>> + }
>>> + *num_buffers = min(max(*num_buffers, fmt_out->min_buffers),
>>> + fmt_out->max_buffers);
>>
>> You can use clamp here. That's easier to read.
>>
>
> Ack
>
>>> + /* The HW needs all buffers to be configured during startup */
>>
>> Why? I kind of expected to see 'q->min_buffers_needed = fmt_out->min_buffers'
>> here. I think some more information is needed here in the comment.
>>
>
> I'll extend the comments to reflect the following:
>
> All codecs in the Amlogic vdec need the full available buffer list to
> be configured at startup, i.e all buffer phy addrs must be written to
> registers prior to decoding.
> The firmwares then decide how they use those buffers and the
> interrupts only tell me "the decoder has written a frame to buffer N°
> X".
>
> fmt_out->min_buffers and fmt_out->max_buffers refer to the min/max
> amount of buffers that can be setup during initialization. In the case
> of MPEG2, the firmware expects 8 buffers, no more no less, so both
> min_buffers and max_buffers have the value "8".
>
> But even if those values differ (as for H.264 that will come later),
> the firmware still expects all allocated buffers to be setup in
> registers. As such, q->min_buffers_needed must reflect the total
> amount of CAPTURE buffers.
That's much better, thank you! Now I understand why you do this :-)
Regards,
Hans
