| Message ID | 20240118105934.137919-3-quic_sachinku@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | add MBR type rate control for encoder | expand |
On 1/18/2024 11:14 PM, Konrad Dybcio wrote: > > > On 1/18/24 11:59, Sachin Kumar Garg wrote: >> There is no limit on the maximum level of the bit rate with >> the existing VBR rate control. >> V4L2_MPEG_VIDEO_BITRATE_MODE_MBR rate control will limit the >> frame maximum bit rate range to the +/- 10% of the configured >> bit-rate value. Encoder will choose appropriate quantization >> parameter and do the smart bit allocation to set the frame >> maximum bitrate level. >> >> Signed-off-by: Sachin Kumar Garg <quic_sachinku@quicinc.com> >> --- >> drivers/media/platform/qcom/venus/hfi_cmds.c | 38 +++++++++++++------ >> .../media/platform/qcom/venus/hfi_helper.h | 1 + >> drivers/media/platform/qcom/venus/venc.c | 2 + >> .../media/platform/qcom/venus/venc_ctrls.c | 5 ++- >> 4 files changed, 33 insertions(+), 13 deletions(-) >> >> diff --git a/drivers/media/platform/qcom/venus/hfi_cmds.c >> b/drivers/media/platform/qcom/venus/hfi_cmds.c >> index 3418d2dd9371..95fc27e0dc7d 100644 >> --- a/drivers/media/platform/qcom/venus/hfi_cmds.c >> +++ b/drivers/media/platform/qcom/venus/hfi_cmds.c >> @@ -645,17 +645,33 @@ static int pkt_session_set_property_1x(struct >> hfi_session_set_property_pkt *pkt, >> case HFI_PROPERTY_PARAM_VENC_RATE_CONTROL: { >> u32 *in = pdata; >> - switch (*in) { >> - case HFI_RATE_CONTROL_OFF: >> - case HFI_RATE_CONTROL_CBR_CFR: >> - case HFI_RATE_CONTROL_CBR_VFR: >> - case HFI_RATE_CONTROL_VBR_CFR: >> - case HFI_RATE_CONTROL_VBR_VFR: >> - case HFI_RATE_CONTROL_CQ: >> - break; >> - default: >> - ret = -EINVAL; >> - break; >> + if (hfi_ver == HFI_VERSION_4XX) { > > So, only sdm845/sc7180 and friends support it, but the newer > SoCs (like 8250 don't)? Thats correct. Supported only in AR50 generations. Not available in 8250. > > [...] > >> --- a/drivers/media/platform/qcom/venus/venc_ctrls.c >> +++ b/drivers/media/platform/qcom/venus/venc_ctrls.c >> @@ -387,10 +387,11 @@ int venc_ctrl_init(struct venus_inst *inst) >> v4l2_ctrl_new_std_menu(&inst->ctrl_handler, &venc_ctrl_ops, >> V4L2_CID_MPEG_VIDEO_BITRATE_MODE, >> - V4L2_MPEG_VIDEO_BITRATE_MODE_CBR, >> + V4L2_MPEG_VIDEO_BITRATE_MODE_MBR, > > Is this okay, since you're claiming only v4 supports it? This looks okay to extend the support for new RC mode. I see an issue in handling this new RC for non supported SOCs. This needs to be fixed in hfi_cmds.c while preparing the packet. MBR for unsupported SOC should be treated as -ENOTSUPP instead of -EINVAL which would terminate the session. This need to be fixed. Regards, Vikash
diff --git a/drivers/media/platform/qcom/venus/hfi_cmds.c b/drivers/media/platform/qcom/venus/hfi_cmds.c index 3418d2dd9371..95fc27e0dc7d 100644 --- a/drivers/media/platform/qcom/venus/hfi_cmds.c +++ b/drivers/media/platform/qcom/venus/hfi_cmds.c @@ -645,17 +645,33 @@ static int pkt_session_set_property_1x(struct hfi_session_set_property_pkt *pkt, case HFI_PROPERTY_PARAM_VENC_RATE_CONTROL: { u32 *in = pdata; - switch (*in) { - case HFI_RATE_CONTROL_OFF: - case HFI_RATE_CONTROL_CBR_CFR: - case HFI_RATE_CONTROL_CBR_VFR: - case HFI_RATE_CONTROL_VBR_CFR: - case HFI_RATE_CONTROL_VBR_VFR: - case HFI_RATE_CONTROL_CQ: - break; - default: - ret = -EINVAL; - break; + if (hfi_ver == HFI_VERSION_4XX) { + switch (*in) { + case HFI_RATE_CONTROL_OFF: + case HFI_RATE_CONTROL_CBR_CFR: + case HFI_RATE_CONTROL_CBR_VFR: + case HFI_RATE_CONTROL_VBR_CFR: + case HFI_RATE_CONTROL_VBR_VFR: + case HFI_RATE_CONTROL_CQ: + case HFI_RATE_CONTROL_MBR_CFR: + break; + default: + ret = -EINVAL; + break; + } + } else { + switch (*in) { + case HFI_RATE_CONTROL_OFF: + case HFI_RATE_CONTROL_CBR_CFR: + case HFI_RATE_CONTROL_CBR_VFR: + case HFI_RATE_CONTROL_VBR_CFR: + case HFI_RATE_CONTROL_VBR_VFR: + case HFI_RATE_CONTROL_CQ: + break; + default: + ret = -EINVAL; + break; + } } pkt->data[1] = *in; diff --git a/drivers/media/platform/qcom/venus/hfi_helper.h b/drivers/media/platform/qcom/venus/hfi_helper.h index e4c05d62cfc7..a0fd857f5c4b 100644 --- a/drivers/media/platform/qcom/venus/hfi_helper.h +++ b/drivers/media/platform/qcom/venus/hfi_helper.h @@ -232,6 +232,7 @@ #define HFI_RATE_CONTROL_VBR_CFR 0x1000003 #define HFI_RATE_CONTROL_CBR_VFR 0x1000004 #define HFI_RATE_CONTROL_CBR_CFR 0x1000005 +#define HFI_RATE_CONTROL_MBR_CFR 0x1000006 #define HFI_RATE_CONTROL_CQ 0x1000008 #define HFI_VIDEO_CODEC_H264 0x00000002 diff --git a/drivers/media/platform/qcom/venus/venc.c b/drivers/media/platform/qcom/venus/venc.c index 3ec2fb8d9fab..8acbb05f6ce8 100644 --- a/drivers/media/platform/qcom/venus/venc.c +++ b/drivers/media/platform/qcom/venus/venc.c @@ -807,6 +807,8 @@ static int venc_set_properties(struct venus_inst *inst) HFI_RATE_CONTROL_CBR_CFR; else if (ctr->bitrate_mode == V4L2_MPEG_VIDEO_BITRATE_MODE_CQ) rate_control = HFI_RATE_CONTROL_CQ; + else if (ctr->bitrate_mode == V4L2_MPEG_VIDEO_BITRATE_MODE_MBR) + rate_control = HFI_RATE_CONTROL_MBR_CFR; ptype = HFI_PROPERTY_PARAM_VENC_RATE_CONTROL; ret = hfi_session_set_property(inst, ptype, &rate_control); diff --git a/drivers/media/platform/qcom/venus/venc_ctrls.c b/drivers/media/platform/qcom/venus/venc_ctrls.c index d9d2a293f3ef..c9c3b1b45525 100644 --- a/drivers/media/platform/qcom/venus/venc_ctrls.c +++ b/drivers/media/platform/qcom/venus/venc_ctrls.c @@ -387,10 +387,11 @@ int venc_ctrl_init(struct venus_inst *inst) v4l2_ctrl_new_std_menu(&inst->ctrl_handler, &venc_ctrl_ops, V4L2_CID_MPEG_VIDEO_BITRATE_MODE, - V4L2_MPEG_VIDEO_BITRATE_MODE_CBR, + V4L2_MPEG_VIDEO_BITRATE_MODE_MBR, ~((1 << V4L2_MPEG_VIDEO_BITRATE_MODE_VBR) | (1 << V4L2_MPEG_VIDEO_BITRATE_MODE_CBR) | - (1 << V4L2_MPEG_VIDEO_BITRATE_MODE_CQ)), + (1 << V4L2_MPEG_VIDEO_BITRATE_MODE_CQ) | + (1 << V4L2_MPEG_VIDEO_BITRATE_MODE_MBR)), V4L2_MPEG_VIDEO_BITRATE_MODE_VBR); v4l2_ctrl_new_std_menu(&inst->ctrl_handler, &venc_ctrl_ops,
There is no limit on the maximum level of the bit rate with the existing VBR rate control. V4L2_MPEG_VIDEO_BITRATE_MODE_MBR rate control will limit the frame maximum bit rate range to the +/- 10% of the configured bit-rate value. Encoder will choose appropriate quantization parameter and do the smart bit allocation to set the frame maximum bitrate level. Signed-off-by: Sachin Kumar Garg <quic_sachinku@quicinc.com> --- drivers/media/platform/qcom/venus/hfi_cmds.c | 38 +++++++++++++------ .../media/platform/qcom/venus/hfi_helper.h | 1 + drivers/media/platform/qcom/venus/venc.c | 2 + .../media/platform/qcom/venus/venc_ctrls.c | 5 ++- 4 files changed, 33 insertions(+), 13 deletions(-)