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

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]

Message-ID: <7ae41d9d-d5fb-367c-ea08-c9a1bc818ffe@linaro.org>
Date:   Mon, 27 Mar 2017 14:49:21 +0300
From:   Stanimir Varbanov <stanimir.varbanov@...aro.org>
To:     Hans Verkuil <hverkuil@...all.nl>,
        Stanimir Varbanov <stanimir.varbanov@...aro.org>,
        Mauro Carvalho Chehab <mchehab@...nel.org>
Cc:     Andy Gross <andy.gross@...aro.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Stephen Boyd <sboyd@...eaurora.org>,
        Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
        linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-arm-msm@...r.kernel.org
Subject: Re: [PATCH v7 5/9] media: venus: vdec: add video decoder files

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