Re: [PATCH v0 2/3] media: chips-media: wave5: Support CBP profile

[Brandon Brnich]

Hi Jackson,

On 3/19/26 20:23, jackson.lee wrote:
> Hi Nicolas

[...]

> > *ctrl)
> > >   		case V4L2_MPEG_VIDEO_H264_PROFILE_CONSTRAINED_BASELINE:
> > >   			inst->enc_param.profile = H264_PROFILE_BP;
> > >   			inst->bit_depth = 8;
> > > +			if (ctrl->val ==
> > V4L2_MPEG_VIDEO_H264_PROFILE_CONSTRAINED_BASELINE)
> > > +				inst->enc_param.constraint_set1_flag = 1;
> > >   			break;
> > >   		case V4L2_MPEG_VIDEO_H264_PROFILE_MAIN:
> > >   			inst->enc_param.profile = H264_PROFILE_MP; @@ -1214,9
> > +1216,11 @@
> > > static int wave5_set_enc_openparam(struct enc_open_param *open_param,
> > >   			open_param->wave_param.decoding_refresh_type =
> > DEC_REFRESH_TYPE_IDR;
> > >   			open_param->wave_param.intra_period =
> > input.avc_idr_period;
> > >   		}
> > > -	} else {
> > > +	} else if (inst->std == W_AVC_ENC)
> > > +		open_param->wave_param.constraint_set1_flag =
> > input.constraint_set1_flag;
> > > +	else
> > >   		open_param->wave_param.avc_idr_period = input.avc_idr_period;
> > > -	}

The above code breaks setting GOP size for the AVC case. Any reason this 
shouldn't remain just an if else where the else sets both constraint 
flag and avc period?

Best,
Brandon

> > nit: Just keep the bracket, so that all branches have brackets.
> >
> > cheers,
> > Nicolas
> >
> > > +
> > >   	open_param->wave_param.entropy_coding_mode =
> > input.entropy_coding_mode;
> > >   	open_param->wave_param.lossless_enable = input.lossless_enable;
> > >   	open_param->wave_param.const_intra_pred_flag =
> > > input.const_intra_pred_flag; @@ -1687,7 +1691,7 @@ static int
> > wave5_vpu_open_enc(struct file *filp)
> > >   			  -6, 6, 1, 0);
> > >   	v4l2_ctrl_new_std(v4l2_ctrl_hdl, &wave5_vpu_enc_ctrl_ops,
> > >   			  V4L2_CID_MPEG_VIDEO_H264_8X8_TRANSFORM,
> > > -			  0, 1, 1, 1);
> > > +			  0, 1, 1, 0);
> > >   	v4l2_ctrl_new_std(v4l2_ctrl_hdl, &wave5_vpu_enc_ctrl_ops,
> > 	  V4L2_CID_MPEG_VIDEO_H264_CONSTRAINED_INTRA_PREDICTION,
> > >   			  0, 1, 1, 0);
> > > diff --git a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
> > > b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
> > > index dc31689e0d27..7b08fef58217 100644
> > > --- a/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
> > > +++ b/drivers/media/platform/chips-media/wave5/wave5-vpuapi.h
> > > @@ -570,6 +570,7 @@ struct enc_wave_param {
> > >   	u32 transform8x8_enable: 1; /* enable 8x8 intra prediction and 8x8
> > transform */
> > >   	u32 mb_level_rc_enable: 1; /* enable MB-level rate control */
> > >   	u32 forced_idr_header_enable: 1; /* enable header encoding before
> > > IDR frame */
> > > +	u32 constraint_set1_flag: 1; /* enable CBP */
> > >   	u32 bg_detection: 1; /* enable background detection */
> > >   };