Re: [PATCH v7 5/9] media: venus: vdec: add video decoder files

[Stanimir Varbanov]

Hi Hans,

On 03/27/2017 11:45 AM, Hans Verkuil wrote:
> On 25/03/17 23:30, Stanimir Varbanov wrote:
>> Thanks for the comments!

<snip>

>>>> +static void vdec_buf_done(struct venus_inst *inst, unsigned int buf_type,
>>>> +              u32 tag, u32 bytesused, u32 data_offset, u32 flags,
>>>> +              u64 timestamp_us)
>>>> +{
>>>> +    struct vb2_v4l2_buffer *vbuf;
>>>> +    struct vb2_buffer *vb;
>>>> +    unsigned int type;
>>>> +
>>>> +    if (buf_type == HFI_BUFFER_INPUT)
>>>> +        type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
>>>> +    else
>>>> +        type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
>>>> +
>>>> +    vbuf = helper_find_buf(inst, type, tag);
>>>> +    if (!vbuf)
>>>> +        return;
>>>> +
>>>> +    vbuf->flags = flags;
>>>> +
>>>> +    if (type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) {
>>>> +        vb = &vbuf->vb2_buf;
>>>> +        vb->planes[0].bytesused =
>>>> +            max_t(unsigned int, inst->output_buf_size, bytesused);
>>>> +        vb->planes[0].data_offset = data_offset;
>>>> +        vb->timestamp = timestamp_us * NSEC_PER_USEC;
>>>> +        vbuf->sequence = inst->sequence++;
>>>
>>> timestamp and sequence are only set for CAPTURE, not OUTPUT. Is that correct?
>>
>> Correct. I can add sequence for the OUTPUT queue too, but I have no idea how that sequence is used by userspace.
> 
> You set V4L2_BUF_FLAG_TIMESTAMP_COPY, so you have to copy the timestamp from the output buffer
> to the capture buffer, if that makes sense for this codec. If not, then you shouldn't use that

The timestamp_us is filled by firmware and it is the timestamp of the
output buffer which is used to produce the uncompressed capture buffer.
So I think V4L2_BUF_FLAG_TIMESTAMP_COPY is correctly used here.

> V4L2_BUF_FLAG and just generate new timestamps whenever a capture buffer is ready.
> 
> For sequence numbering just give the output queue its own sequence counter.

OK will do.

-- 
regards,
Stan