linux-kernel - Re: [PATCH] media: venus: add support for key frame

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]

Date:   Mon, 22 Oct 2018 15:14:45 +0900
From:   Alexandre Courbot <acourbot@...omium.org>
To:     Stanimir Varbanov <stanimir.varbanov@...aro.org>
Cc:     mgottam@...eaurora.org, Hans Verkuil <hverkuil@...all.nl>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Linux Media Mailing List <linux-media@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        linux-arm-msm@...r.kernel.org, vgarodia@...eaurora.org
Subject: Re: [PATCH] media: venus: add support for key frame

On Fri, Oct 12, 2018 at 5:10 PM Stanimir Varbanov
<stanimir.varbanov@...aro.org> wrote:
>
>
>
> On 10/12/2018 11:06 AM, Alexandre Courbot wrote:
> > On Fri, Oct 12, 2018 at 4:37 PM Stanimir Varbanov
> > <stanimir.varbanov@...aro.org> wrote:
> >>
> >> Hi Alex,
> >>
> >> On 10/12/2018 08:26 AM, Alexandre Courbot wrote:
> >>> On Tue, Oct 9, 2018 at 4:54 PM Malathi Gottam <mgottam@...eaurora.org> wrote:
> >>>>
> >>>> When client requests for a keyframe, set the property
> >>>> to hardware to generate the sync frame.
> >>>>
> >>>> Signed-off-by: Malathi Gottam <mgottam@...eaurora.org>
> >>>> ---
> >>>>  drivers/media/platform/qcom/venus/venc_ctrls.c | 13 +++++++++++++
> >>>>  1 file changed, 13 insertions(+)
> >>>>
> >>>> diff --git a/drivers/media/platform/qcom/venus/venc_ctrls.c b/drivers/media/platform/qcom/venus/venc_ctrls.c
> >>>> index 45910172..f332c8e 100644
> >>>> --- a/drivers/media/platform/qcom/venus/venc_ctrls.c
> >>>> +++ b/drivers/media/platform/qcom/venus/venc_ctrls.c
> >>>> @@ -81,6 +81,8 @@ static int venc_op_s_ctrl(struct v4l2_ctrl *ctrl)
> >>>>         struct venc_controls *ctr = &inst->controls.enc;
> >>>>         u32 bframes;
> >>>>         int ret;
> >>>> +       void *ptr;
> >>>> +       u32 ptype;
> >>>>
> >>>>         switch (ctrl->id) {
> >>>>         case V4L2_CID_MPEG_VIDEO_BITRATE_MODE:
> >>>> @@ -173,6 +175,14 @@ static int venc_op_s_ctrl(struct v4l2_ctrl *ctrl)
> >>>>
> >>>>                 ctr->num_b_frames = bframes;
> >>>>                 break;
> >>>> +       case V4L2_CID_MPEG_VIDEO_FORCE_KEY_FRAME:
> >>>> +               ptype = HFI_PROPERTY_CONFIG_VENC_REQUEST_SYNC_FRAME;
> >>>> +               ret = hfi_session_set_property(inst, ptype, ptr);
> >>>
> >>> The test bot already said it, but ptr is passed to
> >>> hfi_session_set_property() uninitialized. And as can be expected the
> >>> call returns -EINVAL on my board.
> >>>
> >>> Looking at other uses of HFI_PROPERTY_CONFIG_VENC_REQUEST_SYNC_FRAME I
> >>> see that the packet sent to the firmware does not have room for an
> >>> argument, so I tried to pass NULL but got the same result.
> >>
> >> yes, because pdata cannot be NULL. I'd suggest to make a pointer to
> >> struct hfi_enable and pass it to the set_property function.
> >
> > FWIW I also tried doing this and got the same error, strange...
> >
>
> OK, when you calling the v4l control? It makes sense when you calling
> it, because set_property checks does the session is on START state (i.e.
> streamon on both queues).

Do you mean that the property won't be actually applied unless both
queues are streaming? In that case maybe it would make sense for the
driver to save controls set before that and apply them when the
conditions allow them to be effective?