* [PATCH 0/2] add MBR type rate control for encoder @ 2024-01-18 10:59 Sachin Kumar Garg 2024-01-18 10:59 ` [PATCH 1/2] media: v4l2-ctrls: add encoder maximum bitrate control Sachin Kumar Garg 2024-01-18 10:59 ` [PATCH 2/2] media: venus: add new rate control type MBR for encoder Sachin Kumar Garg 0 siblings, 2 replies; 6+ messages in thread From: Sachin Kumar Garg @ 2024-01-18 10:59 UTC (permalink / raw) To: hverkuil-cisco, Mauro Carvalho Chehab, Stanimir Varbanov, Vikash Garodia, Andy Gross, Bjorn Andersson, Konrad Dybcio Cc: Bryan O'Donoghue, linux-media, linux-kernel, linux-arm-msm This series adds the support for MBR rate control type in the venus driver. This rate control type will limit the frame level maximum bitrate as per the target bitrate. It will improve the video quality of low motion video at ultra low bit-rates. Sachin Kumar Garg (2): media: v4l2-ctrls: add encoder maximum bitrate control media: venus: add new rate control type MBR for encoder .../media/v4l/ext-ctrls-codec.rst | 2 + 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 ++- drivers/media/v4l2-core/v4l2-ctrls-defs.c | 1 + include/uapi/linux/v4l2-controls.h | 1 + 7 files changed, 37 insertions(+), 13 deletions(-) -- 2.34.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] media: v4l2-ctrls: add encoder maximum bitrate control 2024-01-18 10:59 [PATCH 0/2] add MBR type rate control for encoder Sachin Kumar Garg @ 2024-01-18 10:59 ` Sachin Kumar Garg 2024-01-18 10:59 ` [PATCH 2/2] media: venus: add new rate control type MBR for encoder Sachin Kumar Garg 1 sibling, 0 replies; 6+ messages in thread From: Sachin Kumar Garg @ 2024-01-18 10:59 UTC (permalink / raw) To: hverkuil-cisco, Mauro Carvalho Chehab, Stanimir Varbanov, Vikash Garodia, Andy Gross, Bjorn Andersson, Konrad Dybcio Cc: Bryan O'Donoghue, linux-media, linux-kernel, linux-arm-msm Introduce V4L2_MPEG_VIDEO_BITRATE_MODE_MBR rate control to limit the frame level maximum bit rate. Encoder will choose appropriate quantization parameter and do the smart bit allocation to set the frame maximum bitrate level as per the Bitrate value configured. Signed-off-by: Sachin Kumar Garg <quic_sachinku@quicinc.com> --- Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst | 2 ++ drivers/media/v4l2-core/v4l2-ctrls-defs.c | 1 + include/uapi/linux/v4l2-controls.h | 1 + 3 files changed, 4 insertions(+) diff --git a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst index 2a165ae063fb..05ef4a70e3f5 100644 --- a/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst +++ b/Documentation/userspace-api/media/v4l/ext-ctrls-codec.rst @@ -576,6 +576,8 @@ enum v4l2_mpeg_video_bitrate_mode - - Constant bitrate * - ``V4L2_MPEG_VIDEO_BITRATE_MODE_CQ`` - Constant quality + * - ``V4L2_MPEG_VIDEO_BITRATE_MODE_MBR`` + - Maximum bitrate diff --git a/drivers/media/v4l2-core/v4l2-ctrls-defs.c b/drivers/media/v4l2-core/v4l2-ctrls-defs.c index 8696eb1cdd61..e0597b61ffb9 100644 --- a/drivers/media/v4l2-core/v4l2-ctrls-defs.c +++ b/drivers/media/v4l2-core/v4l2-ctrls-defs.c @@ -154,6 +154,7 @@ const char * const *v4l2_ctrl_get_menu(u32 id) "Variable Bitrate", "Constant Bitrate", "Constant Quality", + "Maximum Bitrate", NULL }; static const char * const mpeg_stream_type[] = { diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h index 99c3f5e99da7..7c74d6c417d1 100644 --- a/include/uapi/linux/v4l2-controls.h +++ b/include/uapi/linux/v4l2-controls.h @@ -393,6 +393,7 @@ enum v4l2_mpeg_video_bitrate_mode { V4L2_MPEG_VIDEO_BITRATE_MODE_VBR = 0, V4L2_MPEG_VIDEO_BITRATE_MODE_CBR = 1, V4L2_MPEG_VIDEO_BITRATE_MODE_CQ = 2, + V4L2_MPEG_VIDEO_BITRATE_MODE_MBR = 3, }; #define V4L2_CID_MPEG_VIDEO_BITRATE (V4L2_CID_CODEC_BASE+207) #define V4L2_CID_MPEG_VIDEO_BITRATE_PEAK (V4L2_CID_CODEC_BASE+208) -- 2.34.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] media: venus: add new rate control type MBR for encoder 2024-01-18 10:59 [PATCH 0/2] add MBR type rate control for encoder Sachin Kumar Garg 2024-01-18 10:59 ` [PATCH 1/2] media: v4l2-ctrls: add encoder maximum bitrate control Sachin Kumar Garg @ 2024-01-18 10:59 ` Sachin Kumar Garg 2024-01-18 13:34 ` Bryan O'Donoghue 2024-01-18 17:44 ` Konrad Dybcio 1 sibling, 2 replies; 6+ messages in thread From: Sachin Kumar Garg @ 2024-01-18 10:59 UTC (permalink / raw) To: hverkuil-cisco, Mauro Carvalho Chehab, Stanimir Varbanov, Vikash Garodia, Andy Gross, Bjorn Andersson, Konrad Dybcio Cc: Bryan O'Donoghue, linux-media, linux-kernel, linux-arm-msm 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) { + 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, -- 2.34.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] media: venus: add new rate control type MBR for encoder 2024-01-18 10:59 ` [PATCH 2/2] media: venus: add new rate control type MBR for encoder Sachin Kumar Garg @ 2024-01-18 13:34 ` Bryan O'Donoghue 2024-01-18 17:44 ` Konrad Dybcio 1 sibling, 0 replies; 6+ messages in thread From: Bryan O'Donoghue @ 2024-01-18 13:34 UTC (permalink / raw) To: Sachin Kumar Garg, hverkuil-cisco, Mauro Carvalho Chehab, Stanimir Varbanov, Vikash Garodia, Andy Gross, Bjorn Andersson, Konrad Dybcio Cc: linux-media, linux-kernel, linux-arm-msm On 18/01/2024 10:59, Sachin Kumar Garg wrote: > > - 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; > + } The if/else you have here seems like a needless replication Just have => case HFI_RATE_CONTROL_MBR_CFR: if (hfi_ver == HFI_VERSION_4XX) --- bod ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] media: venus: add new rate control type MBR for encoder 2024-01-18 10:59 ` [PATCH 2/2] media: venus: add new rate control type MBR for encoder Sachin Kumar Garg 2024-01-18 13:34 ` Bryan O'Donoghue @ 2024-01-18 17:44 ` Konrad Dybcio 2024-01-19 8:05 ` Vikash Garodia 1 sibling, 1 reply; 6+ messages in thread From: Konrad Dybcio @ 2024-01-18 17:44 UTC (permalink / raw) To: Sachin Kumar Garg, hverkuil-cisco, Mauro Carvalho Chehab, Stanimir Varbanov, Vikash Garodia, Andy Gross, Bjorn Andersson Cc: Bryan O'Donoghue, linux-media, linux-kernel, linux-arm-msm 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)? [...] > --- 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? Konrad ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] media: venus: add new rate control type MBR for encoder 2024-01-18 17:44 ` Konrad Dybcio @ 2024-01-19 8:05 ` Vikash Garodia 0 siblings, 0 replies; 6+ messages in thread From: Vikash Garodia @ 2024-01-19 8:05 UTC (permalink / raw) To: Konrad Dybcio, Sachin Kumar Garg, hverkuil-cisco, Mauro Carvalho Chehab, Stanimir Varbanov, Andy Gross, Bjorn Andersson Cc: Bryan O'Donoghue, linux-media, linux-kernel, linux-arm-msm 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-01-19 8:05 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-01-18 10:59 [PATCH 0/2] add MBR type rate control for encoder Sachin Kumar Garg 2024-01-18 10:59 ` [PATCH 1/2] media: v4l2-ctrls: add encoder maximum bitrate control Sachin Kumar Garg 2024-01-18 10:59 ` [PATCH 2/2] media: venus: add new rate control type MBR for encoder Sachin Kumar Garg 2024-01-18 13:34 ` Bryan O'Donoghue 2024-01-18 17:44 ` Konrad Dybcio 2024-01-19 8:05 ` Vikash Garodia
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®