mbox series

[v4,0/2] media: venus: Add hierarchical h.264 controls

Message ID 20241017-submit-v4-0-d852bc7f7fdc@chromium.org
Headers show
Series media: venus: Add hierarchical h.264 controls | expand

Message

Fritz Koenig Oct. 17, 2024, 8:04 p.m. UTC 
v4:
- addressed feedback from Dikshita
- included reviewed by for patch 1
- Link to v3: https://lore.kernel.org/r/20240814-submit-v3-0-f7d05e3e8560@chromium.org

v3:
- dropped reordering patch
- updated cover letter

v2:
- cover letter
- testing methodology
- Signed-off-by

V4L2 has support for encoding with hierarchical frames using the
V4L2_CID_MPEG_VIDEO_H264_HIERARCHICAL_CODING* controls. This allows for
specifing frame references needed for temporal scalability. Encoding a
single stream with a single layer allows for the layer to be dropped and
the stream to be decoded without artifacts.

ChromeOS is planning to use this feature for the L1T2 web standard[1].
This allows video conferencing apps to encode once for a clients with
different performance/bandwidth capabilities. As the application is a
real time encoder only P frame support is added.

The ChromeOS test framework ("tast") was used to verify that no
regressions are present. This was done on SC7180 ("trogdor"). These
patches were also run on SC7280, but not with as an extensive test set.

Verification of the added controls was done with a bitstream analyser to
make sure that reference frame management is correct.

[1]: https://www.w3.org/TR/webrtc-svc/#L1T2*

Signed-off-by: Fritz Koenig <frkoenig@chromium.org>
---
Fritz Koenig (2):
      media: venus: Helper function for dynamically updating bitrate
      media: venus: Enable h.264 hierarchical coding

 drivers/media/platform/qcom/venus/core.h       |   4 +
 drivers/media/platform/qcom/venus/venc.c       |  73 +++++++++-----
 drivers/media/platform/qcom/venus/venc_ctrls.c | 126 ++++++++++++++++++++++---
 3 files changed, 165 insertions(+), 38 deletions(-)
---
base-commit: d07b43284ab356daf7ec5ae1858a16c1c7b6adab
change-id: 20240814-submit-05880a074377

Best regards,

Comments

Dikshita Agarwal Oct. 18, 2024, 4:50 a.m. UTC | #1 
On 10/18/2024 1:34 AM, Fritz Koenig wrote:
> Move the dynamic bitrate updating functionality to a separate function
> so that it can be shared.
> 
> No functionality changes.
> 
> Signed-off-by: Fritz Koenig <frkoenig@chromium.org>
> Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
>  drivers/media/platform/qcom/venus/venc_ctrls.c | 34 +++++++++++++++-----------
>  1 file changed, 20 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/venus/venc_ctrls.c b/drivers/media/platform/qcom/venus/venc_ctrls.c
> index d9d2a293f3ef..3e1f6f26eddf 100644
> --- a/drivers/media/platform/qcom/venus/venc_ctrls.c
> +++ b/drivers/media/platform/qcom/venus/venc_ctrls.c
> @@ -67,12 +67,28 @@ static int venc_calc_bpframes(u32 gop_size, u32 conseq_b, u32 *bf, u32 *pf)
>  	return 0;
>  }
>  
> +static int dynamic_bitrate_update(struct venus_inst *inst, u32 bitrate,
> +				  u32 layer_id)
> +{
> +	int ret = 0;
> +
> +	mutex_lock(&inst->lock);
> +	if (inst->streamon_out && inst->streamon_cap) {
> +		u32 ptype = HFI_PROPERTY_CONFIG_VENC_TARGET_BITRATE;
> +		struct hfi_bitrate brate = { .bitrate = bitrate, .layer_id = layer_id };
> +
> +		ret = hfi_session_set_property(inst, ptype, &brate);
> +	}
> +	mutex_unlock(&inst->lock);
> +
> +	return ret;
> +}
> +
>  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 };
> -	struct hfi_bitrate brate;
>  	struct hfi_ltr_use ltr_use;
>  	struct hfi_ltr_mark ltr_mark;
>  	u32 bframes;
> @@ -85,19 +101,9 @@ static int venc_op_s_ctrl(struct v4l2_ctrl *ctrl)
>  		break;
>  	case V4L2_CID_MPEG_VIDEO_BITRATE:
>  		ctr->bitrate = ctrl->val;
> -		mutex_lock(&inst->lock);
> -		if (inst->streamon_out && inst->streamon_cap) {
> -			ptype = HFI_PROPERTY_CONFIG_VENC_TARGET_BITRATE;
> -			brate.bitrate = ctr->bitrate;
> -			brate.layer_id = 0;
> -
> -			ret = hfi_session_set_property(inst, ptype, &brate);
> -			if (ret) {
> -				mutex_unlock(&inst->lock);
> -				return ret;
> -			}
> -		}
> -		mutex_unlock(&inst->lock);
> +		ret = dynamic_bitrate_update(inst, ctr->bitrate, 0);
> +		if (ret)
> +			return ret;
>  		break;
>  	case V4L2_CID_MPEG_VIDEO_BITRATE_PEAK:
>  		ctr->bitrate_peak = ctrl->val;
> 
Reviewed-by: Dikshita Agarwal <quic_dikshita@quicinc.com>
Dikshita Agarwal Oct. 18, 2024, 5:01 a.m. UTC | #2 
On 10/18/2024 1:34 AM, Fritz Koenig wrote:
> HFI supports hierarchical P encoding and the ability to specify the
> bitrate for the different layers.
> 
> Connect the controls that V4L2 provides and HFI supports.
> 
> Signed-off-by: Fritz Koenig <frkoenig@chromium.org>
> ---
>  drivers/media/platform/qcom/venus/core.h       |  4 ++
>  drivers/media/platform/qcom/venus/venc.c       | 73 +++++++++++++-------
>  drivers/media/platform/qcom/venus/venc_ctrls.c | 92 ++++++++++++++++++++++++++
>  3 files changed, 145 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/venus/core.h b/drivers/media/platform/qcom/venus/core.h
> index 55202b89e1b9..fd46a7778d8c 100644
> --- a/drivers/media/platform/qcom/venus/core.h
> +++ b/drivers/media/platform/qcom/venus/core.h
> @@ -26,6 +26,7 @@
>  #define VIDC_CLKS_NUM_MAX		4
>  #define VIDC_VCODEC_CLKS_NUM_MAX	2
>  #define VIDC_RESETS_NUM_MAX		2
> +#define VIDC_MAX_HIER_CODING_LAYER 6
>  
>  extern int venus_fw_debug;
>  
> @@ -255,6 +256,7 @@ struct venc_controls {
>  	u32 rc_enable;
>  	u32 const_quality;
>  	u32 frame_skip_mode;
> +	u32 layer_bitrate;
>  
>  	u32 h264_i_period;
>  	u32 h264_entropy_mode;
> @@ -273,6 +275,8 @@ struct venc_controls {
>  	s32 h264_loop_filter_alpha;
>  	s32 h264_loop_filter_beta;
>  	u32 h264_8x8_transform;
> +	u32 h264_hier_layers;
> +	u32 h264_hier_layer_bitrate[VIDC_MAX_HIER_CODING_LAYER];
>  
>  	u32 hevc_i_qp;
>  	u32 hevc_p_qp;
> diff --git a/drivers/media/platform/qcom/venus/venc.c b/drivers/media/platform/qcom/venus/venc.c
> index 3ec2fb8d9fab..d717e5dd6a39 100644
> --- a/drivers/media/platform/qcom/venus/venc.c
> +++ b/drivers/media/platform/qcom/venus/venc.c
> @@ -734,6 +734,29 @@ static int venc_set_properties(struct venus_inst *inst)
>  		if (ret)
>  			return ret;
>  
> +		if (ctr->layer_bitrate) {
> +			unsigned int i;
> +
> +			ptype = HFI_PROPERTY_PARAM_VENC_HIER_P_MAX_NUM_ENH_LAYER;
> +			ret = hfi_session_set_property(inst, ptype, &ctr->h264_hier_layers);
> +			if (ret)
> +				return ret;
> +
> +			ptype = HFI_PROPERTY_CONFIG_VENC_HIER_P_ENH_LAYER;
> +			ret = hfi_session_set_property(inst, ptype, &ctr->layer_bitrate);
> +			if (ret)
> +				return ret;
> +
> +			for (i = 0; i < ctr->h264_hier_layers; ++i) {
> +				ptype = HFI_PROPERTY_CONFIG_VENC_TARGET_BITRATE;
> +				brate.bitrate = ctr->h264_hier_layer_bitrate[i];
> +				brate.layer_id = i;
> +
> +				ret = hfi_session_set_property(inst, ptype, &brate);
> +				if (ret)
> +					return ret;
> +			}
> +		}
>  	}
>  
>  	if (inst->fmt_cap->pixfmt == V4L2_PIX_FMT_H264 ||
> @@ -823,21 +846,36 @@ static int venc_set_properties(struct venus_inst *inst)
>  			return ret;
>  	}
>  
> -	if (!ctr->bitrate)
> -		bitrate = 64000;
> -	else
> -		bitrate = ctr->bitrate;
> +	if (!ctr->layer_bitrate) {
> +		if (!ctr->bitrate)
> +			bitrate = 64000;
> +		else
> +			bitrate = ctr->bitrate;
>  
> -	ptype = HFI_PROPERTY_CONFIG_VENC_TARGET_BITRATE;
> -	brate.bitrate = bitrate;
> -	brate.layer_id = 0;
> +		ptype = HFI_PROPERTY_CONFIG_VENC_TARGET_BITRATE;
> +		brate.bitrate = bitrate;
> +		brate.layer_id = 0;
>  
> -	ret = hfi_session_set_property(inst, ptype, &brate);
> -	if (ret)
> -		return ret;
> +		ret = hfi_session_set_property(inst, ptype, &brate);
> +		if (ret)
> +			return ret;
> +
> +		if (!ctr->bitrate_peak)
> +			bitrate *= 2;
> +		else
> +			bitrate = ctr->bitrate_peak;
> +
> +		ptype = HFI_PROPERTY_CONFIG_VENC_MAX_BITRATE;
> +		brate.bitrate = bitrate;
> +		brate.layer_id = 0;
> +
> +		ret = hfi_session_set_property(inst, ptype, &brate);
> +		if (ret)
> +			return ret;
> +	}
>  
>  	if (inst->fmt_cap->pixfmt == V4L2_PIX_FMT_H264 ||
> -	    inst->fmt_cap->pixfmt == V4L2_PIX_FMT_HEVC) {
> +			inst->fmt_cap->pixfmt == V4L2_PIX_FMT_HEVC) {
This is not needed, pls fix.
with above taken care.

Reviewed-by: Dikshita Agarwal <quic_dikshita@quicinc.com>

>  		ptype = HFI_PROPERTY_CONFIG_VENC_SYNC_FRAME_SEQUENCE_HEADER;
>  		if (ctr->header_mode == V4L2_MPEG_VIDEO_HEADER_MODE_SEPARATE)
>  			en.enable = 0;
> @@ -849,19 +887,6 @@ static int venc_set_properties(struct venus_inst *inst)
>  			return ret;
>  	}
>  
> -	if (!ctr->bitrate_peak)
> -		bitrate *= 2;
> -	else
> -		bitrate = ctr->bitrate_peak;
> -
> -	ptype = HFI_PROPERTY_CONFIG_VENC_MAX_BITRATE;
> -	brate.bitrate = bitrate;
> -	brate.layer_id = 0;
> -
> -	ret = hfi_session_set_property(inst, ptype, &brate);
> -	if (ret)
> -		return ret;
> -
>  	ptype = HFI_PROPERTY_PARAM_VENC_SESSION_QP;
>  	if (inst->fmt_cap->pixfmt == V4L2_PIX_FMT_HEVC) {
>  		quant.qp_i = ctr->hevc_i_qp;
> diff --git a/drivers/media/platform/qcom/venus/venc_ctrls.c b/drivers/media/platform/qcom/venus/venc_ctrls.c
> index 3e1f6f26eddf..e340783a4ef2 100644
> --- a/drivers/media/platform/qcom/venus/venc_ctrls.c
> +++ b/drivers/media/platform/qcom/venus/venc_ctrls.c
> @@ -346,6 +346,55 @@ static int venc_op_s_ctrl(struct v4l2_ctrl *ctrl)
>  
>  		ctr->h264_8x8_transform = ctrl->val;
>  		break;
> +	case V4L2_CID_MPEG_VIDEO_H264_HIERARCHICAL_CODING_TYPE:
> +		if (ctrl->val != V4L2_MPEG_VIDEO_H264_HIERARCHICAL_CODING_P)
> +			return -EINVAL;
> +		break;
> +	case V4L2_CID_MPEG_VIDEO_H264_HIERARCHICAL_CODING:
> +		ctr->layer_bitrate = ctrl->val;
> +		break;
> +	case V4L2_CID_MPEG_VIDEO_H264_HIERARCHICAL_CODING_LAYER:
> +		if (ctrl->val > VIDC_MAX_HIER_CODING_LAYER)
> +			return -EINVAL;
> +		ctr->h264_hier_layers = ctrl->val;
> +		break;
> +	case V4L2_CID_MPEG_VIDEO_H264_HIER_CODING_L0_BR:
> +		ctr->h264_hier_layer_bitrate[0] = ctrl->val;
> +		ret = dynamic_bitrate_update(inst, ctr->h264_hier_layer_bitrate[0], 0);
> +		if (ret)
> +			return ret;
> +		break;
> +	case V4L2_CID_MPEG_VIDEO_H264_HIER_CODING_L1_BR:
> +		ctr->h264_hier_layer_bitrate[1] = ctrl->val;
> +		ret = dynamic_bitrate_update(inst, ctr->h264_hier_layer_bitrate[1], 1);
> +		if (ret)
> +			return ret;
> +		break;
> +	case V4L2_CID_MPEG_VIDEO_H264_HIER_CODING_L2_BR:
> +		ctr->h264_hier_layer_bitrate[2] = ctrl->val;
> +		ret = dynamic_bitrate_update(inst, ctr->h264_hier_layer_bitrate[2], 2);
> +		if (ret)
> +			return ret;
> +		break;
> +	case V4L2_CID_MPEG_VIDEO_H264_HIER_CODING_L3_BR:
> +		ctr->h264_hier_layer_bitrate[3] = ctrl->val;
> +		ret = dynamic_bitrate_update(inst, ctr->h264_hier_layer_bitrate[3], 3);
> +		if (ret)
> +			return ret;
> +		break;
> +	case V4L2_CID_MPEG_VIDEO_H264_HIER_CODING_L4_BR:
> +		ctr->h264_hier_layer_bitrate[4] = ctrl->val;
> +		ret = dynamic_bitrate_update(inst, ctr->h264_hier_layer_bitrate[4], 4);
> +		if (ret)
> +			return ret;
> +		break;
> +	case V4L2_CID_MPEG_VIDEO_H264_HIER_CODING_L5_BR:
> +		ctr->h264_hier_layer_bitrate[5] = ctrl->val;
> +		ret = dynamic_bitrate_update(inst, ctr->h264_hier_layer_bitrate[5], 5);
> +		if (ret)
> +			return ret;
> +		break;
> +
>  	default:
>  		return -EINVAL;
>  	}
> @@ -628,6 +677,49 @@ int venc_ctrl_init(struct venus_inst *inst)
>  			  V4L2_CID_MPEG_VIDEO_INTRA_REFRESH_PERIOD, 0,
>  			  ((4096 * 2304) >> 8), 1, 0);
>  
> +	if (IS_V4(inst->core) || IS_V6(inst->core)) {
> +		v4l2_ctrl_new_std_menu(&inst->ctrl_handler, &venc_ctrl_ops,
> +				       V4L2_CID_MPEG_VIDEO_H264_HIERARCHICAL_CODING_TYPE,
> +				       V4L2_MPEG_VIDEO_H264_HIERARCHICAL_CODING_P,
> +				       1, V4L2_MPEG_VIDEO_H264_HIERARCHICAL_CODING_P);
> +
> +		v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops,
> +				  V4L2_CID_MPEG_VIDEO_H264_HIERARCHICAL_CODING, 0, 1, 1, 0);
> +
> +		v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops,
> +				  V4L2_CID_MPEG_VIDEO_H264_HIERARCHICAL_CODING_LAYER, 0,
> +				  VIDC_MAX_HIER_CODING_LAYER, 1, 0);
> +
> +		v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops,
> +				  V4L2_CID_MPEG_VIDEO_H264_HIER_CODING_L0_BR,
> +				  BITRATE_MIN, BITRATE_MAX, BITRATE_STEP, BITRATE_DEFAULT);
> +
> +		v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops,
> +				  V4L2_CID_MPEG_VIDEO_H264_HIER_CODING_L1_BR,
> +				  BITRATE_MIN, BITRATE_MAX,
> +				  BITRATE_STEP, BITRATE_DEFAULT);
> +
> +		v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops,
> +				  V4L2_CID_MPEG_VIDEO_H264_HIER_CODING_L2_BR,
> +				  BITRATE_MIN, BITRATE_MAX,
> +				  BITRATE_STEP, BITRATE_DEFAULT);
> +
> +		v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops,
> +				  V4L2_CID_MPEG_VIDEO_H264_HIER_CODING_L3_BR,
> +				  BITRATE_MIN, BITRATE_MAX,
> +				  BITRATE_STEP, BITRATE_DEFAULT);
> +
> +		v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops,
> +				  V4L2_CID_MPEG_VIDEO_H264_HIER_CODING_L4_BR,
> +				  BITRATE_MIN, BITRATE_MAX,
> +				  BITRATE_STEP, BITRATE_DEFAULT);
> +
> +		v4l2_ctrl_new_std(&inst->ctrl_handler, &venc_ctrl_ops,
> +				  V4L2_CID_MPEG_VIDEO_H264_HIER_CODING_L5_BR,
> +				  BITRATE_MIN, BITRATE_MAX,
> +				  BITRATE_STEP, BITRATE_DEFAULT);
> +	}
> +
>  	ret = inst->ctrl_handler.error;
>  	if (ret)
>  		goto err;
>