On Tue, Oct 9, 2018 at 4:54 PM Malathi Gottam <mgottam@xxxxxxxxxxxxxx> wrote: > > When client requests for a keyframe, set the property > to hardware to generate the sync frame. > > Signed-off-by: Malathi Gottam <mgottam@xxxxxxxxxxxxxx> > --- > 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. > + This newline is unnecessary. > + if (ret) > + return ret; > + > + break; > default: > return -EINVAL; > } > @@ -309,6 +319,9 @@ int venc_ctrl_init(struct venus_inst *inst) > v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops, > V4L2_CID_MPEG_VIDEO_H264_I_PERIOD, 0, (1 << 16) - 1, 1, 0); > > + v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops, > + V4L2_CID_MPEG_VIDEO_FORCE_KEY_FRAME, 0, 0, 0, 0); > + > ret = inst->ctrl_handler.error; > if (ret) > goto err; > -- > 1.9.1 >