media: venus: add support for key frame
Subsystems (4)
PATCH v2 1/1 media: venus: add support for key frame
When client requests for a keyframe, set the property to hardware to generate the sync frame.
--- drivers/media/platform/qcom/venus/venc_ctrls.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-)
drivers/media/platform/qcom/venus/venc_ctrls.c +15 -1
@@ -79,8 +79,10 @@ static int venc_op_s_ctrl(struct v4l2_ctrl *ctrl) {struct venus_inst *inst =ctrl_to_inst (ctrl);struct venc_controls *ctr = &inst->controls.enc; +struct hfi_enable en = { .enable =1 };u32 bframes;int ret; +u32 ptype;switch (ctrl->id) {case V4L2_CID_MPEG_VIDEO_BITRATE_MODE :
@@ -173,6 +175,15 @@ static int venc_op_s_ctrl(struct v4l2_ctrl *ctrl) ctr->num_b_frames = bframes;break ; +case V4L2_CID_MPEG_VIDEO_FORCE_KEY_FRAME : +if (inst->streamon_out && inst->streamon_cap) { + ptype =HFI_PROPERTY_CONFIG_VENC_REQUEST_SYNC_FRAME ; + ret =hfi_session_set_property (inst, ptype, &en); + +if (ret) +return ret; + } +break ;default :return -EINVAL ; }
@@ -188,7 +199,7 @@ int venc_ctrl_init(struct venus_inst *inst) {int ret; - ret =v4l2_ctrl_handler_init (&inst->ctrl_handler,27 ); + ret =v4l2_ctrl_handler_init (&inst->ctrl_handler,28 );if (ret)return ret;
@@ -309,6 +320,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;
Signature · 1 line
1.9.1
Quoted 27 lines · Malathi Gottam
When client requests for a keyframe, set the property to hardware to generate the sync frame.
Signed-off-by Malathi Gottam <[email protected]>--- drivers/media/platform/qcom/venus/venc_ctrls.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-)drivers/media/platform/qcom/venus/venc_ctrls.c +2
-0@@ -79,8 +79,10 @@ static int venc_op_s_ctrl(struct v4l2_ctrl *ctrl) {struct venus_inst *inst =ctrl_to_inst (ctrl);struct venc_controls *ctr = &inst->controls.enc; +struct hfi_enable en = { .enable =1 };u32 bframes;int ret; +u32 ptype;switch (ctrl->id) {case V4L2_CID_MPEG_VIDEO_BITRATE_MODE :@@ -173,6 +175,15 @@ static int venc_op_s_ctrl(struct v4l2_ctrl *ctrl) ctr->num_b_frames = bframes;
drivers/media/platform/qcom/venus/venc_ctrls.c static int venc_op_s_ctrl(struct v4l2_ctrl *ctrl) · line 186
Quoted 10 lines
drivers/media/platform/qcom/venus/venc_ctrls.c +9
-0break ; +case V4L2_CID_MPEG_VIDEO_FORCE_KEY_FRAME : +if (inst->streamon_out && inst->streamon_cap) { + ptype =HFI_PROPERTY_CONFIG_VENC_REQUEST_SYNC_FRAME ; + ret =hfi_session_set_property (inst, ptype, &en); + +if (ret) +return ret; + } +break ;
This is still not the right way to handle this.
Please see the documentation of this control [1]:
"V4L2_CID_MPEG_VIDEO_FORCE_KEY_FRAME (button)
Force a key frame for the next queued buffer. Applicable to encoders. This is a general, codec-agnostic keyframe control."
Even if the driver is not streaming, it must remember that the keyframe was requested for next buffer. The next time userspace QBUFs an OUTPUT buffer, it should ask the hardware to encode that OUTPUT buffer into a keyframe.
But generally, the proper modern way for the userspace to request a keyframe is to set the V4L2_BUF_FLAG_KEYFRAME flag in the vb2_buffer_flag when queuing an OUTPUT buffer. It's the only guaranteed way to ensure that the keyframe will be encoded exactly for the selected frame. (The V4L2 control API doesn't guarantee any synchronization between controls and buffers itself.)
Best regards,
Tomasz
Quoted 45 lines · Tomasz Figa
Quoted 29 lines · Malathi Gottam
When client requests for a keyframe, set the property to hardware to generate the sync frame.
Signed-off-by Malathi Gottam <[email protected]>--- drivers/media/platform/qcom/venus/venc_ctrls.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-)drivers/media/platform/qcom/venus/venc_ctrls.c +3
-0@@ -79,8 +79,10 @@ static int venc_op_s_ctrl(struct v4l2_ctrl *ctrl) {struct venus_inst *inst =ctrl_to_inst (ctrl);struct venc_controls *ctr = &inst->controls.enc; +struct hfi_enable en = { .enable =1 };u32 bframes;int ret; +u32 ptype;switch (ctrl->id) {case V4L2_CID_MPEG_VIDEO_BITRATE_MODE :@@ -173,6 +175,15 @@ static int venc_op_s_ctrl(struct v4l2_ctrl *ctrl) ctr->num_b_frames = bframes;break ; +case V4L2_CID_MPEG_VIDEO_FORCE_KEY_FRAME :drivers/media/platform/qcom/venus/venc_ctrls.c static int venc_op_s_ctrl(struct v4l2_ctrl *ctrl) · line 188
Quoted 10 lines
drivers/media/platform/qcom/venus/venc_ctrls.c +8
-0+if (inst->streamon_out && inst->streamon_cap) { + ptype =HFI_PROPERTY_CONFIG_VENC_REQUEST_SYNC_FRAME ; + ret =hfi_session_set_property (inst, ptype, &en); + +if (ret) +return ret; + } +break ;This is still not the right way to handle this.
Please see the documentation of this control [1]:
Quoted 8 lines
"
V4L2_CID_MPEG_VIDEO_FORCE_KEY_FRAME(button)
Force a key frame for the next queued buffer. Applicable to encoders. This is a general, codec-agnostic keyframe control."Even if the driver is not streaming, it must remember that the keyframe was requested for next buffer. The next time userspace QBUFs an OUTPUT buffer, it should ask the hardware to encode that OUTPUT buffer into a keyframe.
That's correct. Driver can cache the client request and set it when the hardware is capable of accepting the property.
Still the issue having the requested OUTPUT buffer to be encoded as sync frame will be there. If there are few frames queued before streamon, driver will only keep a note that it has to set the request for keyframe, but not the exact one which was requested.
Quoted 9 lines
But generally, the proper modern way for the userspace to request a keyframe is to set the
V4L2_BUF_FLAG_KEYFRAMEflag in thevb2_buffer_flagwhen queuing an OUTPUT buffer. It's the only guaranteed way to ensure that the keyframe will be encoded exactly for the selected frame. (The V4L2 control API doesn't guarantee any synchronization between controls and buffers itself.)
This is a better way to handle it to ensure exact buffer gets encoded as sync frame.
Quoted 2 lines
Best regards,
Tomasz
Quoted 58 lines · Vikash Garodia
Quoted 47 lines · Tomasz Figa
Quoted 30 lines · Malathi Gottam
When client requests for a keyframe, set the property to hardware to generate the sync frame.
Signed-off-by Malathi Gottam <[email protected]>--- drivers/media/platform/qcom/venus/venc_ctrls.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-)drivers/media/platform/qcom/venus/venc_ctrls.c +3
-0@@ -79,8 +79,10 @@ static int venc_op_s_ctrl(struct v4l2_ctrl *ctrl) {struct venus_inst *inst =ctrl_to_inst (ctrl);struct venc_controls *ctr = &inst->controls.enc; +struct hfi_enable en = { .enable =1 };u32 bframes;int ret; +u32 ptype;switch (ctrl->id) {case V4L2_CID_MPEG_VIDEO_BITRATE_MODE :@@ -173,6 +175,15 @@ static int venc_op_s_ctrl(struct v4l2_ctrl *ctrl) ctr->num_b_frames = bframes;break ; +case V4L2_CID_MPEG_VIDEO_FORCE_KEY_FRAME :drivers/media/platform/qcom/venus/venc_ctrls.c static int venc_op_s_ctrl(struct v4l2_ctrl *ctrl) · line 188
Quoted 10 lines
drivers/media/platform/qcom/venus/venc_ctrls.c +8
-0+if (inst->streamon_out && inst->streamon_cap) { + ptype =HFI_PROPERTY_CONFIG_VENC_REQUEST_SYNC_FRAME ; + ret =hfi_session_set_property (inst, ptype, &en); + +if (ret) +return ret; + } +break ;This is still not the right way to handle this.
Please see the documentation of this control [1]:
Quoted 8 lines
"
V4L2_CID_MPEG_VIDEO_FORCE_KEY_FRAME(button)
Force a key frame for the next queued buffer. Applicable to encoders. This is a general, codec-agnostic keyframe control."Even if the driver is not streaming, it must remember that the keyframe was requested for next buffer. The next time userspace QBUFs an OUTPUT buffer, it should ask the hardware to encode that OUTPUT buffer into a keyframe.
Quoted 10 lines
That's correct. Driver can cache the client request and set it when the hardware
is capable of accepting the property.
Still the issue having the requested OUTPUT buffer to be encoded as sync frame will
be there. If there are few frames queued before streamon, driver will only keep a
note that it has to set the request for keyframe, but not the exact one which was
requested.
The description (quoted above) specifies exactly that the control applies only to the next queued buffer. It's a "button" control, so when the application sets it (to 1), it triggers a call to driver's s_ctrl callback and then resets to 0 automatically.
Quoted 11 lines
Quoted 10 lines
But generally, the proper modern way for the userspace to request a keyframe is to set the
V4L2_BUF_FLAG_KEYFRAMEflag in thevb2_buffer_flagwhen queuing an OUTPUT buffer. It's the only guaranteed way to ensure that the keyframe will be encoded exactly for the selected frame. (The V4L2 control API doesn't guarantee any synchronization between controls and buffers itself.)
Quoted 2 lines
This is a better way to handle it to ensure exact buffer gets encoded as sync frame.
It was created later to solve this problem. For compatibility, we have to keep supporting the control too.
Best regards,
Tomasz